[PATCH v3 qemu 00/11] acpi: NUMA nodes for CXL HB as GP + complex NUMA test

[PATCH v3 qemu 00/11] acpi: NUMA nodes for CXL HB as GP + complex NUMA test

[Jonathan Cameron via]

v3: Thanks to Richard for help debugging BE issue and to Igor for
    finding a bunch of other thing to improve via the context in
    the fix patch.
    
- Fix the big endian host/little endian guest issue in the HID being
  written to the Generic Port Affinity Structure ACPI Device Handle.
- Fix a bug in the ordering of bus vs devfn in the BDF field which is
  reversed in the ACPI table wrt to QEMU's internal handling. Note the
  fix is minimal and refactored later in the series.
- Move original GI code to hw/acpi/aml-build.c and hw/acpi/pc.c as
  no need for a separate file and this keeps the SRAT entry building
  all in one place.
- Use properties for the pci bus number and the ACPI UID to avoid
  using pci internal implementation details in hw/acpi.
- Drop the GenericNode base object as much less code is unified with
  the new approach to the aml building and that approach did not bring
  sufficient advantages to be worthwhile after other refactors.
  A little more duplication occurs in v3 but the code is easier to read.

ACPI 6.5 introduced Generic Port Affinity Structures to close a system
description gap that was a problem for CXL memory systems.
It defines an new SRAT Affinity structure (and hence allows creation of an
ACPI Proximity Node which can only be defined via an SRAT structure)
for the boundary between a discoverable fabric and a non discoverable
system interconnects etc.

The HMAT data on latency and bandwidth is combined with discoverable
information from the CXL bus (link speeds, lane counts) and CXL devices
(switch port to port characteristics and USP to memory, via CDAT tables
read from the device).  QEMU has supported the rest of the elements
of this chain for a while but now the kernel has caught up and we need
the missing element of Generic Ports (this code has been used extensively
in testing and debugging that kernel support, some resulting fixes
currently under review).

Generic Port Affinity Structures are very similar to the recently
added Generic Initiator Affinity Structures (GI) so this series
factors out and reuses much of that infrastructure for reuse
There are subtle differences (beyond the obvious structure ID change).

- The ACPI spec example (and linux kernel support) has a Generic
  Port not as associated with the CXL root port, but rather with
  the CXL Host bridge. As a result, an ACPI handle is used (rather
  than the PCI SBDF option for GIs). In QEMU the easiest way
  to get to this is to target the root bridge PCI Bus, and
  conveniently the root bridge bus number is used for the UID allowing
  us to construct an appropriate entry.

A key addition of this series is a complex NUMA topology example that
stretches the QEMU emulation code for GI, GP and nodes with just
CPUS, just memory, just hot pluggable memory, mixture of memory and CPUs.

A similar test showed up a few NUMA related bugs with fixes applied for
9.0 (note that one of these needs linux booted to identify that it
rejects the HMAT table and this test is a regression test for the
table generation only).

https://lore.kernel.org/qemu-devel/2eb6672cfdaea7dacd8e9bb0523887f13b9f85ce.1710282274.git.mst@redhat.com/
https://lore.kernel.org/qemu-devel/74e2845c5f95b0c139c79233ddb65bb17f2dd679.1710282274.git.mst@redhat.com/


Jonathan Cameron (11):
  hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle.
  hw/acpi/GI: Fix trivial parameter alignment issue.
  hw/acpi: Move AML building code for Generic Initiators to aml_build.c
  hw/acpi: Rename build_all_acpi_generic_initiators() to
    build_acpi_generic_initiator()
  hw/pci: Add a bus property to pci_props and use for acpi/gi
  acpi/pci: Move Generic Initiator object handling into acpi/pci.*
  hw/pci-bridge: Add acpi_uid property to CXL PXB
  hw/acpi: Generic Port Affinity Structure support
  bios-tables-test: Allow for new acpihmat-generic-x test data.
  bios-tables-test: Add complex SRAT / HMAT test for GI GP
  bios-tables-test: Add data for complex numa test (GI, GP etc)

 qapi/qom.json                               |  34 +++
 include/hw/acpi/acpi_generic_initiator.h    |  30 +--
 include/hw/acpi/aml-build.h                 |   8 +
 include/hw/acpi/pci.h                       |   7 +
 include/hw/pci/pci_bridge.h                 |   1 +
 hw/acpi/acpi_generic_initiator.c            | 132 +++++++++---
 hw/acpi/aml-build.c                         |  84 ++++++++
 hw/acpi/pci.c                               | 226 ++++++++++++++++++++
 hw/arm/virt-acpi-build.c                    |   3 +-
 hw/i386/acpi-build.c                        |   3 +-
 hw/pci-bridge/pci_expander_bridge.c         |  18 +-
 hw/pci/pci.c                                |  14 ++
 tests/qtest/bios-tables-test.c              |  96 +++++++++
 hw/acpi/meson.build                         |   1 -
 tests/data/acpi/q35/APIC.acpihmat-generic-x | Bin 0 -> 136 bytes
 tests/data/acpi/q35/CEDT.acpihmat-generic-x | Bin 0 -> 68 bytes
 tests/data/acpi/q35/DSDT.acpihmat-generic-x | Bin 0 -> 10849 bytes
 tests/data/acpi/q35/HMAT.acpihmat-generic-x | Bin 0 -> 360 bytes
 tests/data/acpi/q35/SRAT.acpihmat-generic-x | Bin 0 -> 520 bytes
 19 files changed, 597 insertions(+), 60 deletions(-)
 create mode 100644 tests/data/acpi/q35/APIC.acpihmat-generic-x
 create mode 100644 tests/data/acpi/q35/CEDT.acpihmat-generic-x
 create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-generic-x
 create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-generic-x
 create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-generic-x

-- 
2.43.0

[PATCH v3 01/11] hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle.

[Jonathan Cameron via]

The ordering in ACPI specification [1] has bus number in the lowest byte.
As ACPI tables are little endian this is the reverse of the ordering
used by PCI_BUILD_BDF().  As a minimal fix split the QEMU BDF up
into bus and devfn and write them as single bytes in the correct
order.

[1] ACPI Spec 6.3, Table 5.80

Fixes: 0a5b5acdf2d8 ("hw/acpi: Implement the SRAT GI affinity structure")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: New patch.  Note this code will go away, so this is intended for
backporting purposes
---
 hw/acpi/acpi_generic_initiator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
index 17b9a052f5..3d2b567999 100644
--- a/hw/acpi/acpi_generic_initiator.c
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -92,7 +92,8 @@ build_srat_generic_pci_initiator_affinity(GArray *table_data, int node,
 
     /* Device Handle - PCI */
     build_append_int_noprefix(table_data, handle->segment, 2);
-    build_append_int_noprefix(table_data, handle->bdf, 2);
+    build_append_int_noprefix(table_data, PCI_BUS_NUM(handle->bdf), 1);
+    build_append_int_noprefix(table_data, PCI_BDF_TO_DEVFN(handle->bdf), 1);
     for (index = 0; index < 12; index++) {
         build_append_int_noprefix(table_data, 0, 1);
     }
-- 
2.43.0

Re: [PATCH v3 01/11] hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle.

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:09 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> The ordering in ACPI specification [1] has bus number in the lowest byte.
> As ACPI tables are little endian this is the reverse of the ordering
> used by PCI_BUILD_BDF().  As a minimal fix split the QEMU BDF up
> into bus and devfn and write them as single bytes in the correct
> order.
> 
> [1] ACPI Spec 6.3, Table 5.80
> 
> Fixes: 0a5b5acdf2d8 ("hw/acpi: Implement the SRAT GI affinity structure")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> ---
> v3: New patch.  Note this code will go away, so this is intended for
> backporting purposes
> ---
>  hw/acpi/acpi_generic_initiator.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> index 17b9a052f5..3d2b567999 100644
> --- a/hw/acpi/acpi_generic_initiator.c
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -92,7 +92,8 @@ build_srat_generic_pci_initiator_affinity(GArray *table_data, int node,
>  
>      /* Device Handle - PCI */
>      build_append_int_noprefix(table_data, handle->segment, 2);
> -    build_append_int_noprefix(table_data, handle->bdf, 2);
> +    build_append_int_noprefix(table_data, PCI_BUS_NUM(handle->bdf), 1);
> +    build_append_int_noprefix(table_data, PCI_BDF_TO_DEVFN(handle->bdf), 1);
>      for (index = 0; index < 12; index++) {
>          build_append_int_noprefix(table_data, 0, 1);
>      }

[PATCH v3 02/11] hw/acpi/GI: Fix trivial parameter alignment issue.

[Jonathan Cameron via]

Before making additional modification, tidy up this misleading indentation.

Reviewed-by: Ankit Agrawal <ankita@nvidia.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: Unchanged
---
 hw/acpi/acpi_generic_initiator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
index 3d2b567999..4a02c19468 100644
--- a/hw/acpi/acpi_generic_initiator.c
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -133,7 +133,7 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
 
     dev_handle.segment = 0;
     dev_handle.bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
-                                               pci_dev->devfn);
+                                   pci_dev->devfn);
 
     build_srat_generic_pci_initiator_affinity(table_data,
                                               gi->node, &dev_handle);
-- 
2.43.0

Re: [PATCH v3 02/11] hw/acpi/GI: Fix trivial parameter alignment issue.

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:10 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Before making additional modification, tidy up this misleading indentation.
> 
> Reviewed-by: Ankit Agrawal <ankita@nvidia.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v3: Unchanged
> ---
>  hw/acpi/acpi_generic_initiator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> index 3d2b567999..4a02c19468 100644
> --- a/hw/acpi/acpi_generic_initiator.c
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -133,7 +133,7 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
>  
>      dev_handle.segment = 0;
>      dev_handle.bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
> -                                               pci_dev->devfn);
> +                                   pci_dev->devfn);
>  
>      build_srat_generic_pci_initiator_affinity(table_data,
>                                                gi->node, &dev_handle);

[PATCH v3 03/11] hw/acpi: Move AML building code for Generic Initiators to aml_build.c

[Jonathan Cameron via]

Rather than attempting to create a generic function with mess of the two
different device handle types, use a PCI handle specific variant.  If the
ACPI handle form is needed then that can be introduced alongside this
with little duplicated code.

Drop the PCIDeviceHandle in favor of just passing the bus, devfn
and segment directly.  devfn kept as a single byte because ARI means
that in cases this is just an 8 bit function number.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20240618142333.102be976@imammedo.users.ipa.redhat.com/
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: New patch based on Igor's comments on the endian fix.
---
 include/hw/acpi/acpi_generic_initiator.h | 23 -------------
 include/hw/acpi/aml-build.h              |  4 +++
 hw/acpi/acpi_generic_initiator.c         | 39 ++-------------------
 hw/acpi/aml-build.c                      | 44 ++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 59 deletions(-)

diff --git a/include/hw/acpi/acpi_generic_initiator.h b/include/hw/acpi/acpi_generic_initiator.h
index a304bad73e..7b98676713 100644
--- a/include/hw/acpi/acpi_generic_initiator.h
+++ b/include/hw/acpi/acpi_generic_initiator.h
@@ -19,29 +19,6 @@ typedef struct AcpiGenericInitiator {
     uint16_t node;
 } AcpiGenericInitiator;
 
-/*
- * ACPI 6.3:
- * Table 5-81 Flags – Generic Initiator Affinity Structure
- */
-typedef enum {
-    /*
-     * If clear, the OSPM ignores the contents of the Generic
-     * Initiator/Port Affinity Structure. This allows system firmware
-     * to populate the SRAT with a static number of structures, but only
-     * enable them as necessary.
-     */
-    GEN_AFFINITY_ENABLED = (1 << 0),
-} GenericAffinityFlags;
-
-/*
- * ACPI 6.3:
- * Table 5-80 Device Handle - PCI
- */
-typedef struct PCIDeviceHandle {
-    uint16_t segment;
-    uint16_t bdf;
-} PCIDeviceHandle;
-
 void build_srat_generic_pci_initiator(GArray *table_data);
 
 #endif
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index a3784155cb..9ba3a21c13 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -486,6 +486,10 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set, uint32_t io_offset,
 void build_srat_memory(GArray *table_data, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_srat_pci_generic_initiator(GArray *table_date, int node,
+                                      uint16_t segment, uint8_t bus,
+                                      uint8_t devfn);
+
 void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
                 const char *oem_id, const char *oem_table_id);
 
diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
index 4a02c19468..7665b16107 100644
--- a/hw/acpi/acpi_generic_initiator.c
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -74,40 +74,11 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
         acpi_generic_initiator_set_node, NULL, NULL);
 }
 
-/*
- * ACPI 6.3:
- * Table 5-78 Generic Initiator Affinity Structure
- */
-static void
-build_srat_generic_pci_initiator_affinity(GArray *table_data, int node,
-                                          PCIDeviceHandle *handle)
-{
-    uint8_t index;
-
-    build_append_int_noprefix(table_data, 5, 1);  /* Type */
-    build_append_int_noprefix(table_data, 32, 1); /* Length */
-    build_append_int_noprefix(table_data, 0, 1);  /* Reserved */
-    build_append_int_noprefix(table_data, 1, 1);  /* Device Handle Type: PCI */
-    build_append_int_noprefix(table_data, node, 4);  /* Proximity Domain */
-
-    /* Device Handle - PCI */
-    build_append_int_noprefix(table_data, handle->segment, 2);
-    build_append_int_noprefix(table_data, PCI_BUS_NUM(handle->bdf), 1);
-    build_append_int_noprefix(table_data, PCI_BDF_TO_DEVFN(handle->bdf), 1);
-    for (index = 0; index < 12; index++) {
-        build_append_int_noprefix(table_data, 0, 1);
-    }
-
-    build_append_int_noprefix(table_data, GEN_AFFINITY_ENABLED, 4); /* Flags */
-    build_append_int_noprefix(table_data, 0, 4);     /* Reserved */
-}
-
 static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
     AcpiGenericInitiator *gi;
     GArray *table_data = opaque;
-    PCIDeviceHandle dev_handle;
     PCIDevice *pci_dev;
     Object *o;
 
@@ -130,13 +101,9 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
     }
 
     pci_dev = PCI_DEVICE(o);
-
-    dev_handle.segment = 0;
-    dev_handle.bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
-                                   pci_dev->devfn);
-
-    build_srat_generic_pci_initiator_affinity(table_data,
-                                              gi->node, &dev_handle);
+    build_srat_pci_generic_initiator(table_data, gi->node, 0,
+                                     pci_bus_num(pci_get_bus(pci_dev)),
+                                     pci_dev->devfn);
 
     return 0;
 }
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 6d4517cfbe..968b654e58 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1938,6 +1938,50 @@ void build_srat_memory(GArray *table_data, uint64_t base,
     build_append_int_noprefix(table_data, 0, 8); /* Reserved */
 }
 
+/*
+ * ACPI Spec Revision 6.3
+ * Table 5-80 Device Handle - PCI
+ */
+static void build_append_srat_pci_device_handle(GArray *table_data,
+                                                uint16_t segment,
+                                                uint8_t bus, uint8_t devfn)
+{
+    /* PCI segment number */
+    build_append_int_noprefix(table_data, segment, 2);
+    /* PCI Bus Device Function */
+    build_append_int_noprefix(table_data, bus, 1);
+    build_append_int_noprefix(table_data, devfn, 1);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 12);
+}
+
+/*
+ * ACPI spec, Revision 6.3
+ * 5.2.16.6 Generic Initiator Affinity Structure
+ *    With PCI Device Handle.
+ */
+void build_srat_pci_generic_initiator(GArray *table_data, int node,
+                                      uint16_t segment, uint8_t bus,
+                                      uint8_t devfn)
+{
+    /* Type */
+    build_append_int_noprefix(table_data, 5, 1);
+    /* Length */
+    build_append_int_noprefix(table_data, 32, 1);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 1);
+    /* Device Handle Type: PCI */
+    build_append_int_noprefix(table_data, 1, 1);
+    /* Proximity Domain */
+    build_append_int_noprefix(table_data, node, 4);
+    /* Device Handle */
+    build_append_srat_pci_device_handle(table_data, segment, bus, devfn);
+    /* Flags - GI Enabled */
+    build_append_int_noprefix(table_data, 1, 4);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 4);
+}
+
 /*
  * ACPI spec 5.2.17 System Locality Distance Information Table
  * (Revision 2.0 or later)
-- 
2.43.0

Re: [PATCH v3 03/11] hw/acpi: Move AML building code for Generic Initiators to aml_build.c

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:11 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Rather than attempting to create a generic function with mess of the two
> different device handle types, use a PCI handle specific variant.  If the
> ACPI handle form is needed then that can be introduced alongside this
> with little duplicated code.
> 
> Drop the PCIDeviceHandle in favor of just passing the bus, devfn
> and segment directly.  devfn kept as a single byte because ARI means
> that in cases this is just an 8 bit function number.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Link: https://lore.kernel.org/qemu-devel/20240618142333.102be976@imammedo.users.ipa.redhat.com/
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

with typo fixed

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> ---
> v3: New patch based on Igor's comments on the endian fix.
> ---
>  include/hw/acpi/acpi_generic_initiator.h | 23 -------------
>  include/hw/acpi/aml-build.h              |  4 +++
>  hw/acpi/acpi_generic_initiator.c         | 39 ++-------------------
>  hw/acpi/aml-build.c                      | 44 ++++++++++++++++++++++++
>  4 files changed, 51 insertions(+), 59 deletions(-)
> 
> diff --git a/include/hw/acpi/acpi_generic_initiator.h b/include/hw/acpi/acpi_generic_initiator.h
> index a304bad73e..7b98676713 100644
> --- a/include/hw/acpi/acpi_generic_initiator.h
> +++ b/include/hw/acpi/acpi_generic_initiator.h
> @@ -19,29 +19,6 @@ typedef struct AcpiGenericInitiator {
>      uint16_t node;
>  } AcpiGenericInitiator;
>  
> -/*
> - * ACPI 6.3:
> - * Table 5-81 Flags – Generic Initiator Affinity Structure
> - */
> -typedef enum {
> -    /*
> -     * If clear, the OSPM ignores the contents of the Generic
> -     * Initiator/Port Affinity Structure. This allows system firmware
> -     * to populate the SRAT with a static number of structures, but only
> -     * enable them as necessary.
> -     */
> -    GEN_AFFINITY_ENABLED = (1 << 0),
> -} GenericAffinityFlags;
> -
> -/*
> - * ACPI 6.3:
> - * Table 5-80 Device Handle - PCI
> - */
> -typedef struct PCIDeviceHandle {
> -    uint16_t segment;
> -    uint16_t bdf;
> -} PCIDeviceHandle;
> -
>  void build_srat_generic_pci_initiator(GArray *table_data);
>  
>  #endif
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index a3784155cb..9ba3a21c13 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -486,6 +486,10 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set, uint32_t io_offset,
>  void build_srat_memory(GArray *table_data, uint64_t base,
>                         uint64_t len, int node, MemoryAffinityFlags flags);
>  
> +void build_srat_pci_generic_initiator(GArray * table_date, int node,

s/table_date/table_data/

> +                                      uint16_t segment, uint8_t bus,
> +                                      uint8_t devfn);
> +
>  void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>                  const char *oem_id, const char *oem_table_id);
>  
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> index 4a02c19468..7665b16107 100644
> --- a/hw/acpi/acpi_generic_initiator.c
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -74,40 +74,11 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
>          acpi_generic_initiator_set_node, NULL, NULL);
>  }
>  
> -/*
> - * ACPI 6.3:
> - * Table 5-78 Generic Initiator Affinity Structure
> - */
> -static void
> -build_srat_generic_pci_initiator_affinity(GArray *table_data, int node,
> -                                          PCIDeviceHandle *handle)
> -{
> -    uint8_t index;
> -
> -    build_append_int_noprefix(table_data, 5, 1);  /* Type */
> -    build_append_int_noprefix(table_data, 32, 1); /* Length */
> -    build_append_int_noprefix(table_data, 0, 1);  /* Reserved */
> -    build_append_int_noprefix(table_data, 1, 1);  /* Device Handle Type: PCI */
> -    build_append_int_noprefix(table_data, node, 4);  /* Proximity Domain */
> -
> -    /* Device Handle - PCI */
> -    build_append_int_noprefix(table_data, handle->segment, 2);
> -    build_append_int_noprefix(table_data, PCI_BUS_NUM(handle->bdf), 1);
> -    build_append_int_noprefix(table_data, PCI_BDF_TO_DEVFN(handle->bdf), 1);
> -    for (index = 0; index < 12; index++) {
> -        build_append_int_noprefix(table_data, 0, 1);
> -    }
> -
> -    build_append_int_noprefix(table_data, GEN_AFFINITY_ENABLED, 4); /* Flags */
> -    build_append_int_noprefix(table_data, 0, 4);     /* Reserved */
> -}
> -
>  static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
>      AcpiGenericInitiator *gi;
>      GArray *table_data = opaque;
> -    PCIDeviceHandle dev_handle;
>      PCIDevice *pci_dev;
>      Object *o;
>  
> @@ -130,13 +101,9 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
>      }
>  
>      pci_dev = PCI_DEVICE(o);
> -
> -    dev_handle.segment = 0;
> -    dev_handle.bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
> -                                   pci_dev->devfn);
> -
> -    build_srat_generic_pci_initiator_affinity(table_data,
> -                                              gi->node, &dev_handle);
> +    build_srat_pci_generic_initiator(table_data, gi->node, 0,
> +                                     pci_bus_num(pci_get_bus(pci_dev)),
> +                                     pci_dev->devfn);
>  
>      return 0;
>  }
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 6d4517cfbe..968b654e58 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1938,6 +1938,50 @@ void build_srat_memory(GArray *table_data, uint64_t base,
>      build_append_int_noprefix(table_data, 0, 8); /* Reserved */
>  }
>  
> +/*
> + * ACPI Spec Revision 6.3
> + * Table 5-80 Device Handle - PCI
> + */
> +static void build_append_srat_pci_device_handle(GArray *table_data,
> +                                                uint16_t segment,
> +                                                uint8_t bus, uint8_t devfn)
> +{
> +    /* PCI segment number */
> +    build_append_int_noprefix(table_data, segment, 2);
> +    /* PCI Bus Device Function */
> +    build_append_int_noprefix(table_data, bus, 1);
> +    build_append_int_noprefix(table_data, devfn, 1);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 12);
> +}
> +
> +/*
> + * ACPI spec, Revision 6.3
> + * 5.2.16.6 Generic Initiator Affinity Structure
> + *    With PCI Device Handle.
> + */
> +void build_srat_pci_generic_initiator(GArray *table_data, int node,
> +                                      uint16_t segment, uint8_t bus,
> +                                      uint8_t devfn)
> +{
> +    /* Type */
> +    build_append_int_noprefix(table_data, 5, 1);
> +    /* Length */
> +    build_append_int_noprefix(table_data, 32, 1);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 1);
> +    /* Device Handle Type: PCI */
> +    build_append_int_noprefix(table_data, 1, 1);
> +    /* Proximity Domain */
> +    build_append_int_noprefix(table_data, node, 4);
> +    /* Device Handle */
> +    build_append_srat_pci_device_handle(table_data, segment, bus, devfn);
> +    /* Flags - GI Enabled */
> +    build_append_int_noprefix(table_data, 1, 4);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 4);
> +}
> +
>  /*
>   * ACPI spec 5.2.17 System Locality Distance Information Table
>   * (Revision 2.0 or later)

Re: [PATCH v3 03/11] hw/acpi: Move AML building code for Generic Initiators to aml_build.c

[Michael S. Tsirkin]

On Thu, Jun 27, 2024 at 02:42:44PM +0200, Igor Mammedov wrote:
> On Thu, 20 Jun 2024 17:03:11 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > Rather than attempting to create a generic function with mess of the two
> > different device handle types, use a PCI handle specific variant.  If the
> > ACPI handle form is needed then that can be introduced alongside this
> > with little duplicated code.
> > 
> > Drop the PCIDeviceHandle in favor of just passing the bus, devfn
> > and segment directly.  devfn kept as a single byte because ARI means
> > that in cases this is just an 8 bit function number.
> > 
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > Link: https://lore.kernel.org/qemu-devel/20240618142333.102be976@imammedo.users.ipa.redhat.com/
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> with typo fixed

typo being "in cases"?

> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 

Re: [PATCH v3 03/11] hw/acpi: Move AML building code for Generic Initiators to aml_build.c

[Igor Mammedov]

On Thu, 27 Jun 2024 08:44:14 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jun 27, 2024 at 02:42:44PM +0200, Igor Mammedov wrote:
> > On Thu, 20 Jun 2024 17:03:11 +0100
> > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >   
> > > Rather than attempting to create a generic function with mess of the two
> > > different device handle types, use a PCI handle specific variant.  If the
> > > ACPI handle form is needed then that can be introduced alongside this
> > > with little duplicated code.
> > > 
> > > Drop the PCIDeviceHandle in favor of just passing the bus, devfn
> > > and segment directly.  devfn kept as a single byte because ARI means
> > > that in cases this is just an 8 bit function number.
> > > 
> > > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > > Link: https://lore.kernel.org/qemu-devel/20240618142333.102be976@imammedo.users.ipa.redhat.com/
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> > 
> > with typo fixed  
> 
> typo being "in cases"?
> 
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> >   
> 

nope, I highlighted it the patch
  > +void build_srat_pci_generic_initiator(GArray * table_date, int node,  
  s/table_date/table_data/

[PATCH v3 04/11] hw/acpi: Rename build_all_acpi_generic_initiators() to build_acpi_generic_initiator()

[Jonathan Cameron via]

Igor noted that this function only builds one instance, so was rather
misleadingly named. Fix that.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: New patch
---
 hw/acpi/acpi_generic_initiator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
index 7665b16107..73bafaaaea 100644
--- a/hw/acpi/acpi_generic_initiator.c
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -74,7 +74,7 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
         acpi_generic_initiator_set_node, NULL, NULL);
 }
 
-static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
+static int build_acpi_generic_initiator(Object *obj, void *opaque)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
     AcpiGenericInitiator *gi;
@@ -111,6 +111,6 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
 void build_srat_generic_pci_initiator(GArray *table_data)
 {
     object_child_foreach_recursive(object_get_root(),
-                                   build_all_acpi_generic_initiators,
+                                   build_acpi_generic_initiator,
                                    table_data);
 }
-- 
2.43.0

Re: [PATCH v3 04/11] hw/acpi: Rename build_all_acpi_generic_initiators() to build_acpi_generic_initiator()

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:12 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Igor noted that this function only builds one instance, so was rather
> misleadingly named. Fix that.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> ---
> v3: New patch
> ---
>  hw/acpi/acpi_generic_initiator.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> index 7665b16107..73bafaaaea 100644
> --- a/hw/acpi/acpi_generic_initiator.c
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -74,7 +74,7 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
>          acpi_generic_initiator_set_node, NULL, NULL);
>  }
>  
> -static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
> +static int build_acpi_generic_initiator(Object *obj, void *opaque)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
>      AcpiGenericInitiator *gi;
> @@ -111,6 +111,6 @@ static int build_all_acpi_generic_initiators(Object *obj, void *opaque)
>  void build_srat_generic_pci_initiator(GArray *table_data)
>  {
>      object_child_foreach_recursive(object_get_root(),
> -                                   build_all_acpi_generic_initiators,
> +                                   build_acpi_generic_initiator,
>                                     table_data);
>  }

[PATCH v3 05/11] hw/pci: Add a bus property to pci_props and use for acpi/gi

[Jonathan Cameron via]

Using a property allows us to hide the internal details of the PCI device
from the code to build a SRAT Generic Initiator Affinity Structure with
PCI Device Handle.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
V3: New patch
---
 hw/acpi/acpi_generic_initiator.c | 11 ++++++-----
 hw/pci/pci.c                     | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
index 73bafaaaea..34284359f0 100644
--- a/hw/acpi/acpi_generic_initiator.c
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -9,6 +9,7 @@
 #include "hw/boards.h"
 #include "hw/pci/pci_device.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 
 typedef struct AcpiGenericInitiatorClass {
     ObjectClass parent_class;
@@ -79,7 +80,7 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
     MachineState *ms = MACHINE(qdev_get_machine());
     AcpiGenericInitiator *gi;
     GArray *table_data = opaque;
-    PCIDevice *pci_dev;
+    uint8_t bus, devfn;
     Object *o;
 
     if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
@@ -100,10 +101,10 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
         exit(1);
     }
 
-    pci_dev = PCI_DEVICE(o);
-    build_srat_pci_generic_initiator(table_data, gi->node, 0,
-                                     pci_bus_num(pci_get_bus(pci_dev)),
-                                     pci_dev->devfn);
+    bus = object_property_get_uint(o, "bus", &error_fatal);
+    devfn = object_property_get_uint(o, "addr", &error_fatal);
+
+    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
 
     return 0;
 }
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 324c1302d2..b4b499b172 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -67,6 +67,19 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static void pcibus_reset_hold(Object *obj, ResetType type);
 static bool pcie_has_upstream_port(PCIDevice *dev);
 
+static void prop_pci_bus_get(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    uint8_t bus = pci_dev_bus_num(PCI_DEVICE(obj));
+
+    visit_type_uint8(v, name, &bus, errp);
+}
+
+static const PropertyInfo prop_pci_bus = {
+    .name = "bus",
+    .get = prop_pci_bus_get,
+};
+
 static Property pci_props[] = {
     DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
     DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
@@ -85,6 +98,7 @@ static Property pci_props[] = {
                     QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
     DEFINE_PROP_BIT("x-pcie-ari-nextfn-1", PCIDevice, cap_present,
                     QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
+    { .name = "bus", .info = &prop_pci_bus },
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.43.0

Re: [PATCH v3 05/11] hw/pci: Add a bus property to pci_props and use for acpi/gi

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:13 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Using a property allows us to hide the internal details of the PCI device
> from the code to build a SRAT Generic Initiator Affinity Structure with
> PCI Device Handle.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> V3: New patch
> ---
>  hw/acpi/acpi_generic_initiator.c | 11 ++++++-----
>  hw/pci/pci.c                     | 14 ++++++++++++++
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> index 73bafaaaea..34284359f0 100644
> --- a/hw/acpi/acpi_generic_initiator.c
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -9,6 +9,7 @@
>  #include "hw/boards.h"
>  #include "hw/pci/pci_device.h"
>  #include "qemu/error-report.h"
> +#include "qapi/error.h"
>  
>  typedef struct AcpiGenericInitiatorClass {
>      ObjectClass parent_class;
> @@ -79,7 +80,7 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
>      MachineState *ms = MACHINE(qdev_get_machine());
>      AcpiGenericInitiator *gi;
>      GArray *table_data = opaque;
> -    PCIDevice *pci_dev;
> +    uint8_t bus, devfn;
>      Object *o;
>  
>      if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
> @@ -100,10 +101,10 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
>          exit(1);
>      }
>  
> -    pci_dev = PCI_DEVICE(o);
> -    build_srat_pci_generic_initiator(table_data, gi->node, 0,
> -                                     pci_bus_num(pci_get_bus(pci_dev)),
> -                                     pci_dev->devfn);
> +    bus = object_property_get_uint(o, "bus", &error_fatal);
> +    devfn = object_property_get_uint(o, "addr", &error_fatal);
> +
> +    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
>  
>      return 0;
>  }
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 324c1302d2..b4b499b172 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -67,6 +67,19 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
>  static void pcibus_reset_hold(Object *obj, ResetType type);
>  static bool pcie_has_upstream_port(PCIDevice *dev);
>  
> +static void prop_pci_bus_get(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)
> +{
> +    uint8_t bus = pci_dev_bus_num(PCI_DEVICE(obj));
> +
> +    visit_type_uint8(v, name, &bus, errp);
> +}
> +
> +static const PropertyInfo prop_pci_bus = {
> +    .name = "bus",

/me confused,
didn't we have 'bus' property for PCI devices already?

i.e. I can add PCI device like this
  -device e1000,bus=pci.0,addr=0x6,...
  

> +    .get = prop_pci_bus_get,
> +};
> +
>  static Property pci_props[] = {
>      DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
>      DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> @@ -85,6 +98,7 @@ static Property pci_props[] = {
>                      QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
>      DEFINE_PROP_BIT("x-pcie-ari-nextfn-1", PCIDevice, cap_present,
>                      QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
> +    { .name = "bus", .info = &prop_pci_bus },
>      DEFINE_PROP_END_OF_LIST()
>  };
>  

Re: [PATCH v3 05/11] hw/pci: Add a bus property to pci_props and use for acpi/gi

[Igor Mammedov]

On Thu, 27 Jun 2024 15:09:12 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 20 Jun 2024 17:03:13 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > Using a property allows us to hide the internal details of the PCI device
> > from the code to build a SRAT Generic Initiator Affinity Structure with
> > PCI Device Handle.
> > 
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > ---
> > V3: New patch
> > ---
> >  hw/acpi/acpi_generic_initiator.c | 11 ++++++-----
> >  hw/pci/pci.c                     | 14 ++++++++++++++
> >  2 files changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> > index 73bafaaaea..34284359f0 100644
> > --- a/hw/acpi/acpi_generic_initiator.c
> > +++ b/hw/acpi/acpi_generic_initiator.c
> > @@ -9,6 +9,7 @@
> >  #include "hw/boards.h"
> >  #include "hw/pci/pci_device.h"
> >  #include "qemu/error-report.h"
> > +#include "qapi/error.h"
> >  
> >  typedef struct AcpiGenericInitiatorClass {
> >      ObjectClass parent_class;
> > @@ -79,7 +80,7 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
> >      MachineState *ms = MACHINE(qdev_get_machine());
> >      AcpiGenericInitiator *gi;
> >      GArray *table_data = opaque;
> > -    PCIDevice *pci_dev;
> > +    uint8_t bus, devfn;
> >      Object *o;
> >  
> >      if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
> > @@ -100,10 +101,10 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
> >          exit(1);
> >      }
> >  
> > -    pci_dev = PCI_DEVICE(o);
> > -    build_srat_pci_generic_initiator(table_data, gi->node, 0,
> > -                                     pci_bus_num(pci_get_bus(pci_dev)),
> > -                                     pci_dev->devfn);
> > +    bus = object_property_get_uint(o, "bus", &error_fatal);
> > +    devfn = object_property_get_uint(o, "addr", &error_fatal);
> > +
> > +    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
> >  
> >      return 0;
> >  }
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 324c1302d2..b4b499b172 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -67,6 +67,19 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
> >  static void pcibus_reset_hold(Object *obj, ResetType type);
> >  static bool pcie_has_upstream_port(PCIDevice *dev);
> >  
> > +static void prop_pci_bus_get(Object *obj, Visitor *v, const char *name,
> > +                             void *opaque, Error **errp)
> > +{
> > +    uint8_t bus = pci_dev_bus_num(PCI_DEVICE(obj));
> > +
> > +    visit_type_uint8(v, name, &bus, errp);
> > +}
> > +
> > +static const PropertyInfo prop_pci_bus = {
> > +    .name = "bus",  
> 
> /me confused,
> didn't we have 'bus' property for PCI devices already?
> 
> i.e. I can add PCI device like this
>   -device e1000,bus=pci.0,addr=0x6,...

to avoid confusion, I'd suggest to name it to 'busnr'
(or be more specific (primary|secondary)_busnr if applicable)

>   
> 
> > +    .get = prop_pci_bus_get,
> > +};
> > +
> >  static Property pci_props[] = {
> >      DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> >      DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > @@ -85,6 +98,7 @@ static Property pci_props[] = {
> >                      QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
> >      DEFINE_PROP_BIT("x-pcie-ari-nextfn-1", PCIDevice, cap_present,
> >                      QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
> > +    { .name = "bus", .info = &prop_pci_bus },
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >    
> 

Re: [PATCH v3 05/11] hw/pci: Add a bus property to pci_props and use for acpi/gi

[Jonathan Cameron via]

On Fri, 28 Jun 2024 13:58:04 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 27 Jun 2024 15:09:12 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Thu, 20 Jun 2024 17:03:13 +0100
> > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >   
> > > Using a property allows us to hide the internal details of the PCI device
> > > from the code to build a SRAT Generic Initiator Affinity Structure with
> > > PCI Device Handle.
> > > 
> > > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > ---
> > > V3: New patch
> > > ---
> > >  hw/acpi/acpi_generic_initiator.c | 11 ++++++-----
> > >  hw/pci/pci.c                     | 14 ++++++++++++++
> > >  2 files changed, 20 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> > > index 73bafaaaea..34284359f0 100644
> > > --- a/hw/acpi/acpi_generic_initiator.c
> > > +++ b/hw/acpi/acpi_generic_initiator.c
> > > @@ -9,6 +9,7 @@
> > >  #include "hw/boards.h"
> > >  #include "hw/pci/pci_device.h"
> > >  #include "qemu/error-report.h"
> > > +#include "qapi/error.h"
> > >  
> > >  typedef struct AcpiGenericInitiatorClass {
> > >      ObjectClass parent_class;
> > > @@ -79,7 +80,7 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
> > >      MachineState *ms = MACHINE(qdev_get_machine());
> > >      AcpiGenericInitiator *gi;
> > >      GArray *table_data = opaque;
> > > -    PCIDevice *pci_dev;
> > > +    uint8_t bus, devfn;
> > >      Object *o;
> > >  
> > >      if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
> > > @@ -100,10 +101,10 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
> > >          exit(1);
> > >      }
> > >  
> > > -    pci_dev = PCI_DEVICE(o);
> > > -    build_srat_pci_generic_initiator(table_data, gi->node, 0,
> > > -                                     pci_bus_num(pci_get_bus(pci_dev)),
> > > -                                     pci_dev->devfn);
> > > +    bus = object_property_get_uint(o, "bus", &error_fatal);
> > > +    devfn = object_property_get_uint(o, "addr", &error_fatal);
> > > +
> > > +    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
> > >  
> > >      return 0;
> > >  }
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 324c1302d2..b4b499b172 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -67,6 +67,19 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
> > >  static void pcibus_reset_hold(Object *obj, ResetType type);
> > >  static bool pcie_has_upstream_port(PCIDevice *dev);
> > >  
> > > +static void prop_pci_bus_get(Object *obj, Visitor *v, const char *name,
> > > +                             void *opaque, Error **errp)
> > > +{
> > > +    uint8_t bus = pci_dev_bus_num(PCI_DEVICE(obj));
> > > +
> > > +    visit_type_uint8(v, name, &bus, errp);
> > > +}
> > > +
> > > +static const PropertyInfo prop_pci_bus = {
> > > +    .name = "bus",    
> > 
> > /me confused,
> > didn't we have 'bus' property for PCI devices already?
> > 
> > i.e. I can add PCI device like this
> >   -device e1000,bus=pci.0,addr=0x6,...  
> 
> to avoid confusion, I'd suggest to name it to 'busnr'
> (or be more specific (primary|secondary)_busnr if applicable)
For generic initiators we are always dealing with an EP so I think
busnr alone is appropriate. If we need similar for bridges we
can add that later.


> 
> >   
> >   
> > > +    .get = prop_pci_bus_get,
> > > +};
> > > +
> > >  static Property pci_props[] = {
> > >      DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > >      DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > > @@ -85,6 +98,7 @@ static Property pci_props[] = {
> > >                      QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
> > >      DEFINE_PROP_BIT("x-pcie-ari-nextfn-1", PCIDevice, cap_present,
> > >                      QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
> > > +    { .name = "bus", .info = &prop_pci_bus },
> > >      DEFINE_PROP_END_OF_LIST()
> > >  };
> > >      
> >   
> 
> 

[PATCH v3 06/11] acpi/pci: Move Generic Initiator object handling into acpi/pci.*

[Jonathan Cameron via]

Whilst ACPI SRAT Generic Initiator Afinity Structures are able to refer to
both PCI and ACPI Device Handles, the QEMU implementation only implements
the PCI Device Handle case.  For now move the code into the existing
hw/acpi/pci.c file and header.  If support for ACPI Device Handles is
added in the future, perhaps this will be moved again.

Also push the struct AcpiGenericInitiator down into the c file as not
used outside pci.c.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: New patch.
---
 include/hw/acpi/acpi_generic_initiator.h |  24 -----
 include/hw/acpi/pci.h                    |   6 ++
 hw/acpi/acpi_generic_initiator.c         | 117 ----------------------
 hw/acpi/pci.c                            | 118 +++++++++++++++++++++++
 hw/arm/virt-acpi-build.c                 |   1 -
 hw/i386/acpi-build.c                     |   1 -
 hw/acpi/meson.build                      |   1 -
 7 files changed, 124 insertions(+), 144 deletions(-)

diff --git a/include/hw/acpi/acpi_generic_initiator.h b/include/hw/acpi/acpi_generic_initiator.h
deleted file mode 100644
index 7b98676713..0000000000
--- a/include/hw/acpi/acpi_generic_initiator.h
+++ /dev/null
@@ -1,24 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
- */
-
-#ifndef ACPI_GENERIC_INITIATOR_H
-#define ACPI_GENERIC_INITIATOR_H
-
-#include "qom/object_interfaces.h"
-
-#define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
-
-typedef struct AcpiGenericInitiator {
-    /* private */
-    Object parent;
-
-    /* public */
-    char *pci_dev;
-    uint16_t node;
-} AcpiGenericInitiator;
-
-void build_srat_generic_pci_initiator(GArray *table_data);
-
-#endif
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index 467a99461c..9adf1887da 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -28,6 +28,7 @@
 
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/acpi/acpi_aml_interface.h"
+#include "qom/object_interfaces.h"
 
 typedef struct AcpiMcfgInfo {
     uint64_t base;
@@ -40,4 +41,9 @@ Aml *aml_pci_device_dsm(void);
 
 void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
 void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope);
