netdev - Re: [PATCH v9 net-next 05/14] pds_core: set up device and adminq

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: <5635d3b3-0355-f22c-64c3-34036d743ec2@amd.com>
Date:   Mon, 10 Apr 2023 12:27:28 -0700
From:   Shannon Nelson <shannon.nelson@....com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
        kuba@...nel.org, drivers@...sando.io, jiri@...nulli.us
Subject: Re: [PATCH v9 net-next 05/14] pds_core: set up device and adminq

On 4/9/23 5:03 AM, Leon Romanovsky wrote:
> 
> On Thu, Apr 06, 2023 at 04:41:34PM -0700, Shannon Nelson wrote:
>> Set up the basic adminq and notifyq queue structures.  These are
>> used mostly by the client drivers for feature configuration.
>> These are essentially the same adminq and notifyq as in the
>> ionic driver.
>>
>> Part of this includes querying for device identity and FW
>> information, so we can make that available to devlink dev info.
>>
>>    $ devlink dev info pci/0000:b5:00.0
>>    pci/0000:b5:00.0:
>>      driver pds_core
>>      serial_number FLM18420073
>>      versions:
>>          fixed:
>>            asic.id 0x0
>>            asic.rev 0x0
>>          running:
>>            fw 1.51.0-73
>>          stored:
>>            fw.goldfw 1.15.9-C-22
>>            fw.mainfwa 1.60.0-73
>>            fw.mainfwb 1.60.0-57
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>> ---
>>   .../device_drivers/ethernet/amd/pds_core.rst  |  47 ++
>>   drivers/net/ethernet/amd/pds_core/core.c      | 450 ++++++++++++-
>>   drivers/net/ethernet/amd/pds_core/core.h      | 154 +++++
>>   drivers/net/ethernet/amd/pds_core/debugfs.c   |  76 +++
>>   drivers/net/ethernet/amd/pds_core/devlink.c   |  61 ++
>>   drivers/net/ethernet/amd/pds_core/main.c      |  17 +-
>>   include/linux/pds/pds_adminq.h                | 637 ++++++++++++++++++
>>   7 files changed, 1438 insertions(+), 4 deletions(-)
>>   create mode 100644 include/linux/pds/pds_adminq.h
> 
> <...>
> 
>> +void pdsc_stop(struct pdsc *pdsc)
>> +{
>> +     if (pdsc->wq)
>> +             flush_workqueue(pdsc->wq);
>> +
>> +     pdsc_mask_interrupts(pdsc);
>> +}
>> +
> 
> <...>
> 
>>   static const struct devlink_ops pdsc_dl_vf_ops = {
>> @@ -332,6 +346,7 @@ static void pdsc_remove(struct pci_dev *pdev)
>>                mutex_lock(&pdsc->config_lock);
>>                set_bit(PDSC_S_STOPPING_DRIVER, &pdsc->state);
>>
>> +             pdsc_stop(pdsc);
> 
> You are calling to flush this workqueue in a couple of line above.

I guess we really want it flushed...
I'll look at that to see if we really need the extra flushing.


> 
>>                pdsc_teardown(pdsc, PDSC_TEARDOWN_REMOVING);
>>                mutex_unlock(&pdsc->config_lock);
>>                mutex_destroy(&pdsc->config_lock);
>> diff --git a/include/linux/pds/pds_adminq.h b/include/linux/pds/pds_adminq.h
>> new file mode 100644
>> index 000000000000..9cd58b7f5fb2
>> --- /dev/null
>> +++ b/include/linux/pds/pds_adminq.h
>> @@ -0,0 +1,637 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2023 Advanced Micro Devices, Inc */
>> +
> 
> <...>
> 
>> +/* The color bit is a 'done' bit for the completion descriptors
>> + * where the meaning alternates between '1' and '0' for alternating
>> + * passes through the completion descriptor ring.
>> + */
>> +static inline u8 pdsc_color_match(u8 color, u8 done_color)
> 
> static inline bool?

Sure.

> 
>> +{
>> +     return (!!(color & PDS_COMP_COLOR_MASK)) == done_color;
>> +}
>> +#endif /* _PDS_CORE_ADMINQ_H_ */
>> --
>> 2.17.1
>>