Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1700914 > unrolled thread

[PATCH v9 00/13] Fix the on-flight DMA issue on system with amd iommu

Started byBaoquan He <bhe@redhat.com>
First post2017-08-01 13:40 +0200
Last post2017-08-01 13:50 +0200
Articles 9 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v9 00/13] Fix the on-flight DMA issue on system with amd iommu Baoquan He <bhe@redhat.com> - 2017-08-01 13:40 +0200
    [PATCH v9 02/13] iommu/amd: add several helper functions Baoquan He <bhe@redhat.com> - 2017-08-01 13:40 +0200
    [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Baoquan He <bhe@redhat.com> - 2017-08-01 13:50 +0200
      Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Joerg Roedel <jroedel@suse.de> - 2017-08-04 14:10 +0200
        Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Baoquan He <bhe@redhat.com> - 2017-08-04 14:30 +0200
        Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Baoquan He <bhe@redhat.com> - 2017-08-04 14:40 +0200
          Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Joerg Roedel <jroedel@suse.de> - 2017-08-04 15:00 +0200
            Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables() Baoquan He <bhe@redhat.com> - 2017-08-04 15:10 +0200
    [PATCH v9 01/13] iommu/amd: Detect pre enabled translation Baoquan He <bhe@redhat.com> - 2017-08-01 13:50 +0200

#1700914 — [PATCH v9 00/13] Fix the on-flight DMA issue on system with amd iommu

FromBaoquan He <bhe@redhat.com>
Date2017-08-01 13:40 +0200
Subject[PATCH v9 00/13] Fix the on-flight DMA issue on system with amd iommu
Message-ID<u9DGO-3pR-9@gated-at.bofh.it>
When kernel panicked and jump into the kdump kernel, DMA started by the
1st kernel is not stopped, this is called on-flight DMA. In the current
code it will disable iommu and build new translation table and attach
device to it. This will cause:

 1. IO_PAGE_FAULT warning message can be seen.
 2. transfer data to or from incorrect areas of memory.

Sometime it causes the dump failure or kernel hang.

The principle of the fix is to copy the old device table to let the old-flight
DMA continue looking up to get correct address translation and irq remap result,
meanwhile to defer the assignment of device to domain to device driver initializtion
stage. The old domain ids used in 1st kernel are reserved. And a new call-back
is_attach_deferred() is added to iommu-ops, will check whether we need defer the
domain attach/detach in iommu-core code. If defer is needed, just return directly
from amd iommu attach/detach function. The attachment will be done in device driver
initializaiton stage when calling get_domain().

Change history:
v8->v9:
    Made changes according to Joerg's reviewing comments and suggestions:
    - Check if all IOMMUs are pre-enabled, otherwise do not copy dev table
      and just continue as normal kernel does.

    - Add a new global old_dev_tbl_cpy to point to a newly allocated device
      table. The content of old device table will be copied to the specific
      device table for copying which old_dev_tbl_cpy points at. If copy failed
      we can still use the amd_iommu_dev_table which is allocated in
      early_amd_iommu_init(). This is for better rolling back if copy failed,
      the amd_iommu_dev_table has got necessary initialization since iommu init. 

    - Always allocate device table with GFP_DMA32 flag to make sure that they
      are under 4G. This tries to work around the issue mentioned in patch 10/13.
      Meanwhile double check if the address of device table is above 4G since
      it could be touched accidentally in corrupted 1st kernel and not trustworthy
      any more.

v7->v8:
    Rebase patchset v7 on the latest v4.13-rc1.
    - And re-enable printing IO_PAGE_FAULT message in kdump kernel.

    - Only disable iommu if amd_iommu=off is specified in kdump kernel.


v6->v7:
    Two main changes are made according to Joerg's suggestion:
    - Add is_attach_deferred call-back to iommu-ops. With this domain
      can be deferred to device driver init cleanly.

    - Allocate memory below 4G for dev table if translation pre-enabled.
      AMD engineer pointed out that it's unsafe to update the device-table
      while iommu is enabled. device-table pointer update is split up into
      two 32bit writes in the IOMMU hardware. So updating it while the IOMMU
      is enabled could have some nasty side effects.

v5->v6:
    According to Joerg's comments made several below main changes:
    - Add sanity check when copy old dev tables.

    - If a device is set up with guest translations (DTE.GV=1), then don't
      copy that information but move the device over to an empty guest-cr3
      table and handle the faults in the PPR log (which just answer them
      with INVALID).

v5:
    bnx2 NIC can't reset itself during driver init. Post patch to reset
    it during driver init. IO_PAGE_FAULT can't be seen anymore.

    Below is link of v5 post.
    https://lists.linuxfoundation.org/pipermail/iommu/2016-September/018527.html

Baoquan He (13):
  iommu/amd: Detect pre enabled translation
  iommu/amd: add several helper functions
  Revert "iommu/amd: Suppress IO_PAGE_FAULTs in kdump kernel"
  iommu/amd: Define bit fields for DTE particularly
  iommu/amd: Add function copy_dev_tables()
  iommu/amd: copy old trans table from old kernel
  iommu/amd: Do sanity check for address translation and irq remap of
    old dev table entry
  iommu: Add is_attach_deferred call-back to iommu-ops
  iommu/amd: Use is_attach_deferred call-back
  iommu/amd: Allocate memory below 4G for dev table if translation
    pre-enabled
  iommu/amd: Don't copy GCR3 table root pointer
  iommu/amd: Clear out the GV flag when handle deferred domain attach
  iommu/amd: Disable iommu only if amd_iommu=off is specified

 drivers/iommu/amd_iommu.c       |  81 ++++++++-------
 drivers/iommu/amd_iommu_init.c  | 222 ++++++++++++++++++++++++++++++++++++----
 drivers/iommu/amd_iommu_proto.h |   2 +
 drivers/iommu/amd_iommu_types.h |  55 +++++++++-
 drivers/iommu/amd_iommu_v2.c    |  18 +++-
 drivers/iommu/iommu.c           |   8 ++
 include/linux/iommu.h           |   1 +
 7 files changed, 323 insertions(+), 64 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]