+
+#define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
+
+void build_srat_generic_pci_initiator(GArray *table_data);
+
 #endif
diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
deleted file mode 100644
index 34284359f0..0000000000
--- a/hw/acpi/acpi_generic_initiator.c
+++ /dev/null
@@ -1,117 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
- */
-
-#include "qemu/osdep.h"
-#include "hw/acpi/acpi_generic_initiator.h"
-#include "hw/acpi/aml-build.h"
-#include "hw/boards.h"
-#include "hw/pci/pci_device.h"
-#include "qemu/error-report.h"
-#include "qapi/error.h"
-
-typedef struct AcpiGenericInitiatorClass {
-    ObjectClass parent_class;
-} AcpiGenericInitiatorClass;
-
-OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
-                   ACPI_GENERIC_INITIATOR, OBJECT,
-                   { TYPE_USER_CREATABLE },
-                   { NULL })
-
-OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
-
-static void acpi_generic_initiator_init(Object *obj)
-{
-    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
-
-    gi->node = MAX_NODES;
-    gi->pci_dev = NULL;
-}
-
-static void acpi_generic_initiator_finalize(Object *obj)
-{
-    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
-
-    g_free(gi->pci_dev);
-}
-
-static void acpi_generic_initiator_set_pci_device(Object *obj, const char *val,
-                                                  Error **errp)
-{
-    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
-
-    gi->pci_dev = g_strdup(val);
-}
-
-static void acpi_generic_initiator_set_node(Object *obj, Visitor *v,
-                                            const char *name, void *opaque,
-                                            Error **errp)
-{
-    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
-    MachineState *ms = MACHINE(qdev_get_machine());
-    uint32_t value;
-
-    if (!visit_type_uint32(v, name, &value, errp)) {
-        return;
-    }
-
-    if (value >= MAX_NODES) {
-        error_printf("%s: Invalid NUMA node specified\n",
-                     TYPE_ACPI_GENERIC_INITIATOR);
-        exit(1);
-    }
-
-    gi->node = value;
-    ms->numa_state->nodes[gi->node].has_gi = true;
-}
-
-static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
-{
-    object_class_property_add_str(oc, "pci-dev", NULL,
-        acpi_generic_initiator_set_pci_device);
-    object_class_property_add(oc, "node", "int", NULL,
-        acpi_generic_initiator_set_node, NULL, NULL);
-}
-
-static int build_acpi_generic_initiator(Object *obj, void *opaque)
-{
-    MachineState *ms = MACHINE(qdev_get_machine());
-    AcpiGenericInitiator *gi;
-    GArray *table_data = opaque;
-    uint8_t bus, devfn;
-    Object *o;
-
-    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
-        return 0;
-    }
-
-    gi = ACPI_GENERIC_INITIATOR(obj);
-    if (gi->node >= ms->numa_state->num_nodes) {
-        error_printf("%s: Specified node %d is invalid.\n",
-                     TYPE_ACPI_GENERIC_INITIATOR, gi->node);
-        exit(1);
-    }
-
-    o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL);
-    if (!o) {
-        error_printf("%s: Specified device must be a PCI device.\n",
-                     TYPE_ACPI_GENERIC_INITIATOR);
-        exit(1);
-    }
-
-    bus = object_property_get_uint(o, "bus", &error_fatal);
-    devfn = object_property_get_uint(o, "addr", &error_fatal);
-
-    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
-
-    return 0;
-}
-
-void build_srat_generic_pci_initiator(GArray *table_data)
-{
-    object_child_foreach_recursive(object_get_root(),
-                                   build_acpi_generic_initiator,
-                                   table_data);
-}
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index 20b70dcd81..f68be9ea17 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -24,8 +24,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "hw/boards.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/pci.h"
+#include "hw/pci/pci_device.h"
 #include "hw/pci/pcie_host.h"
 
 /*
@@ -59,3 +63,117 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
 
     acpi_table_end(linker, &table);
 }
+
+typedef struct AcpiGenericInitiator {
+    /* private */
+    Object parent;
+
+    /* public */
+    char *pci_dev;
+    uint16_t node;
+} AcpiGenericInitiator;
+
+typedef struct AcpiGenericInitiatorClass {
+    ObjectClass parent_class;
+} AcpiGenericInitiatorClass;
+
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
+                   ACPI_GENERIC_INITIATOR, OBJECT,
+                   { TYPE_USER_CREATABLE },
+                   { NULL })
+
+OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
+
+static void acpi_generic_initiator_init(Object *obj)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    gi->node = MAX_NODES;
+    gi->pci_dev = NULL;
+}
+
+static void acpi_generic_initiator_finalize(Object *obj)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    g_free(gi->pci_dev);
+}
+
+static void acpi_generic_initiator_set_pci_device(Object *obj, const char *val,
+                                                  Error **errp)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    gi->pci_dev = g_strdup(val);
+}
+
+static void acpi_generic_initiator_set_node(Object *obj, Visitor *v,
+                                            const char *name, void *opaque,
+                                            Error **errp)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    if (value >= MAX_NODES) {
+        error_printf("%s: Invalid NUMA node specified\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    gi->node = value;
+    ms->numa_state->nodes[gi->node].has_gi = true;
+}
+
+static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
+{
+    object_class_property_add_str(oc, "pci-dev", NULL,
+        acpi_generic_initiator_set_pci_device);
+    object_class_property_add(oc, "node", "int", NULL,
+        acpi_generic_initiator_set_node, NULL, NULL);
+}
+
+static int build_acpi_generic_initiator(Object *obj, void *opaque)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    AcpiGenericInitiator *gi;
+    GArray *table_data = opaque;
+    uint8_t bus, devfn;
+    Object *o;
+
+    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
+        return 0;
+    }
+
+    gi = ACPI_GENERIC_INITIATOR(obj);
+    if (gi->node >= ms->numa_state->num_nodes) {
+        error_printf("%s: Specified node %d is invalid.\n",
+                     TYPE_ACPI_GENERIC_INITIATOR, gi->node);
+        exit(1);
+    }
+
+    o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL);
+    if (!o) {
+        error_printf("%s: Specified device must be a PCI device.\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    bus = object_property_get_uint(o, "bus", &error_fatal);
+    devfn = object_property_get_uint(o, "addr", &error_fatal);
+
+    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
+
+    return 0;
+}
+
+void build_srat_generic_pci_initiator(GArray *table_data)
+{
+    object_child_foreach_recursive(object_get_root(),
+                                   build_acpi_generic_initiator,
+                                   table_data);
+}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c3ccfef026..1a5aa0d55d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -57,7 +57,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/ghes.h"
 #include "hw/acpi/viot.h"
-#include "hw/acpi/acpi_generic_initiator.h"
 #include "hw/virtio/virtio-acpi.h"
 #include "target/arm/multiprocessing.h"
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 53f804ac16..0d37b9238a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -68,7 +68,6 @@
 #include "hw/acpi/utils.h"
 #include "hw/acpi/pci.h"
 #include "hw/acpi/cxl.h"
-#include "hw/acpi/acpi_generic_initiator.h"
 
 #include "qom/qom-qobject.h"
 #include "hw/i386/amd_iommu.h"
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index fa5c07db90..5441c9b1e4 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -1,6 +1,5 @@
 acpi_ss = ss.source_set()
 acpi_ss.add(files(
-  'acpi_generic_initiator.c',
   'acpi_interface.c',
   'aml-build.c',
   'bios-linker-loader.c',
-- 
2.43.0

[PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Jonathan Cameron via]

This allows the ACPI SRAT Generic Port Affinity Structure
creation to be independent of PCI internals. Note that
the UID is currently the PCI bus number.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: New patch
---
 hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 0411ad31ea..92d39b917a 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
     pbc->numa_node = pxb_bus_numa_node;
 }
 
+static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    uint32_t uid = pci_bus_num(PCI_BUS(obj));
+
+    visit_type_uint32(v, name, &uid, errp);
+}
+
+static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
+{
+    pxb_bus_class_init(class, data);
+    object_class_property_add(class, "acpi_uid", "uint32",
+                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
+}
+
 static const TypeInfo pxb_bus_info = {
     .name          = TYPE_PXB_BUS,
     .parent        = TYPE_PCI_BUS,
@@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
     .name          = TYPE_PXB_CXL_BUS,
     .parent        = TYPE_CXL_BUS,
     .instance_size = sizeof(PXBBus),
-    .class_init    = pxb_bus_class_init,
+    .class_init    = pxb_cxl_bus_class_init,
 };
 
 static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,
-- 
2.43.0

Re: [PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:15 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> This allows the ACPI SRAT Generic Port Affinity Structure
> creation to be independent of PCI internals. Note that
> the UID is currently the PCI bus number.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> v3: New patch
> ---
>  hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 0411ad31ea..92d39b917a 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
>      pbc->numa_node = pxb_bus_numa_node;
>  }
>  
> +static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)
> +{
> +    uint32_t uid = pci_bus_num(PCI_BUS(obj));
> +
> +    visit_type_uint32(v, name, &uid, errp);
> +}
> +
> +static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
> +{
> +    pxb_bus_class_init(class, data);
> +    object_class_property_add(class, "acpi_uid", "uint32",
> +                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
> +}
> +
>  static const TypeInfo pxb_bus_info = {
>      .name          = TYPE_PXB_BUS,
>      .parent        = TYPE_PCI_BUS,
> @@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
>      .name          = TYPE_PXB_CXL_BUS,
>      .parent        = TYPE_CXL_BUS,
>      .instance_size = sizeof(PXBBus),
> -    .class_init    = pxb_bus_class_init,
> +    .class_init    = pxb_cxl_bus_class_init,

why it's CXL only, doesn't the same UID rules apply to other PCI buses?
>  };
>  
>  static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,

Re: [PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Jonathan Cameron via]

On Thu, 27 Jun 2024 15:27:58 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 20 Jun 2024 17:03:15 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > This allows the ACPI SRAT Generic Port Affinity Structure
> > creation to be independent of PCI internals. Note that
> > the UID is currently the PCI bus number.
> > 
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > ---
> > v3: New patch
> > ---
> >  hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 0411ad31ea..92d39b917a 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> >      pbc->numa_node = pxb_bus_numa_node;
> >  }
> >  
> > +static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
> > +                             void *opaque, Error **errp)
> > +{
> > +    uint32_t uid = pci_bus_num(PCI_BUS(obj));
> > +
> > +    visit_type_uint32(v, name, &uid, errp);
> > +}
> > +
> > +static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
> > +{
> > +    pxb_bus_class_init(class, data);
> > +    object_class_property_add(class, "acpi_uid", "uint32",
> > +                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
> > +}
> > +
> >  static const TypeInfo pxb_bus_info = {
> >      .name          = TYPE_PXB_BUS,
> >      .parent        = TYPE_PCI_BUS,
> > @@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
> >      .name          = TYPE_PXB_CXL_BUS,
> >      .parent        = TYPE_CXL_BUS,
> >      .instance_size = sizeof(PXBBus),
> > -    .class_init    = pxb_bus_class_init,
> > +    .class_init    = pxb_cxl_bus_class_init,  
> 
> why it's CXL only, doesn't the same UID rules apply to other PCI buses?

In principle, yes.  My nervousness is that we can only test anything
using this infrastructure today with CXL root bridges.

So I was thinking we should keep it limited and broaden the scope
if anyone ever cares.  I don't mind broadening it from the start though.

Jonathan


> >  };
> >  
> >  static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,  
> 
> 

Re: [PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Igor Mammedov]

On Thu, 27 Jun 2024 14:46:14 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Thu, 27 Jun 2024 15:27:58 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Thu, 20 Jun 2024 17:03:15 +0100
> > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >   
> > > This allows the ACPI SRAT Generic Port Affinity Structure
> > > creation to be independent of PCI internals. Note that
> > > the UID is currently the PCI bus number.
> > > 
> > > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > ---
> > > v3: New patch
> > > ---
> > >  hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
> > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > > index 0411ad31ea..92d39b917a 100644
> > > --- a/hw/pci-bridge/pci_expander_bridge.c
> > > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > > @@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> > >      pbc->numa_node = pxb_bus_numa_node;
> > >  }
> > >  
> > > +static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
> > > +                             void *opaque, Error **errp)
> > > +{
> > > +    uint32_t uid = pci_bus_num(PCI_BUS(obj));
> > > +
> > > +    visit_type_uint32(v, name, &uid, errp);
> > > +}
> > > +
> > > +static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
> > > +{
> > > +    pxb_bus_class_init(class, data);
> > > +    object_class_property_add(class, "acpi_uid", "uint32",
> > > +                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
> > > +}
> > > +
> > >  static const TypeInfo pxb_bus_info = {
> > >      .name          = TYPE_PXB_BUS,
> > >      .parent        = TYPE_PCI_BUS,
> > > @@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
> > >      .name          = TYPE_PXB_CXL_BUS,
> > >      .parent        = TYPE_CXL_BUS,
> > >      .instance_size = sizeof(PXBBus),
> > > -    .class_init    = pxb_bus_class_init,
> > > +    .class_init    = pxb_cxl_bus_class_init,    
> > 
> > why it's CXL only, doesn't the same UID rules apply to other PCI buses?  
> 
> In principle, yes.  My nervousness is that we can only test anything
> using this infrastructure today with CXL root bridges.
> 
> So I was thinking we should keep it limited and broaden the scope
> if anyone ever cares.  I don't mind broadening it from the start though.

Then I'd use it everywhere and cleanup ACPI code to use it as well
just to be consistent.
 
> Jonathan
> 
> 
> > >  };
> > >  
> > >  static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,    
> > 
> >   
> 

Re: [PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Jonathan Cameron via]

On Fri, 28 Jun 2024 13:55:25 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 27 Jun 2024 14:46:14 +0100
> Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> > On Thu, 27 Jun 2024 15:27:58 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >   
> > > On Thu, 20 Jun 2024 17:03:15 +0100
> > > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> > >     
> > > > This allows the ACPI SRAT Generic Port Affinity Structure
> > > > creation to be independent of PCI internals. Note that
> > > > the UID is currently the PCI bus number.
> > > > 
> > > > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > 
> > > > ---
> > > > v3: New patch
> > > > ---
> > > >  hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
> > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > > > index 0411ad31ea..92d39b917a 100644
> > > > --- a/hw/pci-bridge/pci_expander_bridge.c
> > > > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > > > @@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> > > >      pbc->numa_node = pxb_bus_numa_node;
> > > >  }
> > > >  
> > > > +static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
> > > > +                             void *opaque, Error **errp)
> > > > +{
> > > > +    uint32_t uid = pci_bus_num(PCI_BUS(obj));
> > > > +
> > > > +    visit_type_uint32(v, name, &uid, errp);
> > > > +}
> > > > +
> > > > +static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
> > > > +{
> > > > +    pxb_bus_class_init(class, data);
> > > > +    object_class_property_add(class, "acpi_uid", "uint32",
> > > > +                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
> > > > +}
> > > > +
> > > >  static const TypeInfo pxb_bus_info = {
> > > >      .name          = TYPE_PXB_BUS,
> > > >      .parent        = TYPE_PCI_BUS,
> > > > @@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
> > > >      .name          = TYPE_PXB_CXL_BUS,
> > > >      .parent        = TYPE_CXL_BUS,
> > > >      .instance_size = sizeof(PXBBus),
> > > > -    .class_init    = pxb_bus_class_init,
> > > > +    .class_init    = pxb_cxl_bus_class_init,      
> > > 
> > > why it's CXL only, doesn't the same UID rules apply to other PCI buses?    
> > 
> > In principle, yes.  My nervousness is that we can only test anything
> > using this infrastructure today with CXL root bridges.
> > 
> > So I was thinking we should keep it limited and broaden the scope
> > if anyone ever cares.  I don't mind broadening it from the start though.  
> 
> Then I'd use it everywhere and cleanup ACPI code to use it as well
> just to be consistent.
That is easy to do for all the TYPE_PXB_BUS types and they have separate
handling in the various ACPI table builds from other host bridgesn anyway.

Ultimately it might be nice to do for the host bridges in general but
that needs to be separate I think as there isn't a simple common
ancestor to use.  For at least some cases (gpex-acpi.c) it's hard
coded as 0 directly with no look up at all.

Jonathan

>  
> > Jonathan
> > 
> >   
> > > >  };
> > > >  
> > > >  static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,      
> > > 
> > >     
> >   
> 
> 

Re: [PATCH v3 07/11] hw/pci-bridge: Add acpi_uid property to CXL PXB

[Jonathan Cameron via]

On Mon, 1 Jul 2024 18:52:03 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Fri, 28 Jun 2024 13:55:25 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Thu, 27 Jun 2024 14:46:14 +0100
> > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> >   
> > > On Thu, 27 Jun 2024 15:27:58 +0200
> > > Igor Mammedov <imammedo@redhat.com> wrote:
> > >     
> > > > On Thu, 20 Jun 2024 17:03:15 +0100
> > > > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> > > >       
> > > > > This allows the ACPI SRAT Generic Port Affinity Structure
> > > > > creation to be independent of PCI internals. Note that
> > > > > the UID is currently the PCI bus number.
> > > > > 
> > > > > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > > 
> > > > > ---
> > > > > v3: New patch
> > > > > ---
> > > > >  hw/pci-bridge/pci_expander_bridge.c | 17 ++++++++++++++++-
> > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > > > > index 0411ad31ea..92d39b917a 100644
> > > > > --- a/hw/pci-bridge/pci_expander_bridge.c
> > > > > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > > > > @@ -93,6 +93,21 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> > > > >      pbc->numa_node = pxb_bus_numa_node;
> > > > >  }
> > > > >  
> > > > > +static void prop_pxb_cxl_uid_get(Object *obj, Visitor *v, const char *name,
> > > > > +                             void *opaque, Error **errp)
> > > > > +{
> > > > > +    uint32_t uid = pci_bus_num(PCI_BUS(obj));
> > > > > +
> > > > > +    visit_type_uint32(v, name, &uid, errp);
> > > > > +}
> > > > > +
> > > > > +static void pxb_cxl_bus_class_init(ObjectClass *class, void *data)
> > > > > +{
> > > > > +    pxb_bus_class_init(class, data);
> > > > > +    object_class_property_add(class, "acpi_uid", "uint32",
> > > > > +                              prop_pxb_cxl_uid_get, NULL, NULL, NULL);
> > > > > +}
> > > > > +
> > > > >  static const TypeInfo pxb_bus_info = {
> > > > >      .name          = TYPE_PXB_BUS,
> > > > >      .parent        = TYPE_PCI_BUS,
> > > > > @@ -111,7 +126,7 @@ static const TypeInfo pxb_cxl_bus_info = {
> > > > >      .name          = TYPE_PXB_CXL_BUS,
> > > > >      .parent        = TYPE_CXL_BUS,
> > > > >      .instance_size = sizeof(PXBBus),
> > > > > -    .class_init    = pxb_bus_class_init,
> > > > > +    .class_init    = pxb_cxl_bus_class_init,        
> > > > 
> > > > why it's CXL only, doesn't the same UID rules apply to other PCI buses?      
> > > 
> > > In principle, yes.  My nervousness is that we can only test anything
> > > using this infrastructure today with CXL root bridges.
> > > 
> > > So I was thinking we should keep it limited and broaden the scope
> > > if anyone ever cares.  I don't mind broadening it from the start though.    
> > 
> > Then I'd use it everywhere and cleanup ACPI code to use it as well
> > just to be consistent.  
> That is easy to do for all the TYPE_PXB_BUS types and they have separate
> handling in the various ACPI table builds from other host bridgesn anyway.
> 
> Ultimately it might be nice to do for the host bridges in general but
> that needs to be separate I think as there isn't a simple common
> ancestor to use.  For at least some cases (gpex-acpi.c) it's hard
> coded as 0 directly with no look up at all.
Also worth noting that we could take the approach of not using pci internals
in ACPI building further and deal with things like numa nodes.
I don't mind doing that in the longer term, but I don't want that to be
a dependency for this series.

Jonathan

> 
> Jonathan
> 
> >    
> > > Jonathan
> > > 
> > >     
> > > > >  };
> > > > >  
> > > > >  static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,        
> > > > 
> > > >       
> > >     
> > 
> >   
> 
> 
> 

[PATCH v3 08/11] hw/acpi: Generic Port Affinity Structure support

[Jonathan Cameron via]

These are very similar to the recently added Generic Initiators
but instead of representing an initiator of memory traffic they
represent an edge point beyond which may lie either targets or
initiators.  Here we add these ports such that they may
be targets of hmat_lb records to describe the latency and
bandwidth from host side initiators to the port.  A discoverable
mechanism such as UEFI CDAT read from CXL devices and switches
is used to discover the remainder of the path, and the OS can build
up full latency and bandwidth numbers as need for work and data
placement decisions.

Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: Move to hw/acpi/pci.c
    Rename the funciton to actually registers both types
    of generic nodes to reflect it isn't GI only.
    Note that the qom part is unchanged and other changes are mostly
    code movement so I've kept Markus' Ack.
---
 qapi/qom.json                            |  34 ++++
 include/hw/acpi/acpi_generic_initiator.h |  35 ++++
 include/hw/acpi/aml-build.h              |   4 +
 include/hw/acpi/pci.h                    |   3 +-
 include/hw/pci/pci_bridge.h              |   1 +
 hw/acpi/acpi_generic_initiator.c         | 216 +++++++++++++++++++++++
 hw/acpi/aml-build.c                      |  40 +++++
 hw/acpi/pci.c                            | 110 +++++++++++-
 hw/arm/virt-acpi-build.c                 |   2 +-
 hw/i386/acpi-build.c                     |   2 +-
 hw/pci-bridge/pci_expander_bridge.c      |   1 -
 11 files changed, 443 insertions(+), 5 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 8bd299265e..8fa6bbd9a7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -826,6 +826,38 @@
   'data': { 'pci-dev': 'str',
             'node': 'uint32' } }
 
+##
+# @AcpiGenericPortProperties:
+#
+# Properties for acpi-generic-port objects.
+#
+# @pci-bus: QOM path of the PCI bus of the hostbridge associated with
+#     this SRAT Generic Port Affinity Structure.  This is the same as
+#     the bus parameter for the root ports attached to this host
+#     bridge.  The resulting SRAT Generic Port Affinity Structure will
+#     refer to the ACPI object in DSDT that represents the host bridge
+#     (e.g.  ACPI0016 for CXL host bridges).  See ACPI 6.5 Section
+#     5.2.16.7 for more information.
+#
+# @node: Similar to a NUMA node ID, but instead of providing a
+#     reference point used for defining NUMA distances and access
+#     characteristics to memory or from an initiator (e.g. CPU), this
+#     node defines the boundary point between non-discoverable system
+#     buses which must be described by firmware, and a discoverable
+#     bus.  NUMA distances and access characteristics are defined to
+#     and from that point.  For system software to establish full
+#     initiator to target characteristics this information must be
+#     combined with information retrieved from the discoverable part
+#     of the path.  An example would use CDAT (see UEFI.org)
+#     information read from devices and switches in conjunction with
+#     link characteristics read from PCIe Configuration space.
+#
+# Since: 9.1
+##
+{ 'struct': 'AcpiGenericPortProperties',
+  'data': { 'pci-bus': 'str',
+            'node': 'uint32' } }
+
 ##
 # @RngProperties:
 #
@@ -1019,6 +1051,7 @@
 { 'enum': 'ObjectType',
   'data': [
     'acpi-generic-initiator',
+    'acpi-generic-port',
     'authz-list',
     'authz-listfile',
     'authz-pam',
@@ -1092,6 +1125,7 @@
   'discriminator': 'qom-type',
   'data': {
       'acpi-generic-initiator':     'AcpiGenericInitiatorProperties',
+      'acpi-generic-port':          'AcpiGenericPortProperties',
       'authz-list':                 'AuthZListProperties',
       'authz-listfile':             'AuthZListFileProperties',
       'authz-pam':                  'AuthZPAMProperties',
diff --git a/include/hw/acpi/acpi_generic_initiator.h b/include/hw/acpi/acpi_generic_initiator.h
new file mode 100644
index 0000000000..92a39ad303
--- /dev/null
+++ b/include/hw/acpi/acpi_generic_initiator.h
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ */
+
+#ifndef ACPI_GENERIC_INITIATOR_H
+#define ACPI_GENERIC_INITIATOR_H
+
+#include "qom/object_interfaces.h"
+
+#define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
+
+typedef struct AcpiGenericInitiator {
+    /* private */
+    Object parent;
+
+    /* public */
+    char *pci_dev;
+    uint16_t node;
+} AcpiGenericInitiator;
+
+#define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
+
+typedef struct AcpiGenericPort {
+    /* private */
+    Object parent;
+
+    /* public */
+    char *pci_bus;
+    uint16_t node;
+} AcpiGenericPort;
+
+void build_srat_generic_pci_initiator(GArray *table_data);
+
+#endif
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 9ba3a21c13..fb8cf6c415 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -490,6 +490,10 @@ void build_srat_pci_generic_initiator(GArray *table_date, int node,
                                       uint16_t segment, uint8_t bus,
                                       uint8_t devfn);
 
+void build_srat_acpi_generic_port(GArray *table_data, int node,
+                                  const char *hid,
+                                  uint32_t uid);
+
 void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
                 const char *oem_id, const char *oem_table_id);
 
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index 9adf1887da..182095d104 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -43,7 +43,8 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
 void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope);
 
 #define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
+#define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
 
-void build_srat_generic_pci_initiator(GArray *table_data);
+void build_srat_generic_affinity_structures(GArray *table_data);
 
 #endif
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 5cd452115a..5456e24883 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -102,6 +102,7 @@ typedef struct PXBPCIEDev {
     PXBDev parent_obj;
 } PXBPCIEDev;
 
+#define TYPE_PXB_CXL_BUS "pxb-cxl-bus"
 #define TYPE_PXB_DEV "pxb"
 OBJECT_DECLARE_SIMPLE_TYPE(PXBDev, PXB_DEV)
 
diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
new file mode 100644
index 0000000000..8491aaf5ec
--- /dev/null
+++ b/hw/acpi/acpi_generic_initiator.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi_generic_initiator.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/boards.h"
+#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_device.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+
+typedef struct AcpiGenericInitiatorClass {
+    ObjectClass parent_class;
+} AcpiGenericInitiatorClass;
+
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
+                   ACPI_GENERIC_INITIATOR, OBJECT,
+                   { TYPE_USER_CREATABLE },
+                   { NULL })
+
+OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
+
+static void acpi_generic_initiator_init(Object *obj)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    gi->node = MAX_NODES;
+    gi->pci_dev = NULL;
+}
+
+static void acpi_generic_initiator_finalize(Object *obj)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    g_free(gi->pci_dev);
+}
+
+static void acpi_generic_initiator_set_pci_device(Object *obj, const char *val,
+                                                  Error **errp)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+    gi->pci_dev = g_strdup(val);
+}
+
+static void acpi_generic_initiator_set_node(Object *obj, Visitor *v,
+                                            const char *name, void *opaque,
+                                            Error **errp)
+{
+    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    if (value >= MAX_NODES) {
+        error_printf("%s: Invalid NUMA node specified\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    gi->node = value;
+    ms->numa_state->nodes[gi->node].has_gi = true;
+}
+
+static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
+{
+    object_class_property_add_str(oc, "pci-dev", NULL,
+        acpi_generic_initiator_set_pci_device);
+    object_class_property_add(oc, "node", "int", NULL,
+        acpi_generic_initiator_set_node, NULL, NULL);
+}
+
+typedef struct AcpiGenericPortClass {
+    ObjectClass parent_class;
+} AcpiGenericPortClass;
+
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
+                   ACPI_GENERIC_PORT, OBJECT,
+                   { TYPE_USER_CREATABLE },
+                   { NULL })
+
+OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
+
+static void acpi_generic_port_init(Object *obj)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    gp->node = MAX_NODES;
+    gp->pci_bus = NULL;
+}
+
+static void acpi_generic_port_finalize(Object *obj)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    g_free(gp->pci_bus);
+}
+
+static void acpi_generic_port_set_pci_bus(Object *obj, const char *val,
+                                          Error **errp)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    gp->pci_bus = g_strdup(val);
+}
+
+static void acpi_generic_port_set_node(Object *obj, Visitor *v,
+                                       const char *name, void *opaque,
+                                       Error **errp)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    if (value >= MAX_NODES) {
+        error_printf("%s: Invalid NUMA node specified\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    gp->node = value;
+}
+
+static void acpi_generic_port_class_init(ObjectClass *oc, void *data)
+{
+    object_class_property_add_str(oc, "pci-bus", NULL,
+        acpi_generic_port_set_pci_bus);
+    object_class_property_add(oc, "node", "int", NULL,
+        acpi_generic_port_set_node, NULL, NULL);
+}
+
+static int build_acpi_generic_initiator(Object *obj, void *opaque)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    AcpiGenericInitiator *gi;
+    GArray *table_data = opaque;
+    uint8_t bus, devfn;
+    Object *o;
+
+    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
+        return 0;
+    }
+
+    gi = ACPI_GENERIC_INITIATOR(obj);
+    if (gi->node >= ms->numa_state->num_nodes) {
+        error_printf("%s: Specified node %d is invalid.\n",
+                     TYPE_ACPI_GENERIC_INITIATOR, gi->node);
+        exit(1);
+    }
+
+    o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL);
+    if (!o) {
+        error_printf("%s: Specified device must be a PCI device.\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    bus = object_property_get_uint(o, "bus", &error_fatal);
+    devfn = object_property_get_uint(o, "addr", &error_fatal);
+
+    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
+
+    return 0;
+}
+
+static int build_acpi_generic_port(Object *obj, void *opaque)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    const char *hid = "ACPI0016";
+    GArray *table_data = opaque;
+    AcpiGenericPort *gp;
+    uint32_t uid;
+    Object *o;
+
+    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_PORT)) {
+        return 0;
+    }
+
+    gp = ACPI_GENERIC_PORT(obj);
+
+    if (gp->node >= ms->numa_state->num_nodes) {
+        error_printf("%s: node %d is invalid.\n",
+                     TYPE_ACPI_GENERIC_PORT, gp->node);
+        exit(1);
+    }
+
+    o = object_resolve_path_type(gp->pci_bus, TYPE_PXB_CXL_BUS, NULL);
+    if (!o) {
+        error_printf("%s: device must be a CXL host bridge.\n",
+                     TYPE_ACPI_GENERIC_PORT);
+        exit(1);
+    }
+
+    uid = object_property_get_uint(o, "acpi_uid", &error_fatal);
+    build_srat_acpi_generic_port(table_data, gp->node, hid, uid);
+
+    return 0;
+}
+
+void build_srat_generic_pci_initiator(GArray *table_data)
+{
+    object_child_foreach_recursive(object_get_root(),
+                                   build_acpi_generic_initiator,
+                                   table_data);
+    object_child_foreach_recursive(object_get_root(), build_acpi_generic_port,
+                                   table_data);
+}
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 968b654e58..4067100dd6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1955,6 +1955,19 @@ static void build_append_srat_pci_device_handle(GArray *table_data,
     build_append_int_noprefix(table_data, 0, 12);
 }
 
