Re: [RFC PATCH 0/3] QEMU ACPI generic port support

[Jonathan Cameron via <qemu-devel@nongnu.org>]

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
> 
> --
> 
>