| Message ID | 1425868467-9667-4-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Mon, Mar 09, 2015 at 10:34:00AM +0800, Yijing Wang wrote: > Export out busn_resource. Xen pcifront module need it. > > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > --- > drivers/pci/pci.h | 2 ++ > drivers/pci/probe.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 4091f82..eeacab9 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -10,6 +10,8 @@ bool pcie_cap_has_lnkctl(const struct pci_dev *dev); > > /* Functions internal to the PCI core code */ > > +extern struct resource busn_resource; > + > int pci_create_sysfs_dev_files(struct pci_dev *pdev); > void pci_remove_sysfs_dev_files(struct pci_dev *pdev); > #if !defined(CONFIG_DMI) && !defined(CONFIG_ACPI) > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 8ef0375..b97ea81 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -17,12 +17,13 @@ > #define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */ > #define CARDBUS_RESERVE_BUSNR 3 > > -static struct resource busn_resource = { > +struct resource busn_resource = { > .name = "PCI busn", > .start = 0, > .end = 255, > .flags = IORESOURCE_BUS, > }; > +EXPORT_SYMBOL(busn_resource); I don't think this is a good idea. We support multiple PCI domains, and each domain has its own 0-255 bus number range. This busn_resource is only for domain 0 and probably should be handled in arch code instead of the PCI core. Right now, I think it's possible to call pci_scan_bus_parented() or pci_scan_bus() several times for buses in different domains, and they would all share the same busn_resource, which would cause corruption. So it's already broken, but I don't want to make it harder to fix by exporting this stuff. > > /* Ugh. Need to stop exporting this to modules. */ > LIST_HEAD(pci_root_buses); > -- > 1.7.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> -static struct resource busn_resource = { >> +struct resource busn_resource = { >> .name = "PCI busn", >> .start = 0, >> .end = 255, >> .flags = IORESOURCE_BUS, >> }; >> +EXPORT_SYMBOL(busn_resource); > > I don't think this is a good idea. We support multiple PCI domains, and > each domain has its own 0-255 bus number range. This busn_resource is > only for domain 0 and probably should be handled in arch code instead of > the PCI core. > > Right now, I think it's possible to call pci_scan_bus_parented() or > pci_scan_bus() several times for buses in different domains, and they would > all share the same busn_resource, which would cause corruption. > > So it's already broken, but I don't want to make it harder to fix by > exporting this stuff. Hi Bjorn, busn_resource may would not be shared by multi domains, We insert bus number resource like: pci_add_resource(&resources, &busn_resource); pci_bus_insert_busn_res(root_bus, start, bus_max); //start is the root bus number provided by arch pci host driver, bus max here == 255 get_pci_domain_busn_res(domain) //for root bus //try to get a domain specific pci_domain_busn_res, if not exist, create it. request_resource_conflict(domain_specific_busn_res, res) //request busn res(start, bus_max) from the pci_domain_busn_res. So every domain has its own pci_domain_busn_res , different domain would not share the same bus number resource. Do I understand correct ? Thanks! Yijing. > >> >> /* Ugh. Need to stop exporting this to modules. */ >> LIST_HEAD(pci_root_buses); >> -- >> 1.7.1 >> > > . >
On Thu, Mar 12, 2015 at 07:42:25PM +0800, Yijing Wang wrote: > >> -static struct resource busn_resource = { > >> +struct resource busn_resource = { > >> .name = "PCI busn", > >> .start = 0, > >> .end = 255, > >> .flags = IORESOURCE_BUS, > >> }; > >> +EXPORT_SYMBOL(busn_resource); > > > > I don't think this is a good idea. We support multiple PCI domains, and > > each domain has its own 0-255 bus number range. This busn_resource is > > only for domain 0 and probably should be handled in arch code instead of > > the PCI core. > > > > Right now, I think it's possible to call pci_scan_bus_parented() or > > pci_scan_bus() several times for buses in different domains, and they would > > all share the same busn_resource, which would cause corruption. > > > > So it's already broken, but I don't want to make it harder to fix by > > exporting this stuff. > > Hi Bjorn, busn_resource may would not be shared by multi domains, > > We insert bus number resource like: > > pci_add_resource(&resources, &busn_resource); > pci_bus_insert_busn_res(root_bus, start, bus_max); //start is the root bus number provided by arch pci host driver, bus max here == 255 > get_pci_domain_busn_res(domain) //for root bus //try to get a domain specific pci_domain_busn_res, if not exist, create it. > request_resource_conflict(domain_specific_busn_res, res) //request busn res(start, bus_max) from the pci_domain_busn_res. > > So every domain has its own pci_domain_busn_res , different domain would not share the same bus number resource. The intent of pci_add_resource() and passing the resulting resource list into pci_scan_root_bus(), etc., is that the host bridge may consume any resources described by the list. If we pass the same resource to multiple calls, that means the resource must be shared between the multiple host bridges involved. For bus numbers, that makes some sense if the platform considers the bridges to be in the same domain, but not if they are in different domains. If we export busn_resource, we have no control over what domains it becomes associated with because the domain is passed into pci_scan_root_bus() by the caller. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Hi Bjorn, busn_resource may would not be shared by multi domains, >> >> We insert bus number resource like: >> >> pci_add_resource(&resources, &busn_resource); >> pci_bus_insert_busn_res(root_bus, start, bus_max); //start is the root bus number provided by arch pci host driver, bus max here == 255 >> get_pci_domain_busn_res(domain) //for root bus //try to get a domain specific pci_domain_busn_res, if not exist, create it. >> request_resource_conflict(domain_specific_busn_res, res) //request busn res(start, bus_max) from the pci_domain_busn_res. >> >> So every domain has its own pci_domain_busn_res , different domain would not share the same bus number resource. > > The intent of pci_add_resource() and passing the resulting resource list > into pci_scan_root_bus(), etc., is that the host bridge may consume any > resources described by the list. If we pass the same resource to multiple > calls, that means the resource must be shared between the multiple host > bridges involved. For bus numbers, that makes some sense if the platform > considers the bridges to be in the same domain, but not if they are in > different domains. > > If we export busn_resource, we have no control over what domains it becomes > associated with because the domain is passed into pci_scan_root_bus() by > the caller. Agree, I will drop this patch, thanks! > > Bjorn > > . >
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 4091f82..eeacab9 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -10,6 +10,8 @@ bool pcie_cap_has_lnkctl(const struct pci_dev *dev); /* Functions internal to the PCI core code */ +extern struct resource busn_resource; + int pci_create_sysfs_dev_files(struct pci_dev *pdev); void pci_remove_sysfs_dev_files(struct pci_dev *pdev); #if !defined(CONFIG_DMI) && !defined(CONFIG_ACPI) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 8ef0375..b97ea81 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -17,12 +17,13 @@ #define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */ #define CARDBUS_RESERVE_BUSNR 3 -static struct resource busn_resource = { +struct resource busn_resource = { .name = "PCI busn", .start = 0, .end = 255, .flags = IORESOURCE_BUS, }; +EXPORT_SYMBOL(busn_resource); /* Ugh. Need to stop exporting this to modules. */ LIST_HEAD(pci_root_buses);
Export out busn_resource. Xen pcifront module need it. Signed-off-by: Yijing Wang <wangyijing@huawei.com> --- drivers/pci/pci.h | 2 ++ drivers/pci/probe.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletions(-)