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

Groups > linux.kernel > #1406666 > unrolled thread

[Patch v4 0/9] *** Fix kdump failure in system with amd iommu***

Back to article view | Back to linux.kernel

Contents

  [Patch v4 0/9] *** Fix kdump failure in system with amd iommu*** Baoquan He <bhe@redhat.com> - 2016-05-25 08:30 +0200
    [Patch v4 8/9] iommu/amd: Do not initialize dev tables again in kdump Baoquan He <bhe@redhat.com> - 2016-05-25 08:30 +0200
    [Patch v4 9/9] iommu/amd: Check the validation of irq table and domain id Baoquan He <bhe@redhat.com> - 2016-05-25 08:30 +0200
      Re: [Patch v4 9/9] iommu/amd: Check the validation of irq table and  domain id Wan Zongshun <vw@iommu.org> - 2016-05-28 15:40 +0200
        Re: [Patch v4 9/9] iommu/amd: Check the validation of irq table and  domain id Baoquan He <bhe@redhat.com> - 2016-05-30 07:20 +0200
    [Patch v4 6/9] iommu/amd: Add function copy_dev_tables Baoquan He <bhe@redhat.com> - 2016-05-25 08:30 +0200
      Re: [Patch v4 6/9] iommu/amd: Add function copy_dev_tables Wan Zongshun <vw@iommu.org> - 2016-05-28 15:10 +0200
        Re: [Patch v4 6/9] iommu/amd: Add function copy_dev_tables Baoquan He <bhe@redhat.com> - 2016-05-30 05:40 +0200
    [Patch v4 1/9] iommu/amd: clean up the cmpxchg64 invocation Baoquan He <bhe@redhat.com> - 2016-05-25 08:40 +0200
    [Patch v4 5/9] iommu/amd: Define bit fields for DTE particularly Baoquan He <bhe@redhat.com> - 2016-05-25 08:40 +0200
    [Patch v4 2/9] iommu/amd: Use standard bitmap operation to set bitmap Baoquan He <bhe@redhat.com> - 2016-05-25 08:40 +0200
    [Patch v4 7/9] iommu/amd: copy old trans table from old kernel Baoquan He <bhe@redhat.com> - 2016-05-25 08:40 +0200
      Re: [Patch v4 7/9] iommu/amd: copy old trans table from old kernel Wan Zongshun <vw@iommu.org> - 2016-05-28 15:30 +0200
    Re: [Patch v4 0/9] *** Fix kdump failure in system with amd iommu*** Baoquan He <bhe@redhat.com> - 2016-05-25 09:20 +0200

#1406666 — [Patch v4 0/9] *** Fix kdump failure in system with amd iommu***

Hi Joerg,

Recently I have time to continue the work of fixing AMD IOMMU faults in
kdump kernel. The situation is I tried to make change at the time point
as Intel iommu has done, but still Ethernet NIC will trigger the printing
of IO_PAGE_FAULT. I got 2 machines with AMD IOMMU v1 and v2 separately,
the test result is similar, always there are IO_PAGE_FAULT message for
Ethernet network card. I have no idea why it happened though I did it
just like Intel IOMMU did.

In this v4 change, I just made changes as follows:

1) Several clean up patches when I reviewed AMD IOMMU code.
2) Detect if it's pre-enabled in kdump kernel.
3) Copy dev tables in kdump kernel. Since dev table is per device we just
   need to copy the content in dev table, the io page table pointer and
   irq table pointer are contained in each dev table entry.
4) Reserved the domain id which has been allocated in 1st kernel
5) Do not re-enable or re-init the dev table entry bits or control bit
   of AMD IOMMU.

Post this to mailing list, hope it can be figured out which need be further
changed.

v3->v4:
    1)Define several PTE/DTE bits MACRO definition according to Zongshun's
      comments.
    2)Learned the implementation of vt-d fix done by Joerg and did the AMD
      IOMMU change similiarly.

