| Message ID | 20240223124223.800078-3-ankita@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acpi: report numa nodes for device memory using GI | expand |
On Fri, 23 Feb 2024 12:42:23 +0000 <ankita@nvidia.com> wrote: > From: Ankit Agrawal <ankita@nvidia.com> > > ACPI spec provides a scheme to associate "Generic Initiators" [1] > (e.g. heterogeneous processors and accelerators, GPUs, and I/O devices with > integrated compute or DMA engines GPUs) with Proximity Domains. This is > achieved using Generic Initiator Affinity Structure in SRAT. During bootup, > Linux kernel parse the ACPI SRAT to determine the PXM ids and create a NUMA > node for each unique PXM ID encountered. Qemu currently do not implement > these structures while building SRAT. > > Add GI structures while building VM ACPI SRAT. The association between > device and node are stored using acpi-generic-initiator object. Lookup > presence of all such objects and use them to build these structures. > > The structure needs a PCI device handle [2] that consists of the device BDF. > The vfio-pci device corresponding to the acpi-generic-initiator object is > located to determine the BDF. > > [1] ACPI Spec 6.3, Section 5.2.16.6 > [2] ACPI Spec 6.3, Table 5.80 > > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> Hi Ankit, As the code stands the use of a list seems overkill. Otherwise looks good to me. I need Generic Ports support for CXL stuff so will copy your approach for that as it's ended up nice and simple. Jonathan > --- > hw/acpi/acpi-generic-initiator.c | 84 ++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 3 + > include/hw/acpi/acpi-generic-initiator.h | 26 ++++++++ > 3 files changed, 113 insertions(+) > > diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c > index 1ade2f723f..d78382bc63 100644 > --- a/hw/acpi/acpi-generic-initiator.c > +++ b/hw/acpi/acpi-generic-initiator.c > @@ -68,3 +68,87 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data) > object_class_property_add(oc, "node", "int", NULL, > acpi_generic_initiator_set_node, NULL, NULL); > } > + > +static int acpi_generic_initiator_list(Object *obj, void *opaque) > +{ > + GSList **list = opaque; > + > + if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { > + *list = g_slist_append(*list, ACPI_GENERIC_INITIATOR(obj)); > + } > + > + object_child_foreach(obj, acpi_generic_initiator_list, opaque); See below. There is a recursive helper that avoids need for this. > + return 0; > +} > + > +/* > + * Identify Generic Initiator objects and link them into the list which is > + * returned to the caller. > + * > + * Note: it is the caller's responsibility to free the list to avoid > + * memory leak. > + */ > +static GSList *acpi_generic_initiator_get_list(void) > +{ > + GSList *list = NULL; > + > + object_child_foreach(object_get_root(), > + acpi_generic_initiator_list, &list); I think you can use object_child_foreach_recursive() and skip the manual calling above? > + return list; > +} > + > +/* > + * 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, handle->bdf, 2); > + 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 */ > +} > + > +void build_srat_generic_pci_initiator(GArray *table_data) > +{ > + GSList *gi_list, *list = acpi_generic_initiator_get_list(); Did you consider just have the functional called in the scan do this? Not sure you need anything as a parameter beyond the GArray *table_data Something like... static int acpi_generic_initiator_list(Object *obj, void *opaque) { uint8_t index; AcpiGenericInitiator *gi; GArray *table_data = opaque; PCIDeviceHandle dev_handle; PCIDevice *pci_dev; Object *o; if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { return 0; } gi = ACPI_GENERIC_INITIATOR(obj); o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL); if (!o) { error_setg(&error_abort, "GI: Specified device must be a PCI device.\n") return 1; } 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); } + a call to. object_child_foreach_recursive(object_get_root(), acpi_generic_srat, table_data); > + AcpiGenericInitiator *gi; > + > + for (gi_list = list; gi_list; gi_list = gi_list->next) { > + PCIDeviceHandle dev_handle; > + PCIDevice *pci_dev; > + Object *o; > + > + gi = gi_list->data; > + > + o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL); > + if (!o) { > + error_printf("Specified device must be a PCI device.\n"); as above, use an errp rather than exit(1); > + exit(1); > + } > + 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); Should we check for consistency of gi->node and -numa node,id=X entries? Maybe just check less than numa_state->num_nodes as that's the variable used to walk the other structures when building srat. > + } > + > + g_slist_free(list); > +} > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 8bc35a483c..00d77327e0 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -58,6 +58,7 @@ > #include "migration/vmstate.h" > #include "hw/acpi/ghes.h" > #include "hw/acpi/viot.h" > +#include "hw/acpi/acpi-generic-initiator.h" > > #define ARM_SPI_BASE 32 > > @@ -558,6 +559,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > } > } > > + build_srat_generic_pci_initiator(table_data); Perhaps passing in a suitable Error ** would be sensible. > + > if (ms->nvdimms_state->is_enabled) { > nvdimm_build_srat(table_data); > } > diff --git a/include/hw/acpi/acpi-generic-initiator.h b/include/hw/acpi/acpi-generic-initiator.h > index 2f183b029a..213545e614 100644 > --- a/include/hw/acpi/acpi-generic-initiator.h > +++ b/include/hw/acpi/acpi-generic-initiator.h > @@ -29,4 +29,30 @@ typedef struct AcpiGenericInitiatorClass { > ObjectClass parent_class; > } AcpiGenericInitiatorClass; > > +/* > + * ACPI 6.3: > + * Table 5-81 Flags – Generic Initiator Affinity Structure > + */ > +typedef enum { > + GEN_AFFINITY_ENABLED = (1 << 0), /* > + * 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. > + */ I'd put the comment above the definition to avoid wrapping so much! > +} 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
On Fri, 23 Feb 2024 12:42:23 +0000 <ankita@nvidia.com> wrote: > From: Ankit Agrawal <ankita@nvidia.com> > > ACPI spec provides a scheme to associate "Generic Initiators" [1] > (e.g. heterogeneous processors and accelerators, GPUs, and I/O devices with > integrated compute or DMA engines GPUs) with Proximity Domains. This is > achieved using Generic Initiator Affinity Structure in SRAT. During bootup, > Linux kernel parse the ACPI SRAT to determine the PXM ids and create a NUMA > node for each unique PXM ID encountered. Qemu currently do not implement > these structures while building SRAT. > > Add GI structures while building VM ACPI SRAT. The association between > device and node are stored using acpi-generic-initiator object. Lookup > presence of all such objects and use them to build these structures. > > The structure needs a PCI device handle [2] that consists of the device BDF. > The vfio-pci device corresponding to the acpi-generic-initiator object is > located to determine the BDF. > > [1] ACPI Spec 6.3, Section 5.2.16.6 > [2] ACPI Spec 6.3, Table 5.80 > > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> One thing I forgot. Please add a test. tests/qtest/bios-tables-test.c + relevant table dumps. Could also hook this up for x86 with a oneline addition and improve test coverage. If not, I'll do it when I add Generic Ports as annoyingly people still care about x86 for some reason.
Thanks Jonathan for reviewing the change. Comments inline. >> The structure needs a PCI device handle [2] that consists of the device BDF. >> The vfio-pci device corresponding to the acpi-generic-initiator object is >> located to determine the BDF. >> >> [1] ACPI Spec 6.3, Section 5.2.16.6 >> [2] ACPI Spec 6.3, Table 5.80 >> >> Signed-off-by: Ankit Agrawal <ankita@nvidia.com> >Hi Ankit, > > As the code stands the use of a list seems overkill. Yeah, I will try out your suggestion. > Otherwise looks good to me. I need Generic Ports support for CXL > stuff so will copy your approach for that as it's ended up nice > and simple. > > Jonathan Nice, would be good to have uniform implementations. >> +/* >> + * Identify Generic Initiator objects and link them into the list which is >> + * returned to the caller. >> + * >> + * Note: it is the caller's responsibility to free the list to avoid >> + * memory leak. >> + */ >> +static GSList *acpi_generic_initiator_get_list(void) >> +{ >> + GSList *list = NULL; >> + >> + object_child_foreach(object_get_root(), >> + acpi_generic_initiator_list, &list); > > I think you can use object_child_foreach_recursive() and skip the manual > calling above? Great suggestion, will give that a try. >> + * 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, handle->bdf, 2); >> + 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 */ >> +} >> + >> +void build_srat_generic_pci_initiator(GArray *table_data) >> +{ >> + GSList *gi_list, *list = acpi_generic_initiator_get_list(); > > > Did you consider just have the functional called in the scan do this? > Not sure you need anything as a parameter beyond the GArray *table_data > > Something like... > > static int acpi_generic_initiator_list(Object *obj, void *opaque) > { > uint8_t index; > AcpiGenericInitiator *gi; > GArray *table_data = opaque; > PCIDeviceHandle dev_handle; > PCIDevice *pci_dev; > Object *o; > > if (!object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { > return 0; > } > > gi = ACPI_GENERIC_INITIATOR(obj); > o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL); > if (!o) { > error_setg(&error_abort, "GI: Specified device must be a PCI device.\n") > return 1; > } > 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); > } > > + a call to. > object_child_foreach_recursive(object_get_root(), > acpi_generic_srat, table_data); > Thanks. That does seem better. >> + 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); > Should we check for consistency of gi->node and > -numa node,id=X entries? > > Maybe just check less than numa_state->num_nodes as that's the variable > used to walk the other structures when building srat. Ack, will add a check to compare against numa_state->num_nodes. >> +/* >> + * ACPI 6.3: >> + * Table 5-81 Flags – Generic Initiator Affinity Structure >> + */ >> +typedef enum { >> + GEN_AFFINITY_ENABLED = (1 << 0), /* >> + * 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. >> + */ > I'd put the comment above the definition to avoid wrapping so much! Yeah, it does look kind of silly. Will fix it. >> +} GenericAffinityFlags; >> +
On Fri, 23 Feb 2024 12:42:23 +0000 <ankita@nvidia.com> wrote: > From: Ankit Agrawal <ankita@nvidia.com> > > ACPI spec provides a scheme to associate "Generic Initiators" [1] > (e.g. heterogeneous processors and accelerators, GPUs, and I/O devices with > integrated compute or DMA engines GPUs) with Proximity Domains. This is > achieved using Generic Initiator Affinity Structure in SRAT. During bootup, > Linux kernel parse the ACPI SRAT to determine the PXM ids and create a NUMA > node for each unique PXM ID encountered. Qemu currently do not implement > these structures while building SRAT. > > Add GI structures while building VM ACPI SRAT. The association between > device and node are stored using acpi-generic-initiator object. Lookup > presence of all such objects and use them to build these structures. > > The structure needs a PCI device handle [2] that consists of the device BDF. > The vfio-pci device corresponding to the acpi-generic-initiator object is > located to determine the BDF. > > [1] ACPI Spec 6.3, Section 5.2.16.6 > [2] ACPI Spec 6.3, Table 5.80 > > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > --- > hw/acpi/acpi-generic-initiator.c | 84 ++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 3 + > include/hw/acpi/acpi-generic-initiator.h | 26 ++++++++ A few more comments. Maybe _ rather than - as more common for acpi include naming. I also wonder if we need the acpi prefix for file names given context?
On Tue, 27 Feb 2024 08:37:15 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > Thanks Jonathan for reviewing the change. > > Comments inline. > > >> The structure needs a PCI device handle [2] that consists of the device BDF. > >> The vfio-pci device corresponding to the acpi-generic-initiator object is > >> located to determine the BDF. > >> > >> [1] ACPI Spec 6.3, Section 5.2.16.6 > >> [2] ACPI Spec 6.3, Table 5.80 > >> > >> Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > >Hi Ankit, > > > > As the code stands the use of a list seems overkill. > > Yeah, I will try out your suggestion. > > > Otherwise looks good to me. I need Generic Ports support for CXL > > stuff so will copy your approach for that as it's ended up nice > > and simple. > > > > Jonathan > > Nice, would be good to have uniform implementations. I've been messing around with this today. They differ only very trivially. 2 Options. 1) Have acpi-generic-port inherit from acpi-generic-initiator. Works but implies a relationship that isn't really true. 2) Add an abstract base class. I've called it acpi-generic-node and have bother acpi-generic-initiator and acpi-generic-port inherit from it. The second feels more natural but is a tiny bit more code (a few more empty init / finalize functions. If we are going to end up with an abstract base 'object' it will be cleaner to do this all as one series if you don't mind carrying the generic port stuff as well? Or I can smash the two series together and send out an updated version that hopefully meets both our requirements (+ tests etc). I'm just running tests against the CXL qos / generic port code but assuming all goes well can share my additional changes in next day or two. Jonathan
On Tue, 27 Feb 2024 17:11:15 +0000 Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > On Tue, 27 Feb 2024 08:37:15 +0000 > Ankit Agrawal <ankita@nvidia.com> wrote: > > > Thanks Jonathan for reviewing the change. > > > > Comments inline. > > > > >> The structure needs a PCI device handle [2] that consists of the device BDF. > > >> The vfio-pci device corresponding to the acpi-generic-initiator object is > > >> located to determine the BDF. > > >> > > >> [1] ACPI Spec 6.3, Section 5.2.16.6 > > >> [2] ACPI Spec 6.3, Table 5.80 > > >> > > >> Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > > >Hi Ankit, > > > > > > As the code stands the use of a list seems overkill. > > > > Yeah, I will try out your suggestion. > > > > > Otherwise looks good to me. I need Generic Ports support for CXL > > > stuff so will copy your approach for that as it's ended up nice > > > and simple. > > > > > > Jonathan > > > > Nice, would be good to have uniform implementations. > > I've been messing around with this today. > > They differ only very trivially. > 2 Options. > 1) Have acpi-generic-port inherit from acpi-generic-initiator. > Works but implies a relationship that isn't really true. > 2) Add an abstract base class. I've called it acpi-generic-node > and have bother acpi-generic-initiator and acpi-generic-port > inherit from it. > > The second feels more natural but is a tiny bit more code (a few > more empty init / finalize functions. > > If we are going to end up with an abstract base 'object' it > will be cleaner to do this all as one series if you don't mind > carrying the generic port stuff as well? Or I can smash the > two series together and send out an updated version that hopefully > meets both our requirements (+ tests etc). > > I'm just running tests against the CXL qos / generic port code > but assuming all goes well can share my additional changes > in next day or two. > > Jonathan One more thing. Right now we can't use Generic Initiators as HMAT initiators. That also wants fixing given that's their normal usecase rather than what you are using them for so it should 'work'. Jonathan > > >
> One thing I forgot. > > Please add a test. tests/qtest/bios-tables-test.c IIUC, we need to add a test for aarch64 to test the interface with the acpi-generic-initiator object. > + relevant table dumps. Sorry it isn't clear where do you want me to add this. In the git commit message?
>> --- >> hw/acpi/acpi-generic-initiator.c | 84 ++++++++++++++++++++++++ >> hw/arm/virt-acpi-build.c | 3 + >> include/hw/acpi/acpi-generic-initiator.h | 26 ++++++++ > A few more comments. > > Maybe _ rather than - as more common for acpi include naming. Ack. will change the name. > I also wonder if we need the acpi prefix for file names given context? I tried to keep it to match the object name. If you have preference for not having it, I can change that too.
On Thu, 29 Feb 2024 11:43:44 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > > One thing I forgot. > > > > Please add a test. tests/qtest/bios-tables-test.c > > IIUC, we need to add a test for aarch64 to test the interface with the > acpi-generic-initiator object. > > > + relevant table dumps. > > Sorry it isn't clear where do you want me to add this. In the git commit > message? I meant as part of the test. Typically you then include the relevant snippet as part of the commit message to show what the key part of the test is. Thanks, Jonathan
On Thu, 29 Feb 2024 11:46:27 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > >> --- > >> hw/acpi/acpi-generic-initiator.c | 84 ++++++++++++++++++++++++ > >> hw/arm/virt-acpi-build.c | 3 + > >> include/hw/acpi/acpi-generic-initiator.h | 26 ++++++++ > > A few more comments. > > > > Maybe _ rather than - as more common for acpi include naming. > > Ack. will change the name. > > > I also wonder if we need the acpi prefix for file names given context? > > I tried to keep it to match the object name. If you have preference for > not having it, I can change that too. Not my area, so up to maintainers of hw/acpi to comment if they care! I'd not wait for that though before v8. *impatient user who has some other stuff queued on top of this!* :) Jonathan
>> > One thing I forgot. >> > >> > Please add a test. tests/qtest/bios-tables-test.c >> >> IIUC, we need to add a test for aarch64 to test the interface with the >> acpi-generic-initiator object. >> >> > + relevant table dumps. >> >> Sorry it isn't clear where do you want me to add this. In the git commit >> message? > > I meant as part of the test. Typically you then include the relevant snippet > as part of the commit message to show what the key part of the test is. Got it, thanks! > Thanks, > > Jonathan
On Tue, 27 Feb 2024 17:36:21 +0000 Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > On Tue, 27 Feb 2024 17:11:15 +0000 > Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > > > On Tue, 27 Feb 2024 08:37:15 +0000 > > Ankit Agrawal <ankita@nvidia.com> wrote: > > > > > Thanks Jonathan for reviewing the change. > > > > > > Comments inline. > > > > > > >> The structure needs a PCI device handle [2] that consists of the device BDF. > > > >> The vfio-pci device corresponding to the acpi-generic-initiator object is > > > >> located to determine the BDF. > > > >> > > > >> [1] ACPI Spec 6.3, Section 5.2.16.6 > > > >> [2] ACPI Spec 6.3, Table 5.80 > > > >> > > > >> Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > > > >Hi Ankit, > > > > > > > > As the code stands the use of a list seems overkill. > > > > > > Yeah, I will try out your suggestion. > > > > > > > Otherwise looks good to me. I need Generic Ports support for CXL > > > > stuff so will copy your approach for that as it's ended up nice > > > > and simple. > > > > > > > > Jonathan > > > > > > Nice, would be good to have uniform implementations. > > > > I've been messing around with this today. > > > > They differ only very trivially. > > 2 Options. > > 1) Have acpi-generic-port inherit from acpi-generic-initiator. > > Works but implies a relationship that isn't really true. > > 2) Add an abstract base class. I've called it acpi-generic-node > > and have bother acpi-generic-initiator and acpi-generic-port > > inherit from it. > > > > The second feels more natural but is a tiny bit more code (a few > > more empty init / finalize functions. > > > > If we are going to end up with an abstract base 'object' it > > will be cleaner to do this all as one series if you don't mind > > carrying the generic port stuff as well? Or I can smash the > > two series together and send out an updated version that hopefully > > meets both our requirements (+ tests etc). > > > > I'm just running tests against the CXL qos / generic port code > > but assuming all goes well can share my additional changes > > in next day or two. > > > > Jonathan > > One more thing. Right now we can't use Generic Initiators as > HMAT initiators. That also wants fixing given that's their > normal usecase rather than what you are using them for so it > should 'work'. Something along the lines of this will do the job. diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index 4173ef2afa..825cfe86bc 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -41,6 +41,7 @@ struct NodeInfo { struct HostMemoryBackend *node_memdev; bool present; bool has_cpu; + bool has_gi; uint8_t lb_info_provided; uint16_t initiator; uint8_t distance[MAX_NODES]; diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c index 9179590a42..8a67300320 100644 --- a/hw/acpi/acpi-generic-initiator.c +++ b/hw/acpi/acpi-generic-initiator.c @@ -6,6 +6,7 @@ #include "qemu/osdep.h" #include "hw/acpi/acpi-generic-initiator.h" #include "hw/pci/pci_device.h" +#include "hw/boards.h" #include "qapi/error.h" #include "qapi/qapi-builtin-visit.h" #include "qapi/visitor.h" @@ -58,6 +59,7 @@ static void acpi_generic_node_set_node(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { + MachineState *ms = MACHINE(qdev_get_machine()); AcpiGenericNode *gn = ACPI_GENERIC_NODE(obj); uint32_t value; @@ -72,6 +74,10 @@ static void acpi_generic_node_set_node(Object *obj, Visitor *v, } gn->node = value; + + if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { + ms->numa_state->nodes[gn->node].has_gi = true; + } } static void acpi_generic_node_class_init(ObjectClass *oc, void *data) diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c index b933ae3c06..9b1662b6b8 100644 --- a/hw/acpi/hmat.c +++ b/hw/acpi/hmat.c @@ -225,7 +225,7 @@ static void hmat_build_table_structs(GArray *table_data, NumaState *numa_state) } for (i = 0; i < numa_state->num_nodes; i++) { - if (numa_state->nodes[i].has_cpu) { + if (numa_state->nodes[i].has_cpu || numa_state->nodes[i].has_gi) { initiator_list[num_initiator++] = i; } } diff --git a/hw/core/numa.c b/hw/core/numa.c index f08956ddb0..58a32f1564 100644 --- a/hw/core/numa.c +++ b/hw/core/numa.c @@ -229,7 +229,8 @@ void parse_numa_hmat_lb(NumaState *numa_state, NumaHmatLBOptions *node, node->target, numa_state->num_nodes); return; } - if (!numa_info[node->initiator].has_cpu) { + if (!numa_info[node->initiator].has_cpu && + !numa_info[node->initiator].has_gi) { error_setg(errp, "Invalid initiator=%d, it isn't an " "initiator proximity domain", node->initiator); return; > > Jonathan > > > > > > > >
>> >> One more thing. Right now we can't use Generic Initiators as >> HMAT initiators. That also wants fixing given that's their >> normal usecase rather than what you are using them for so it >> should 'work'. > > Something along the lines of this will do the job. Thanks! Will incorporate the patch in the next posting. > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > index 4173ef2afa..825cfe86bc 100644 > --- a/include/sysemu/numa.h > +++ b/include/sysemu/numa.h > @@ -41,6 +41,7 @@ struct NodeInfo { > struct HostMemoryBackend *node_memdev; > bool present; > bool has_cpu; >+ bool has_gi; > uint8_t lb_info_provided; > uint16_t initiator; > uint8_t distance[MAX_NODES]; > diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c > index 9179590a42..8a67300320 100644 > --- a/hw/acpi/acpi-generic-initiator.c > +++ b/hw/acpi/acpi-generic-initiator.c > @@ -6,6 +6,7 @@ > #include "qemu/osdep.h" > #include "hw/acpi/acpi-generic-initiator.h" > #include "hw/pci/pci_device.h" > +#include "hw/boards.h" > #include "qapi/error.h" > #include "qapi/qapi-builtin-visit.h" > #include "qapi/visitor.h" > @@ -58,6 +59,7 @@ static void acpi_generic_node_set_node(Object *obj, Visitor *v, > const char *name, void *opaque, > Error **errp) > { > + MachineState *ms = MACHINE(qdev_get_machine()); > AcpiGenericNode *gn = ACPI_GENERIC_NODE(obj); > uint32_t value; > > @@ -72,6 +74,10 @@ static void acpi_generic_node_set_node(Object *obj, Visitor *v, > } > > gn->node = value; > + > + if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { > + ms->numa_state->nodes[gn->node].has_gi = true; > + } > } > > static void acpi_generic_node_class_init(ObjectClass *oc, void *data) > diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c > index b933ae3c06..9b1662b6b8 100644 > --- a/hw/acpi/hmat.c > +++ b/hw/acpi/hmat.c > @@ -225,7 +225,7 @@ static void hmat_build_table_structs(GArray *table_data, NumaState *numa_state) > } > > for (i = 0; i < numa_state->num_nodes; i++) { > - if (numa_state->nodes[i].has_cpu) { > + if (numa_state->nodes[i].has_cpu || numa_state->nodes[i].has_gi) { > initiator_list[num_initiator++] = i; > } > } > diff --git a/hw/core/numa.c b/hw/core/numa.c > index f08956ddb0..58a32f1564 100644 > --- a/hw/core/numa.c > +++ b/hw/core/numa.c > @@ -229,7 +229,8 @@ void parse_numa_hmat_lb(NumaState *numa_state, NumaHmatLBOptions *node, > node->target, numa_state->num_nodes); > return; > } > - if (!numa_info[node->initiator].has_cpu) { > + if (!numa_info[node->initiator].has_cpu && > + !numa_info[node->initiator].has_gi) { > error_setg(errp, "Invalid initiator=%d, it isn't an " > "initiator proximity domain", node->initiator); > return; > > Jonathan > > > > > > > >
> One thing I forgot. > > Please add a test. tests/qtest/bios-tables-test.c > + relevant table dumps. Here I need to add a test that creates a vfio-pci device and numa nodes and link using the acpi-generic-initiator object. One thing here is that the -device vfio-pci needs a host=<bdf> argument. I probably cannot provide the device bdf from my local setup. So I am not sure how can I add this test to tests/qtest/bios-tables-test.c. FYI, the following is a sample args we use for the acpi-generic-initiator object. -numa node,nodeid=2 -device vfio-pci-nohotplug,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \ -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=2 \ Moreover based on a quick grep, I don't see any other test that have -device vfio-pci argument. Jonathan, Alex, do you know how we may add tests that is dependent on the vfio-pci device?
On 3/5/24 06:59, Ankit Agrawal wrote: >> One thing I forgot. >> >> Please add a test. tests/qtest/bios-tables-test.c >> + relevant table dumps. > > Here I need to add a test that creates a vfio-pci device and numa > nodes and link using the acpi-generic-initiator object. One thing > here is that the -device vfio-pci needs a host=<bdf> argument. I > probably cannot provide the device bdf from my local setup. So > I am not sure how can I add this test to tests/qtest/bios-tables-test.c. > FYI, the following is a sample args we use for the > acpi-generic-initiator object. > > -numa node,nodeid=2 > -device vfio-pci-nohotplug,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \ > -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=2 \ > > Moreover based on a quick grep, I don't see any other test that > have -device vfio-pci argument. > > Jonathan, Alex, do you know how we may add tests that is dependent > on the vfio-pci device? There are none. This would require a host device always available for passthrough and there is no simple solution for this problem. Such tests would need to run in a nested environment under avocado: a pc/virt machine with an igb device and use the PF and/or VFs to check device assignment in a nested guests. PPC just introduced new tests to check nested guest support on two different HV implementations. If you have time, please take a look at tests/avocado/ppc_hv_tests.py for the framework. I will try to propose a new test when I am done with the reviews, not before 9.0 soft freeze though. Thanks, C.
>>> Please add a test. tests/qtest/bios-tables-test.c >>> + relevant table dumps. >> >> Here I need to add a test that creates a vfio-pci device and numa >> nodes and link using the acpi-generic-initiator object. One thing >> here is that the -device vfio-pci needs a host=<bdf> argument. I >> probably cannot provide the device bdf from my local setup. So >> I am not sure how can I add this test to tests/qtest/bios-tables-test.c. >> FYI, the following is a sample args we use for the >> acpi-generic-initiator object. >> >> -numa node,nodeid=2 >> -device vfio-pci-nohotplug,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \ >> -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=2 \ >> >> Moreover based on a quick grep, I don't see any other test that >> have -device vfio-pci argument. >> >> Jonathan, Alex, do you know how we may add tests that is dependent >> on the vfio-pci device? > > There are none. > > This would require a host device always available for passthrough and > there is no simple solution for this problem. Such tests would need to > run in a nested environment under avocado: a pc/virt machine with an > igb device and use the PF and/or VFs to check device assignment in a > nested guests. > > PPC just introduced new tests to check nested guest support on two > different HV implementations. If you have time, please take a look > at tests/avocado/ppc_hv_tests.py for the framework. > > I will try to propose a new test when I am done with the reviews, > not before 9.0 soft freeze though. Thanks for the information. As part of this patch, I'll leave out this test change then.
On Tue, 5 Mar 2024 08:17:18 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > >>> Please add a test. tests/qtest/bios-tables-test.c > >>> + relevant table dumps. > >> > >> Here I need to add a test that creates a vfio-pci device and numa > >> nodes and link using the acpi-generic-initiator object. One thing > >> here is that the -device vfio-pci needs a host=<bdf> argument. I > >> probably cannot provide the device bdf from my local setup. So > >> I am not sure how can I add this test to tests/qtest/bios-tables-test.c. > >> FYI, the following is a sample args we use for the > >> acpi-generic-initiator object. > >> > >> -numa node,nodeid=2 > >> -device vfio-pci-nohotplug,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \ > >> -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=2 \ > >> > >> Moreover based on a quick grep, I don't see any other test that > >> have -device vfio-pci argument. > >> > >> Jonathan, Alex, do you know how we may add tests that is dependent > >> on the vfio-pci device? > > > > There are none. > > > > This would require a host device always available for passthrough and > > there is no simple solution for this problem. Such tests would need to > > run in a nested environment under avocado: a pc/virt machine with an > > igb device and use the PF and/or VFs to check device assignment in a > > nested guests. > > > > PPC just introduced new tests to check nested guest support on two > > different HV implementations. If you have time, please take a look > > at tests/avocado/ppc_hv_tests.py for the framework. > > > > I will try to propose a new test when I am done with the reviews, > > not before 9.0 soft freeze though. > > Thanks for the information. As part of this patch, I'll leave out > this test change then. For BIOS table purposes it can be any PCI device. I've been testing this with a virtio-net-pci but something like virtio-rng-pci will do fine. The table contents doesn't care if it's vfio or not. I can spin a test as part of the follow up Generic Port series that incorporates both and pushes the limits of the hmat code in general. Current tests are too tame ;) Given I don't think we have clarification from ACPI spec side on the many to one mapping you are using, I'd just use a 1-1 in any test. Jonathan
On Tue, 5 Mar 2024 05:59:46 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > > One thing I forgot. > > > > Please add a test. tests/qtest/bios-tables-test.c > > + relevant table dumps. > > Here I need to add a test that creates a vfio-pci device and numa > nodes and link using the acpi-generic-initiator object. One thing > here is that the -device vfio-pci needs a host=<bdf> argument. I > probably cannot provide the device bdf from my local setup. So > I am not sure how can I add this test to tests/qtest/bios-tables-test.c. > FYI, the following is a sample args we use for the > acpi-generic-initiator object. > > -numa node,nodeid=2 > -device vfio-pci-nohotplug,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \ > -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=2 \ > > Moreover based on a quick grep, I don't see any other test that > have -device vfio-pci argument. > > Jonathan, Alex, do you know how we may add tests that is dependent > on the vfio-pci device? > As Jonathan notes, we've decoupled this from vfio-pci, the pci-dev= arg can point to any PCI device. For example, any emulated PCI NIC could be a stand-in for the vfio-pci device used in practice. Thanks, Alex
On Mon, 26 Feb 2024 16:42:29 +0000 Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > On Fri, 23 Feb 2024 12:42:23 +0000 > <ankita@nvidia.com> wrote: > > > From: Ankit Agrawal <ankita@nvidia.com> > > > > ACPI spec provides a scheme to associate "Generic Initiators" [1] > > (e.g. heterogeneous processors and accelerators, GPUs, and I/O devices with > > integrated compute or DMA engines GPUs) with Proximity Domains. This is > > achieved using Generic Initiator Affinity Structure in SRAT. During bootup, > > Linux kernel parse the ACPI SRAT to determine the PXM ids and create a NUMA > > node for each unique PXM ID encountered. Qemu currently do not implement > > these structures while building SRAT. > > > > Add GI structures while building VM ACPI SRAT. The association between > > device and node are stored using acpi-generic-initiator object. Lookup > > presence of all such objects and use them to build these structures. > > > > The structure needs a PCI device handle [2] that consists of the device BDF. > > The vfio-pci device corresponding to the acpi-generic-initiator object is > > located to determine the BDF. > > > > [1] ACPI Spec 6.3, Section 5.2.16.6 > > [2] ACPI Spec 6.3, Table 5.80 > > > > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > > One thing I forgot. And another :) It might be nice to also support x86 from the start (apparently people still care about that old architecture) https://gitlab.com/jic23/qemu/-/commit/ccfb4fe22167e035173390cf147d9c226951b9b6 is what I'm carrying for this (see below) We could do this later as part of the generic ports series (which is also on that tree if you are curious). diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 2d2bb0a325b83f5e3fb4666a462a693aea1a2220..54462d3a46c379a4159b4d71d7689a107745fa4c 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -68,6 +68,7 @@ #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" @@ -2097,6 +2098,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) } } + build_srat_generic_pci_initiator(table_data); if (machine->nvdimms_state->is_enabled) { nvdimm_build_srat(table_data); } > > Please add a test. tests/qtest/bios-tables-test.c > + relevant table dumps. > > Could also hook this up for x86 with a oneline addition and improve > test coverage. If not, I'll do it when I add Generic Ports as annoyingly > people still care about x86 for some reason. >
>> >> Jonathan, Alex, do you know how we may add tests that is dependent >> >> on the vfio-pci device? >> > >> > There are none. >> > >> > This would require a host device always available for passthrough and >> > there is no simple solution for this problem. Such tests would need to >> > run in a nested environment under avocado: a pc/virt machine with an >> > igb device and use the PF and/or VFs to check device assignment in a >> > nested guests. >> > >> > PPC just introduced new tests to check nested guest support on two >> > different HV implementations. If you have time, please take a look >> > at tests/avocado/ppc_hv_tests.py for the framework. >> > >> > I will try to propose a new test when I am done with the reviews, >> > not before 9.0 soft freeze though. >> >> Thanks for the information. As part of this patch, I'll leave out >> this test change then. > > For BIOS table purposes it can be any PCI device. I've been testing > this with a virtio-net-pci but something like virtio-rng-pci will > do fine. The table contents doesn't care if it's vfio or not. Thanks, I was able to work this out with the virtio-rng-pci device. > I can spin a test as part of the follow up Generic Port series that > incorporates both and pushes the limits of the hmat code in general. > Current tests are too tame ;) Sure, that is fine by me. FYI, this is how the test change looked like in case you were wondering. diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index fe6a9a8563..8ac8e3f048 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -2157,6 +2157,29 @@ static void test_acpi_virt_oem_fields(void) g_free(args); } +static void test_acpi_virt_srat_gi(void) +{ + test_data data = { + .machine = "virt", + .tcg_only = true, + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", + .ram_start = 0x40000000ULL, + .scan_len = 128ULL * 1024 * 1024, + }; + + data.variant = ".gi"; + test_acpi_one(" -cpu cortex-a57" + " -object memory-backend-ram,id=ram0,size=128M" + " -numa node,memdev=ram0,nodeid=0" + " -numa node,nodeid=1" + " -device virtio-rng-pci,id=dev0" + " -object acpi-generic-initiator,id=gi0,pci-dev=dev0,node=1", + &data); + + free_test_data(&data); +} int main(int argc, char *argv[]) { @@ -2312,6 +2335,7 @@ int main(int argc, char *argv[]) if (qtest_has_device("virtio-iommu-pci")) { qtest_add_func("acpi/virt/viot", test_acpi_virt_viot); } + qtest_add_func("acpi/virt/gi", test_acpi_virt_srat_gi); } } ret = g_test_run(); -- 2.34.1 > Given I don't think we have clarification from ACPI spec side on > the many to one mapping you are using, I'd just use a 1-1 in any > test. Ack.
>> >> Jonathan, Alex, do you know how we may add tests that is dependent >> on the vfio-pci device? >> > > As Jonathan notes, we've decoupled this from vfio-pci, the pci-dev= arg > can point to any PCI device. For example, any emulated PCI NIC could > be a stand-in for the vfio-pci device used in practice. Thanks, Yeah, -device virtio-rng-pci could be used as suggested by Jonathan.
>> > The structure needs a PCI device handle [2] that consists of the device BDF. >> > The vfio-pci device corresponding to the acpi-generic-initiator object is >> > located to determine the BDF. >> > >> > [1] ACPI Spec 6.3, Section 5.2.16.6 >> > [2] ACPI Spec 6.3, Table 5.80 >> > >> > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> >> >> One thing I forgot. > And another :) > > It might be nice to also support x86 from the start (apparently people still > care about that old architecture) > > https://gitlab.com/jic23/qemu/-/commit/ccfb4fe22167e035173390cf147d9c226951b9b6 > is what I'm carrying for this (see below) Ack. > We could do this later as part of the generic ports series (which is also on > that tree if you are curious). Yeah, that sounds fine to me.
On Wed, 6 Mar 2024 10:33:17 +0000 Ankit Agrawal <ankita@nvidia.com> wrote: > >> >> Jonathan, Alex, do you know how we may add tests that is dependent > >> >> on the vfio-pci device? > >> > > >> > There are none. > >> > > >> > This would require a host device always available for passthrough and > >> > there is no simple solution for this problem. Such tests would need to > >> > run in a nested environment under avocado: a pc/virt machine with an > >> > igb device and use the PF and/or VFs to check device assignment in a > >> > nested guests. > >> > > >> > PPC just introduced new tests to check nested guest support on two > >> > different HV implementations. If you have time, please take a look > >> > at tests/avocado/ppc_hv_tests.py for the framework. > >> > > >> > I will try to propose a new test when I am done with the reviews, > >> > not before 9.0 soft freeze though. > >> > >> Thanks for the information. As part of this patch, I'll leave out > >> this test change then. > > > > For BIOS table purposes it can be any PCI device. I've been testing > > this with a virtio-net-pci but something like virtio-rng-pci will > > do fine. The table contents doesn't care if it's vfio or not. > > Thanks, I was able to work this out with the virtio-rng-pci device. > > > I can spin a test as part of the follow up Generic Port series that > > incorporates both and pushes the limits of the hmat code in general. > > Current tests are too tame ;) > > Sure, that is fine by me. > FYI, this is how the test change looked like in case you were wondering. Looks good as a starting point. Ideally I'd like HMAT + a few bandwidth and latency values so we test that GIs work with that as well part. Think you'd just need "-machine hmat=on " //some values for cpu to local memory "-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=10G" //some values for the GI node to main memory. "-numa hmat-lb,initiator=1,target=0,hierarchy-memory,data-type=access_latency,latency=200" "-numa hmat-lb,initiator=1,target=0,hierarchy-memory,data-type=access_bandwidth,bandwidth=5G"
diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c index 1ade2f723f..d78382bc63 100644 --- a/hw/acpi/acpi-generic-initiator.c +++ b/hw/acpi/acpi-generic-initiator.c @@ -68,3 +68,87 @@ static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data) object_class_property_add(oc, "node", "int", NULL, acpi_generic_initiator_set_node, NULL, NULL); } + +static int acpi_generic_initiator_list(Object *obj, void *opaque) +{ + GSList **list = opaque; + + if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) { + *list = g_slist_append(*list, ACPI_GENERIC_INITIATOR(obj)); + } + + object_child_foreach(obj, acpi_generic_initiator_list, opaque); + return 0; +} + +/* + * Identify Generic Initiator objects and link them into the list which is + * returned to the caller. + * + * Note: it is the caller's responsibility to free the list to avoid + * memory leak. + */ +static GSList *acpi_generic_initiator_get_list(void) +{ + GSList *list = NULL; + + object_child_foreach(object_get_root(), + acpi_generic_initiator_list, &list); + return list; +} + +/* + * 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, handle->bdf, 2); + 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 */ +} + +void build_srat_generic_pci_initiator(GArray *table_data) +{ + GSList *gi_list, *list = acpi_generic_initiator_get_list(); + AcpiGenericInitiator *gi; + + for (gi_list = list; gi_list; gi_list = gi_list->next) { + PCIDeviceHandle dev_handle; + PCIDevice *pci_dev; + Object *o; + + gi = gi_list->data; + + o = object_resolve_path_type(gi->pci_dev, TYPE_PCI_DEVICE, NULL); + if (!o) { + error_printf("Specified device must be a PCI device.\n"); + exit(1); + } + 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); + } + + g_slist_free(list); +} diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 8bc35a483c..00d77327e0 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -58,6 +58,7 @@ #include "migration/vmstate.h" #include "hw/acpi/ghes.h" #include "hw/acpi/viot.h" +#include "hw/acpi/acpi-generic-initiator.h" #define ARM_SPI_BASE 32 @@ -558,6 +559,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) } } + build_srat_generic_pci_initiator(table_data); + if (ms->nvdimms_state->is_enabled) { nvdimm_build_srat(table_data); } diff --git a/include/hw/acpi/acpi-generic-initiator.h b/include/hw/acpi/acpi-generic-initiator.h index 2f183b029a..213545e614 100644 --- a/include/hw/acpi/acpi-generic-initiator.h +++ b/include/hw/acpi/acpi-generic-initiator.h @@ -29,4 +29,30 @@ typedef struct AcpiGenericInitiatorClass { ObjectClass parent_class; } AcpiGenericInitiatorClass; +/* + * ACPI 6.3: + * Table 5-81 Flags – Generic Initiator Affinity Structure + */ +typedef enum { + GEN_AFFINITY_ENABLED = (1 << 0), /* + * 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. + */ +} 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