Re: [PATCH v2 02/37] iommu/amd: Introduce pci segment structure

2022-04-29 Thread Vasant Hegde via iommu
Joerg,

On 4/28/2022 3:24 PM, Joerg Roedel wrote:
> Hi Vasant,
> 
> On Mon, Apr 25, 2022 at 05:03:40PM +0530, Vasant Hegde wrote:
>> +/*
>> + * This structure contains information about one PCI segment in the system.
>> + */
>> +struct amd_iommu_pci_seg {
>> +struct list_head list;
> 
> The purpose of this list_head needs a comment.

Sure.

> 
>> +
>> +/* PCI segment number */
>> +u16 id;
>> +};
>> +/*
>> + * List with all PCI segments in the system. This list is not locked because
>> + * it is only written at driver initialization time
>> + */
>> +extern struct list_head amd_iommu_pci_seg_list;
> 
> So there will never be hotplug of a PCI segment? Say together with
> hotplugging a CPU?

As I understood currently we don't support hotplugging.

> 
>> +static void __init free_pci_segment(void)
> 
> This needs plural: free_pci_segments(), as it frees all segments.
> 

Sure. Will fix it in v3.

-Vasant

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 02/37] iommu/amd: Introduce pci segment structure

2022-04-28 Thread Joerg Roedel
Hi Vasant,

On Mon, Apr 25, 2022 at 05:03:40PM +0530, Vasant Hegde wrote:
> +/*
> + * This structure contains information about one PCI segment in the system.
> + */
> +struct amd_iommu_pci_seg {
> + struct list_head list;

The purpose of this list_head needs a comment.

> +
> + /* PCI segment number */
> + u16 id;
> +};
> +/*
> + * List with all PCI segments in the system. This list is not locked because
> + * it is only written at driver initialization time
> + */
> +extern struct list_head amd_iommu_pci_seg_list;

So there will never be hotplug of a PCI segment? Say together with
hotplugging a CPU?

> +static void __init free_pci_segment(void)

This needs plural: free_pci_segments(), as it frees all segments.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 02/37] iommu/amd: Introduce pci segment structure

2022-04-25 Thread Vasant Hegde via iommu
Newer AMD systems can support multiple PCI segments, where each segment
contains one or more IOMMU instances. However, an IOMMU instance can only
support a single PCI segment.

Current code assumes that system contains only one pci segment (segment 0)
and creates global data structures such as device table, rlookup table,
etc.

Introducing per PCI segment data structure, which contains segment
specific data structures. This will eventually replace the global
data structures.

Also update `amd_iommu->pci_seg` variable to point to PCI segment
structure instead of PCI segment ID.

Co-developed-by: Suravee Suthikulpanit 
Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Vasant Hegde 
---
 drivers/iommu/amd/amd_iommu_types.h | 23 ++-
 drivers/iommu/amd/init.c| 46 -
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h 
b/drivers/iommu/amd/amd_iommu_types.h
index 06235b7cb13d..62442d88978f 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -452,6 +452,11 @@ extern bool amd_iommu_irq_remap;
 /* kmem_cache to get tables with 128 byte alignement */
 extern struct kmem_cache *amd_iommu_irq_cache;
 
+/* Make iterating over all pci segment easier */
+#define for_each_pci_segment(pci_seg) \
+   list_for_each_entry((pci_seg), _iommu_pci_seg_list, list)
+#define for_each_pci_segment_safe(pci_seg, next) \
+   list_for_each_entry_safe((pci_seg), (next), _iommu_pci_seg_list, 
list)
 /*
  * Make iterating over all IOMMUs easier
  */
@@ -526,6 +531,16 @@ struct protection_domain {
unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
 };
 
+/*
+ * This structure contains information about one PCI segment in the system.
+ */
+struct amd_iommu_pci_seg {
+   struct list_head list;
+
+   /* PCI segment number */
+   u16 id;
+};
+
 /*
  * Structure where we save information about one hardware AMD IOMMU in the
  * system.
@@ -577,7 +592,7 @@ struct amd_iommu {
u16 cap_ptr;
 
/* pci domain of this IOMMU */
-   u16 pci_seg;
+   struct amd_iommu_pci_seg *pci_seg;
 
/* start of exclusion range of that IOMMU */
u64 exclusion_start;
@@ -705,6 +720,12 @@ extern struct list_head ioapic_map;
 extern struct list_head hpet_map;
 extern struct list_head acpihid_map;
 
+/*
+ * List with all PCI segments in the system. This list is not locked because
+ * it is only written at driver initialization time
+ */
+extern struct list_head amd_iommu_pci_seg_list;
+
 /*
  * List with all IOMMUs in the system. This list is not locked because it is
  * only written and read at driver initialization or suspend time
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index b4a798c7b347..e01eae9dcbc1 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -165,6 +165,7 @@ u16 amd_iommu_last_bdf; /* largest PCI 
device id we have
 LIST_HEAD(amd_iommu_unity_map);/* a list of required unity 
mappings
   we find in ACPI */
 
+LIST_HEAD(amd_iommu_pci_seg_list); /* list of all PCI segments */
 LIST_HEAD(amd_iommu_list); /* list of all AMD IOMMUs in the
   system */
 
@@ -1456,6 +1457,43 @@ static int __init init_iommu_from_acpi(struct amd_iommu 
*iommu,
return 0;
 }
 
+/* Allocate PCI segment data structure */
+static struct amd_iommu_pci_seg *__init alloc_pci_segment(u16 id)
+{
+   struct amd_iommu_pci_seg *pci_seg;
+
+   pci_seg = kzalloc(sizeof(struct amd_iommu_pci_seg), GFP_KERNEL);
+   if (pci_seg == NULL)
+   return NULL;
+
+   pci_seg->id = id;
+   list_add_tail(_seg->list, _iommu_pci_seg_list);
+
+   return pci_seg;
+}
+
+static struct amd_iommu_pci_seg *__init get_pci_segment(u16 id)
+{
+   struct amd_iommu_pci_seg *pci_seg;
+
+   for_each_pci_segment(pci_seg) {
+   if (pci_seg->id == id)
+   return pci_seg;
+   }
+
+   return alloc_pci_segment(id);
+}
+
+static void __init free_pci_segment(void)
+{
+   struct amd_iommu_pci_seg *pci_seg, *next;
+
+   for_each_pci_segment_safe(pci_seg, next) {
+   list_del(_seg->list);
+   kfree(pci_seg);
+   }
+}
+
 static void __init free_iommu_one(struct amd_iommu *iommu)
 {
free_cwwb_sem(iommu);
@@ -1542,8 +1580,14 @@ static void amd_iommu_ats_write_check_workaround(struct 
amd_iommu *iommu)
  */
 static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header 
*h)
 {
+   struct amd_iommu_pci_seg *pci_seg;
int ret;
 
+   pci_seg = get_pci_segment(h->pci_seg);
+   if (pci_seg == NULL)
+   return -ENOMEM;
+   iommu->pci_seg = pci_seg;
+
raw_spin_lock_init(>lock);
iommu->cmd_sem_val = 0;
 
@@ -1564,7 +1608,6 @@ static int __init