netdev - Re: [PATCH v3 net-next 1/3] amd-xgbe: reorganize the code of XPCS access

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: <2ba8b168-627c-48f6-ac63-e83ab8056864@amd.com>
Date: Wed, 13 Dec 2023 10:37:40 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Raju Rangoju <Raju.Rangoju@....com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, Shyam-sundar.S-k@....com
Subject: Re: [PATCH v3 net-next 1/3] amd-xgbe: reorganize the code of XPCS
 access

On 12/13/23 09:35, Raju Rangoju wrote:
> On 12/12/2023 8:03 PM, Tom Lendacky wrote:
>> On 12/11/23 23:37, Raju Rangoju wrote:
>>> The xgbe_{read/write}_mmd_regs_v* functions have common code which can
>>> be moved to helper functions. Also, the xgbe_pci_probe() needs
>>> reorganization.
>>>
>>> Add new helper functions to calculate the mmd_address for v1/v2 of xpcs
>>> access. And, convert if/else statements in xgbe_pci_probe() to switch
>>> case. This helps code look cleaner.
>>>
>>> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>
>>> ---
>>>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 43 ++++++++++++------------
>>>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 18 +++++++---
>>>   2 files changed, 34 insertions(+), 27 deletions(-)
>>>

>>> @@ -274,12 +274,18 @@ 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)) {
>>> +
>>> +    if (!(rdev && rdev->vendor == PCI_VENDOR_ID_AMD)) {
>>> +        ret = -ENODEV;
>>> +        goto err_pci_enable;
>>> +    }
>>
>> This is different behavior compared to today. Today, everything would
>> default to the final "else" statement. With this patch you have a
>> possibility of failing probe now.
> 
> This was done to skip the cases where rdev is NULL or vendor != AMD. Not 
> sure if I'm missing something.

Right, I'm just pointing out that if rdev == NULL or vendor != AMD then, 
today, the else path is taken. With this patch you'll fail the probe with 
-ENODEV.

Thanks,
Tom

>