On Fri, Mar 13, 2015 at 9:01 AM, Konrad Rzeszutek Wilk <konrad.w...@oracle.com> wrote: > On Fri, Mar 13, 2015 at 08:24:58AM -0500, Bjorn Helgaas wrote: >> On Thu, Mar 12, 2015 at 9:36 PM, Yijing Wang <wangyij...@huawei.com> wrote: >> >>>>> + pci_add_resource(&resources, &ioport_resource); >> >>>>> + pci_add_resource(&resources, &iomem_resource); >> >>>>> + pci_add_resource(&resources, &busn_resource); >> >>>> >> >>>> Since I don't want to export busn_resource, you might have to allocate >> >>>> your >> >>>> own struct resource for it here. And, of course, figure out the >> >>>> details of >> >>>> which PCI domain you're in and whether you need to share one struct >> >>>> resource across several host bridges in the same domain. >> >>> >> >>> Allocate its own resource here is ok for me, as I mentioned in previous >> >>> reply, >> >>> so do we still need to add additional info to figure out which domain >> >>> own the bus resource ? >> >> >> >> That's up to the caller. Only the platform knows which bridges it wants >> >> to >> >> have in the same domain. In principle, every host bridge could be in its >> >> own domain, since each bridge is the root of a unique PCI hierarchy. But >> >> some platforms have firmware that assumes otherwise. I have no idea what >> >> xen assumes. >> > >> > I'm not xen guy, so I don't know much about it, but because it call >> > pci_scan_bus_parented() >> > before, and in which busn_resource is always shared for different host >> > bridges(same domain or not), >> > I think add a static bus resource(0,255) should be safe, at least, it >> > would not introduce new risk. >> > >> > Something like: >> > >> > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c >> > index b1ffebe..a69e529 100644 >> > --- a/drivers/pci/xen-pcifront.c >> > +++ b/drivers/pci/xen-pcifront.c >> > @@ -446,9 +446,15 @@ static int pcifront_scan_root(struct pcifront_device >> > *pdev, >> > unsigned int domain, unsigned int bus) >> > { >> > struct pci_bus *b; >> > + LIST_HEAD(resources); >> > struct pcifront_sd *sd = NULL; >> > struct pci_bus_entry *bus_entry = NULL; >> > int err = 0; >> > + static struct resource busn_res = { >> > + .start = 0, >> > + .end = 255, >> > + .flags = IORESOURCE_BUS, >> > + }; >> > >> > #ifndef CONFIG_PCI_DOMAINS >> > if (domain != 0) { >> > @@ -470,17 +476,21 @@ static int pcifront_scan_root(struct pcifront_device >> > *pdev, >> > err = -ENOMEM; >> > goto err_out; >> > } >> > + pci_add_resource(&resources, &ioport_resource); >> > + pci_add_resource(&resources, &iomem_resource); >> > + pci_add_resource(&resources, &busn_res); >> > pcifront_init_sd(sd, domain, bus, pdev); >> > >> > pci_lock_rescan_remove(); >> > >> > - b = pci_scan_bus_parented(&pdev->xdev->dev, bus, >> > - &pcifront_bus_ops, sd); >> > + b = pci_scan_root_bus(&pdev->xdev->dev, bus, >> > + &pcifront_bus_ops, sd, &resources); >> > if (!b) { >> > >> > Bjorn, what do you think about ? >> >> That seems OK to me. Probably still wrong, but no worse than it was before. > > Interesting. The mechanism for PCI passthrough can either synthesize > and PCI bus number starting at zero (so first device is always 0:0:0.0) > or it can replicate the backend PCI topology. That means you > could have segment values passed in, so: ab:ff:00.1). I've to admin > I hadn't tried the 'physical' replication on an machine with > domains (err, segments). > > Is there an git tree with this so I can just try it out?
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/enumeration-yw6 has similar code (it exports the single busn_resource and makes xen use it). That should be functionally identical to what v4.0-rc1 does. Yijing hasn't posted the static busn_res proposal above yet, so I don't have a branch with that in it. Bjorn _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel