netdev - Re: [PATCH net-next 1/3] net: amd-xgbe: Add Support for Yellow Carp Ethernet device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <24152108-701d-7f0d-6b60-8bde179846d5@amd.com>
Date:   Mon, 20 Dec 2021 15:43:12 +0530
From:   Raju Rangoju <Raju.Rangoju@....com>
To:     Tom Lendacky <thomas.lendacky@....com>,
        Raju Rangoju <rrangoju@....com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     netdev@...r.kernel.org, Shyam-sundar.S-k@....com,
        Sudheesh.Mavila@....com
Subject: Re: [PATCH net-next 1/3] net: amd-xgbe: Add Support for Yellow Carp
 Ethernet device



On 17/12/2021 19:58, Tom Lendacky wrote:
> On 12/17/21 5:15 AM, Raju Rangoju wrote:
>> From: Raju Rangoju <Raju.Rangoju@....com>
>>
>> Yellow Carp Ethernet devices use the existing PCI ID but
>> the window settings for the indirect PCS access have been
>> altered. Add the check for Yellow Carp Ethernet devices to
>> use the new register values.
>>
>> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>
>> ---
>>   drivers/net/ethernet/amd/xgbe/xgbe-common.h |  2 ++
>>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 12 ++++++++----
>>   2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h 
>> b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> index 533b8519ec35..0075939121d1 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> @@ -898,6 +898,8 @@
>>   #define PCS_V2_WINDOW_SELECT        0x9064
>>   #define PCS_V2_RV_WINDOW_DEF        0x1060
>>   #define PCS_V2_RV_WINDOW_SELECT        0x1064
>> +#define PCS_V2_YC_WINDOW_DEF        0x18060
>> +#define PCS_V2_YC_WINDOW_SELECT        0x18064
>>   /* PCS register entry bit positions and sizes */
>>   #define PCS_V2_WINDOW_DEF_OFFSET_INDEX    6
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c 
>> b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
>> index 90cb55eb5466..39e606c4d653 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
>> @@ -274,10 +274,14 @@ static int xgbe_pci_probe(struct pci_dev *pdev, 
>> const struct pci_device_id *id)
>>       /* Set the PCS indirect addressing definition registers */
>>       rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>> -    if (rdev &&
>> -        (rdev->vendor == PCI_VENDOR_ID_AMD) && (rdev->device == 
>> 0x15d0)) {
>> -        pdata->xpcs_window_def_reg = PCS_V2_RV_WINDOW_DEF;
>> -        pdata->xpcs_window_sel_reg = PCS_V2_RV_WINDOW_SELECT;
>> +    if (rdev && rdev->vendor == PCI_VENDOR_ID_AMD) {
>> +        if (rdev->device == 0x15d0) {
>> +            pdata->xpcs_window_def_reg = PCS_V2_RV_WINDOW_DEF;
>> +            pdata->xpcs_window_sel_reg = PCS_V2_RV_WINDOW_SELECT;
>> +        } else if (rdev->device == 0x14b5) {
>> +            pdata->xpcs_window_def_reg = PCS_V2_YC_WINDOW_DEF;
>> +            pdata->xpcs_window_sel_reg = PCS_V2_YC_WINDOW_SELECT;
>> +        }
> 
> Hmmm... now that I look at this, pdata->xpcs_window... won't be set for 
> non RV or YC platforms, right?  All rdev devices should have an AMD 
> vendor ID and therefore always take the if path. But only RV and YC will 
> set the values. So this needs to be reworked.
> 

Hi Tom,

Good catch, thanks! I missed it.

I'll re-spin the next version with the rework.

Thanks,
Raju


> Thanks,
> Tom
> 
>>       } else {
>>           pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF;
>>           pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT;
>>