#1700915 — [PATCH v9 02/13] iommu/amd: add several helper functions

FromBaoquan He <bhe@redhat.com>
Date2017-08-01 13:40 +0200
Subject[PATCH v9 02/13] iommu/amd: add several helper functions
Message-ID<u9DGP-3pR-51@gated-at.bofh.it>
In reply to#1700914
Move single iommu enabling codes into a wrapper function early_enable_iommu().
This can make later kdump change easier.

And also add iommu_disable_command_buffer and iommu_disable_event_buffer
for later usage.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu_init.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e39857ce6481..b36c145c498d 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -634,6 +634,14 @@ static void iommu_enable_command_buffer(struct amd_iommu *iommu)
 	amd_iommu_reset_cmd_buffer(iommu);
 }
 
+/*
+ * This function disables the command buffer
+ */
+static void iommu_disable_command_buffer(struct amd_iommu *iommu)
+{
+	iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
+}
+
 static void __init free_command_buffer(struct amd_iommu *iommu)
 {
 	free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
@@ -666,6 +674,14 @@ static void iommu_enable_event_buffer(struct amd_iommu *iommu)
 	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
 }
 
+/*
+ * This function disables the event log buffer
+ */
+static void iommu_disable_event_buffer(struct amd_iommu *iommu)
+{
+	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
+}
+
 static void __init free_event_buffer(struct amd_iommu *iommu)
 {
 	free_pages((unsigned long)iommu->evt_buf, get_order(EVT_BUFFER_SIZE));
@@ -2046,6 +2062,19 @@ static void iommu_enable_ga(struct amd_iommu *iommu)
 #endif
 }
 
