netdev - Re: [RFC PATCH fwctl 2/5] pds_core: add new fwctl auxilary

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6fc8dae5-9bfb-4b74-b741-b85159a0daef@amd.com>
Date: Thu, 20 Feb 2025 15:20:14 -0800
From: "Nelson, Shannon" <shannon.nelson@....com>
To: Leon Romanovsky <leon@...nel.org>
Cc: jgg@...dia.com, andrew.gospodarek@...adcom.com,
 aron.silverton@...cle.com, dan.j.williams@...el.com, daniel.vetter@...ll.ch,
 dave.jiang@...el.com, dsahern@...nel.org, gospo@...adcom.com,
 hch@...radead.org, itayavr@...dia.com, jiri@...dia.com,
 Jonathan.Cameron@...wei.com, kuba@...nel.org, lbloch@...dia.com,
 saeedm@...dia.com, linux-cxl@...r.kernel.org, linux-rdma@...r.kernel.org,
 netdev@...r.kernel.org, brett.creeley@....com
Subject: Re: [RFC PATCH fwctl 2/5] pds_core: add new fwctl auxilary_device

On 2/19/2025 12:24 AM, Leon Romanovsky wrote:
> 
> On Tue, Feb 18, 2025 at 12:00:52PM -0800, Nelson, Shannon wrote:
>> On 2/18/2025 11:28 AM, Leon Romanovsky wrote:
>>>
>>> On Tue, Feb 11, 2025 at 03:48:51PM -0800, Shannon Nelson wrote:
>>>> Add support for a new fwctl-based auxiliary_device for creating a
>>>> channel for fwctl support into the AMD/Pensando DSC.
>>>>
>>>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>>>> ---
>>>>    drivers/net/ethernet/amd/pds_core/auxbus.c |  3 +--
>>>>    drivers/net/ethernet/amd/pds_core/core.c   |  7 +++++++
>>>>    drivers/net/ethernet/amd/pds_core/core.h   |  1 +
>>>>    drivers/net/ethernet/amd/pds_core/main.c   | 10 ++++++++++
>>>>    include/linux/pds/pds_common.h             |  2 ++
>>>>    5 files changed, 21 insertions(+), 2 deletions(-)
>>>
>>> <...>
>>>
>>> My comment is only slightly related to the patch itself, but worth to
>>> write it anyway.
>>>
>>>> diff --git a/include/linux/pds/pds_common.h b/include/linux/pds/pds_common.h
>>>> index 5802e1deef24..b193adbe7cc3 100644
>>>> --- a/include/linux/pds/pds_common.h
>>>> +++ b/include/linux/pds/pds_common.h
>>>> @@ -29,6 +29,7 @@ enum pds_core_vif_types {
>>>>         PDS_DEV_TYPE_ETH        = 3,
>>>>         PDS_DEV_TYPE_RDMA       = 4,
>>>>         PDS_DEV_TYPE_LM         = 5,
>>>> +     PDS_DEV_TYPE_FWCTL      = 6,
>>>
>>> This enum and defines below should be cleaned from unsupported types.
>>> I don't see any code for RDMA, LM and ETH.
>>>
>>> Thanks
>>
>> I've looked at those a few times over the life of this code, but I continue
>> to leave them there because they are part of the firmware interface
>> definition, whether we use them or not.
> 
> How? You are passing some number which FW is not aware of it. You can
> pass any number you want there. Even it is not true, you can
> PDS_DEV_TYPE_FWCTL = 6 here, but remove rest of enums and *_STR defines.

When pds_core starts up it gets ident/config information from the 
firmware in a struct pds_core_dev_identity, which includes the 
vif_types[] array which tells us how many of each PDS_DEV_TYPE_xx are 
supported in the FW.  This is indexed by enum pds_core_vif_types.

> 
>>
>> You're right, there is no ETH or RDMA type code, they exist as historical
>> artifacts of the early interface design.
> 
> This make me wonder why netdev merged this code which has nothing to do
> with netdev subsystem at all.

The pds_core was originally brought in for supporting pds_vdpa and 
pds_vfio_pci.  At the time, it was essentially following the example of 
the mlx core module; at the time there wasn't any push back or 
suggestions of a different place to land.  Maybe further fwctl and 
"core" related discussions will suggest another approach.

sln

> 
> Thanks