mbox series

[RFC,0/3] QEMU ACPI generic port support

Message ID 168185633821.899932.322047053764766056.stgit@djiang5-mobl3 (mailing list archive)
Headers show
Series QEMU ACPI generic port support | expand

Message

Dave Jiang April 18, 2023, 10:21 p.m. UTC
s small RFC patch series is really a hack on what I need from qemu rather
than a proper implementation. I'm hoping to get some guidance from the list on
how to implement this correctly for qemu upstream. Thank you!

The patch series provides support for the ACPI Generic Port support that's
defined by ACPI spec 6.5 5.2.16.7 (Generic Port Affinity Structure). The
series also adds a genport object that allows locality data to be injected via
qemu commandline to the HMAT tables. The generic port support is to allow a hot
plugged CXL memory device to calculate the locality data from the CPU to
the CXL device. The generic port related data provides the locality data from
the CPU to the CXL host bridge (latency and bandwidth). These data in
addition to the PCIe link data, CDAT from device, and CXL switch CDAT if switch
exist, provides the locality data for the entire path.

Patch1: Adds Generic Port Affinity Structure sub-tables to the SRAT. For
each CXL Host Bridge (HB) a GPAS entry is created with a unique proximity
domain. For example, if the system is created with 4 proximity domains (PXM) for
system memory, then the next GPAS will get PXM 4 and so on.

Patch2: Add the json support for generic port. Split out because
clang-format really clobbers the json files.

Patch3: Add a generic port object. The intention here is to allow setup of
numa nodes, add hmat-lb data and node distance for the generic targets. I had to
add a hack in qemu_create_cli_devices() to realize the genport objects. I need
guidance on where and how to do this properly so the genport objects
realize at the correct place and time.

Example of genport setup:
-object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
-numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
-numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM
for ((i = 0; i < total_nodes; i++)); do
        for ((j = 0; j < cxl_hbs; j++ )); do        # 2 CXL HBs
                -numa dist,src=$i,dst=$X,val=$dist
        done
done
Linux kernel support:
https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t

---

Dave Jiang (3):
      hw/acpi: Add support for Generic Port Affinity Structure to SRAT
      genport: Add json support for generic port
      acpi: add generic port device object


 hw/acpi/aml-build.c         | 21 +++++++++++++
 hw/acpi/genport.c           | 61 +++++++++++++++++++++++++++++++++++++
 hw/acpi/meson.build         |  1 +
 hw/i386/acpi-build.c        | 45 +++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h | 27 ++++++++++++++++
 qapi/machine.json           |  3 +-
 qapi/qom.json               | 12 ++++++++
 softmmu/vl.c                | 26 ++++++++++++++++
 8 files changed, 195 insertions(+), 1 deletion(-)
 create mode 100644 hw/acpi/genport.c

--

Comments

Jonathan Cameron May 3, 2023, 10:42 a.m. UTC | #1
On Tue, 18 Apr 2023 15:21:36 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> s small RFC patch series is really a hack on what I need from qemu rather
> than a proper implementation. I'm hoping to get some guidance from the list on
> how to implement this correctly for qemu upstream. Thank you!
> 
> The patch series provides support for the ACPI Generic Port support that's
> defined by ACPI spec 6.5 5.2.16.7 (Generic Port Affinity Structure). The
> series also adds a genport object that allows locality data to be injected via
> qemu commandline to the HMAT tables. The generic port support is to allow a hot
> plugged CXL memory device to calculate the locality data from the CPU to
> the CXL device. The generic port related data provides the locality data from
> the CPU to the CXL host bridge (latency and bandwidth). These data in
> addition to the PCIe link data, CDAT from device, and CXL switch CDAT if switch
> exist, provides the locality data for the entire path.
> 
> Patch1: Adds Generic Port Affinity Structure sub-tables to the SRAT. For
> each CXL Host Bridge (HB) a GPAS entry is created with a unique proximity
> domain. For example, if the system is created with 4 proximity domains (PXM) for
> system memory, then the next GPAS will get PXM 4 and so on.

I may be going crazy but I'm not seeing an increment on the numa node. So I think
they all get 4 at the moment. Found it increment in patch 3.

> 
> Patch2: Add the json support for generic port. Split out because
> clang-format really clobbers the json files.
> 
> Patch3: Add a generic port object. The intention here is to allow setup of
> numa nodes, add hmat-lb data and node distance for the generic targets. I had to
> add a hack in qemu_create_cli_devices() to realize the genport objects. I need
> guidance on where and how to do this properly so the genport objects
> realize at the correct place and time.
> 
> Example of genport setup:
> -object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM

I think we should be using some links to the host bridges in here.
So I don't think there should be an explicit genport object at all.
Instead we should be able to point at the pxb itself.  Perhaps also
allowing other port types in future.

Something like

-device pxb-cxl,id=cxl1.1
-numa node,genport=cxl1.1,nodeid=$X
-numa hmat-lb,initiator=$Z,target=$X,...
-numa hmat-lb,initiator=$X,target=$Y,...
//as generic port goes both ways.