+static void early_enable_iommu(struct amd_iommu *iommu)
+{
+	iommu_disable(iommu);
+	iommu_init_flags(iommu);
+	iommu_set_device_table(iommu);
+	iommu_enable_command_buffer(iommu);
+	iommu_enable_event_buffer(iommu);
+	iommu_set_exclusion_range(iommu);
+	iommu_enable_ga(iommu);
+	iommu_enable(iommu);
+	iommu_flush_all_caches(iommu);
+}
+
 /*
  * This function finally enables all IOMMUs found in the system after
  * they have been initialized
@@ -2054,17 +2083,8 @@ static void early_enable_iommus(void)
 {
 	struct amd_iommu *iommu;
 
-	for_each_iommu(iommu) {
-		iommu_disable(iommu);
-		iommu_init_flags(iommu);
-		iommu_set_device_table(iommu);
-		iommu_enable_command_buffer(iommu);
-		iommu_enable_event_buffer(iommu);
-		iommu_set_exclusion_range(iommu);
-		iommu_enable_ga(iommu);
-		iommu_enable(iommu);
-		iommu_flush_all_caches(iommu);
-	}
+	for_each_iommu(iommu)
+		early_enable_iommu(iommu);
 
 #ifdef CONFIG_IRQ_REMAP
 	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]


#1700921 — [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromBaoquan He <bhe@redhat.com>
Date2017-08-01 13:50 +0200
Subject[PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<u9DQu-3tn-19@gated-at.bofh.it>
In reply to#1700914
Add function copy_dev_tables to copy the old DEV table entries of the panicked
kernel to the new allocated device table. Since all iommus share the same device
table the copy only need be done one time. Here add a new global old_dev_tbl_cpy
to point to the newly allocated device table which the content of old device
table will be copied to. Besides, we also need to:

  - Check whether all IOMMUs actually use the same device table with the same size

  - Verify that the size of the old device table is the expected size.

  - Reserve the old domain id occupied in 1st kernel to avoid touching the old
    io-page tables. Then on-flight DMA can continue looking it up.

And also define MACRO DEV_DOMID_MASK to replace magic number 0xffffULL, it can be
reused in copy_dev_tables().

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu.c       |  2 +-
 drivers/iommu/amd_iommu_init.c  | 64 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |  1 +
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index e5a03f259986..4d00f1bda900 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2086,7 +2086,7 @@ static void set_dte_entry(u16 devid, struct protection_domain *domain, bool ats)
 		flags    |= tmp;
 	}
 
-	flags &= ~(0xffffUL);
+	flags &= ~DEV_DOMID_MASK;
 	flags |= domain->id;
 
 	amd_iommu_dev_table[devid].data[1]  = flags;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 8896137f17af..ab6794539527 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -195,6 +195,11 @@ spinlock_t amd_iommu_pd_lock;
  * page table root pointer.
  */
 struct dev_table_entry *amd_iommu_dev_table;
+/*
+ * Pointer to a device table which the content of old device table
+ * will be copied to. It's only be used in kdump kernel.
+ */
+static struct dev_table_entry *old_dev_tbl_cpy;
 
 /*
  * The alias table is a driver specific data structure which contains the
@@ -842,6 +847,65 @@ static int get_dev_entry_bit(u16 devid, u8 bit)
 }
 
 
+static int copy_device_table(void)
+{
+	struct dev_table_entry *old_devtb = NULL;
+	u32 lo, hi, devid, old_devtb_size;
+	phys_addr_t old_devtb_phys;
+	u64 entry, last_entry = 0;
+	struct amd_iommu *iommu;
+	u16 dom_id, dte_v;
+	static int copied;
+	gfp_t gfp_flag;
+
+	for_each_iommu(iommu) {
+		/* All IOMMUs should use the same device table with the same size */
+		lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
+		hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
+		entry = (((u64) hi) << 32) + lo;
+		if (last_entry && last_entry != entry) {
+			pr_err("IOMMU:%d should use the same dev table as others!/n",
+				iommu->index);
+			return -1;
+		}
+		last_entry = entry;
+
+		old_devtb_size = ((entry & ~PAGE_MASK) + 1) << 12;
+		if (old_devtb_size != dev_table_size) {
+			pr_err("The device table size of IOMMU:%d is not expected!/n",
+				iommu->index);
+			return -1;
+		}
+
+		if (copied)
+			continue;
+
+		old_devtb_phys = entry & PAGE_MASK;
+		old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
+		if (!old_devtb)
+			return -1;
+
+		gfp_flag = GFP_KERNEL | __GFP_ZERO;
+		old_dev_tbl_cpy = (void *)__get_free_pages(gfp_flag,
+					get_order(dev_table_size));
+		if (old_dev_tbl_cpy == NULL) {
+			pr_err("Failed to allocate memory for copying old device table!/n");
+			return -1;
+		}
+
+		for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
+			old_dev_tbl_cpy[devid] = old_devtb[devid];
+			dom_id = old_devtb[devid].data[1] & DEV_DOMID_MASK;
+			dte_v = old_devtb[devid].data[0] & DTE_FLAG_V;
+			if (dte_v && dom_id)
+				__set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
+		}
+		memunmap(old_devtb);
+		copied = 1;
+	}
+	return 0;
+}
+
 void amd_iommu_apply_erratum_63(u16 devid)
 {
 	int sysmgt;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index f88e802481a3..a7f6cf8c841e 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -336,6 +336,7 @@
 #define DTE_FLAG_MASK	(0x3ffULL << 32)
 #define DTE_GLX_SHIFT	(56)
 #define DTE_GLX_MASK	(3)
+#define DEV_DOMID_MASK	0xffffULL
 
 #define DTE_GCR3_VAL_A(x)	(((x) >> 12) & 0x00007ULL)
 #define DTE_GCR3_VAL_B(x)	(((x) >> 15) & 0x0ffffULL)
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]