Baoquan HE (4):
  iommu/amd: add early_enable_iommu() helper function
  iommu/amd: copy old trans table from old kernel
  iommu/amd: Do not initialize dev tables again in kdump
  iommu/amd: Check the validation of irq table and domain id

Baoquan He (5):
  iommu/amd: clean up the cmpxchg64 invocation
  iommu/amd: Use standard bitmap operation to set bitmap
  iommu/amd: Detect pre enabled translation
  iommu/amd: Define bit fields for DTE particularly
  iommu/amd: Add function copy_dev_tables

 drivers/iommu/amd_iommu.c       |  21 +++----
 drivers/iommu/amd_iommu_init.c  | 136 +++++++++++++++++++++++++++++++++++-----
 drivers/iommu/amd_iommu_types.h |  29 +++++++--
 3 files changed, 156 insertions(+), 30 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1406667 — [Patch v4 8/9] iommu/amd: Do not initialize dev tables again in kdump

From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>

The init should have been done in normal kernel, skip it in kdump
kernel. And clean up the function comments.

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

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 66a1fa5..47e5972 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1432,7 +1432,11 @@ static int __init amd_iommu_init_pci(void)
 			break;
 	}
 
-	init_device_table_dma();
+	for_each_iommu(iommu) {
+		if ( !translation_pre_enabled(iommu) )
+			init_device_table_dma();
+			break;
+	}
 
 	for_each_iommu(iommu)
 		iommu_flush_all_caches(iommu);
