Re: [RFC 0/2] GENL interface for ACPI _DSM methods

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,

On 1/6/25 12:01 PM, Jonathan Cameron wrote:
> On Mon, 6 Jan 2025 16:30:43 +0000
> Wathsala Vithanage <wathsala.vithanage@arm.com> wrote:
> 
>> Linux v6.13-rc1 added support for PCIe TPH and direct cache injection.
>> As already described in the patch set[1] that introduced this feature,
>> the cache injection in supported hardware allows optimal utilization of
>> platform resources for specific requests on the PCIe bus. However, the
>> patch set [1] implements the functionality for usage within the kernel.
>> But certain user space applications, especially those whose performance
>> is sensitive to the latency of inbound writes as seen by a CPU core, may
>> benefit from using this information (E.g., the DPDK cache stashing
>> feature discussed in RFC [2]). This RFC is an attempt to obtain the PCIe
>> steering tag information from the kernel to be used by user mode
>> applications. We understand that there is more than one way to provide
>> this information. Please review and suggest alternatives if necessary.
>>
>> The first of the two patches introduced in this RFC attempts to overcome
>> the kernel-only limitation by providing an API to kernel subsystems to
>> hook up relevant _DSM methods to a GENL interface. User space
>> applications can invoke a _DSM hooked up to this interface via the
>> "acpi-event" GENL family socket, granted they have the minimum
>> capabilities and message formats demanded by the kernel subsystem that
>> hooked up the _DSM method. This feature is added by extending the
>> "acpi-event" GENL family that multicasts ACPI events to the user-space
>> applications such as acpid.
>>
>> The second patch of this RFC hooks up the PCIe root-port TLP Processing
>> Hints (TPH) _DSM to the ACPI GENL interface. User space applications
>> like [2] can now request the kernel to execute the _DSM on their behalf
>> and return steering-tag information.
>>
>> [1] lore.kernel.org/linux-pci/20241002165954.128085-1-wei.huang2@amd.com
>> [2] inbox.dpdk.org/dev/20241021015246.304431-2-wathsala.vithanage@arm.com
> 
> Hi Wathsala,
> 
> Superficially this feels like another potential interface that could be wrapped
> up under appropriate fwctl. Jason, what do you think?
> 
> Mind you I'm not personally convinced that an interface that focuses on
> exposing _DSM calls to userspace makes sense as opposed to subsystem specific
> stuff.
> 
> Maybe consider associating the actual interface with the individual PCI functions
> (which provides the first chunk of the message directly).

Right,

I think this was similar to a conversation we had internally, which was 
basically to detect the PCIe extended capability and export a 'steering' 
entry in sysfs on each PCIe device which can take a logical cpu/cache 
value, translate those on write to the ACPI cpu/cache id's, make the 
firmware call, then directly update the PCIe device's capability with 
the result. This also leaves the door open for future 
cpu/cache->steering tag translation methods to transparently replace the 
_DSM call while leaving the userspace API the same.


> 
> Also, _DSM is just one form of firmware interface used for PCI supporting
> system. Tying the userspace interface to that feels unwise.  I can certainly
> foresee a PSCI/SCMI or similar interface for this on ARM platforms
> wrapped up in _DSM where ACPI is present but directly accessed when DT
> is in use.
> 
> I'd also request that you break out what goes in ARG0,1,2 as that is all
> stuff that the kernel is aware of and not all reviewers have access to the
> ECN (I do though).  In particular the fact there are ACPI UIDs may
> need a more generic solution.
> 
> Jonathan
> 
>> Wathsala Vithanage (2):
>>    ACPI: Add support for invoking select _DSM methods from user space
>>    PCI: Add generic netlink interface to TPH _DSM
>>
>>   drivers/acpi/Makefile                 |   3 +-
>>   drivers/acpi/{event.c => acpi_genl.c} | 110 ++++++++++++++++++++++-
>>   drivers/acpi/acpi_genl_dsm.c          |  76 ++++++++++++++++
>>   drivers/pci/tph.c                     | 121 ++++++++++++++++++++++++++
>>   include/acpi/acpi_genl.h              |  54 ++++++++++++
>>   include/linux/acpi.h                  |   1 +
>>   6 files changed, 360 insertions(+), 5 deletions(-)
>>   rename drivers/acpi/{event.c => acpi_genl.c} (63%)
>>   create mode 100644 drivers/acpi/acpi_genl_dsm.c
>>   create mode 100644 include/acpi/acpi_genl.h
>>
>