+static void build_append_srat_acpi_device_handle(GArray *table_data,
+                                                 const char *hid,
+                                                 uint32_t uid)
+{
+    assert(strlen(hid) == 8);
+    /* Device Handle - ACPI */
+    for (int i = 0; i < sizeof(hid); i++) {
+        build_append_int_noprefix(table_data, hid[i], 1);
+    }
+    build_append_int_noprefix(table_data, uid, 4);
+    build_append_int_noprefix(table_data, 0, 4);
+}
+
 /*
  * ACPI spec, Revision 6.3
  * 5.2.16.6 Generic Initiator Affinity Structure
@@ -1982,6 +1995,33 @@ void build_srat_pci_generic_initiator(GArray *table_data, int node,
     build_append_int_noprefix(table_data, 0, 4);
 }
 
+/*
+ * ACPI spec, Revision 6.5
+ * 5.2.16.7 Generic Port Affinity Structure
+ *   With ACPI Device Handle.
+ */
+void build_srat_acpi_generic_port(GArray *table_data, int node,
+                                  const char *hid,
+                                  uint32_t uid)
+{
+    /* Type */
+    build_append_int_noprefix(table_data, 6, 1);
+    /* Length */
+    build_append_int_noprefix(table_data, 32, 1);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 1);
+    /* Device Handle Type: ACPI */
+    build_append_int_noprefix(table_data, 0, 1);
+    /* Proximity Domain */
+    build_append_int_noprefix(table_data, node, 4);
+    /* Device Handle */
+    build_append_srat_acpi_device_handle(table_data, hid, uid);
+    /* Flags - GP Enabled */
+    build_append_int_noprefix(table_data, 1, 4);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 4);
+}
+
 /*
  * ACPI spec 5.2.17 System Locality Distance Information Table
  * (Revision 2.0 or later)
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index f68be9ea17..b9e2776966 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -29,6 +29,7 @@
 #include "hw/boards.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_device.h"
 #include "hw/pci/pcie_host.h"
 
@@ -171,9 +172,116 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
     return 0;
 }
 
-void build_srat_generic_pci_initiator(GArray *table_data)
+typedef struct AcpiGenericPort {
+    /* private */
+    Object parent;
+
+    /* public */
+    char *pci_bus;
+    uint16_t node;
+} AcpiGenericPort;
+
+typedef struct AcpiGenericPortClass {
+    ObjectClass parent_class;
+} AcpiGenericPortClass;
+
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
+                   ACPI_GENERIC_PORT, OBJECT,
+                   { TYPE_USER_CREATABLE },
+                   { NULL })
+
+OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
+
+static void acpi_generic_port_init(Object *obj)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    gp->node = MAX_NODES;
+    gp->pci_bus = NULL;
+}
+
+static void acpi_generic_port_finalize(Object *obj)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    g_free(gp->pci_bus);
+}
+
+static void acpi_generic_port_set_pci_bus(Object *obj, const char *val,
+                                          Error **errp)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+
+    gp->pci_bus = g_strdup(val);
+}
+
+static void acpi_generic_port_set_node(Object *obj, Visitor *v,
+                                       const char *name, void *opaque,
+                                       Error **errp)
+{
+    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    if (value >= MAX_NODES) {
+        error_printf("%s: Invalid NUMA node specified\n",
+                     TYPE_ACPI_GENERIC_INITIATOR);
+        exit(1);
+    }
+
+    gp->node = value;
+}
+
+static void acpi_generic_port_class_init(ObjectClass *oc, void *data)
+{
+    object_class_property_add_str(oc, "pci-bus", NULL,
+        acpi_generic_port_set_pci_bus);
+    object_class_property_add(oc, "node", "int", NULL,
+        acpi_generic_port_set_node, NULL, NULL);
+}
+
+static int build_acpi_generic_port(Object *obj, void *opaque)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    const char *hid = "ACPI0016";
+    GArray *table_data = opaque;
+    AcpiGenericPort *gp;
+    uint32_t uid;
+    Object *o;
+
+    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_PORT)) {
+        return 0;
+    }
+
+    gp = ACPI_GENERIC_PORT(obj);
+
+    if (gp->node >= ms->numa_state->num_nodes) {
+        error_printf("%s: node %d is invalid.\n",
+                     TYPE_ACPI_GENERIC_PORT, gp->node);
+        exit(1);
+    }
+
+    o = object_resolve_path_type(gp->pci_bus, TYPE_PXB_CXL_BUS, NULL);
+    if (!o) {
+        error_printf("%s: device must be a CXL host bridge.\n",
+                     TYPE_ACPI_GENERIC_PORT);
+       exit(1);
+    }
+
+    uid = object_property_get_uint(o, "acpi_uid", &error_fatal);
+    build_srat_acpi_generic_port(table_data, gp->node, hid, uid);
+
+    return 0;
+}
+
+void build_srat_generic_affinity_structures(GArray *table_data)
 {
     object_child_foreach_recursive(object_get_root(),
                                    build_acpi_generic_initiator,
                                    table_data);
+    object_child_foreach_recursive(object_get_root(), build_acpi_generic_port,
+                                   table_data);
 }
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1a5aa0d55d..31956410ed 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -504,7 +504,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         }
     }
 