@@ -1612,8 +1616,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
 }
 
 /*
- * Init the device table to not allow DMA access for devices and
- * suppress all page faults
+ * Init the device table to not allow DMA access for devices.
  */
 static void init_device_table_dma(void)
 {
-- 
2.5.5

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

#1406668 — [Patch v4 9/9] iommu/amd: Check the validation of irq table and domain id

From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>

If not valid just skip reserving the old domain id.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu.c       | 4 ----
 drivers/iommu/amd_iommu_init.c  | 5 +++--
 drivers/iommu/amd_iommu_types.h | 5 +++++
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index f3bd7fd..40c4a05 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3697,10 +3697,6 @@ struct amd_ir_data {
 
 static struct irq_chip amd_ir_chip;
 
-#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
-#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
-#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
-#define DTE_IRQ_REMAP_ENABLE    1ULL
 
 static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
 {
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 47e5972..263704a 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -670,7 +670,7 @@ static int copy_dev_tables(void)
 	u32 lo, hi, devid;
 	phys_addr_t old_devtb_phys;
 	struct dev_table_entry *old_devtb;
-	u16 dom_id, dte_v;
+	u16 dom_id, dte_v, irq_v;
 	struct amd_iommu *iommu;
 	static int copied;
 
@@ -692,7 +692,8 @@ static int copy_dev_tables(void)
                         amd_iommu_dev_table[devid] = old_devtb[devid];
                         dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
 			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
-			if (!dte_v)
+			irq_v = amd_iommu_dev_table[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
+			if (!dte_v || !irq_v || !dom_id)
 				continue;
                         __set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
                 }
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 34acd73..08340f5 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -224,6 +224,11 @@
 
 #define PPR_REQ_FAULT		0x01
 
+#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
+#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
+#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
+#define DTE_IRQ_REMAP_ENABLE    1ULL
+
 #define PAGE_MODE_NONE    0x00
 #define PAGE_MODE_1_LEVEL 0x01
 #define PAGE_MODE_2_LEVEL 0x02
-- 
2.5.5

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

#1408470 — Re: [Patch v4 9/9] iommu/amd: Check the validation of irq table and domain id

-------- Original Message --------
> From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>
>
> If not valid just skip reserving the old domain id.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  drivers/iommu/amd_iommu.c       | 4 ----
>  drivers/iommu/amd_iommu_init.c  | 5 +++--
>  drivers/iommu/amd_iommu_types.h | 5 +++++
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index f3bd7fd..40c4a05 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3697,10 +3697,6 @@ struct amd_ir_data {
>
>  static struct irq_chip amd_ir_chip;
>
> -#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> -#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
> -#define DTE_IRQ_REMAP_ENABLE    1ULL

At least, you should give reason comments to why you want move it.

Any drivers files you want to use them as well?

>
>  static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
>  {
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 47e5972..263704a 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -670,7 +670,7 @@ static int copy_dev_tables(void)
>  	u32 lo, hi, devid;
>  	phys_addr_t old_devtb_phys;
>  	struct dev_table_entry *old_devtb;
> -	u16 dom_id, dte_v;
> +	u16 dom_id, dte_v, irq_v;
>  	struct amd_iommu *iommu;
>  	static int copied;
>
> @@ -692,7 +692,8 @@ static int copy_dev_tables(void)
>                          amd_iommu_dev_table[devid] = old_devtb[devid];
>                          dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
>  			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
> -			if (!dte_v)
> +			irq_v = amd_iommu_dev_table[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
> +			if (!dte_v || !irq_v || !dom_id)
>  				continue;
>                          __set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
>                  }
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 34acd73..08340f5 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -224,6 +224,11 @@
>
>  #define PPR_REQ_FAULT		0x01
>
> +#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> +#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> +#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
> +#define DTE_IRQ_REMAP_ENABLE    1ULL
> +
>  #define PAGE_MODE_NONE    0x00
>  #define PAGE_MODE_1_LEVEL 0x01
>  #define PAGE_MODE_2_LEVEL 0x02
>

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

#1408759 — Re: [Patch v4 9/9] iommu/amd: Check the validation of irq table and domain id

On 05/28/16 at 09:30pm, Wan Zongshun wrote:
> 
> 
> -------- Original Message --------
> >From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>
> >diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> >index f3bd7fd..40c4a05 100644
> >--- a/drivers/iommu/amd_iommu.c
> >+++ b/drivers/iommu/amd_iommu.c
> >@@ -3697,10 +3697,6 @@ struct amd_ir_data {
> >
> > static struct irq_chip amd_ir_chip;
> >
> >-#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> >-#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> >-#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
> >-#define DTE_IRQ_REMAP_ENABLE    1ULL
> 
> At least, you should give reason comments to why you want move it.
> 
> Any drivers files you want to use them as well?

Yes, sorry fot this. I have several test machines. I made change on
them and made patches. In the machine I posted this patch was not
arranged well.

> 
> >
> > static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
> > {
> >diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> >index 47e5972..263704a 100644
> >--- a/drivers/iommu/amd_iommu_init.c
> >+++ b/drivers/iommu/amd_iommu_init.c
> >@@ -670,7 +670,7 @@ static int copy_dev_tables(void)
> > 	u32 lo, hi, devid;
> > 	phys_addr_t old_devtb_phys;
> > 	struct dev_table_entry *old_devtb;
> >-	u16 dom_id, dte_v;
> >+	u16 dom_id, dte_v, irq_v;
> > 	struct amd_iommu *iommu;
> > 	static int copied;
> >
> >@@ -692,7 +692,8 @@ static int copy_dev_tables(void)
> >                         amd_iommu_dev_table[devid] = old_devtb[devid];
> >                         dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
> > 			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
> >-			if (!dte_v)
> >+			irq_v = amd_iommu_dev_table[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
> >+			if (!dte_v || !irq_v || !dom_id)
> > 				continue;
> >                         __set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
> >                 }
> >diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> >index 34acd73..08340f5 100644
> >--- a/drivers/iommu/amd_iommu_types.h
> >+++ b/drivers/iommu/amd_iommu_types.h
> >@@ -224,6 +224,11 @@
> >
> > #define PPR_REQ_FAULT		0x01
> >
> >+#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> >+#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> >+#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
> >+#define DTE_IRQ_REMAP_ENABLE    1ULL
> >+
> > #define PAGE_MODE_NONE    0x00
> > #define PAGE_MODE_1_LEVEL 0x01
> > #define PAGE_MODE_2_LEVEL 0x02
> >

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

#1406671 — [Patch v4 6/9] iommu/amd: Add function copy_dev_tables

Add function copy_dev_tables to copy old DTE of the 1st kernel to
the new DTE table. Since all iommu share the same DTE table the
copy only need be done once as long as the physical address of
old DTE table is retrieved from iommu reg. Besides the old domain
id occupied in 1st kernel need be reserved in order to avoid touch
the old translation tables.

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

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1c916cc..f3bd7fd 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2081,7 +2081,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 9c1aa54..71c7ac9 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -664,6 +664,44 @@ static int get_dev_entry_bit(u16 devid, u8 bit)
 }
 
 
+static int copy_dev_tables(void)
+{
+	u64 entry;
+	u32 lo, hi, devid;
+	phys_addr_t old_devtb_phys;
+	struct dev_table_entry *old_devtb;
+	u16 dom_id, dte_v;
+	struct amd_iommu *iommu;
+	static int copied;
+
+        for_each_iommu(iommu) {
+		if (!translation_pre_enabled()) {
+			pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
+			return -1;
+		}
+
+		if (copied)
+			continue;
+
+                lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
+                hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
+                entry = (((u64) hi) << 32) + lo;
+                old_devtb_phys = entry & PAGE_MASK;
+                old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
+                for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
+                        amd_iommu_dev_table[devid] = old_devtb[devid];
+                        dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
+			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
+			if (!dte_v)
+				continue;
+                        __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 7796edf..34acd73 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -311,6 +311,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]

#1408464 — Re: [Patch v4 6/9] iommu/amd: Add function copy_dev_tables

-------- Original Message --------
> Add function copy_dev_tables to copy old DTE of the 1st kernel to
> the new DTE table. Since all iommu share the same DTE table the
> copy only need be done once as long as the physical address of
> old DTE table is retrieved from iommu reg. Besides the old domain
> id occupied in 1st kernel need be reserved in order to avoid touch
> the old translation tables.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  drivers/iommu/amd_iommu.c       |  2 +-
>  drivers/iommu/amd_iommu_init.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/amd_iommu_types.h |  1 +
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 1c916cc..f3bd7fd 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -2081,7 +2081,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 9c1aa54..71c7ac9 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -664,6 +664,44 @@ static int get_dev_entry_bit(u16 devid, u8 bit)
>  }
>
>
> +static int copy_dev_tables(void)
> +{
> +	u64 entry;
> +	u32 lo, hi, devid;
> +	phys_addr_t old_devtb_phys;
> +	struct dev_table_entry *old_devtb;
> +	u16 dom_id, dte_v;
> +	struct amd_iommu *iommu;
> +	static int copied;
> +
> +        for_each_iommu(iommu) {
> +		if (!translation_pre_enabled()) {
> +			pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
> +			return -1;
> +		}

If one iommu is not pre-enabled, all iommus will be exit the copy.

> +
> +		if (copied)
> +			continue;
> +
> +                lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> +                hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
> +                entry = (((u64) hi) << 32) + lo;
> +                old_devtb_phys = entry & PAGE_MASK;
> +                old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
> +                for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
> +                        amd_iommu_dev_table[devid] = old_devtb[devid];
> +                        dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
> +			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
> +			if (!dte_v)
> +				continue;
> +                        __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 7796edf..34acd73 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -311,6 +311,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)
>

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

#1408747 — Re: [Patch v4 6/9] iommu/amd: Add function copy_dev_tables

On 05/28/16 at 09:08pm, Wan Zongshun wrote:
> >+static int copy_dev_tables(void)
> >+{
> >+	u64 entry;
> >+	u32 lo, hi, devid;
> >+	phys_addr_t old_devtb_phys;
> >+	struct dev_table_entry *old_devtb;
> >+	u16 dom_id, dte_v;
> >+	struct amd_iommu *iommu;
> >+	static int copied;
> >+
> >+        for_each_iommu(iommu) {
> >+		if (!translation_pre_enabled()) {
> >+			pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
> >+			return -1;
> >+		}
> 
> If one iommu is not pre-enabled, all iommus will be exit the copy.

Currently amd iommu driver make all iommu-s share a single device table.
When handling this code, I am struggling to take what way to make this
look better. Say we have two iommus A and B on a system, A is detected
to be pre_enabled, but B is not, I didn't think of a good way to do. Any
suggestion?

> 
> >+
> >+		if (copied)
> >+			continue;
> >+
> >+                lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> >+                hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 4);
> >+                entry = (((u64) hi) << 32) + lo;
> >+                old_devtb_phys = entry & PAGE_MASK;
> >+                old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
> >+                for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
> >+                        amd_iommu_dev_table[devid] = old_devtb[devid];
> >+                        dom_id = amd_iommu_dev_table[devid].data[1] & DEV_DOMID_MASK;
> >+			dte_v = amd_iommu_dev_table[devid].data[0] & DTE_FLAG_V;
> >+			if (!dte_v)
> >+				continue;
> >+                        __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 7796edf..34acd73 100644
> >--- a/drivers/iommu/amd_iommu_types.h
> >+++ b/drivers/iommu/amd_iommu_types.h
> >@@ -311,6 +311,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)
> >

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

#1406673 — [Patch v4 1/9] iommu/amd: clean up the cmpxchg64 invocation

Change it as it's designed for and keep it consistent with other
places.

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

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 5efadad..9ec7cad 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1287,7 +1287,8 @@ static u64 *alloc_pte(struct protection_domain *domain,
 
 			__npte = PM_LEVEL_PDE(level, virt_to_phys(page));
 
-			if (cmpxchg64(pte, __pte, __npte)) {
+			/* pte could have been changed somewhere. */
+			if (cmpxchg64(pte, __pte, __npte) != __pte) {
 				free_page((unsigned long)page);
 				continue;
 			}
-- 
2.5.5

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

#1406675 — [Patch v4 5/9] iommu/amd: Define bit fields for DTE particularly

In amd-vi spec several bits of IO PTE fields and DTE fields are similar
so that both of them can share the same MACRO definition. However
defining their respecitve bit fields can make code more read-able. So
do it in this patch.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu.c       | 10 +++++-----
 drivers/iommu/amd_iommu_types.h | 19 +++++++++++++++----
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index cc636e6..1c916cc 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1401,9 +1401,9 @@ static int iommu_map_page(struct protection_domain *dom,
 
 	if (count > 1) {
 		__pte = PAGE_SIZE_PTE(phys_addr, page_size);
-		__pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_P | IOMMU_PTE_FC;
+		__pte |= PM_LEVEL_ENC(7) | IOMMU_PTE_PR | IOMMU_PTE_FC;
 	} else
-		__pte = phys_addr | IOMMU_PTE_P | IOMMU_PTE_FC;
+		__pte = phys_addr | IOMMU_PTE_PR | IOMMU_PTE_FC;
 
 	if (prot & IOMMU_PROT_IR)
 		__pte |= IOMMU_PTE_IR;
@@ -2048,7 +2048,7 @@ static void set_dte_entry(u16 devid, struct protection_domain *domain, bool ats)
 
 	pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
 		    << DEV_ENTRY_MODE_SHIFT;
-	pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
+	pte_root |= DTE_FLAG_IR | DTE_FLAG_IW | DTE_FLAG_V | DTE_FLAG_TV;
 
 	flags = amd_iommu_dev_table[devid].data[1];
 
@@ -2091,7 +2091,7 @@ static void set_dte_entry(u16 devid, struct protection_domain *domain, bool ats)
 static void clear_dte_entry(u16 devid)
 {
 	/* remove entry from the device table seen by the hardware */
-	amd_iommu_dev_table[devid].data[0]  = IOMMU_PTE_P | IOMMU_PTE_TV;
+	amd_iommu_dev_table[devid].data[0]  = DTE_FLAG_V | DTE_FLAG_TV;
 	amd_iommu_dev_table[devid].data[1] &= DTE_FLAG_MASK;
 
 	amd_iommu_apply_erratum_63(devid);
@@ -2533,7 +2533,7 @@ static dma_addr_t dma_ops_domain_map(struct dma_ops_domain *dom,
 	if (!pte)
 		return DMA_ERROR_CODE;
 
-	__pte = paddr | IOMMU_PTE_P | IOMMU_PTE_FC;
+	__pte = paddr | IOMMU_PTE_PR | IOMMU_PTE_FC;
 
 	if (direction == DMA_TO_DEVICE)
 		__pte |= IOMMU_PTE_IR;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 01783cc..7796edf 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -239,7 +239,7 @@
 #define PM_LEVEL_INDEX(x, a)	(((a) >> PM_LEVEL_SHIFT((x))) & 0x1ffULL)
 #define PM_LEVEL_ENC(x)		(((x) << 9) & 0xe00ULL)
 #define PM_LEVEL_PDE(x, a)	((a) | PM_LEVEL_ENC((x)) | \
-				 IOMMU_PTE_P | IOMMU_PTE_IR | IOMMU_PTE_IW)
+				 IOMMU_PTE_PR | IOMMU_PTE_IR | IOMMU_PTE_IW)
 #define PM_PTE_LEVEL(pte)	(((pte) >> 9) & 0x7ULL)
 
 #define PM_MAP_4k		0
@@ -288,13 +288,24 @@
 #define PTE_LEVEL_PAGE_SIZE(level)			\
 	(1ULL << (12 + (9 * (level))))
 
-#define IOMMU_PTE_P  (1ULL << 0)
-#define IOMMU_PTE_TV (1ULL << 1)
+/*
+ * Bit value definition for I/O PTE fields
+ */
+#define IOMMU_PTE_PR (1ULL << 0)
 #define IOMMU_PTE_U  (1ULL << 59)
 #define IOMMU_PTE_FC (1ULL << 60)
 #define IOMMU_PTE_IR (1ULL << 61)
 #define IOMMU_PTE_IW (1ULL << 62)
 
+
+/*
+ * Bit value definition for DTE fields
+ */
+#define DTE_FLAG_V  (1ULL << 0)
+#define DTE_FLAG_TV (1ULL << 1)
+#define DTE_FLAG_IR (1ULL << 61)
+#define DTE_FLAG_IW (1ULL << 62)
+
 #define DTE_FLAG_IOTLB	(1ULL << 32)
 #define DTE_FLAG_GV	(1ULL << 55)
 #define DTE_FLAG_MASK	(0x3ffULL << 32)
@@ -316,7 +327,7 @@
 #define GCR3_VALID		0x01ULL
 
 #define IOMMU_PAGE_MASK (((1ULL << 52) - 1) & ~0xfffULL)
-#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_P)
+#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_PR)
 #define IOMMU_PTE_PAGE(pte) (phys_to_virt((pte) & IOMMU_PAGE_MASK))
 #define IOMMU_PTE_MODE(pte) (((pte) >> 9) & 0x07)
 
-- 
2.5.5

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

#1406678 — [Patch v4 2/9] iommu/amd: Use standard bitmap operation to set bitmap

It will be more readable then the old setting.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu.c      | 2 +-
 drivers/iommu/amd_iommu_init.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9ec7cad..cc636e6 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2016,7 +2016,7 @@ static struct dma_ops_domain *dma_ops_domain_alloc(void)
 	 * mark the first page as allocated so we never return 0 as
 	 * a valid dma-address. So we can use 0 as error value
 	 */
-	dma_dom->aperture[0]->bitmap[0] = 1;
+	__set_bit(0, dma_dom->aperture[0]->bitmap);
 
 	for_each_possible_cpu(cpu)
 		*per_cpu_ptr(dma_dom->next_index, cpu) = 0;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index bf4959f..8361367d 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -20,6 +20,7 @@
 #include <linux/pci.h>
 #include <linux/acpi.h>
 #include <linux/list.h>
+#include <linux/bitmap.h>
 #include <linux/slab.h>
 #include <linux/syscore_ops.h>
 #include <linux/interrupt.h>
@@ -1908,8 +1909,7 @@ static int __init early_amd_iommu_init(void)
 	 * never allocate domain 0 because its used as the non-allocated and
 	 * error value placeholder
 	 */
-	amd_iommu_pd_alloc_bitmap[0] = 1;
-
+	 __set_bit(0, amd_iommu_pd_alloc_bitmap);
 	spin_lock_init(&amd_iommu_pd_lock);
 
 	/*
-- 
2.5.5

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

#1406681 — [Patch v4 7/9] iommu/amd: copy old trans table from old kernel

From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>

Here several things need be done:
1) Initialize amd_iommu_dev_table because it was set several times
   since kdump kernel reboot. We don't need the set because we will
   copy the content from old kernel.
2) Re-enable event/cmd buffer
3) Install the DTE table to reg
4) Flush all caches

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

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 71c7ac9..66a1fa5 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -35,7 +35,7 @@
 #include <asm/iommu_table.h>
 #include <asm/io_apic.h>
 #include <asm/irq_remapping.h>
-
+#include <linux/crash_dump.h>
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
 #include "irq_remapping.h"
@@ -675,7 +675,7 @@ static int copy_dev_tables(void)
 	static int copied;
 
         for_each_iommu(iommu) {
-		if (!translation_pre_enabled()) {
+		if (!translation_pre_enabled(iommu)) {
 			pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
 			return -1;
 		}
@@ -1160,8 +1160,13 @@ 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) && !is_kdump_kernel()) {
+                        clear_translation_pre_enabled(iommu);
+                        pr_warn("Translation was enabled for IOMMU:%d but we are not in kdump mode\n",
+                                iommu->index);
+                }
 
-	if (translation_pre_enabled())
+	if (translation_pre_enabled(iommu))
 		pr_warn("Translation is already enabled - trying to copy translation structures\n");
 
 	ret = init_iommu_from_acpi(iommu, h);
@@ -1730,9 +1735,41 @@ static void early_enable_iommu(struct amd_iommu *iommu)
 static void early_enable_iommus(void)
 {
 	struct amd_iommu *iommu;
+	bool is_pre_enabled=false;
 
-	for_each_iommu(iommu)
-		early_enable_iommu(iommu);
+	for_each_iommu(iommu) {
+		if ( translation_pre_enabled(iommu) ) {
+			is_pre_enabled = true;
+			break;
+		}
+	}
+
+	if ( !is_pre_enabled) {
+		for_each_iommu(iommu)
+			early_enable_iommu(iommu);
+	} else {
+		if (copy_dev_tables()) {
+			pr_err("Failed to copy translation tables from previous kernel.\n");
+			/*
+			 * If failed to copy dev tables from old kernel, continue to proceed
+			 * as it does in normal kernel.
+			 */
+			for_each_iommu(iommu) {
+				clear_translation_pre_enabled(iommu);
+				early_enable_iommu(iommu);
+			}
+		} else {
+			pr_info("Copied translation tables from previous kernel.\n");
+			for_each_iommu(iommu) {
+				iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
+		                iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
+		                iommu_enable_command_buffer(iommu);
+		                iommu_enable_event_buffer(iommu);
+		                iommu_set_device_table(iommu);
+		                iommu_flush_all_caches(iommu);
+			}
+		}
+	}
 }
 
 static void enable_iommus_v2(void)
-- 
2.5.5

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

#1408469 — Re: [Patch v4 7/9] iommu/amd: copy old trans table from old kernel

-------- Original Message --------
> From: Baoquan HE <bhe@dhcp-129-10.nay.redhat.com>
>
> Here several things need be done:
> 1) Initialize amd_iommu_dev_table because it was set several times
>    since kdump kernel reboot. We don't need the set because we will
>    copy the content from old kernel.
> 2) Re-enable event/cmd buffer
> 3) Install the DTE table to reg
> 4) Flush all caches
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  drivers/iommu/amd_iommu_init.c | 47 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 71c7ac9..66a1fa5 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -35,7 +35,7 @@
>  #include <asm/iommu_table.h>
>  #include <asm/io_apic.h>
>  #include <asm/irq_remapping.h>
> -
> +#include <linux/crash_dump.h>
>  #include "amd_iommu_proto.h"
>  #include "amd_iommu_types.h"
>  #include "irq_remapping.h"
> @@ -675,7 +675,7 @@ static int copy_dev_tables(void)
>  	static int copied;
>
>          for_each_iommu(iommu) {
> -		if (!translation_pre_enabled()) {
> +		if (!translation_pre_enabled(iommu)) {
>  			pr_err("IOMMU:%d is not pre-enabled!/n", iommu->index);
>  			return -1;
>  		}
> @@ -1160,8 +1160,13 @@ 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) && !is_kdump_kernel()) {
> +                        clear_translation_pre_enabled(iommu);
> +                        pr_warn("Translation was enabled for IOMMU:%d but we are not in kdump mode\n",
> +                                iommu->index);
> +                }
>
> -	if (translation_pre_enabled())
> +	if (translation_pre_enabled(iommu))
>  		pr_warn("Translation is already enabled - trying to copy translation structures\n");
>
>  	ret = init_iommu_from_acpi(iommu, h);
> @@ -1730,9 +1735,41 @@ static void early_enable_iommu(struct amd_iommu *iommu)
>  static void early_enable_iommus(void)
>  {
>  	struct amd_iommu *iommu;
> +	bool is_pre_enabled=false;
>
> -	for_each_iommu(iommu)
> -		early_enable_iommu(iommu);
> +	for_each_iommu(iommu) {
> +		if ( translation_pre_enabled(iommu) ) {
> +			is_pre_enabled = true;
> +			break;
> +		}
> +	}

I wonder if you consider multi-iommus condition for your those series 
patches?

> +
> +	if ( !is_pre_enabled) {

Why not use translation_pre_enabled(iommu) to judge pre-enable directly?

> +		for_each_iommu(iommu)
> +			early_enable_iommu(iommu);
> +	} else {
> +		if (copy_dev_tables()) {
> +			pr_err("Failed to copy translation tables from previous kernel.\n");
> +			/*
> +			 * If failed to copy dev tables from old kernel, continue to proceed
> +			 * as it does in normal kernel.
> +			 */
> +			for_each_iommu(iommu) {
> +				clear_translation_pre_enabled(iommu);
> +				early_enable_iommu(iommu);
> +			}
> +		} else {
> +			pr_info("Copied translation tables from previous kernel.\n");
> +			for_each_iommu(iommu) {
> +				iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
> +		                iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
> +		                iommu_enable_command_buffer(iommu);
> +		                iommu_enable_event_buffer(iommu);
> +		                iommu_set_device_table(iommu);
> +		                iommu_flush_all_caches(iommu);
> +			}
> +		}
> +	}
>  }
>
>  static void enable_iommus_v2(void)
>

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

#1406692

[Multipart message — attachments visible in raw view] — view raw

Sorry, log of 'lspci -vvv' is not attatched correclty. Re-attach it here.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web