As we are currently using bus_nr for UID (which is admittedly a somewhat dirty hack that
just happened to be convenient) the ACPI building code can use that to fill in the SRAT
entry at appropriate point.

I haven't tried implementing it so there may well be some ordering issues that
require some late binding etc, but it should be possible to make it work.

> for ((i = 0; i < total_nodes; i++)); do
>         for ((j = 0; j < cxl_hbs; j++ )); do        # 2 CXL HBs
>                 -numa dist,src=$i,dst=$X,val=$dist
>         done
> done
> Linux kernel support:
> https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
> 
> ---
> 
> Dave Jiang (3):
>       hw/acpi: Add support for Generic Port Affinity Structure to SRAT
>       genport: Add json support for generic port
>       acpi: add generic port device object
> 
> 
>  hw/acpi/aml-build.c         | 21 +++++++++++++
>  hw/acpi/genport.c           | 61 +++++++++++++++++++++++++++++++++++++
>  hw/acpi/meson.build         |  1 +
>  hw/i386/acpi-build.c        | 45 +++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h | 27 ++++++++++++++++
>  qapi/machine.json           |  3 +-
>  qapi/qom.json               | 12 ++++++++
>  softmmu/vl.c                | 26 ++++++++++++++++
>  8 files changed, 195 insertions(+), 1 deletion(-)
>  create mode 100644 hw/acpi/genport.c
> 
> --
> 
>
Dave Jiang May 3, 2023, 3:25 p.m. UTC | #2
On 5/3/23 3:42 AM, Jonathan Cameron wrote:
> On Tue, 18 Apr 2023 15:21:36 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> s small RFC patch series is really a hack on what I need from qemu rather
>> than a proper implementation. I'm hoping to get some guidance from the list on
>> how to implement this correctly for qemu upstream. Thank you!
>>
>> The patch series provides support for the ACPI Generic Port support that's
>> defined by ACPI spec 6.5 5.2.16.7 (Generic Port Affinity Structure). The
>> series also adds a genport object that allows locality data to be injected via
>> qemu commandline to the HMAT tables. The generic port support is to allow a hot
>> plugged CXL memory device to calculate the locality data from the CPU to
>> the CXL device. The generic port related data provides the locality data from
>> the CPU to the CXL host bridge (latency and bandwidth). These data in
>> addition to the PCIe link data, CDAT from device, and CXL switch CDAT if switch
>> exist, provides the locality data for the entire path.
>>
>> Patch1: Adds Generic Port Affinity Structure sub-tables to the SRAT. For
>> each CXL Host Bridge (HB) a GPAS entry is created with a unique proximity
>> domain. For example, if the system is created with 4 proximity domains (PXM) for
>> system memory, then the next GPAS will get PXM 4 and so on.
> 
> I may be going crazy but I'm not seeing an increment on the numa node. So I think
> they all get 4 at the moment. Found it increment in patch 3.
> 
>>
>> Patch2: Add the json support for generic port. Split out because
>> clang-format really clobbers the json files.
>>
>> Patch3: Add a generic port object. The intention here is to allow setup of
>> numa nodes, add hmat-lb data and node distance for the generic targets. I had to
>> add a hack in qemu_create_cli_devices() to realize the genport objects. I need
>> guidance on where and how to do this properly so the genport objects
>> realize at the correct place and time.
>>
>> Example of genport setup:
>> -object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
>> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
>> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM
> 
> I think we should be using some links to the host bridges in here.
> So I don't think there should be an explicit genport object at all.
> Instead we should be able to point at the pxb itself.  Perhaps also
> allowing other port types in future.
> 
> Something like
> 
> -device pxb-cxl,id=cxl1.1
> -numa node,genport=cxl1.1,nodeid=$X
> -numa hmat-lb,initiator=$Z,target=$X,...
> -numa hmat-lb,initiator=$X,target=$Y,...
> //as generic port goes both ways.
> 
> As we are currently using bus_nr for UID (which is admittedly a somewhat dirty hack that
> just happened to be convenient) the ACPI building code can use that to fill in the SRAT
> entry at appropriate point.
> 
> I haven't tried implementing it so there may well be some ordering issues that
> require some late binding etc, but it should be possible to make it work.

Ok thanks for the suggestion Jonathan! I appreciate you looking over 
this. I'll take a look when I get a chance.

