diff mbox series

[v6,2/8] AMD/IOMMU: make phantom functions share interrupt remapping tables

Message ID 06a35251-013f-d215-d70c-70a4c98ac86e@suse.com (mailing list archive)
State Superseded
Headers show
Series AMD IOMMU: further improvements | expand

Commit Message

Jan Beulich Sept. 19, 2019, 1:22 p.m. UTC 
Rather than duplicating entries in amd_iommu_msi_msg_update_ire(), share
the tables. This mainly requires some care while freeing them, to avoid
freeing memory blocks twice.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v5: New.

---
 xen/drivers/passthrough/amd/iommu_init.c      |   43 +++++++++++++++---------
 xen/drivers/passthrough/amd/iommu_intr.c      |   45 +++++++++++++-------------
 xen/drivers/passthrough/amd/pci_amd_iommu.c   |    2 -
 xen/include/asm-x86/amd-iommu.h               |    2 -
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |    2 -
 5 files changed, 53 insertions(+), 41 deletions(-)

Comments

Paul Durrant Sept. 23, 2019, 3:44 p.m. UTC | #1 
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan Beulich
> Sent: 19 September 2019 14:22
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Subject: [Xen-devel] [PATCH v6 2/8] AMD/IOMMU: make phantom functions share interrupt remapping tables
> 
> Rather than duplicating entries in amd_iommu_msi_msg_update_ire(), share
> the tables. This mainly requires some care while freeing them, to avoid
> freeing memory blocks twice.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> v5: New.
> 
> ---
>  xen/drivers/passthrough/amd/iommu_init.c      |   43 +++++++++++++++---------
>  xen/drivers/passthrough/amd/iommu_intr.c      |   45 +++++++++++++-------------
>  xen/drivers/passthrough/amd/pci_amd_iommu.c   |    2 -
>  xen/include/asm-x86/amd-iommu.h               |    2 -
>  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |    2 -
>  5 files changed, 53 insertions(+), 41 deletions(-)
> 
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1111,7 +1111,7 @@ static void __init amd_iommu_init_cleanu
>          amd_iommu_free_intremap_table(list_first_entry(&amd_iommu_head,
>                                                         struct amd_iommu,
>                                                         list),
> -                                      NULL);
> +                                      NULL, 0);
> 
>      /* free amd iommu list */
>      list_for_each_entry_safe ( iommu, next, &amd_iommu_head, list )
> @@ -1176,7 +1176,7 @@ int iterate_ivrs_mappings(int (*handler)
>  }
> 
>  int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *,
> -                                        struct ivrs_mappings *))
> +                                        struct ivrs_mappings *, uint16_t bdf))
>  {
>      u16 seg = 0;
>      int rc = 0;
> @@ -1193,7 +1193,7 @@ int iterate_ivrs_entries(int (*handler)(
>              const struct amd_iommu *iommu = map[bdf].iommu;
> 
>              if ( iommu && map[bdf].dte_requestor_id == bdf )
> -                rc = handler(iommu, &map[bdf]);
> +                rc = handler(iommu, &map[bdf], bdf);
>          }
>      } while ( !rc && ++seg );
> 
> @@ -1286,20 +1286,29 @@ static int __init amd_iommu_setup_device
> 
>              if ( pdev )
>              {
> -                unsigned int req_id = bdf;
> -
> -                do {
> -                    ivrs_mappings[req_id].intremap_table =
> -                        amd_iommu_alloc_intremap_table(
> -                            ivrs_mappings[bdf].iommu,
> -                            &ivrs_mappings[req_id].intremap_inuse);
> -                    if ( !ivrs_mappings[req_id].intremap_table )
> -                        return -ENOMEM;
> -
> -                    if ( !pdev->phantom_stride )
> -                        break;
> -                    req_id += pdev->phantom_stride;
> -                } while ( PCI_SLOT(req_id) == pdev->sbdf.dev );
> +                ivrs_mappings[bdf].intremap_table =
> +                    amd_iommu_alloc_intremap_table(
> +                        ivrs_mappings[bdf].iommu,
> +                        &ivrs_mappings[bdf].intremap_inuse);
> +                if ( !ivrs_mappings[bdf].intremap_table )
> +                    return -ENOMEM;
> +
> +                if ( pdev->phantom_stride )
> +                {
> +                    unsigned int req_id = bdf;
> +
> +                    for ( ; ; )
> +                    {
> +                        req_id += pdev->phantom_stride;
> +                        if ( PCI_SLOT(req_id) != pdev->sbdf.dev )
> +                            break;
> +
> +                        ivrs_mappings[req_id].intremap_table =
> +                            ivrs_mappings[bdf].intremap_table;
> +                        ivrs_mappings[req_id].intremap_inuse =
> +                            ivrs_mappings[bdf].intremap_inuse;
> +                    }
> +                }
>              }
> 
>              amd_iommu_set_intremap_table(
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -711,33 +711,20 @@ int amd_iommu_msi_msg_update_ire(
> 
>      if ( msi_desc->remap_index >= 0 && !msg )
>      {
> -        do {
> -            update_intremap_entry_from_msi_msg(iommu, bdf, nr,
> -                                               &msi_desc->remap_index,
> -                                               NULL, NULL);
> -            if ( !pdev || !pdev->phantom_stride )
> -                break;
> -            bdf += pdev->phantom_stride;
> -        } while ( PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
> +        update_intremap_entry_from_msi_msg(iommu, bdf, nr,
> +                                           &msi_desc->remap_index,
> +                                           NULL, NULL);
> 
>          for ( i = 0; i < nr; ++i )
>              msi_desc[i].remap_index = -1;
> -        if ( pdev )
> -            bdf = PCI_BDF2(pdev->bus, pdev->devfn);
>      }
> 
>      if ( !msg )
>          return 0;
> 
> -    do {
> -        rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr,
> -                                                &msi_desc->remap_index,
> -                                                msg, &data);
> -        if ( rc || !pdev || !pdev->phantom_stride )
> -            break;
> -        bdf += pdev->phantom_stride;
> -    } while ( PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
> -
> +    rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr,
> +                                            &msi_desc->remap_index,
> +                                            msg, &data);
>      if ( !rc )
>      {
>          for ( i = 1; i < nr; ++i )
> @@ -790,12 +777,27 @@ void amd_iommu_read_msi_from_ire(
>  }
> 
>  int amd_iommu_free_intremap_table(
> -    const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping)
> +    const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping,
> +    uint16_t bdf)
>  {
>      void **tblp;
> 
>      if ( ivrs_mapping )
>      {
> +        unsigned int i;
> +
> +        /*
> +         * PCI device phantom functions use the same tables as their "base"
> +         * function: Look ahead to zap the pointers.
> +         */
> +        for ( i = 1; PCI_FUNC(bdf + i) && bdf + i < ivrs_bdf_entries; ++i )
> +            if ( ivrs_mapping[i].intremap_table ==
> +                 ivrs_mapping->intremap_table )
> +            {
> +                ivrs_mapping[i].intremap_table = NULL;
> +                ivrs_mapping[i].intremap_inuse = NULL;
> +            }
> +
>          XFREE(ivrs_mapping->intremap_inuse);
>          tblp = &ivrs_mapping->intremap_table;
>      }
> @@ -934,7 +936,8 @@ static void dump_intremap_table(const st
>  }
> 
>  static int dump_intremap_mapping(const struct amd_iommu *iommu,
> -                                 struct ivrs_mappings *ivrs_mapping)
> +                                 struct ivrs_mappings *ivrs_mapping,
> +                                 uint16_t unused)
>  {
>      unsigned long flags;
> 
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -519,7 +519,7 @@ static int amd_iommu_remove_device(u8 de
>      if ( amd_iommu_perdev_intremap &&
>           ivrs_mappings[bdf].dte_requestor_id == bdf &&
>           ivrs_mappings[bdf].intremap_table )
> -        amd_iommu_free_intremap_table(iommu, &ivrs_mappings[bdf]);
> +        amd_iommu_free_intremap_table(iommu, &ivrs_mappings[bdf], bdf);
> 
>      return 0;
>  }
> --- a/xen/include/asm-x86/amd-iommu.h
> +++ b/xen/include/asm-x86/amd-iommu.h
> @@ -131,7 +131,7 @@ extern u8 ivhd_type;
>  struct ivrs_mappings *get_ivrs_mappings(u16 seg);
>  int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
>  int iterate_ivrs_entries(int (*)(const struct amd_iommu *,
> -                                 struct ivrs_mappings *));
> +                                 struct ivrs_mappings *, uint16_t));
> 
>  /* iommu tables in guest space */
>  struct mmio_reg {
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -101,7 +101,7 @@ int amd_iommu_setup_ioapic_remapping(voi
>  void *amd_iommu_alloc_intremap_table(
>      const struct amd_iommu *, unsigned long **);
>  int amd_iommu_free_intremap_table(
> -    const struct amd_iommu *, struct ivrs_mappings *);
> +    const struct amd_iommu *, struct ivrs_mappings *, uint16_t);
>  void amd_iommu_ioapic_update_ire(
>      unsigned int apic, unsigned int reg, unsigned int value);
>  unsigned int amd_iommu_read_ioapic_from_ire(
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
diff mbox series

Patch

--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1111,7 +1111,7 @@  static void __init amd_iommu_init_cleanu
         amd_iommu_free_intremap_table(list_first_entry(&amd_iommu_head,
                                                        struct amd_iommu,
                                                        list),
-                                      NULL);
+                                      NULL, 0);
 
     /* free amd iommu list */
     list_for_each_entry_safe ( iommu, next, &amd_iommu_head, list )
@@ -1176,7 +1176,7 @@  int iterate_ivrs_mappings(int (*handler)
 }
 
 int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *,
-                                        struct ivrs_mappings *))
+                                        struct ivrs_mappings *, uint16_t bdf))
 {
     u16 seg = 0;
     int rc = 0;
@@ -1193,7 +1193,7 @@  int iterate_ivrs_entries(int (*handler)(
             const struct amd_iommu *iommu = map[bdf].iommu;
 
             if ( iommu && map[bdf].dte_requestor_id == bdf )
-                rc = handler(iommu, &map[bdf]);
+                rc = handler(iommu, &map[bdf], bdf);
         }
     } while ( !rc && ++seg );
 
@@ -1286,20 +1286,29 @@  static int __init amd_iommu_setup_device
 
             if ( pdev )
             {
-                unsigned int req_id = bdf;
-
-                do {
-                    ivrs_mappings[req_id].intremap_table =
-                        amd_iommu_alloc_intremap_table(
-                            ivrs_mappings[bdf].iommu,
-                            &ivrs_mappings[req_id].intremap_inuse);
-                    if ( !ivrs_mappings[req_id].intremap_table )
-                        return -ENOMEM;
-
-                    if ( !pdev->phantom_stride )
-                        break;
-                    req_id += pdev->phantom_stride;
-                } while ( PCI_SLOT(req_id) == pdev->sbdf.dev );
+                ivrs_mappings[bdf].intremap_table =
+                    amd_iommu_alloc_intremap_table(
+                        ivrs_mappings[bdf].iommu,
+                        &ivrs_mappings[bdf].intremap_inuse);
+                if ( !ivrs_mappings[bdf].intremap_table )
+                    return -ENOMEM;
+
+                if ( pdev->phantom_stride )
+                {
+                    unsigned int req_id = bdf;
+
+                    for ( ; ; )
+                    {
+                        req_id += pdev->phantom_stride;
+                        if ( PCI_SLOT(req_id) != pdev->sbdf.dev )
+                            break;
+
+                        ivrs_mappings[req_id].intremap_table =
+                            ivrs_mappings[bdf].intremap_table;
+                        ivrs_mappings[req_id].intremap_inuse =
+                            ivrs_mappings[bdf].intremap_inuse;
+                    }
+                }
             }
 
             amd_iommu_set_intremap_table(
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -711,33 +711,20 @@  int amd_iommu_msi_msg_update_ire(
 
     if ( msi_desc->remap_index >= 0 && !msg )
     {
-        do {
-            update_intremap_entry_from_msi_msg(iommu, bdf, nr,
-                                               &msi_desc->remap_index,
-                                               NULL, NULL);
-            if ( !pdev || !pdev->phantom_stride )
-                break;
-            bdf += pdev->phantom_stride;
-        } while ( PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
+        update_intremap_entry_from_msi_msg(iommu, bdf, nr,
+                                           &msi_desc->remap_index,
+                                           NULL, NULL);
 
         for ( i = 0; i < nr; ++i )
             msi_desc[i].remap_index = -1;
-        if ( pdev )
-            bdf = PCI_BDF2(pdev->bus, pdev->devfn);
     }
 
     if ( !msg )
         return 0;
 
-    do {
-        rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr,
-                                                &msi_desc->remap_index,
-                                                msg, &data);
-        if ( rc || !pdev || !pdev->phantom_stride )
-            break;
-        bdf += pdev->phantom_stride;
-    } while ( PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
-
+    rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr,
+                                            &msi_desc->remap_index,
+                                            msg, &data);
     if ( !rc )
     {
         for ( i = 1; i < nr; ++i )
@@ -790,12 +777,27 @@  void amd_iommu_read_msi_from_ire(
 }
 
 int amd_iommu_free_intremap_table(
-    const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping)
+    const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping,
+    uint16_t bdf)
 {
     void **tblp;
 
     if ( ivrs_mapping )
     {
+        unsigned int i;
+
+        /*
+         * PCI device phantom functions use the same tables as their "base"
+         * function: Look ahead to zap the pointers.
+         */
+        for ( i = 1; PCI_FUNC(bdf + i) && bdf + i < ivrs_bdf_entries; ++i )
+            if ( ivrs_mapping[i].intremap_table ==
+                 ivrs_mapping->intremap_table )
+            {
+                ivrs_mapping[i].intremap_table = NULL;
+                ivrs_mapping[i].intremap_inuse = NULL;
+            }
+
         XFREE(ivrs_mapping->intremap_inuse);
         tblp = &ivrs_mapping->intremap_table;
     }
@@ -934,7 +936,8 @@  static void dump_intremap_table(const st
 }
 
 static int dump_intremap_mapping(const struct amd_iommu *iommu,
-                                 struct ivrs_mappings *ivrs_mapping)
+                                 struct ivrs_mappings *ivrs_mapping,
+                                 uint16_t unused)
 {
     unsigned long flags;
 
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -519,7 +519,7 @@  static int amd_iommu_remove_device(u8 de
     if ( amd_iommu_perdev_intremap &&
          ivrs_mappings[bdf].dte_requestor_id == bdf &&
          ivrs_mappings[bdf].intremap_table )
-        amd_iommu_free_intremap_table(iommu, &ivrs_mappings[bdf]);
+        amd_iommu_free_intremap_table(iommu, &ivrs_mappings[bdf], bdf);
 
     return 0;
 }
--- a/xen/include/asm-x86/amd-iommu.h
+++ b/xen/include/asm-x86/amd-iommu.h
@@ -131,7 +131,7 @@  extern u8 ivhd_type;
 struct ivrs_mappings *get_ivrs_mappings(u16 seg);
 int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
 int iterate_ivrs_entries(int (*)(const struct amd_iommu *,
-                                 struct ivrs_mappings *));
+                                 struct ivrs_mappings *, uint16_t));
 
 /* iommu tables in guest space */
 struct mmio_reg {
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -101,7 +101,7 @@  int amd_iommu_setup_ioapic_remapping(voi
 void *amd_iommu_alloc_intremap_table(
     const struct amd_iommu *, unsigned long **);
 int amd_iommu_free_intremap_table(
-    const struct amd_iommu *, struct ivrs_mappings *);
+    const struct amd_iommu *, struct ivrs_mappings *, uint16_t);
 void amd_iommu_ioapic_update_ire(
     unsigned int apic, unsigned int reg, unsigned int value);
 unsigned int amd_iommu_read_ioapic_from_ire(