netdev - Re: [PATCH net-next 1/2] ixgbe: register a mdiobus

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]

Date:   Fri, 30 Nov 2018 09:43:56 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Steve Douthit <stephend@...icom-usa.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/2] ixgbe: register a mdiobus



On 11/30/2018 9:34 AM, Steve Douthit wrote:
> On 11/30/18 11:34 AM, Andrew Lunn wrote:
>>> Yep, registering multiple interfaces is wrong.  The first board I tested
>>> against only had a single MAC enabled (they can be disabled/hidden via
>>> straps) so it just happened to work.
>>
>> Hi Steve
>>
>> Can you hide any/all via straps, or is 00.0 always guaranteed to
>> exist?
> 
> You can hide all the devices, but if function 1 is enabled then function
> 0 must also be enabled, so not all combinations are valid.
> 
>>> The Intel C3xxx family of SoCs have up to four ixgbe MACs.  These are
>>> structured as two devices of two functions each on fixed internal root
>>> ports.
>>>
>>> from lspci:
>>> <snip>
>>>              +-16.0-[05]--+-00.0
>>>              |            \-00.1
>>>              +-17.0-[06]--+-00.0
>>>              |            \-00.1
>>> <snip>
>>
>> Is there any other hardware resource which is shared between the MAC
>> interfaces? I'm just wondering if the driver has already solved this
>> once. Is there an EEPROM per interface for the MAC address, or one
>> shared EEPROM?
> 
> NVM is shared, it's actually the same SPI flash that holds the BIOS for
> this SoC.  Access is serialized by the swfw_sync semaphore.  I think the
> device firmware is automagically handling offset translation.
> 
> I don't think that helps for this case.
> 
> There might be a better match for shared resources, but nothing springs
> to mind.
> 
>> Ah, how about using the 'cards_found' found variable. It is not
>> perfect, in that it is not decremented in ixgb_remove(), and i wonder
>> about race conditions since there does not appear to be any lock when
>> it is incremented. But if cards_found == 0, register the MDIO bus.
> 
> 'cards_found' doesn't exist for the ixgbe driver.  I could add it and
> fix the race/decrement issues you mention, but it'd have to be a device
> type specific count.  It's still possible there are other non-x550em_a
> ixgbe devices in external PCIe slots that have different resource
> sharing setups.
> 
> It's still a lighter weight solution than poking around the parent bus
> so I'll add a 'x550em_a_devs_found' counter to v2.
> 

If the MDIO bus block is hardwired to function 0, would it be acceptable
to walk the PCI bus hierarchy from any driver's probe routine where PCI
function != 0 and see if it is claimed/enabled already, and if so, skip
registering the MDIO bus entirely?

There is also possibly a problem if you have a shared MDIO device, and
you turn off/power off the PCI function that does provide it. How can we
make sure it remains alive for other functions to use it?
-- 
Florian