Re: [PATCH v2 3/6] hw/acpi: Generic Port Affinity Structure support

[Markus Armbruster <armbru@redhat.com>]

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

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

I figure your mean "discoverable", and ...

> mechanism such as UEFI CDAT read from CXL devices and switches
> is used to discover the remainder fo the path and the OS can build

... " of the path, and the OS".

> up full latency and bandwidth numbers as need for work and data
> placement decisions.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: Updates to QMP documentation to provide a lot more information
>     on the parameters.
> ---
>  qapi/qom.json                            |  35 ++++++
>  include/hw/acpi/acpi_generic_initiator.h |  18 ++-
>  include/hw/pci/pci_bridge.h              |   1 +
>  hw/acpi/acpi_generic_initiator.c         | 141 +++++++++++++++++------
>  hw/pci-bridge/pci_expander_bridge.c      |   1 -
>  5 files changed, 158 insertions(+), 38 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 38dde6d785..9d1d86bdad 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -826,6 +826,39 @@
>    'data': { 'pci-dev': 'str',
>              'node': 'uint32' } }
>  
> +

Extra blank line.

> +##
> +# @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

I'd put the period behind the parenthesis: "bridges).  See ACPI"

> +#     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.

Lines are slightly wide in places.  

Together:

   # @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:
>  #
> @@ -953,6 +986,7 @@
>  { 'enum': 'ObjectType',
>    'data': [
>      'acpi-generic-initiator',
> +    'acpi-generic-port',
>      'authz-list',
>      'authz-listfile',
>      'authz-pam',
> @@ -1025,6 +1059,7 @@
>    'discriminator': 'qom-type',
>    'data': {
>        'acpi-generic-initiator':     'AcpiGenericInitiatorProperties',
> +      'acpi-generic-port':          'AcpiGenericPortProperties',
>        'authz-list':                 'AuthZListProperties',
>        'authz-listfile':             'AuthZListFileProperties',
>        'authz-pam':                  'AuthZPAMProperties',

Preferably with these touch-ups
Acked-by: Markus Armbruster <armbru@redhat.com>

[...]