#1703917 — Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromJoerg Roedel <jroedel@suse.de>
Date2017-08-04 14:10 +0200
SubjectRe: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<uaJAv-6MJ-51@gated-at.bofh.it>
In reply to#1700921
Hi Baoquan,

On Tue, Aug 01, 2017 at 07:37:21PM +0800, Baoquan He wrote:
> +	for_each_iommu(iommu) {
> +		/* All IOMMUs should use the same device table with the same size */
> +		lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> +		hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
> +		entry = (((u64) hi) << 32) + lo;
> +		if (last_entry && last_entry != entry) {
> +			pr_err("IOMMU:%d should use the same dev table as others!/n",
> +				iommu->index);
> +			return -1;
> +		}
> +		last_entry = entry;
> +
> +		old_devtb_size = ((entry & ~PAGE_MASK) + 1) << 12;
> +		if (old_devtb_size != dev_table_size) {
> +			pr_err("The device table size of IOMMU:%d is not expected!/n",
> +				iommu->index);
> +			return -1;
> +		}
> +
> +		if (copied)
> +			continue;
> +
> +		old_devtb_phys = entry & PAGE_MASK;
> +		old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
> +		if (!old_devtb)
> +			return -1;

You forgot to check whether the old device table is also below 4GB.

> +
> +		gfp_flag = GFP_KERNEL | __GFP_ZERO;
> +		old_dev_tbl_cpy = (void *)__get_free_pages(gfp_flag,
> +					get_order(dev_table_size));
> +		if (old_dev_tbl_cpy == NULL) {
> +			pr_err("Failed to allocate memory for copying old device table!/n");
> +			return -1;
> +		}
> +
> +		for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
> +			old_dev_tbl_cpy[devid] = old_devtb[devid];
> +			dom_id = old_devtb[devid].data[1] & DEV_DOMID_MASK;
> +			dte_v = old_devtb[devid].data[0] & DTE_FLAG_V;
> +			if (dte_v && dom_id)
> +				__set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
> +		}
> +		memunmap(old_devtb);
> +		copied = 1;

And this one should be outside of the loop, then you can get rid of the
'copied' variable. Also I don't really understand why you need a
temporary copy of the old device-table. Can't you just smart-copy the
contents of the old table to the real new one?


Regards,

	Joerg
	

[toc] | [prev] | [next] | [standalone]


#1703932 — Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromBaoquan He <bhe@redhat.com>
Date2017-08-04 14:30 +0200
SubjectRe: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<uaJTQ-6TP-11@gated-at.bofh.it>
In reply to#1703917
Hi Joerg,

Thanks for your reviewing!

