Re: [PATCH v5 13/18] pci: introduce pci_find_the_only_child()

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 12:35:54PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 02.03.23 11:53, Michael S. Tsirkin wrote:
> > On Thu, Mar 02, 2023 at 11:45:00AM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > On 02.03.23 11:37, Michael S. Tsirkin wrote:
> > > > On Thu, Mar 02, 2023 at 11:28:44AM +0300, Vladimir Sementsov-Ogievskiy 
> > > > wrote:
> > > > > On 02.03.23 00:09, Michael S. Tsirkin wrote:
> > > > > > On Thu, Feb 16, 2023 at 09:03:51PM +0300, Vladimir 
> > > > > > Sementsov-Ogievskiy wrote:
> > > > > > > To be used in further patch to identify the device hot-plugged 
> > > > > > > into
> > > > > > > pcie-root-port.
> > > > > > > 
> > > > > > > Signed-off-by: Vladimir 
> > > > > > > Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
> > > > > > > Reviewed-by: Anton Kuchin<antonkuchin@yandex-team.ru>
> > > > > > Wait a second does this work for multifunction devices correctly?
> > > > > > 
> > > > > 
> > > > > I thought about that and I'm just lost:)
> > > > > 
> > > > > Could several (multifunction?) devices be plugged into one 
> > > > > pcie-root-port device?
> > > > 
> > > > One device per port but one multifunction device is represented as 
> > > > multiple PCIDevice structures.
> > > 
> > > So, it should be OK to send _one_ event for that multifunction device, 
> > > and the code is shpc_reset() is OK, but reporting "Several child devices 
> > > found" is bad idea?
> > 
> > I don't know about your new event, we are discussing it separately.
> > yes all functions are removed together normally on real hardware.
> > 
> > > So, if I change the logic from pci_find_the_only_child() to 
> > > pci_find_first_child() is it OK?
> > 
> > Yes though I don't like this name either - need to make it clear that
> > multifunction is ok, multiple unrelated devices aren't.
> 
> Could we check it somehow that all plugged devices represent the one 
> multifunction device?

You can write a function like this, sure.
But really just return a bus+slot pair.

> > 
> > 
> > > > 
> > > > > Same question for SHPC slots. For example, 
> > > > > shpc_free_devices_in_slot() looks like we can have several devices in 
> > > > > one slot..
> > > > > On the other hand, in shpc_reset() we have construction 
> > > > > shpc->sec_bus->devices[PCI_DEVFN(SHPC_IDX_TO_PCI(i), 0)] to access 
> > > > > the device in slot. The only one device.
> > > > > 
> > > > > -- 
> > > > > Best regards,
> > > > > Vladimir
> > > > 
> > > > Same thing.
> > > > 
> > > > ... and let's not get started about sriov and ari ...
> > > > 
> > > 
> > > -- 
> > > Best regards,
> > > Vladimir
> > 
> 
> -- 
> Best regards,
> Vladimir