linux-kernel - Re: [PATCH] vfio-pci: PCI hot reset interface

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: <CAErSpo50ve+ZMqcX=13hKvrB75LyWmUddC-qyX7Esfr=P5StcA@mail.gmail.com>
Date:	Mon, 19 Aug 2013 14:02:41 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Alex Williamson <alex.williamson@...hat.com>
Cc:	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] vfio-pci: PCI hot reset interface

On Mon, Aug 19, 2013 at 12:41 PM, Alex Williamson
<alex.williamson@...hat.com> wrote:
> On Wed, 2013-08-14 at 17:06 -0600, Alex Williamson wrote:
>> On Wed, 2013-08-14 at 16:42 -0600, Bjorn Helgaas wrote:
>> > On Wed, Aug 14, 2013 at 2:10 PM, Alex Williamson
>> > <alex.williamson@...hat.com> wrote:

>> > > +static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
>> > > +                                        int (*fn)(struct pci_dev *,
>> > > +                                                  void *data), void *data,
>> > > +                                        bool slot)
>> > > +{
>> > > +       struct pci_dev *tmp;
>> > > +       int ret = 0;
>> > > +
>> > > +       list_for_each_entry(tmp, &pdev->bus->devices, bus_list) {
>> > > +               if (slot && tmp->slot != pdev->slot)
>> > > +                       continue;
>> > > +
>> > > +               ret = fn(tmp, data);
>> > > +               if (ret)
>> > > +                       break;
>> > > +
>> > > +               if (tmp->subordinate) {
>> > > +                       ret = vfio_pci_for_each_slot_or_bus(tmp, fn,
>> > > +                                                           data, false);
>> > > +                       if (ret)
>> > > +                               break;
>> > > +               }
>> > > +       }
>> > > +
>> > > +       return ret;
>> > > +}
>> >
>> > vfio_pci_for_each_slot_or_bus() isn't really vfio-specific, is it?
>>
>> It's not, I originally has callbacks split out as PCI patches but I was
>> able to simplify some things in the code by customizing it to my usage,
>> so I left it here.
>>
>> > I mean, traversing the PCI hierarchy doesn't require vfio knowledge.  I
>> > think this loop (walking the bus->devices list) skips devices on
>> > "virtual buses" that may be added for SR-IOV.  I'm not sure that
>> > pci_walk_bus() handles that correctly either, but at least if you used
>> > that, we could fix the problem in one place.
>>
>> I didn't know about pci_walk_bus(), I'll look into switching to it.
>
> It looks like pci_walk_bus() is a poor replacement for when dealing with
> slots.  There might be multiple slots on a bus or a mix of slots and
> non-slots, so for each device pci_walk_bus() finds on a subordinate bus
> I'd need to walk up the tree to find the parent bridge on the original
> bus to figure out if it's in the same slot.

Do you really care about that scenario?  PCIe only supports a single
slot per bus, as far as I know.

> Should we have a pci_walk_slot() function?

I guess.  And supply the pci_slot rather than the pci_dev?  I'm a
little bit worried because the idea of a "slot" is not well-defined in
the spec, and we have sort of an ad hoc method of discovering and
managing them, e.g., acpiphp and pciehp might discover the same slot.
But I guess that's no reason to bury generic code in vfio.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/