On 08/04/17 at 02:09pm, Joerg Roedel wrote:
> Hi Baoquan,
> 
> On Tue, Aug 01, 2017 at 07:37:21PM +0800, Baoquan He wrote:
> > +	for_each_iommu(iommu) {
> > +		/* All IOMMUs should use the same device table with the same size */
> > +		lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> > +		hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
> > +		entry = (((u64) hi) << 32) + lo;
> > +		if (last_entry && last_entry != entry) {
> > +			pr_err("IOMMU:%d should use the same dev table as others!/n",
> > +				iommu->index);
> > +			return -1;
> > +		}
> > +		last_entry = entry;
> > +
> > +		old_devtb_size = ((entry & ~PAGE_MASK) + 1) << 12;
> > +		if (old_devtb_size != dev_table_size) {
> > +			pr_err("The device table size of IOMMU:%d is not expected!/n",
> > +				iommu->index);
> > +			return -1;
> > +		}
> > +
> > +		if (copied)
> > +			continue;
> > +
> > +		old_devtb_phys = entry & PAGE_MASK;
> > +		old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
> > +		if (!old_devtb)
> > +			return -1;
> 
> You forgot to check whether the old device table is also below 4GB.

I did it in patch 10/13. I think it's an sub-issue and can be explained
in a specific patch.

Thanks
Baoquan

[toc] | [prev] | [next] | [standalone]


#1703945 — Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromBaoquan He <bhe@redhat.com>
Date2017-08-04 14:40 +0200
SubjectRe: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<uaK3y-6XA-45@gated-at.bofh.it>
In reply to#1703917
On 08/04/17 at 02:09pm, Joerg Roedel wrote:
> 
> On Tue, Aug 01, 2017 at 07:37:21PM +0800, Baoquan He wrote:
> > +	for_each_iommu(iommu) {

	......

> > +		if (copied)
> > +			continue;
> > +
> > +		old_devtb_phys = entry & PAGE_MASK;
> > +		old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
> > +		if (!old_devtb)
> > +			return -1;
> 
> You forgot to check whether the old device table is also below 4GB.
> 
> > +
> > +		gfp_flag = GFP_KERNEL | __GFP_ZERO;
> > +		old_dev_tbl_cpy = (void *)__get_free_pages(gfp_flag,
> > +					get_order(dev_table_size));
> > +		if (old_dev_tbl_cpy == NULL) {
> > +			pr_err("Failed to allocate memory for copying old device table!/n");
> > +			return -1;
> > +		}
> > +
> > +		for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
> > +			old_dev_tbl_cpy[devid] = old_devtb[devid];
> > +			dom_id = old_devtb[devid].data[1] & DEV_DOMID_MASK;
> > +			dte_v = old_devtb[devid].data[0] & DTE_FLAG_V;
> > +			if (dte_v && dom_id)
> > +				__set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
> > +		}
> > +		memunmap(old_devtb);
> > +		copied = 1;
> 
> And this one should be outside of the loop, then you can get rid of the
> 'copied' variable. Also I don't really understand why you need a
> temporary copy of the old device-table. Can't you just smart-copy the
> contents of the old table to the real new one?

Sorry, I don't get 'this one' meaning, are you suggesting the copy for
loop should be take out of the iommu for loop? 

About the temporary copy of the old device-table, you can see in patch
7/13, if irq sanity check failed, it return -1. This return could happen
in the middle of copy. So I think we should do a whole successful copy,
or don't copy at all. It might not be good do half copy.

[toc] | [prev] | [next] | [standalone]


#1703951 — Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromJoerg Roedel <jroedel@suse.de>
Date2017-08-04 15:00 +0200
SubjectRe: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<uaKmT-76Y-27@gated-at.bofh.it>
In reply to#1703945
On Fri, Aug 04, 2017 at 08:30:39PM +0800, Baoquan He wrote:
> Sorry, I don't get 'this one' meaning, are you suggesting the copy for
> loop should be take out of the iommu for loop? 
> 
> About the temporary copy of the old device-table, you can see in patch
> 7/13, if irq sanity check failed, it return -1. This return could happen
> in the middle of copy. So I think we should do a whole successful copy,
> or don't copy at all. It might not be good do half copy.

No, I mean that you should move the copy of the complete device-table
out of the for_each_iommu() loop. Currently you make sure you copy only
once with the 'copied' flag, but that is not necessary if you move the
code behind the loop.

[toc] | [prev] | [next] | [standalone]


#1703958 — Re: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()

FromBaoquan He <bhe@redhat.com>
Date2017-08-04 15:10 +0200
SubjectRe: [PATCH v9 05/13] iommu/amd: Add function copy_dev_tables()
Message-ID<uaKwy-7py-21@gated-at.bofh.it>
In reply to#1703951
On 08/04/17 at 02:51pm, Joerg Roedel wrote:
> On Fri, Aug 04, 2017 at 08:30:39PM +0800, Baoquan He wrote:
> > Sorry, I don't get 'this one' meaning, are you suggesting the copy for
> > loop should be take out of the iommu for loop? 
> > 
> > About the temporary copy of the old device-table, you can see in patch
> > 7/13, if irq sanity check failed, it return -1. This return could happen
> > in the middle of copy. So I think we should do a whole successful copy,
> > or don't copy at all. It might not be good do half copy.
> 
> No, I mean that you should move the copy of the complete device-table
> out of the for_each_iommu() loop. Currently you make sure you copy only
> once with the 'copied' flag, but that is not necessary if you move the
> code behind the loop.

Ok, will do. Thanks!
> 

[toc] | [prev] | [next] | [standalone]


#1700923 — [PATCH v9 01/13] iommu/amd: Detect pre enabled translation

FromBaoquan He <bhe@redhat.com>
Date2017-08-01 13:50 +0200
Subject[PATCH v9 01/13] iommu/amd: Detect pre enabled translation
Message-ID<u9DQu-3tn-21@gated-at.bofh.it>
In reply to#1700914
Add functions to check whether translation is already enabled in IOMMU.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu_init.c  | 24 ++++++++++++++++++++++++
 drivers/iommu/amd_iommu_proto.h |  1 +
 drivers/iommu/amd_iommu_types.h |  3 +++
 3 files changed, 28 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 5cc597b383c7..e39857ce6481 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -258,6 +258,25 @@ static int amd_iommu_enable_interrupts(void);
 static int __init iommu_go_to_state(enum iommu_init_state state);
 static void init_device_table_dma(void);
 
+bool translation_pre_enabled(struct amd_iommu *iommu)
+{
+	return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
+}
+
+static void clear_translation_pre_enabled(struct amd_iommu *iommu)
+{
+	iommu->flags &= ~AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
+static void init_translation_status(struct amd_iommu *iommu)
+{
+	u32 ctrl;
+
+	ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
+	if (ctrl & (1<<CONTROL_IOMMU_EN))
+		iommu->flags |= AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
 static inline void update_last_devid(u16 devid)
 {
 	if (devid > amd_iommu_last_bdf)
@@ -1399,6 +1418,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 	iommu->int_enabled = false;
 
+	init_translation_status(iommu);
+
+	if (translation_pre_enabled(iommu))
+		pr_warn("Translation is already enabled - trying to copy translation structures\n");
+
 	ret = init_iommu_from_acpi(iommu, h);
 	if (ret)
 		return ret;
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 466260f8a1df..a9666d2005bb 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -87,4 +87,5 @@ static inline bool iommu_feature(struct amd_iommu *iommu, u64 f)
 	return !!(iommu->features & f);
 }
 
+extern bool translation_pre_enabled(struct amd_iommu *iommu);
 #endif /* _ASM_X86_AMD_IOMMU_PROTO_H  */
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 294a409e283b..0c98b2cf04cc 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -435,6 +435,8 @@ struct iommu_domain;
 struct irq_domain;
 struct amd_irte_ops;
 
+#define AMD_IOMMU_FLAG_TRANS_PRE_ENABLED      (1 << 0)
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
@@ -569,6 +571,7 @@ struct amd_iommu {
 	struct amd_irte_ops *irte_ops;
 #endif
 
+	u32 flags;
 	volatile u64 __aligned(8) cmd_sem;
 };
 
-- 
2.5.5

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web