Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751271Ab3HSUDF (ORCPT ); Mon, 19 Aug 2013 16:03:05 -0400 Received: from mail-ie0-f175.google.com ([209.85.223.175]:61496 "EHLO mail-ie0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751105Ab3HSUDD (ORCPT ); Mon, 19 Aug 2013 16:03:03 -0400 MIME-Version: 1.0 In-Reply-To: <1376937682.2657.15.camel@ul30vt.home> References: <20130814200845.21923.64284.stgit@bling.home> <1376521578.13642.65.camel@ul30vt.home> <1376937682.2657.15.camel@ul30vt.home> From: Bjorn Helgaas Date: Mon, 19 Aug 2013 14:02:41 -0600 Message-ID: Subject: Re: [PATCH] vfio-pci: PCI hot reset interface To: Alex Williamson Cc: "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , Benjamin Herrenschmidt , Alexander Viro , linux-fsdevel Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3103 Lines: 71 On Mon, Aug 19, 2013 at 12:41 PM, Alex Williamson 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 >> > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/