-    build_srat_generic_pci_initiator(table_data);
+    build_srat_generic_affinity_structures(table_data);
 
     if (ms->nvdimms_state->is_enabled) {
         nvdimm_build_srat(table_data);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 0d37b9238a..446adcc602 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2046,7 +2046,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
         build_srat_memory(table_data, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
     }
 
-    build_srat_generic_pci_initiator(table_data);
+    build_srat_generic_affinity_structures(table_data);
 
     /*
      * Entry is required for Windows to enable memory hotplug in OS
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 92d39b917a..1c51f3f5b6 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -38,7 +38,6 @@ DECLARE_INSTANCE_CHECKER(PXBBus, PXB_BUS,
 DECLARE_INSTANCE_CHECKER(PXBBus, PXB_PCIE_BUS,
                          TYPE_PXB_PCIE_BUS)
 
-#define TYPE_PXB_CXL_BUS "pxb-cxl-bus"
 DECLARE_INSTANCE_CHECKER(PXBBus, PXB_CXL_BUS,
                          TYPE_PXB_CXL_BUS)
 
-- 
2.43.0

Re: [PATCH v3 08/11] hw/acpi: Generic Port Affinity Structure support

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:16 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> These are very similar to the recently added Generic Initiators
> but instead of representing an initiator of memory traffic they
> represent an edge point beyond which may lie either targets or
> initiators.  Here we add these ports such that they may
> be targets of hmat_lb records to describe the latency and
> bandwidth from host side initiators to the port.  A discoverable
> mechanism such as UEFI CDAT read from CXL devices and switches
> is used to discover the remainder of the path, and the OS can build
> up full latency and bandwidth numbers as need for work and data
> placement decisions.
> 
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v3: Move to hw/acpi/pci.c
>     Rename the funciton to actually registers both types
>     of generic nodes to reflect it isn't GI only.
>     Note that the qom part is unchanged and other changes are mostly
>     code movement so I've kept Markus' Ack.
> ---
>  qapi/qom.json                            |  34 ++++
>  include/hw/acpi/acpi_generic_initiator.h |  35 ++++
>  include/hw/acpi/aml-build.h              |   4 +
>  include/hw/acpi/pci.h                    |   3 +-
>  include/hw/pci/pci_bridge.h              |   1 +
>  hw/acpi/acpi_generic_initiator.c         | 216 +++++++++++++++++++++++
>  hw/acpi/aml-build.c                      |  40 +++++
>  hw/acpi/pci.c                            | 110 +++++++++++-
>  hw/arm/virt-acpi-build.c                 |   2 +-
>  hw/i386/acpi-build.c                     |   2 +-
>  hw/pci-bridge/pci_expander_bridge.c      |   1 -
>  11 files changed, 443 insertions(+), 5 deletions(-)

this is quite large patch, is it possible to split into
a set of smaller patches?

> diff --git a/qapi/qom.json b/qapi/qom.json
> index 8bd299265e..8fa6bbd9a7 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -826,6 +826,38 @@
>    'data': { 'pci-dev': 'str',
>              'node': 'uint32' } }
>  
> +##
> +# @AcpiGenericPortProperties:
> +#
> +# Properties for acpi-generic-port objects.
> +#
> +# @pci-bus: QOM path of the PCI bus of the hostbridge associated with
> +#     this SRAT Generic Port Affinity Structure.  This is the same as
> +#     the bus parameter for the root ports attached to this host
> +#     bridge.  The resulting SRAT Generic Port Affinity Structure will
> +#     refer to the ACPI object in DSDT that represents the host bridge
> +#     (e.g.  ACPI0016 for CXL host bridges).  See ACPI 6.5 Section
> +#     5.2.16.7 for more information.
> +#
> +# @node: Similar to a NUMA node ID, but instead of providing a
> +#     reference point used for defining NUMA distances and access
> +#     characteristics to memory or from an initiator (e.g. CPU), this
> +#     node defines the boundary point between non-discoverable system
> +#     buses which must be described by firmware, and a discoverable
> +#     bus.  NUMA distances and access characteristics are defined to
> +#     and from that point.  For system software to establish full
> +#     initiator to target characteristics this information must be
> +#     combined with information retrieved from the discoverable part
> +#     of the path.  An example would use CDAT (see UEFI.org)
> +#     information read from devices and switches in conjunction with
> +#     link characteristics read from PCIe Configuration space.
> +#
> +# Since: 9.1
> +##
> +{ 'struct': 'AcpiGenericPortProperties',
> +  'data': { 'pci-bus': 'str',
> +            'node': 'uint32' } }
> +
>  ##
>  # @RngProperties:
>  #
> @@ -1019,6 +1051,7 @@
>  { 'enum': 'ObjectType',
>    'data': [
>      'acpi-generic-initiator',
> +    'acpi-generic-port',
>      'authz-list',
>      'authz-listfile',
>      'authz-pam',
> @@ -1092,6 +1125,7 @@
>    'discriminator': 'qom-type',
>    'data': {
>        'acpi-generic-initiator':     'AcpiGenericInitiatorProperties',
> +      'acpi-generic-port':          'AcpiGenericPortProperties',
>        'authz-list':                 'AuthZListProperties',
>        'authz-listfile':             'AuthZListFileProperties',
>        'authz-pam':                  'AuthZPAMProperties',
> diff --git a/include/hw/acpi/acpi_generic_initiator.h b/include/hw/acpi/acpi_generic_initiator.h
> new file mode 100644
> index 0000000000..92a39ad303
> --- /dev/null
> +++ b/include/hw/acpi/acpi_generic_initiator.h
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#ifndef ACPI_GENERIC_INITIATOR_H
> +#define ACPI_GENERIC_INITIATOR_H
> +
> +#include "qom/object_interfaces.h"
> +
> +#define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
> +
> +typedef struct AcpiGenericInitiator {
> +    /* private */
> +    Object parent;
> +
> +    /* public */
> +    char *pci_dev;
> +    uint16_t node;
> +} AcpiGenericInitiator;
> +
> +#define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
> +
> +typedef struct AcpiGenericPort {
> +    /* private */
> +    Object parent;
> +
> +    /* public */
> +    char *pci_bus;
> +    uint16_t node;
> +} AcpiGenericPort;
> +
> +void build_srat_generic_pci_initiator(GArray *table_data);
> +
> +#endif
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 9ba3a21c13..fb8cf6c415 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -490,6 +490,10 @@ void build_srat_pci_generic_initiator(GArray *table_date, int node,
>                                        uint16_t segment, uint8_t bus,
>                                        uint8_t devfn);
>  
> +void build_srat_acpi_generic_port(GArray *table_data, int node,
> +                                  const char *hid,
> +                                  uint32_t uid);
> +
>  void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>                  const char *oem_id, const char *oem_table_id);
>  
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> index 9adf1887da..182095d104 100644
> --- a/include/hw/acpi/pci.h
> +++ b/include/hw/acpi/pci.h
> @@ -43,7 +43,8 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
>  void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope);
>  
>  #define TYPE_ACPI_GENERIC_INITIATOR "acpi-generic-initiator"
> +#define TYPE_ACPI_GENERIC_PORT "acpi-generic-port"
>  
> -void build_srat_generic_pci_initiator(GArray *table_data);
> +void build_srat_generic_affinity_structures(GArray *table_data);
>  
>  #endif
> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> index 5cd452115a..5456e24883 100644
> --- a/include/hw/pci/pci_bridge.h
> +++ b/include/hw/pci/pci_bridge.h
> @@ -102,6 +102,7 @@ typedef struct PXBPCIEDev {
>      PXBDev parent_obj;
>  } PXBPCIEDev;
>  
> +#define TYPE_PXB_CXL_BUS "pxb-cxl-bus"
>  #define TYPE_PXB_DEV "pxb"
>  OBJECT_DECLARE_SIMPLE_TYPE(PXBDev, PXB_DEV)
>  
> diff --git a/hw/acpi/acpi_generic_initiator.c b/hw/acpi/acpi_generic_initiator.c
> new file mode 100644
> index 0000000000..8491aaf5ec
> --- /dev/null
> +++ b/hw/acpi/acpi_generic_initiator.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi_generic_initiator.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/boards.h"
> +#include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_device.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +
> +typedef struct AcpiGenericInitiatorClass {
> +    ObjectClass parent_class;
> +} AcpiGenericInitiatorClass;
> +
> +OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, acpi_generic_initiator,
> +                   ACPI_GENERIC_INITIATOR, OBJECT,
> +                   { TYPE_USER_CREATABLE },
> +                   { NULL })
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
> +
> +static void acpi_generic_initiator_init(Object *obj)
> +{
> +    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
> +
> +    gi->node = MAX_NODES;
> +    gi->pci_dev = NULL;
> +}
> +
> +static void acpi_generic_initiator_finalize(Object *obj)
> +{
> +    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
> +
> +    g_free(gi->pci_dev);
> +}
> +
> +static void acpi_generic_initiator_set_pci_device(Object *obj, const char *val,
> +                                                  Error **errp)
> +{
> +    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
> +
> +    gi->pci_dev = g_strdup(val);
> +}
> +
> +static void acpi_generic_initiator_set_node(Object *obj, Visitor *v,
> +                                            const char *name, void *opaque,
> +                                            Error **errp)
> +{
> +    AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    if (value >= MAX_NODES) {
> +        error_printf("%s: Invalid NUMA node specified\n",
> +                     TYPE_ACPI_GENERIC_INITIATOR);
> +        exit(1);
> +    }
> +
> +    gi->node = value;
> +    ms->numa_state->nodes[gi->node].has_gi = true;
> +}
> +
> +static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
> +{
> +    object_class_property_add_str(oc, "pci-dev", NULL,
> +        acpi_generic_initiator_set_pci_device);
> +    object_class_property_add(oc, "node", "int", NULL,
> +        acpi_generic_initiator_set_node, NULL, NULL);
> +}
> +
> +typedef struct AcpiGenericPortClass {
> +    ObjectClass parent_class;
> +} AcpiGenericPortClass;
> +
> +OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
> +                   ACPI_GENERIC_PORT, OBJECT,
> +                   { TYPE_USER_CREATABLE },
> +                   { NULL })
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
> +
> +static void acpi_generic_port_init(Object *obj)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    gp->node = MAX_NODES;
> +    gp->pci_bus = NULL;
> +}
> +
> +static void acpi_generic_port_finalize(Object *obj)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    g_free(gp->pci_bus);
> +}
> +
> +static void acpi_generic_port_set_pci_bus(Object *obj, const char *val,
> +                                          Error **errp)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    gp->pci_bus = g_strdup(val);
> +}
> +
> +static void acpi_generic_port_set_node(Object *obj, Visitor *v,
> +                                       const char *name, void *opaque,
> +                                       Error **errp)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    if (value >= MAX_NODES) {
> +        error_printf("%s: Invalid NUMA node specified\n",
> +                     TYPE_ACPI_GENERIC_INITIATOR);
> +        exit(1);
> +    }
> +
> +    gp->node = value;
> +}
> +
> +static void acpi_generic_port_class_init(ObjectClass *oc, void *data)
> +{
> +    object_class_property_add_str(oc, "pci-bus", NULL,
> +        acpi_generic_port_set_pci_bus);
> +    object_class_property_add(oc, "node", "int", NULL,
> +        acpi_generic_port_set_node, NULL, NULL);
> +}
> +
> +static int build_acpi_generic_initiator(Object *obj, void *opaque)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    AcpiGenericInitiator *gi;
> +    GArray *table_data = opaque;
> +    uint8_t bus, devfn;
> +    Object *o;
> +
> +    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
> +        return 0;
> +    }
> +
> +    gi = ACPI_GENERIC_INITIATOR(obj);
> +    if (gi->node >= ms->numa_state->num_nodes) {
> +        error_printf("%s: Specified node %d is invalid.\n",
> +                     TYPE_ACPI_GENERIC_INITIATOR, gi->node);
> +        exit(1);
> +    }
> +
> +    o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL);
> +    if (!o) {
> +        error_printf("%s: Specified device must be a PCI device.\n",
> +                     TYPE_ACPI_GENERIC_INITIATOR);
> +        exit(1);
> +    }
> +
> +    bus = object_property_get_uint(o, "bus", &error_fatal);
> +    devfn = object_property_get_uint(o, "addr", &error_fatal);
> +
> +    build_srat_pci_generic_initiator(table_data, gi->node, 0, bus, devfn);
> +
> +    return 0;
> +}
> +
> +static int build_acpi_generic_port(Object *obj, void *opaque)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const char *hid = "ACPI0016";
> +    GArray *table_data = opaque;
> +    AcpiGenericPort *gp;
> +    uint32_t uid;
> +    Object *o;
> +
> +    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_PORT)) {
> +        return 0;
> +    }
> +
> +    gp = ACPI_GENERIC_PORT(obj);
> +
> +    if (gp->node >= ms->numa_state->num_nodes) {
> +        error_printf("%s: node %d is invalid.\n",
> +                     TYPE_ACPI_GENERIC_PORT, gp->node);
> +        exit(1);
> +    }
> +
> +    o = object_resolve_path_type(gp->pci_bus, TYPE_PXB_CXL_BUS, NULL);
> +    if (!o) {
> +        error_printf("%s: device must be a CXL host bridge.\n",
> +                     TYPE_ACPI_GENERIC_PORT);
> +        exit(1);
> +    }
> +
> +    uid = object_property_get_uint(o, "acpi_uid", &error_fatal);
> +    build_srat_acpi_generic_port(table_data, gp->node, hid, uid);
> +
> +    return 0;
> +}
> +
> +void build_srat_generic_pci_initiator(GArray *table_data)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   build_acpi_generic_initiator,
> +                                   table_data);
> +    object_child_foreach_recursive(object_get_root(), build_acpi_generic_port,
> +                                   table_data);
> +}
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 968b654e58..4067100dd6 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1955,6 +1955,19 @@ static void build_append_srat_pci_device_handle(GArray *table_data,
>      build_append_int_noprefix(table_data, 0, 12);
>  }
>  
> +static void build_append_srat_acpi_device_handle(GArray *table_data,
> +                                                 const char *hid,
> +                                                 uint32_t uid)
> +{
> +    assert(strlen(hid) == 8);
> +    /* Device Handle - ACPI */
> +    for (int i = 0; i < sizeof(hid); i++) {
> +        build_append_int_noprefix(table_data, hid[i], 1);
> +    }
> +    build_append_int_noprefix(table_data, uid, 4);
> +    build_append_int_noprefix(table_data, 0, 4);
> +}
> +
>  /*
>   * ACPI spec, Revision 6.3
>   * 5.2.16.6 Generic Initiator Affinity Structure
> @@ -1982,6 +1995,33 @@ void build_srat_pci_generic_initiator(GArray *table_data, int node,
>      build_append_int_noprefix(table_data, 0, 4);
>  }
>  
> +/*
> + * ACPI spec, Revision 6.5
> + * 5.2.16.7 Generic Port Affinity Structure
> + *   With ACPI Device Handle.
> + */
> +void build_srat_acpi_generic_port(GArray *table_data, int node,
> +                                  const char *hid,
> +                                  uint32_t uid)
> +{
> +    /* Type */
> +    build_append_int_noprefix(table_data, 6, 1);
> +    /* Length */
> +    build_append_int_noprefix(table_data, 32, 1);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 1);
> +    /* Device Handle Type: ACPI */
> +    build_append_int_noprefix(table_data, 0, 1);
> +    /* Proximity Domain */
> +    build_append_int_noprefix(table_data, node, 4);
> +    /* Device Handle */
> +    build_append_srat_acpi_device_handle(table_data, hid, uid);
> +    /* Flags - GP Enabled */
> +    build_append_int_noprefix(table_data, 1, 4);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 4);
> +}
> +
>  /*
>   * ACPI spec 5.2.17 System Locality Distance Information Table
>   * (Revision 2.0 or later)
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index f68be9ea17..b9e2776966 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -29,6 +29,7 @@
>  #include "hw/boards.h"
>  #include "hw/acpi/aml-build.h"
>  #include "hw/acpi/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_device.h"
>  #include "hw/pci/pcie_host.h"
>  
> @@ -171,9 +172,116 @@ static int build_acpi_generic_initiator(Object *obj, void *opaque)
>      return 0;
>  }
>  
> -void build_srat_generic_pci_initiator(GArray *table_data)
> +typedef struct AcpiGenericPort {
> +    /* private */
> +    Object parent;
> +
> +    /* public */
> +    char *pci_bus;
> +    uint16_t node;
> +} AcpiGenericPort;
> +
> +typedef struct AcpiGenericPortClass {
> +    ObjectClass parent_class;
> +} AcpiGenericPortClass;
> +
> +OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericPort, acpi_generic_port,
> +                   ACPI_GENERIC_PORT, OBJECT,
> +                   { TYPE_USER_CREATABLE },
> +                   { NULL })
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericPort, ACPI_GENERIC_PORT)
> +
> +static void acpi_generic_port_init(Object *obj)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    gp->node = MAX_NODES;
> +    gp->pci_bus = NULL;
> +}
> +
> +static void acpi_generic_port_finalize(Object *obj)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    g_free(gp->pci_bus);
> +}
> +
> +static void acpi_generic_port_set_pci_bus(Object *obj, const char *val,
> +                                          Error **errp)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +
> +    gp->pci_bus = g_strdup(val);
> +}
> +
> +static void acpi_generic_port_set_node(Object *obj, Visitor *v,
> +                                       const char *name, void *opaque,
> +                                       Error **errp)
> +{
> +    AcpiGenericPort *gp = ACPI_GENERIC_PORT(obj);
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    if (value >= MAX_NODES) {
> +        error_printf("%s: Invalid NUMA node specified\n",
> +                     TYPE_ACPI_GENERIC_INITIATOR);
> +        exit(1);
> +    }
> +
> +    gp->node = value;
> +}
> +
> +static void acpi_generic_port_class_init(ObjectClass *oc, void *data)
> +{
> +    object_class_property_add_str(oc, "pci-bus", NULL,
> +        acpi_generic_port_set_pci_bus);
> +    object_class_property_add(oc, "node", "int", NULL,
> +        acpi_generic_port_set_node, NULL, NULL);
> +}
> +
> +static int build_acpi_generic_port(Object *obj, void *opaque)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const char *hid = "ACPI0016";
> +    GArray *table_data = opaque;
> +    AcpiGenericPort *gp;
> +    uint32_t uid;
> +    Object *o;
> +
> +    if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_PORT)) {
> +        return 0;
> +    }
> +
> +    gp = ACPI_GENERIC_PORT(obj);
> +
> +    if (gp->node >= ms->numa_state->num_nodes) {
> +        error_printf("%s: node %d is invalid.\n",
> +                     TYPE_ACPI_GENERIC_PORT, gp->node);
> +        exit(1);
> +    }
> +
> +    o = object_resolve_path_type(gp->pci_bus, TYPE_PXB_CXL_BUS, NULL);
> +    if (!o) {
> +        error_printf("%s: device must be a CXL host bridge.\n",
> +                     TYPE_ACPI_GENERIC_PORT);
> +       exit(1);
> +    }
> +
> +    uid = object_property_get_uint(o, "acpi_uid", &error_fatal);
> +    build_srat_acpi_generic_port(table_data, gp->node, hid, uid);
> +
> +    return 0;
> +}
> +
> +void build_srat_generic_affinity_structures(GArray *table_data)
>  {
>      object_child_foreach_recursive(object_get_root(),
>                                     build_acpi_generic_initiator,
>                                     table_data);
> +    object_child_foreach_recursive(object_get_root(), build_acpi_generic_port,
> +                                   table_data);
>  }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 1a5aa0d55d..31956410ed 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -504,7 +504,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          }
>      }
>  
> -    build_srat_generic_pci_initiator(table_data);
> +    build_srat_generic_affinity_structures(table_data);
>  
>      if (ms->nvdimms_state->is_enabled) {
>          nvdimm_build_srat(table_data);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 0d37b9238a..446adcc602 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2046,7 +2046,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>          build_srat_memory(table_data, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>      }
>  
> -    build_srat_generic_pci_initiator(table_data);
> +    build_srat_generic_affinity_structures(table_data);
>  
>      /*
>       * Entry is required for Windows to enable memory hotplug in OS
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 92d39b917a..1c51f3f5b6 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -38,7 +38,6 @@ DECLARE_INSTANCE_CHECKER(PXBBus, PXB_BUS,
>  DECLARE_INSTANCE_CHECKER(PXBBus, PXB_PCIE_BUS,
>                           TYPE_PXB_PCIE_BUS)
>  
> -#define TYPE_PXB_CXL_BUS "pxb-cxl-bus"
>  DECLARE_INSTANCE_CHECKER(PXBBus, PXB_CXL_BUS,
>                           TYPE_PXB_CXL_BUS)
>  

Re: [PATCH v3 08/11] hw/acpi: Generic Port Affinity Structure support

[Jonathan Cameron via]

On Mon, 1 Jul 2024 10:52:19 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 20 Jun 2024 17:03:16 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > These are very similar to the recently added Generic Initiators
> > but instead of representing an initiator of memory traffic they
> > represent an edge point beyond which may lie either targets or
> > initiators.  Here we add these ports such that they may
> > be targets of hmat_lb records to describe the latency and
> > bandwidth from host side initiators to the port.  A discoverable
> > mechanism such as UEFI CDAT read from CXL devices and switches
> > is used to discover the remainder of the path, and the OS can build
> > up full latency and bandwidth numbers as need for work and data
> > placement decisions.
> > 
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v3: Move to hw/acpi/pci.c
> >     Rename the funciton to actually registers both types
> >     of generic nodes to reflect it isn't GI only.
> >     Note that the qom part is unchanged and other changes are mostly
> >     code movement so I've kept Markus' Ack.
> > ---
> >  qapi/qom.json                            |  34 ++++
> >  include/hw/acpi/acpi_generic_initiator.h |  35 ++++
> >  include/hw/acpi/aml-build.h              |   4 +
> >  include/hw/acpi/pci.h                    |   3 +-
> >  include/hw/pci/pci_bridge.h              |   1 +
> >  hw/acpi/acpi_generic_initiator.c         | 216 +++++++++++++++++++++++
> >  hw/acpi/aml-build.c                      |  40 +++++
> >  hw/acpi/pci.c                            | 110 +++++++++++-
> >  hw/arm/virt-acpi-build.c                 |   2 +-
> >  hw/i386/acpi-build.c                     |   2 +-
> >  hw/pci-bridge/pci_expander_bridge.c      |   1 -
> >  11 files changed, 443 insertions(+), 5 deletions(-)  
> 
> this is quite large patch, is it possible to split into
> a set of smaller patches?

Oops.

It's bigger that it should due to a messed up rebase.
The acpi_generic_initator.c/.h shouldn't exist!

With those gone will be much more manageable.

[PATCH v3 09/11] bios-tables-test: Allow for new acpihmat-generic-x test data.

[Jonathan Cameron via]

The test to be added exercises many corners of the SRAT and HMAT table
generation.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: No change
---
 tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
 tests/data/acpi/q35/APIC.acpihmat-generic-x | 0
 tests/data/acpi/q35/CEDT.acpihmat-generic-x | 0
 tests/data/acpi/q35/DSDT.acpihmat-generic-x | 0
 tests/data/acpi/q35/HMAT.acpihmat-generic-x | 0
 tests/data/acpi/q35/SRAT.acpihmat-generic-x | 0
 6 files changed, 5 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..a5aa801c99 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,6 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/APIC.acpihmat-generic-x",
+"tests/data/acpi/q35/CEDT.acpihmat-generic-x",
+"tests/data/acpi/q35/DSDT.acpihmat-generic-x",
+"tests/data/acpi/q35/HMAT.acpihmat-generic-x",
+"tests/data/acpi/q35/SRAT.acpihmat-generic-x",
diff --git a/tests/data/acpi/q35/APIC.acpihmat-generic-x b/tests/data/acpi/q35/APIC.acpihmat-generic-x
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/CEDT.acpihmat-generic-x b/tests/data/acpi/q35/CEDT.acpihmat-generic-x
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/DSDT.acpihmat-generic-x b/tests/data/acpi/q35/DSDT.acpihmat-generic-x
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/HMAT.acpihmat-generic-x b/tests/data/acpi/q35/HMAT.acpihmat-generic-x
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/SRAT.acpihmat-generic-x b/tests/data/acpi/q35/SRAT.acpihmat-generic-x
new file mode 100644
index 0000000000..e69de29bb2
-- 
2.43.0

Re: [PATCH v3 09/11] bios-tables-test: Allow for new acpihmat-generic-x test data.

[Igor Mammedov]

On Thu, 20 Jun 2024 17:03:17 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> The test to be added exercises many corners of the SRAT and HMAT table
                                      ^^^^ did you mean 'corner cases"?
> generation.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v3: No change
> ---
>  tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
>  tests/data/acpi/q35/APIC.acpihmat-generic-x | 0
>  tests/data/acpi/q35/CEDT.acpihmat-generic-x | 0
>  tests/data/acpi/q35/DSDT.acpihmat-generic-x | 0
>  tests/data/acpi/q35/HMAT.acpihmat-generic-x | 0
>  tests/data/acpi/q35/SRAT.acpihmat-generic-x | 0
>  6 files changed, 5 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..a5aa801c99 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,6 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/APIC.acpihmat-generic-x",
> +"tests/data/acpi/q35/CEDT.acpihmat-generic-x",
> +"tests/data/acpi/q35/DSDT.acpihmat-generic-x",
> +"tests/data/acpi/q35/HMAT.acpihmat-generic-x",
> +"tests/data/acpi/q35/SRAT.acpihmat-generic-x",
> diff --git a/tests/data/acpi/q35/APIC.acpihmat-generic-x b/tests/data/acpi/q35/APIC.acpihmat-generic-x
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/CEDT.acpihmat-generic-x b/tests/data/acpi/q35/CEDT.acpihmat-generic-x
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/DSDT.acpihmat-generic-x b/tests/data/acpi/q35/DSDT.acpihmat-generic-x
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/HMAT.acpihmat-generic-x b/tests/data/acpi/q35/HMAT.acpihmat-generic-x
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/SRAT.acpihmat-generic-x b/tests/data/acpi/q35/SRAT.acpihmat-generic-x
> new file mode 100644
> index 0000000000..e69de29bb2

Re: [PATCH v3 09/11] bios-tables-test: Allow for new acpihmat-generic-x test data.

[Igor Mammedov]

On Thu, 27 Jun 2024 14:51:55 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 20 Jun 2024 17:03:17 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > The test to be added exercises many corners of the SRAT and HMAT table  
>                                       ^^^^ did you mean 'corner cases"?
> > generation.

another issue is that this and later patches will conflict with
risc-v acpi tests, that along the way change directory structure
of expected tables.

Perhaps, it's better to rebase this series on top of that. 


> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v3: No change
> > ---
> >  tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
> >  tests/data/acpi/q35/APIC.acpihmat-generic-x | 0
> >  tests/data/acpi/q35/CEDT.acpihmat-generic-x | 0
> >  tests/data/acpi/q35/DSDT.acpihmat-generic-x | 0
> >  tests/data/acpi/q35/HMAT.acpihmat-generic-x | 0
> >  tests/data/acpi/q35/SRAT.acpihmat-generic-x | 0
> >  6 files changed, 5 insertions(+)
> > 
> > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > index dfb8523c8b..a5aa801c99 100644
> > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > @@ -1 +1,6 @@
> >  /* List of comma-separated changed AML files to ignore */
> > +"tests/data/acpi/q35/APIC.acpihmat-generic-x",
> > +"tests/data/acpi/q35/CEDT.acpihmat-generic-x",
> > +"tests/data/acpi/q35/DSDT.acpihmat-generic-x",
> > +"tests/data/acpi/q35/HMAT.acpihmat-generic-x",
> > +"tests/data/acpi/q35/SRAT.acpihmat-generic-x",
> > diff --git a/tests/data/acpi/q35/APIC.acpihmat-generic-x b/tests/data/acpi/q35/APIC.acpihmat-generic-x
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/data/acpi/q35/CEDT.acpihmat-generic-x b/tests/data/acpi/q35/CEDT.acpihmat-generic-x
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/data/acpi/q35/DSDT.acpihmat-generic-x b/tests/data/acpi/q35/DSDT.acpihmat-generic-x
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/data/acpi/q35/HMAT.acpihmat-generic-x b/tests/data/acpi/q35/HMAT.acpihmat-generic-x
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/data/acpi/q35/SRAT.acpihmat-generic-x b/tests/data/acpi/q35/SRAT.acpihmat-generic-x
> > new file mode 100644
> > index 0000000000..e69de29bb2  
> 

[PATCH v3 10/11] bios-tables-test: Add complex SRAT / HMAT test for GI GP

[Jonathan Cameron via]

Add a test with 6 nodes to exercise most interesting corner cases of SRAT
and HMAT generation including the new Generic Initiator and Generic Port
Affinity structures.  More details of the set up in the following patch
adding the table data.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v3: Ensure we have recognizable values in the PCI devfn by
    using a multifunction device.  virtio-pci-rng doesn't support
    multifunctional usage, so switch to pci-testdev for this purpose.
---
 tests/qtest/bios-tables-test.c | 96 ++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index d1ff4db7a2..1fb8e3729d 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1862,6 +1862,100 @@ static void test_acpi_q35_tcg_acpi_hmat_noinitiator(void)
     free_test_data(&data);
 }
 
+/* Test intended to hit corner cases of SRAT and HMAT */
+static void test_acpi_q35_tcg_acpi_hmat_generic_x(void)
+{
+    test_data data = {};
+
+    data.machine = MACHINE_Q35;
+    data.variant = ".acpihmat-generic-x";
+    test_acpi_one(" -machine hmat=on,cxl=on"
+                  " -smp 3,sockets=3"
+                  " -m 128M,maxmem=384M,slots=2"
+                  " -device pcie-root-port,chassis=1,id=pci.1"
+                  " -device pci-testdev,bus=pci.1,"
+                  "multifunction=on,addr=00.0"
+                  " -device pci-testdev,bus=pci.1,addr=00.1"
+                  " -device pci-testdev,bus=pci.1,id=gidev,addr=00.2"
+                  " -device pxb-cxl,bus_nr=64,bus=pcie.0,id=cxl.1"
+                  " -object memory-backend-ram,size=64M,id=ram0"
+                  " -object memory-backend-ram,size=64M,id=ram1"
+                  " -numa node,nodeid=0,cpus=0,memdev=ram0"
+                  " -numa node,nodeid=1"
+                  " -object acpi-generic-initiator,id=gi0,pci-dev=gidev,node=1"
+                  " -numa node,nodeid=2"
+                  " -object acpi-generic-port,id=gp0,pci-bus=cxl.1,node=2"
+                  " -numa node,nodeid=3,cpus=1"
+                  " -numa node,nodeid=4,memdev=ram1"
+                  " -numa node,nodeid=5,cpus=2"
+                  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+                  "data-type=access-latency,latency=10"
+                  " -numa hmat-lb,initiator=0,target=0,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=800M"
+                  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+                  "data-type=access-latency,latency=100"
+                  " -numa hmat-lb,initiator=0,target=2,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=0,target=4,hierarchy=memory,"
+                  "data-type=access-latency,latency=100"
+                  " -numa hmat-lb,initiator=0,target=4,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=0,target=5,hierarchy=memory,"
+                  "data-type=access-latency,latency=200"
+                  " -numa hmat-lb,initiator=0,target=5,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=400M"
+                  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+                  "data-type=access-latency,latency=500"
+                  " -numa hmat-lb,initiator=1,target=0,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=100M"
+                  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+                  "data-type=access-latency,latency=50"
+                  " -numa hmat-lb,initiator=1,target=2,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=400M"
+                  " -numa hmat-lb,initiator=1,target=4,hierarchy=memory,"
+                  "data-type=access-latency,latency=50"
+                  " -numa hmat-lb,initiator=1,target=4,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=800M"
+                  " -numa hmat-lb,initiator=1,target=5,hierarchy=memory,"
+                  "data-type=access-latency,latency=500"
+                  " -numa hmat-lb,initiator=1,target=5,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=100M"
+                  " -numa hmat-lb,initiator=3,target=0,hierarchy=memory,"
+                  "data-type=access-latency,latency=20"
+                  " -numa hmat-lb,initiator=3,target=0,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=400M"
+                  " -numa hmat-lb,initiator=3,target=2,hierarchy=memory,"
+                  "data-type=access-latency,latency=80"
+                  " -numa hmat-lb,initiator=3,target=2,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=3,target=4,hierarchy=memory,"
+                  "data-type=access-latency,latency=80"
+                  " -numa hmat-lb,initiator=3,target=4,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=3,target=5,hierarchy=memory,"
+                  "data-type=access-latency,latency=20"
+                  " -numa hmat-lb,initiator=3,target=5,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=400M"
+                  " -numa hmat-lb,initiator=5,target=0,hierarchy=memory,"
+                  "data-type=access-latency,latency=20"
+                  " -numa hmat-lb,initiator=5,target=0,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=400M"
+                  " -numa hmat-lb,initiator=5,target=2,hierarchy=memory,"
+                  "data-type=access-latency,latency=80"
+                  " -numa hmat-lb,initiator=5,target=4,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=5,target=4,hierarchy=memory,"
+                  "data-type=access-latency,latency=80"
+                  " -numa hmat-lb,initiator=5,target=2,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=200M"
+                  " -numa hmat-lb,initiator=5,target=5,hierarchy=memory,"
+                  "data-type=access-latency,latency=10"
+                  " -numa hmat-lb,initiator=5,target=5,hierarchy=memory,"
+                  "data-type=access-bandwidth,bandwidth=800M",
+                  &data);
+    free_test_data(&data);
+}
+
 #ifdef CONFIG_POSIX
 static void test_acpi_erst(const char *machine)
 {
@@ -2304,6 +2398,8 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
             qtest_add_func("acpi/q35/acpihmat-noinitiator",
                            test_acpi_q35_tcg_acpi_hmat_noinitiator);
+            qtest_add_func("acpi/q35/acpihmat-genericx",
+                           test_acpi_q35_tcg_acpi_hmat_generic_x);
 
             /* i386 does not support memory hotplug */
             if (strcmp(arch, "i386")) {
-- 
2.43.0

[PATCH v3 11/11] bios-tables-test: Add data for complex numa test (GI, GP etc)

[Jonathan Cameron via]

Given this is a new configuration, there are affects on APIC, CEDT
and DSDT, but the key elements are in SRAT (plus related data in
HMAT).  The configuration has node to exercise many different combinations.

0) CPUs + Memory
1) GI only
2) GP only
3) CPUS only
4) Memory only
5) CPUs + HP memory

GI node, GP Node, Memory only node, hotplug memory
only node, latency and bandwidth such that in Linux Access0
(any initiator) and Access1 (CPU initiators only) given different
answers.  Following cropped to remove details of each entry.

[000h 0000 004h]                   Signature : "SRAT"    [System Resource Affinity Table]
...
[030h 0048 001h]               Subtable Type : 00 [Processor Local APIC/SAPIC Affinity]
...
[032h 0050 001h]     Proximity Domain Low(8) : 00
[033h 0051 001h]                     Apic ID : 00
...
[040h 0064 001h]               Subtable Type : 00 [Processor Local APIC/SAPIC Affinity]
...
[042h 0066 001h]     Proximity Domain Low(8) : 03
[043h 0067 001h]                     Apic ID : 01
...
[050h 0080 001h]               Subtable Type : 00 [Processor Local APIC/SAPIC Affinity]
...
[052h 0082 001h]     Proximity Domain Low(8) : 05
[053h 0083 001h]                     Apic ID : 02
...
[060h 0096 001h]               Subtable Type : 01 [Memory Affinity]
...
[062h 0098 004h]            Proximity Domain : 00000000
...
[068h 0104 008h]                Base Address : 0000000000000000
[070h 0112 008h]              Address Length : 00000000000A0000
...
[088h 0136 001h]               Subtable Type : 01 [Memory Affinity]
...
[08Ah 0138 004h]            Proximity Domain : 00000000
...
[090h 0144 008h]                Base Address : 0000000000100000
[098h 0152 008h]              Address Length : 0000000003F00000
...
[0B0h 0176 001h]               Subtable Type : 01 [Memory Affinity]
...
[0B2h 0178 004h]            Proximity Domain : 00000004
...
[0B8h 0184 008h]                Base Address : 0000000004000000
[0C0h 0192 008h]              Address Length : 0000000004000000
... some zero length entries follow...

[1A0h 0416 001h]               Subtable Type : 05 [Generic Initiator Affinity]
[1A1h 0417 001h]                      Length : 20

[1A2h 0418 001h]                   Reserved1 : 00
[1A3h 0419 001h]          Device Handle Type : 01
[1A4h 0420 004h]            Proximity Domain : 00000001
[1A8h 0424 010h]               Device Handle : 00 00 01 02 00 00 00 00 00 00 00 00 00 00 00 00
[1B8h 0440 004h]       Flags (decoded below) : 00000001
                                     Enabled : 1
                  Architectural Transactions : 0
[1BCh 0444 004h]                   Reserved2 : 00000000

[1C0h 0448 001h]               Subtable Type : 06 [Generic Port Affinity]
[1C1h 0449 001h]                      Length : 20

[1C2h 0450 001h]                   Reserved1 : 00
[1C3h 0451 001h]          Device Handle Type : 00
[1C4h 0452 004h]            Proximity Domain : 00000002
[1C8h 0456 010h]               Device Handle : 41 43 50 49 30 30 31 36 40 00 00 00 00 00 00 00
[1D8h 0472 004h]       Flags (decoded below) : 00000001
                                     Enabled : 1
                  Architectural Transactions : 0
[1DCh 0476 004h]                   Reserved2 : 00000000

[1E0h 0480 001h]               Subtable Type : 01 [Memory Affinity]
...
[1E2h 0482 004h]            Proximity Domain : 00000005
...
[1E8h 0488 008h]                Base Address : 0000000100000000
[1F0h 0496 008h]              Address Length : 0000000090000000

Example block from HMAT:
[0F0h 0240 002h]              Structure Type : 0001 [System Locality Latency and Bandwidth Information]
[0F2h 0242 002h]                    Reserved : 0000
[0F4h 0244 004h]                      Length : 00000078
[0F8h 0248 001h]       Flags (decoded below) : 00
                            Memory Hierarchy : 0
                   Use Minimum Transfer Size : 0
                    Non-sequential Transfers : 0
[0F9h 0249 001h]                   Data Type : 03
[0FAh 0250 001h]       Minimum Transfer Size : 00
[0FBh 0251 001h]                   Reserved1 : 00
[0FCh 0252 004h] Initiator Proximity Domains # : 00000004
[100h 0256 004h]  Target Proximity Domains # : 00000006
[104h 0260 004h]                   Reserved2 : 00000000
[108h 0264 008h]             Entry Base Unit : 0000000000000004
[110h 0272 004h] Initiator Proximity Domain List : 00000000
[114h 0276 004h] Initiator Proximity Domain List : 00000001
[118h 0280 004h] Initiator Proximity Domain List : 00000003
[11Ch 0284 004h] Initiator Proximity Domain List : 00000005
[120h 0288 004h] Target Proximity Domain List : 00000000
[124h 0292 004h] Target Proximity Domain List : 00000001
[128h 0296 004h] Target Proximity Domain List : 00000002
[12Ch 0300 004h] Target Proximity Domain List : 00000003
[130h 0304 004h] Target Proximity Domain List : 00000004
[134h 0308 004h] Target Proximity Domain List : 00000005
[138h 0312 002h]                       Entry : 00C8
[13Ah 0314 002h]                       Entry : 0000
[13Ch 0316 002h]                       Entry : 0032
[13Eh 0318 002h]                       Entry : 0000
[140h 0320 002h]                       Entry : 0032
[142h 0322 002h]                       Entry : 0064
[144h 0324 002h]                       Entry : 0019
[146h 0326 002h]                       Entry : 0000
[148h 0328 002h]                       Entry : 0064
[14Ah 0330 002h]                       Entry : 0000
[14Ch 0332 002h]                       Entry : 00C8
[14Eh 0334 002h]                       Entry : 0019
[150h 0336 002h]                       Entry : 0064
[152h 0338 002h]                       Entry : 0000
[154h 0340 002h]                       Entry : 0032
[156h 0342 002h]                       Entry : 0000
[158h 0344 002h]                       Entry : 0032
[15Ah 0346 002h]                       Entry : 0064
[15Ch 0348 002h]                       Entry : 0064
[15Eh 0350 002h]                       Entry : 0000
[160h 0352 002h]                       Entry : 0032
[162h 0354 002h]                       Entry : 0000
[164h 0356 002h]                       Entry : 0032
[166h 0358 002h]                       Entry : 00C8

Note the zeros represent entries where the target node has no
memory.  These could be surpressed but it isn't 'wrong' to provide
them and it is (probably) permissible under ACPI to hotplug memory
into these nodes later.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   5 -----
 tests/data/acpi/q35/APIC.acpihmat-generic-x | Bin 0 -> 136 bytes
 tests/data/acpi/q35/CEDT.acpihmat-generic-x | Bin 0 -> 68 bytes
 tests/data/acpi/q35/DSDT.acpihmat-generic-x | Bin 0 -> 10849 bytes
 tests/data/acpi/q35/HMAT.acpihmat-generic-x | Bin 0 -> 360 bytes
 tests/data/acpi/q35/SRAT.acpihmat-generic-x | Bin 0 -> 520 bytes
 6 files changed, 5 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index a5aa801c99..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,6 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/APIC.acpihmat-generic-x",