> 
>> for ((i = 0; i < total_nodes; i++)); do
>>          for ((j = 0; j < cxl_hbs; j++ )); do        # 2 CXL HBs
>>                  -numa dist,src=$i,dst=$X,val=$dist
>>          done
>> done
>> Linux kernel support:
>> https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
>>
>> ---
>>
>> Dave Jiang (3):
>>        hw/acpi: Add support for Generic Port Affinity Structure to SRAT
>>        genport: Add json support for generic port
>>        acpi: add generic port device object
>>
>>
>>   hw/acpi/aml-build.c         | 21 +++++++++++++
>>   hw/acpi/genport.c           | 61 +++++++++++++++++++++++++++++++++++++
>>   hw/acpi/meson.build         |  1 +
>>   hw/i386/acpi-build.c        | 45 +++++++++++++++++++++++++++
>>   include/hw/acpi/aml-build.h | 27 ++++++++++++++++
>>   qapi/machine.json           |  3 +-
>>   qapi/qom.json               | 12 ++++++++
>>   softmmu/vl.c                | 26 ++++++++++++++++
>>   8 files changed, 195 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/acpi/genport.c
>>
>> --
>>
>>
>
Dave Jiang May 11, 2023, 4:14 p.m. UTC | #3
On 5/3/23 3:42 AM, Jonathan Cameron wrote:
> On Tue, 18 Apr 2023 15:21:36 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> s small RFC patch series is really a hack on what I need from qemu rather
>> than a proper implementation. I'm hoping to get some guidance from the list on
>> how to implement this correctly for qemu upstream. Thank you!
>>
>> The patch series provides support for the ACPI Generic Port support that's
>> defined by ACPI spec 6.5 5.2.16.7 (Generic Port Affinity Structure). The
>> series also adds a genport object that allows locality data to be injected via
>> qemu commandline to the HMAT tables. The generic port support is to allow a hot
>> plugged CXL memory device to calculate the locality data from the CPU to
>> the CXL device. The generic port related data provides the locality data from
>> the CPU to the CXL host bridge (latency and bandwidth). These data in
>> addition to the PCIe link data, CDAT from device, and CXL switch CDAT if switch
>> exist, provides the locality data for the entire path.
>>
>> Patch1: Adds Generic Port Affinity Structure sub-tables to the SRAT. For
>> each CXL Host Bridge (HB) a GPAS entry is created with a unique proximity
>> domain. For example, if the system is created with 4 proximity domains (PXM) for
>> system memory, then the next GPAS will get PXM 4 and so on.
> 
> I may be going crazy but I'm not seeing an increment on the numa node. So I think
> they all get 4 at the moment. Found it increment in patch 3.

Sorry about that. There are some changes for 1/3 strayed into 3/3.

> 
>>
>> Patch2: Add the json support for generic port. Split out because
>> clang-format really clobbers the json files.
>>
>> Patch3: Add a generic port object. The intention here is to allow setup of
>> numa nodes, add hmat-lb data and node distance for the generic targets. I had to
>> add a hack in qemu_create_cli_devices() to realize the genport objects. I need
>> guidance on where and how to do this properly so the genport objects
>> realize at the correct place and time.
>>
>> Example of genport setup:
>> -object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
>> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
>> -numa hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM
> 
> I think we should be using some links to the host bridges in here.
> So I don't think there should be an explicit genport object at all.
> Instead we should be able to point at the pxb itself.  Perhaps also
> allowing other port types in future.
> 
> Something like
> 
> -device pxb-cxl,id=cxl1.1
> -numa node,genport=cxl1.1,nodeid=$X

Ok I think that makes sense. So now there's a relation between genport 
being constructed and the passed in numa node. When we are building the 
SRAT, I assume there's a way to get hold of the parsed numa nodes 
attributes and iterate through to attempt a match?

> -numa hmat-lb,initiator=$Z,target=$X,...
> -numa hmat-lb,initiator=$X,target=$Y,...
> //as generic port goes both ways.
> 
> As we are currently using bus_nr for UID (which is admittedly a somewhat dirty hack that
> just happened to be convenient) the ACPI building code can use that to fill in the SRAT
> entry at appropriate point.
> 
> I haven't tried implementing it so there may well be some ordering issues that
> require some late binding etc, but it should be possible to make it work.
> 
>> for ((i = 0; i < total_nodes; i++)); do
>>          for ((j = 0; j < cxl_hbs; j++ )); do        # 2 CXL HBs
>>                  -numa dist,src=$i,dst=$X,val=$dist
>>          done
>> done
>> Linux kernel support:
>> https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
>>
>> ---
>>
>> Dave Jiang (3):
>>        hw/acpi: Add support for Generic Port Affinity Structure to SRAT
>>        genport: Add json support for generic port
>>        acpi: add generic port device object
>>
>>
>>   hw/acpi/aml-build.c         | 21 +++++++++++++
>>   hw/acpi/genport.c           | 61 +++++++++++++++++++++++++++++++++++++
>>   hw/acpi/meson.build         |  1 +
>>   hw/i386/acpi-build.c        | 45 +++++++++++++++++++++++++++
>>   include/hw/acpi/aml-build.h | 27 ++++++++++++++++
>>   qapi/machine.json           |  3 +-
>>   qapi/qom.json               | 12 ++++++++
>>   softmmu/vl.c                | 26 ++++++++++++++++
>>   8 files changed, 195 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/acpi/genport.c
>>
>> --
>>
>>
>