-"tests/data/acpi/q35/CEDT.acpihmat-generic-x",
-"tests/data/acpi/q35/DSDT.acpihmat-generic-x",
-"tests/data/acpi/q35/HMAT.acpihmat-generic-x",
-"tests/data/acpi/q35/SRAT.acpihmat-generic-x",
diff --git a/tests/data/acpi/q35/APIC.acpihmat-generic-x b/tests/data/acpi/q35/APIC.acpihmat-generic-x
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..317ddb3fbed94e4f49a87976fdc7f23b1a6c3fdc 100644
GIT binary patch
literal 136
zcmZ<^@O18AU|?WQaPoKd2v%^42yj*a0!E-1hz+6{7#{os(;N&85Soz@LNhUeXht58
ongjnpBoh}9gBTzdD=U!Z1+h3eVJt470*DwlH<-o3_8({j09efo0RR91

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/q35/CEDT.acpihmat-generic-x b/tests/data/acpi/q35/CEDT.acpihmat-generic-x
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..31c9011663639b4a0f4816f3b2b06398f94682f7 100644
GIT binary patch
literal 68
zcmZ>EbqR4{U|?Xhb@F%i2v%^42yj*a0!E-1hz+6{7!(*BfFy(s;xkNuupuM>Q<et-

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/q35/DSDT.acpihmat-generic-x b/tests/data/acpi/q35/DSDT.acpihmat-generic-x
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..75e926a25b819889977355a6a2dd8e0209487ad4 100644
GIT binary patch
literal 10849
zcmcIqUu+x6d7t4QX|-HQD{3wKd_J4--KCdHdPm8BF8!09$z6)1&6Q}9^0}Z<E+x5@
z>=d{4os07s24o#@ar}_>j&abw(FZ2&Tl-M-Er@~Sp?xz@6g{9ofjsmjj}3~rKoRx#
z&Fs=MBn8AkY6H}Kv)}Lg<~KV#zuDRGnqI5>CS&|(#dWvR%oVTJ-5maL#u%09uU$=D
zW9>b!SnlvlBE{SHGop;2qTzhqD{hqeU+o0n4}uTB9q?|*HoveRZFa*?9t4|=oNjN1
zrbz|+(qgAs?6mi*^L4kHH#(K7XBD2BhS70lDQR>zsvdJ{)1CFEn|gSYx!!xr{k3#&
zXJOH_+y1|w`Q@n-7ry`AhlMBK{I@@Tc+*V5VFiB;{5=!WPjELdPX>Ma^WHAMM{sd%
zxxe^%Ph6IDHeCq=s(Eb5DhjncxanL<m%OGw+dtpC!NP?kXF>n?=duA$-}&eNY@Ykr
z|Lo|`{g-~?`h%c9{b<)r41&<S{~%xt6(@Q(%;_+jPOw=PpclTySU2p-i|zeyc#VQ9
zn9tVN>UlT6%)q|UTsUQ(`1Vo8Eb4Ol_xA?}2S1iYX5V3U&3TJDk2<csS9hB&o_S)K
zM*F@^hKu@*9Dcl9d#_o#h+FkfmYKr6BEv1_OBZpWxaK!_=Bb2{;TG36{9L{2UCp(5
zvF2W4&g*rzbTQF>m8^q<aECm?JA9W>jP`F)^?*5FD*!Xei(-ZJ6GXm&Q=F-b{`96^
zs6)|fltgqOemL92ZMfIlWW7xbeRII>b$Rz*_YOCwpLcNob*=YD0)(6LHW>A)yUHA^
zgTFl7A<N#s!l{34_vd14)*G#Q_R<G?*-R>fT(z}JB__)p<yDrosNPJ#D91(;MulZ(
zQ)nM<qx6d`LcwuPUK2RD#wH;VU-%{j626f|$k;W(1QM<S36qd$jqpthWZZ%zBV_EF
zlt{P=BuqlaSyBQSw;&~uv1>{q<Ho~Od8Rd<X@QJg(-Ijs9;V83O5-^tkg@BO&J(7}
zb6Vp$t?``JdBRkA8X8YS<7wzTVX8c5G@dgW&l#O3OqHjp@iaA_rp^<l$}^+!%xF9_
zI!~A?PfO!zX*?~RCrp*+tj2Rz<2kGIgsJkJ(|FElJm++tFjb!O8qax+=e*7nrpnXS
zc-k6ITjvQ=<+-5oT+nzf=saPnJhK|ltj05|^MtALT-10jYCIQpo-kFOj>gl`cse>y
zm@3aB8qXsd&m%fdm@3aDjpvfab4lk3Q{@R6lXI4sm5ypWkLo;OsyvTrJdbHSkLf&N
zsyts3$U-y|z9f*vXtH}rBHiaEA-%}=xQ0BgA&=`wm@4vR4f(Q$d|5}rRFNk%<OvOV
zLPx?>k*{dTS2W}+IufReJSmXz9Dh<E<2nAMM8*?0OhU#=a!Sj2O3Qgl&k0lJG$5b_
zML^5VDpCtE<tQU@B%m;mRvJkdaZ(7V^b>=KGeX7f<47u9hXM*SrUn6(zOjHRt|Xw+
zbts@P(VAF5rEg?WPDG6rlaO(bk$}QPi-a!(RQg6%#8DtgK&5MJsyrc}GOR#CKowUK
zP?)hQl7PaDc|t&?uRuaT6;~2a=_+!PfWnM<LO`XjKtezjR}xU^Dv%_gFk_w&Q0Xg>
z5KzUH1XQ{TBnc?Y_<lk_rLRCjKowUKQ0Xd=B%m;3o)A#!E07RS#gzn9x(Xx-D9o5A
z1XTJ8Bm`7(B>|PL0!ac2Gv)~amA(QA0aaW{K&7idl7PaDc|t&?uRuaT6;~2a=_-&U
zpfF>e5K!qWkPuMCl>}6}3M2_A%$O$xRQd`e1XOV)0hO)-NdgKp<_Q6nz5)pWRa{9x
zrK>=afWnM<LO`XjKtezjR}xU^Dv%_gFk_w&Q0Xg>5KzUH1XQ{TBnc?Ym?s2O`U)fj
zRB<H%m97Fw0tz$c2?3S90to?CTuDHst3Z-~!i;%BK&7uhLO>N)5>V+XkR+fmW1bLD
z=_`;BP{ox5RJsZz2`J2%Cj?ab3M2$naU}tjt^!E{3Nz*j0hPW22?14HNkFBmK$3vM
zjCn#pVJQL%OBGO<s(`{&1XMvnKoukjsDdN`Rge%+1qlIFkR+fAk_1#iLO>NH1XMwi
zfGS85Pz4DARge%+1xW&`AW1+KBm@*ze0L$BuyDsjXOe)z#BGR8LdJay0fiO!Ed&%+
z+_xm4Fyp=@0fmV@kmDUlDC=82@v=Ig7t+U5^rzq~RxjiIzdv)=+}mLTb7o*>oU_%-
zZes?r+sH4>nVIiQu^F+IaECplIx~&mYIt|YoM%)*phmvEUz>)`7vh~5YhLcZ7)ae*
zSTYyCGsk9<P$-5um|-?$cbGHZX|@V%KuxGOE*G4Ij#q86dsH^}M9n;HXpo275cbhd
zv(e-accL~>da94I7e})57sy^zvKK}6BHE{BFFKR67u)+jahH2B*oBI?n~4`l?<(nC
zk=`9i?;eugjnaD~<W1~={n~x+Dd|0t-Wy5p9g^OQ(w9clr(Pg^Nl9N4=}RN&ONXQ{
zMd?>Z)2Ck`{i>3FRis}XNxynX`qe0Xc{Kgh3#2bA>B}O0c_e-Lko4s!ePuNL^b4e~
zDCsLAePtwl<&g9hq)*XwG@9O)>Ep=*O9MRaa&uCzbEd>2k3aNWN8v^vV{<_dhxNi(
zI9WF_+}LxO)x%*CF&0kNO$;~ooGt3%u(B8nC+jAL8+&dXJsg%BW8q}o#BgKJ!4W+i
z)*xfyWZlGYW8->B4~K=xSU6cXG2GZVJ*tPpYGo{(tkc8Uz9F~&OGc$$YQ@8kW6GmV
z|Mp&`-trk%-cNs(Vc5F1$zHrtTJv)ChS$kC=Y{fwR1<_$)%`dy?VxXc)(coKz<%t2
z_2)k8b=mj27W>(Mrw0&6NE&UgFPL+m;uVB~GrX)}L*40=>+(L%7TkI@pU<CXcI~2U
zV#U1nb~4^?uQy7~xV0wc2()(At#9Ni?|&G6>-L}IZe9Q2{oA*;u7AMV+^yFxMGwua
zX_ony)|VEy?tmSpb?n)HX_;30{u^WzSkA4N*0@_;YjU^YU*Xt!&(qH;XGz!fJ?!ph
z$usFTOSK%_y+SL8ZR_e<)d(3?w)Zc-drf>8K!ao>BR0!<YKHu*l6Ky3TdRMV=$pMA
z_Nfc1yHw;<T;A<xdOK!<$}780WeQ&t3N!6juf3g8`4cbt957mqHiYznq&Iqjn1$v%
zJ_C$hsa+Zbj-MvBFP4aphC+mi@tyLL+jQ5ALueT8TIs!P;vi&OA>aO@l<jTvkZp%N
z2wC?F-}=j5z#nayNiYkyMso_rqm{T>{iBaYYjL}_e$b!kb<G3h`9V18djGYtuA}h~
zZattzcG>MNn)Udd{<$$XF=B9Asi9*%gNwl3-lETOrIh?0M*%n477c|hBxo;BCN~xf
z?YX@<xu5+RcfHxN4(aE~!(}W!IxpOv&^Mu(7_BAO8>5xz=83IqfAR-3*6FEd^tA^A
z+ZfRB#*^U}S_>J|VA#;0WwJAe500%2kzqT#H!#6n^;!FqT8f?!v5wXPV2wHqz_|#J
z=|m52o-4t49L!9%KUt?}E7+c*gtO0^Lu<}qxI{vPvxec!1cW_qoS7H~Ogk^*$;TYl
zDdksKG+ogwKqDWboK|KqzfzY9C_XwCFw@Cd%)kDp@87zU+Zmh4<fJKPFs%Jec8q4Q
zN}WC*GSfWP-Y=Bow1db}sz)MwRo~BTU;p!6nidV;TWgq!_Wie%TSsl8h)KUutmW`i
z!B34!HI(XCJGu7#uTStVunVSNYIe9!&xub@flnVi+xK6a;GOho>h#x3SMbwxa}GX1
zV*ucxq&6{9h)kRN;?rkg%175J*YNzeU!`jY2VeaDqks@$Kmq%UJ((SkQouvel*dbb
zrze|3we(Pubt~YQ11ZRWJHPFd2q<C0zusXYV5TQH$r)342TtLb59z`bDVq@c8h+#a
z*@ZJ^{^8f~y%-<;^nbsikE8e(*GlW)055_BH0RLrnk;5ftjOY5YNfdLO%$WrCBJYr
z@;#1XC8XO#AS%=-5DK*l0jN-;Ah)QR3WOy?Tqbnic|>(|{TA6=R4#~0{^78ON;SGx
z>wD8Zs>T3noLL!uA|FyFszj403VLT&OcUWd;Dq8Fuu*Xd4eErHnJ$MEoGymcPP!D`
zA%YEy=g&H?VI2B=^ag}9I^&xfIB#HJwNhhFyEN#+8#L-<Nar-hX&xWV_1lSIg5fJ&
z@YqiI^*oM7&>AEE5hkyA;bHB?>w6j$bR9Mg<pGVU$fj9qKy4QG0!<r9Y8{!FIDnu@
zikcy6qIAo{baWU&9?JOtNavjJ@nl4+5TY5aLQq`ZRc5m@@oYwu3q6wd`pIi@UAB7C
zt(}$KV_q;i#o`+ML6hTE56x^?^<=GtI9WnJNu#+@uQS7KHEN8?t|+@WW<s;Ra?At^
z$%*GoWNqeO|7&OWrw{!fee-+A?RWp>e-Ax<a_ZMSeS-5QE7UY`Y=v4U&!hCc)S(G2
zV9p{j9bY*|xU<BE3g-~OrGqTzIQ<N6KAzK$wOp7H8%kj|u_u-}YtxUq=3e-5*G#a@
z@b0bdCbpi~R^RSzu`oL$j|6yZn=1IQqJWBp-j<md))b?f<gmuYQtIBFP19yrlCuZf
zjD>f*i2Y!jvvB)CK#Q6|ASLFAnSC(J5?J-K3;0iJjJ5fCTHfw*+Qoya-QahaGb6`6
zHoe~I;ADx_&&&S$;s0F!#ee+GpI!LjjeoE*>)&l)lE1+QIKjjFbG;jUkTnuKn@TWl
z`$wdDiK8Ck{orE8{KvQ<jJ+$GQ|8aCXBIb5&*eP(OSJQuHMD2N^z#dGZYP?QOiV_!
zQAYm~OgqtIV3ztjd4>J0cI64K{6%!->Da9Pgcbzw__26@iQMQ9)-LF*_P(3KDnK5m
z8c|`YS6)x>{?#g&8AaT4Q@JUQ<3)InNYyL#hU{uOne=>-fKHn5sxuzziS@+dSSZ+E
zy@rRdd8U({O3~T~<+Givkunn)X=I(FD@4Hlz2~=~z5mwgd#-#Fp1oZ1H?VSBA@7kE
z+i5#ZFI0A6Ma&6yb(!vBdq306=Q~EH;5ESJ&1U`^(!d?#46o<SWe;jkg_XE)h6*^1
zcBHu+A>o`rjTQ>VvJ8c-4y&18kp>oRMXUrcR#PHzp}DcfoUe)j8PX`jy8}5q2r&HV
z;)8&*0iA@xM0_(qJIFsPCS*EtMZQ!g;vwO0G#=Xgn~LYW>0McCCHk4WJDhz=)bRp2
zx!m5LZ!SN-4M`ehPIL8n%_({<t~M?w%)qp4Hzg<e`@j2kl5Qz^(p65I#n{-sT%;eD
zemsL=PvCNi%B%EKrk{$@-amV(=-~!Q5ldogfyPg?2XM?hQsw~*80Tfa&9?FPbV58q
zuleg6L)1L>h7^=xrccuey!QS|t=@uG%GfE4O{gl~1QcJ}oGtT@k{=}v<Mf`0yyC9=
zLrfaplQGYd&yvhIZ41onjg~uXO$K>r14D!kS;)9?nsI8*d~>xR$D);B=t`<6Miog^
zP=^lB4BD^LAa_pH-L)2i(5XJUkLc$O9u+(@>EJLSuK&?n<V;2TU3$ZCeh2Ru%Z%Rg
ShMOYte4tW6?(`a_dHf$b$99(h

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/q35/HMAT.acpihmat-generic-x b/tests/data/acpi/q35/HMAT.acpihmat-generic-x
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0e5765f6ee4c07638c70647ae145e968718b67cd 100644
GIT binary patch
literal 360
zcmeb9bqvX1WME+Ock*}k2v%^42yj*a0-z8Bhz+7)Km*7?=EKC%X^=V)XaHgs5CaPU
znNtB32dQC$vIW$k3?Kzk!wkf%P$3YX2`UEC0}=;`ae=W2gAr7W703dq;{anOBsL>h
pJ=k8L!N~R^yOS7uPXNsZ*=NL%!XOExQ-JsckOiV);t2K$1^{D&4*>uG

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/q35/SRAT.acpihmat-generic-x b/tests/data/acpi/q35/SRAT.acpihmat-generic-x
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..b45838adb7d304f8a36c38c90d89f3629ec48353 100644
GIT binary patch
literal 520
zcmWFzatz^MVqjn_cJg=j2v%^42yj*a0!9V~1`r!WgD@Njp!1m-QRP{gkok-naGg*F
z7hC|lI-mt$@PQeo5LF!uOc=(1(J1c3v=^ogl^*QsSQQwc;mZh&B?N$l37Y}~14zQr
eIl$Avz|hPAsstv_sKE*qfydhfm;gM0fdT+OoeTj0

literal 0
HcmV?d00001

-- 
2.43.0

Re: [PATCH v3 qemu 00/11] acpi: NUMA nodes for CXL HB as GP + complex NUMA test

[Huang, Ying]

Hi, Jonathan,

Jonathan Cameron <Jonathan.Cameron@huawei.com> writes:

> v3: Thanks to Richard for help debugging BE issue and to Igor for
>     finding a bunch of other thing to improve via the context in
>     the fix patch.
>     
> - Fix the big endian host/little endian guest issue in the HID being
>   written to the Generic Port Affinity Structure ACPI Device Handle.
> - Fix a bug in the ordering of bus vs devfn in the BDF field which is
>   reversed in the ACPI table wrt to QEMU's internal handling. Note the
>   fix is minimal and refactored later in the series.
> - Move original GI code to hw/acpi/aml-build.c and hw/acpi/pc.c as
>   no need for a separate file and this keeps the SRAT entry building
>   all in one place.
> - Use properties for the pci bus number and the ACPI UID to avoid
>   using pci internal implementation details in hw/acpi.
> - Drop the GenericNode base object as much less code is unified with
>   the new approach to the aml building and that approach did not bring
>   sufficient advantages to be worthwhile after other refactors.
>   A little more duplication occurs in v3 but the code is easier to read.
>
> ACPI 6.5 introduced Generic Port Affinity Structures to close a system
> description gap that was a problem for CXL memory systems.
> It defines an new SRAT Affinity structure (and hence allows creation of an
> ACPI Proximity Node which can only be defined via an SRAT structure)
> for the boundary between a discoverable fabric and a non discoverable
> system interconnects etc.
>
> The HMAT data on latency and bandwidth is combined with discoverable
> information from the CXL bus (link speeds, lane counts) and CXL devices
> (switch port to port characteristics and USP to memory, via CDAT tables
> read from the device).  QEMU has supported the rest of the elements
> of this chain for a while but now the kernel has caught up and we need
> the missing element of Generic Ports (this code has been used extensively
> in testing and debugging that kernel support, some resulting fixes
> currently under review).
>
> Generic Port Affinity Structures are very similar to the recently
> added Generic Initiator Affinity Structures (GI) so this series
> factors out and reuses much of that infrastructure for reuse
> There are subtle differences (beyond the obvious structure ID change).
>
> - The ACPI spec example (and linux kernel support) has a Generic
>   Port not as associated with the CXL root port, but rather with
>   the CXL Host bridge. As a result, an ACPI handle is used (rather
>   than the PCI SBDF option for GIs). In QEMU the easiest way
>   to get to this is to target the root bridge PCI Bus, and
>   conveniently the root bridge bus number is used for the UID allowing
>   us to construct an appropriate entry.
>
> A key addition of this series is a complex NUMA topology example that
> stretches the QEMU emulation code for GI, GP and nodes with just
> CPUS, just memory, just hot pluggable memory, mixture of memory and CPUs.
>
> A similar test showed up a few NUMA related bugs with fixes applied for
> 9.0 (note that one of these needs linux booted to identify that it
> rejects the HMAT table and this test is a regression test for the
> table generation only).
>
> https://lore.kernel.org/qemu-devel/2eb6672cfdaea7dacd8e9bb0523887f13b9f85ce.1710282274.git.mst@redhat.com/
> https://lore.kernel.org/qemu-devel/74e2845c5f95b0c139c79233ddb65bb17f2dd679.1710282274.git.mst@redhat.com/

When developing the Linux kernel patchset "[PATCH v3 0/3] cxl/region:
Support to calculate memory tier abstract distance" as in [1].

[1] https://lore.kernel.org/linux-cxl/20240618084639.1419629-1-ying.huang@intel.com/

I use this patchset to test my kernel patchset and it works great!
Thanks!

Feel free to add my

Tested-by: "Huang, Ying" <ying.huang@intel.com>

in the future versions.

>
> Jonathan Cameron (11):
>   hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle.
>   hw/acpi/GI: Fix trivial parameter alignment issue.
>   hw/acpi: Move AML building code for Generic Initiators to aml_build.c
>   hw/acpi: Rename build_all_acpi_generic_initiators() to
>     build_acpi_generic_initiator()
>   hw/pci: Add a bus property to pci_props and use for acpi/gi
>   acpi/pci: Move Generic Initiator object handling into acpi/pci.*
>   hw/pci-bridge: Add acpi_uid property to CXL PXB
>   hw/acpi: Generic Port Affinity Structure support
>   bios-tables-test: Allow for new acpihmat-generic-x test data.
>   bios-tables-test: Add complex SRAT / HMAT test for GI GP
>   bios-tables-test: Add data for complex numa test (GI, GP etc)
>
>  qapi/qom.json                               |  34 +++
>  include/hw/acpi/acpi_generic_initiator.h    |  30 +--
>  include/hw/acpi/aml-build.h                 |   8 +
>  include/hw/acpi/pci.h                       |   7 +
>  include/hw/pci/pci_bridge.h                 |   1 +
>  hw/acpi/acpi_generic_initiator.c            | 132 +++++++++---
>  hw/acpi/aml-build.c                         |  84 ++++++++
>  hw/acpi/pci.c                               | 226 ++++++++++++++++++++
>  hw/arm/virt-acpi-build.c                    |   3 +-
>  hw/i386/acpi-build.c                        |   3 +-
>  hw/pci-bridge/pci_expander_bridge.c         |  18 +-
>  hw/pci/pci.c                                |  14 ++
>  tests/qtest/bios-tables-test.c              |  96 +++++++++
>  hw/acpi/meson.build                         |   1 -
>  tests/data/acpi/q35/APIC.acpihmat-generic-x | Bin 0 -> 136 bytes
>  tests/data/acpi/q35/CEDT.acpihmat-generic-x | Bin 0 -> 68 bytes
>  tests/data/acpi/q35/DSDT.acpihmat-generic-x | Bin 0 -> 10849 bytes
>  tests/data/acpi/q35/HMAT.acpihmat-generic-x | Bin 0 -> 360 bytes
>  tests/data/acpi/q35/SRAT.acpihmat-generic-x | Bin 0 -> 520 bytes
>  19 files changed, 597 insertions(+), 60 deletions(-)
>  create mode 100644 tests/data/acpi/q35/APIC.acpihmat-generic-x
>  create mode 100644 tests/data/acpi/q35/CEDT.acpihmat-generic-x
>  create mode 100644 tests/data/acpi/q35/DSDT.acpihmat-generic-x
>  create mode 100644 tests/data/acpi/q35/HMAT.acpihmat-generic-x
>  create mode 100644 tests/data/acpi/q35/SRAT.acpihmat-generic-x

--
Best Regards,
Huang, Ying

Re: [PATCH v3 qemu 00/11] acpi: NUMA nodes for CXL HB as GP + complex NUMA test

[Jonathan Cameron via]

On Thu, 20 Jun 2024 17:03:08 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> v3: Thanks to Richard for help debugging BE issue and to Igor for
>     finding a bunch of other thing to improve via the context in
>     the fix patch.

I forgot to mention that his time I ran the bios tables test on
an emulated x86_64 machine on top of an emulated s390 (with the timeouts
massively increased as it took about 2 hours).

Hopefully no more surprises!