| Message ID | 1333905129-8776-3-git-send-email-jiang.liu@huawei.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sun, Apr 8, 2012 at 10:12 AM, Jiang Liu <liuj97@gmail.com> wrote: > This patch enhances pci_root driver to update MMCFG information when > hot-plugging PCI root bridges on x86 platforms. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > --- > arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/acpi/pci_root.c | 20 ++++++++++++++++ > include/acpi/acnames.h | 1 + > include/linux/pci-acpi.h | 3 ++ > 4 files changed, 82 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index da0149d..9184970 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) > return bus; > } > > +int arch_acpi_pci_root_add(struct acpi_pci_root *root) > +{ > + int result = 0; > + acpi_status status; > + unsigned long long base_addr; > + struct pci_mmcfg_region *cfg; > + > + /* > + * Try to insert MMCFG information for host bridges with _CBA method > + */ > + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, > + NULL, &base_addr); > + if (ACPI_SUCCESS(status)) { > + result = pci_mmconfig_insert(root->segment, > + root->secondary.start, > + root->secondary.end, > + base_addr); > + /* > + * MMCFG information for hot-pluggable host bridges may have > + * already been added by __pci_mmcfg_init(); > + */ > + if (result == -EEXIST) > + result = 0; > + } else if (status == AE_NOT_FOUND) { > + /* > + * Check whether MMCFG information has been added for > + * host bridges without _CBA method. > + */ > + rcu_read_lock(); > + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); > + if (!cfg || cfg->end_bus < root->secondary.end) > + result = -ENODEV; > + rcu_read_unlock(); > + } else > + result = -ENODEV; > + > + return result; > +} > + > +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) > +{ > + acpi_status status; > + unsigned long long base_addr; > + > + /* Remove MMCFG information for host bridges with _CBA method */ > + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, > + NULL, &base_addr); add one flag in acpi_pci_root about we added mmconf for it before? like mmconf_added? > + if (ACPI_SUCCESS(status)) > + return pci_mmconfig_delete(root->segment, > + root->secondary.start, > + root->secondary.end, > + base_addr); > + else if (status != AE_NOT_FOUND) > + return -ENODEV; > + > + return 0; > +} > + > int __init pci_acpi_init(void) > { > struct pci_dev *dev = NULL; > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 4a7d575..a62bfa8 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -448,6 +448,16 @@ out: > } > EXPORT_SYMBOL(acpi_pci_osc_control_set); > > +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) > +{ > + return 0; > +} > + > +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) > +{ > + return 0; > +} > + > static int __devinit acpi_pci_root_add(struct acpi_device *device) > { > unsigned long long segment, bus; > @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) > strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); > device->driver_data = root; > > + if (arch_acpi_pci_root_add(root)) { > + printk(KERN_ERR PREFIX > + "can't add MMCFG information for Bus %04x:%02x\n", > + root->segment, (unsigned int)root->secondary.start); > + result = -ENODEV; > + goto out_free; > + } > + > /* > * All supported architectures that use ACPI have support for > * PCI domains, so we indicate this in _OSC support capabilities. > @@ -629,6 +647,7 @@ out_del_root: > list_del_rcu(&root->node); > mutex_unlock(&acpi_pci_root_lock); > synchronize_rcu(); > + arch_acpi_pci_root_remove(root); > out_free: > kfree(root); > return result; > @@ -679,6 +698,7 @@ out: > list_del_rcu(&root->node); > mutex_unlock(&acpi_pci_root_lock); > synchronize_rcu(); > + arch_acpi_pci_root_remove(root); > kfree(root); > > return 0; > diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h > index 38f5088..99bda75 100644 > --- a/include/acpi/acnames.h > +++ b/include/acpi/acnames.h > @@ -62,6 +62,7 @@ > #define METHOD_NAME__AEI "_AEI" > #define METHOD_NAME__PRW "_PRW" > #define METHOD_NAME__SRS "_SRS" > +#define METHOD_NAME__CBA "_CBA" > > /* Method names - these methods must appear at the namespace root */ > > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h > index ac93634..816b971 100644 > --- a/include/linux/pci-acpi.h > +++ b/include/linux/pci-acpi.h > @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) > > void acpi_pci_root_rescan(void); > > +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); > +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); > + don't need extern here. > #else > > static inline void acpi_pci_root_rescan(void) { } > -- > 1.7.5.4 > -- 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 Yinghai, Thanks for your comments and will send out another version according to your suggestions. On 2012-4-9 3:19, Yinghai Lu wrote: > On Sun, Apr 8, 2012 at 10:12 AM, Jiang Liu<liuj97@gmail.com> wrote: >> This patch enhances pci_root driver to update MMCFG information when >> hot-plugging PCI root bridges on x86 platforms. >> >> Signed-off-by: Jiang Liu<jiang.liu@huawei.com> >> --- >> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/acpi/pci_root.c | 20 ++++++++++++++++ >> include/acpi/acnames.h | 1 + >> include/linux/pci-acpi.h | 3 ++ >> 4 files changed, 82 insertions(+), 0 deletions(-) >> >> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c >> index da0149d..9184970 100644 >> --- a/arch/x86/pci/acpi.c >> +++ b/arch/x86/pci/acpi.c >> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) >> return bus; >> } >> >> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + int result = 0; >> + acpi_status status; >> + unsigned long long base_addr; >> + struct pci_mmcfg_region *cfg; >> + >> + /* >> + * Try to insert MMCFG information for host bridges with _CBA method >> + */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL,&base_addr); >> + if (ACPI_SUCCESS(status)) { >> + result = pci_mmconfig_insert(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + /* >> + * MMCFG information for hot-pluggable host bridges may have >> + * already been added by __pci_mmcfg_init(); >> + */ >> + if (result == -EEXIST) >> + result = 0; >> + } else if (status == AE_NOT_FOUND) { >> + /* >> + * Check whether MMCFG information has been added for >> + * host bridges without _CBA method. >> + */ >> + rcu_read_lock(); >> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); >> + if (!cfg || cfg->end_bus< root->secondary.end) >> + result = -ENODEV; >> + rcu_read_unlock(); >> + } else >> + result = -ENODEV; >> + >> + return result; >> +} >> + >> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + acpi_status status; >> + unsigned long long base_addr; >> + >> + /* Remove MMCFG information for host bridges with _CBA method */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL,&base_addr); > > add one flag in acpi_pci_root about we added mmconf for it before? > like mmconf_added? > >> + if (ACPI_SUCCESS(status)) >> + return pci_mmconfig_delete(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + else if (status != AE_NOT_FOUND) >> + return -ENODEV; >> + >> + return 0; >> +} >> + >> int __init pci_acpi_init(void) >> { >> struct pci_dev *dev = NULL; >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 4a7d575..a62bfa8 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -448,6 +448,16 @@ out: >> } >> EXPORT_SYMBOL(acpi_pci_osc_control_set); >> >> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> static int __devinit acpi_pci_root_add(struct acpi_device *device) >> { >> unsigned long long segment, bus; >> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >> device->driver_data = root; >> >> + if (arch_acpi_pci_root_add(root)) { >> + printk(KERN_ERR PREFIX >> + "can't add MMCFG information for Bus %04x:%02x\n", >> + root->segment, (unsigned int)root->secondary.start); >> + result = -ENODEV; >> + goto out_free; >> + } >> + >> /* >> * All supported architectures that use ACPI have support for >> * PCI domains, so we indicate this in _OSC support capabilities. >> @@ -629,6 +647,7 @@ out_del_root: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> out_free: >> kfree(root); >> return result; >> @@ -679,6 +698,7 @@ out: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> kfree(root); >> >> return 0; >> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h >> index 38f5088..99bda75 100644 >> --- a/include/acpi/acnames.h >> +++ b/include/acpi/acnames.h >> @@ -62,6 +62,7 @@ >> #define METHOD_NAME__AEI "_AEI" >> #define METHOD_NAME__PRW "_PRW" >> #define METHOD_NAME__SRS "_SRS" >> +#define METHOD_NAME__CBA "_CBA" >> >> /* Method names - these methods must appear at the namespace root */ >> >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index ac93634..816b971 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) >> >> void acpi_pci_root_rescan(void); >> >> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); >> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); >> + > > don't need extern here. > >> #else >> >> static inline void acpi_pci_root_rescan(void) { } >> -- >> 1.7.5.4 >> > > . > -- 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
Your patch looks good to me. I have some comments. (2012/04/09 2:12), Jiang Liu wrote: > This patch enhances pci_root driver to update MMCFG information when > hot-plugging PCI root bridges on x86 platforms. > Do you have the patch that can be applied to Bjorn's pci tree? <snip.> > +int arch_acpi_pci_root_add(struct acpi_pci_root *root) > +{ > + int result = 0; > + acpi_status status; > + unsigned long long base_addr; > + struct pci_mmcfg_region *cfg; > + > + /* > + * Try to insert MMCFG information for host bridges with _CBA method > + */ > + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, > + NULL,&base_addr); > + if (ACPI_SUCCESS(status)) { > + result = pci_mmconfig_insert(root->segment, > + root->secondary.start, > + root->secondary.end, > + base_addr); > + /* > + * MMCFG information for hot-pluggable host bridges may have > + * already been added by __pci_mmcfg_init(); > + */ > + if (result == -EEXIST) > + result = 0; Just for confirmation. From my interpretation of PCI firmware spec, MCFG doesn't have any entry for hot-pluggable hostbridge. So I assume this is for the machine that is not compliant to the spec. Is my understanding same as yours? <snip.> > static int __devinit acpi_pci_root_add(struct acpi_device *device) > { > unsigned long long segment, bus; > @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) > strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); > device->driver_data = root; > > + if (arch_acpi_pci_root_add(root)) { > + printk(KERN_ERR PREFIX > + "can't add MMCFG information for Bus %04x:%02x\n", > + root->segment, (unsigned int)root->secondary.start); > + result = -ENODEV; > + goto out_free; > + } Desn't this break the system that doesn't support MMCONFIG? In my understanding, arch_acpi_pci_root_add() returns -ENODEV if mmconfig information is found neither in MCFG table nor _CBA. And pci root bridge initialization seems to fail arch_acpi_pci_root_add() returns non-zero value. Regards, Kenji Kaneshige -- 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 Yinghai, >> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + acpi_status status; >> + unsigned long long base_addr; >> + >> + /* Remove MMCFG information for host bridges with _CBA method */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL, &base_addr); > > add one flag in acpi_pci_root about we added mmconf for it before? > like mmconf_added? > I feel adding a flag here doesn't help here. For any hot-pluggable PCI host bridge with _CBA method available, arch_acpi_pci_root_remove() should delete the MMCFG information, no matter it's added by __pci_mmcfg_init() or arch_acpi_pci_root_add(). So check existence of _CBA seems simpler here. Thanks! On 04/09/2012 03:19 AM, Yinghai Lu wrote: > On Sun, Apr 8, 2012 at 10:12 AM, Jiang Liu <liuj97@gmail.com> wrote: >> This patch enhances pci_root driver to update MMCFG information when >> hot-plugging PCI root bridges on x86 platforms. >> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >> --- >> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/acpi/pci_root.c | 20 ++++++++++++++++ >> include/acpi/acnames.h | 1 + >> include/linux/pci-acpi.h | 3 ++ >> 4 files changed, 82 insertions(+), 0 deletions(-) >> >> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c >> index da0149d..9184970 100644 >> --- a/arch/x86/pci/acpi.c >> +++ b/arch/x86/pci/acpi.c >> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) >> return bus; >> } >> >> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + int result = 0; >> + acpi_status status; >> + unsigned long long base_addr; >> + struct pci_mmcfg_region *cfg; >> + >> + /* >> + * Try to insert MMCFG information for host bridges with _CBA method >> + */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL, &base_addr); >> + if (ACPI_SUCCESS(status)) { >> + result = pci_mmconfig_insert(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + /* >> + * MMCFG information for hot-pluggable host bridges may have >> + * already been added by __pci_mmcfg_init(); >> + */ >> + if (result == -EEXIST) >> + result = 0; >> + } else if (status == AE_NOT_FOUND) { >> + /* >> + * Check whether MMCFG information has been added for >> + * host bridges without _CBA method. >> + */ >> + rcu_read_lock(); >> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); >> + if (!cfg || cfg->end_bus < root->secondary.end) >> + result = -ENODEV; >> + rcu_read_unlock(); >> + } else >> + result = -ENODEV; >> + >> + return result; >> +} >> + >> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + acpi_status status; >> + unsigned long long base_addr; >> + >> + /* Remove MMCFG information for host bridges with _CBA method */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL, &base_addr); > > add one flag in acpi_pci_root about we added mmconf for it before? > like mmconf_added? > >> + if (ACPI_SUCCESS(status)) >> + return pci_mmconfig_delete(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + else if (status != AE_NOT_FOUND) >> + return -ENODEV; >> + >> + return 0; >> +} >> + >> int __init pci_acpi_init(void) >> { >> struct pci_dev *dev = NULL; >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 4a7d575..a62bfa8 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -448,6 +448,16 @@ out: >> } >> EXPORT_SYMBOL(acpi_pci_osc_control_set); >> >> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> static int __devinit acpi_pci_root_add(struct acpi_device *device) >> { >> unsigned long long segment, bus; >> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >> device->driver_data = root; >> >> + if (arch_acpi_pci_root_add(root)) { >> + printk(KERN_ERR PREFIX >> + "can't add MMCFG information for Bus %04x:%02x\n", >> + root->segment, (unsigned int)root->secondary.start); >> + result = -ENODEV; >> + goto out_free; >> + } >> + >> /* >> * All supported architectures that use ACPI have support for >> * PCI domains, so we indicate this in _OSC support capabilities. >> @@ -629,6 +647,7 @@ out_del_root: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> out_free: >> kfree(root); >> return result; >> @@ -679,6 +698,7 @@ out: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> kfree(root); >> >> return 0; >> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h >> index 38f5088..99bda75 100644 >> --- a/include/acpi/acnames.h >> +++ b/include/acpi/acnames.h >> @@ -62,6 +62,7 @@ >> #define METHOD_NAME__AEI "_AEI" >> #define METHOD_NAME__PRW "_PRW" >> #define METHOD_NAME__SRS "_SRS" >> +#define METHOD_NAME__CBA "_CBA" >> >> /* Method names - these methods must appear at the namespace root */ >> >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index ac93634..816b971 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) >> >> void acpi_pci_root_rescan(void); >> >> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); >> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); >> + > > don't need extern here. > >> #else >> >> static inline void acpi_pci_root_rescan(void) { } >> -- >> 1.7.5.4 >> -- 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
On Mon, Apr 9, 2012 at 7:37 AM, Jiang Liu <liuj97@gmail.com> wrote: > Hi Yinghai, > >>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >>> +{ >>> + acpi_status status; >>> + unsigned long long base_addr; >>> + >>> + /* Remove MMCFG information for host bridges with _CBA method */ >>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >>> + NULL, &base_addr); >> >> add one flag in acpi_pci_root about we added mmconf for it before? >> like mmconf_added? >> > > I feel adding a flag here doesn't help here. For any hot-pluggable PCI host bridge > with _CBA method available, arch_acpi_pci_root_remove() should delete the MMCFG > information, no matter it's added by __pci_mmcfg_init() or arch_acpi_pci_root_add(). > So check existence of _CBA seems simpler here. _remove should not care about _CBA, it should only care about if it gets added on _add path. Also like Kenji said, We should let it go through even no MMCONF is added at that time. like somehow _CBA does not exis for some root bus or evaluating failing etc. Yinghai -- 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 Kenji, Thanks for your careful review and comments. On 04/09/2012 07:43 PM, Kenji Kaneshige wrote: > Your patch looks good to me. > > I have some comments. > > (2012/04/09 2:12), Jiang Liu wrote: >> This patch enhances pci_root driver to update MMCFG information when >> hot-plugging PCI root bridges on x86 platforms. >> > > Do you have the patch that can be applied to Bjorn's pci tree? > > <snip.> Will try to generate a version against Bjorn's version. Could you please tell me the exact git link for that? I haven't pull from Bjorn's tree yet. > >> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + int result = 0; >> + acpi_status status; >> + unsigned long long base_addr; >> + struct pci_mmcfg_region *cfg; >> + >> + /* >> + * Try to insert MMCFG information for host bridges with _CBA method >> + */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL,&base_addr); >> + if (ACPI_SUCCESS(status)) { >> + result = pci_mmconfig_insert(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + /* >> + * MMCFG information for hot-pluggable host bridges may have >> + * already been added by __pci_mmcfg_init(); >> + */ >> + if (result == -EEXIST) >> + result = 0; > > Just for confirmation. > From my interpretation of PCI firmware spec, MCFG doesn't have any entry > for hot-pluggable hostbridge. So I assume this is for the machine that > is not compliant to the spec. Is my understanding same as yours? > > <snip.> You are right, it's defined to that way in PCI FW Spec 3.1. Here I have some concerns about the PCI buses to host all Ubox components on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare MMCFG information for those host bridges by MCFG table instead of _CBA method, though those host bridge will disappear after hot-removing a physical processor. > >> static int __devinit acpi_pci_root_add(struct acpi_device *device) >> { >> unsigned long long segment, bus; >> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >> device->driver_data = root; >> >> + if (arch_acpi_pci_root_add(root)) { >> + printk(KERN_ERR PREFIX >> + "can't add MMCFG information for Bus %04x:%02x\n", >> + root->segment, (unsigned int)root->secondary.start); >> + result = -ENODEV; >> + goto out_free; >> + } > > Desn't this break the system that doesn't support MMCONFIG? > > In my understanding, arch_acpi_pci_root_add() returns -ENODEV if > mmconfig information is found neither in MCFG table nor _CBA. And > pci root bridge initialization seems to fail arch_acpi_pci_root_add() > returns non-zero value. Good catch, will add following code into arch_acpi_pci_root_add() and arch_acpi_pci_root_remove() to solve this issue. --- /* MMCONFIG disabled */ if ((pci_probe & PCI_PROBE_MMCONF) == 0) return 0; --- > > Regards, > Kenji Kaneshige -- 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
On Mon, Apr 9, 2012 at 7:37 AM, Jiang Liu <liuj97@gmail.com> wrote: > Hi Yinghai, > >>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >>> +{ >>> + acpi_status status; >>> + unsigned long long base_addr; >>> + >>> + /* Remove MMCFG information for host bridges with _CBA method */ >>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >>> + NULL, &base_addr); >> >> add one flag in acpi_pci_root about we added mmconf for it before? >> like mmconf_added? >> > > I feel adding a flag here doesn't help here. For any hot-pluggable PCI host bridge > with _CBA method available, arch_acpi_pci_root_remove() should delete the MMCFG > information, no matter it's added by __pci_mmcfg_init() or arch_acpi_pci_root_add(). > So check existence of _CBA seems simpler here. please check updated patch that will use mmconf_added. it will make sure only remove mmconf with _CBA. other than that if BIOS have duplicated one in MCFG table, and _CBA, will bail out early. Line added will be 68 instead of 90. also some changes could be merged to patch 1-5. Thanks Yinghai
(2012/04/10 1:02), Jiang Liu wrote: > Hi Kenji, > Thanks for your careful review and comments. > > On 04/09/2012 07:43 PM, Kenji Kaneshige wrote: >> Your patch looks good to me. >> >> I have some comments. >> >> (2012/04/09 2:12), Jiang Liu wrote: >>> This patch enhances pci_root driver to update MMCFG information when >>> hot-plugging PCI root bridges on x86 platforms. >>> >> >> Do you have the patch that can be applied to Bjorn's pci tree? >> >> <snip.> > Will try to generate a version against Bjorn's version. Could you please tell > me the exact git link for that? I haven't pull from Bjorn's tree yet. As you may know, it was announced _today_ (sorry:). http://www.spinics.net/lists/linux-pci/msg14626.html > >> >>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >>> +{ >>> + int result = 0; >>> + acpi_status status; >>> + unsigned long long base_addr; >>> + struct pci_mmcfg_region *cfg; >>> + >>> + /* >>> + * Try to insert MMCFG information for host bridges with _CBA method >>> + */ >>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >>> + NULL,&base_addr); >>> + if (ACPI_SUCCESS(status)) { >>> + result = pci_mmconfig_insert(root->segment, >>> + root->secondary.start, >>> + root->secondary.end, >>> + base_addr); >>> + /* >>> + * MMCFG information for hot-pluggable host bridges may have >>> + * already been added by __pci_mmcfg_init(); >>> + */ >>> + if (result == -EEXIST) >>> + result = 0; >> >> Just for confirmation. >> From my interpretation of PCI firmware spec, MCFG doesn't have any entry >> for hot-pluggable hostbridge. So I assume this is for the machine that >> is not compliant to the spec. Is my understanding same as yours? >> >> <snip.> > You are right, it's defined to that way in PCI FW Spec 3.1. > Here I have some concerns about the PCI buses to host all Ubox components > on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare > MMCFG information for those host bridges by MCFG table instead of _CBA method, > though those host bridge will disappear after hot-removing a physical processor. Ok, thank you for clarification. > >> >>> static int __devinit acpi_pci_root_add(struct acpi_device *device) >>> { >>> unsigned long long segment, bus; >>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >>> device->driver_data = root; >>> >>> + if (arch_acpi_pci_root_add(root)) { >>> + printk(KERN_ERR PREFIX >>> + "can't add MMCFG information for Bus %04x:%02x\n", >>> + root->segment, (unsigned int)root->secondary.start); Additional comment. This printk message looks strange because arch_acpi_pci_root_add() is not a mmconfig specific function. So I think this message should be moved to arch specific code (arch_acpi_pci_root_add()). >>> + result = -ENODEV; >>> + goto out_free; >>> + } >> >> Desn't this break the system that doesn't support MMCONFIG? >> >> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if >> mmconfig information is found neither in MCFG table nor _CBA. And >> pci root bridge initialization seems to fail arch_acpi_pci_root_add() >> returns non-zero value. > Good catch, will add following code into arch_acpi_pci_root_add() and > arch_acpi_pci_root_remove() to solve this issue. > --- > /* MMCONFIG disabled */ > if ((pci_probe& PCI_PROBE_MMCONF) == 0) > return 0; > --- My understanding is that PCI_PROBE_MMCONF is set even if the system doesn't have MCFG table. So I don't think this solves the issue. I guess this is what Yinghai pointed out on your V2 patch [6/6]. Additionally, I think there is a remaining issue even if we change this check like below. if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF)) return 0; I think this check has an assumption that system has at least one MCFG table entry and it has been initialized before acpi_pci_root_add() is called. I think this doesn't work on the system that doesn't have MCFG and all the pci root bridge have _CBA (that is, all host bridges are hot-pluggable and BIOS is implemented in the way PCI FW spec defines). As a result, MMCONFIG would never be enabled on such systems. Could you double check this? Regards, Kenji Kaneshige -- 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
On Tue, Apr 10, 2012 at 3:32 AM, Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> wrote: > Additionally, I think there is a remaining issue even if we change > this check like below. > > > if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF)) > return 0; > > I think this check has an assumption that system has at least one > MCFG table entry and it has been initialized before > acpi_pci_root_add() is called. I think this doesn't work on the > system that doesn't have MCFG and all the pci root bridge have > _CBA (that is, all host bridges are hot-pluggable and BIOS is > implemented in the way PCI FW spec defines). As a result, MMCONFIG > would never be enabled on such systems. Could you double check this? You are right. We can just remove that checking. But wonder if current x86_64 system support that. All peer root buses can be physically removed ? Thanks Yinghai -- 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
On 04/10/2012 06:32 PM, Kenji Kaneshige wrote: > (2012/04/10 1:02), Jiang Liu wrote: >> Hi Kenji, >> Thanks for your careful review and comments. >> >> On 04/09/2012 07:43 PM, Kenji Kaneshige wrote: >>> Your patch looks good to me. >>> >>> I have some comments. >>> >>> (2012/04/09 2:12), Jiang Liu wrote: >>>> This patch enhances pci_root driver to update MMCFG information when >>>> hot-plugging PCI root bridges on x86 platforms. >>>> >>> >>> Do you have the patch that can be applied to Bjorn's pci tree? >>> >>> <snip.> >> Will try to generate a version against Bjorn's version. Could you please tell >> me the exact git link for that? I haven't pull from Bjorn's tree yet. > > As you may know, it was announced _today_ (sorry:). > > http://www.spinics.net/lists/linux-pci/msg14626.html > >> >>> >>>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >>>> +{ >>>> + int result = 0; >>>> + acpi_status status; >>>> + unsigned long long base_addr; >>>> + struct pci_mmcfg_region *cfg; >>>> + >>>> + /* >>>> + * Try to insert MMCFG information for host bridges with _CBA method >>>> + */ >>>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >>>> + NULL,&base_addr); >>>> + if (ACPI_SUCCESS(status)) { >>>> + result = pci_mmconfig_insert(root->segment, >>>> + root->secondary.start, >>>> + root->secondary.end, >>>> + base_addr); >>>> + /* >>>> + * MMCFG information for hot-pluggable host bridges may have >>>> + * already been added by __pci_mmcfg_init(); >>>> + */ >>>> + if (result == -EEXIST) >>>> + result = 0; >>> >>> Just for confirmation. >>> From my interpretation of PCI firmware spec, MCFG doesn't have any entry >>> for hot-pluggable hostbridge. So I assume this is for the machine that >>> is not compliant to the spec. Is my understanding same as yours? >>> >>> <snip.> >> You are right, it's defined to that way in PCI FW Spec 3.1. >> Here I have some concerns about the PCI buses to host all Ubox components >> on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare >> MMCFG information for those host bridges by MCFG table instead of _CBA method, >> though those host bridge will disappear after hot-removing a physical processor. > > Ok, thank you for clarification. > >> >>> >>>> static int __devinit acpi_pci_root_add(struct acpi_device *device) >>>> { >>>> unsigned long long segment, bus; >>>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >>>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >>>> device->driver_data = root; >>>> >>>> + if (arch_acpi_pci_root_add(root)) { >>>> + printk(KERN_ERR PREFIX >>>> + "can't add MMCFG information for Bus %04x:%02x\n", >>>> + root->segment, (unsigned int)root->secondary.start); > > Additional comment. > > This printk message looks strange because arch_acpi_pci_root_add() > is not a mmconfig specific function. So I think this message > should be moved to arch specific code (arch_acpi_pci_root_add()). > >>>> + result = -ENODEV; >>>> + goto out_free; >>>> + } >>> >>> Desn't this break the system that doesn't support MMCONFIG? >>> >>> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if >>> mmconfig information is found neither in MCFG table nor _CBA. And >>> pci root bridge initialization seems to fail arch_acpi_pci_root_add() >>> returns non-zero value. >> Good catch, will add following code into arch_acpi_pci_root_add() and >> arch_acpi_pci_root_remove() to solve this issue. >> --- >> /* MMCONFIG disabled */ >> if ((pci_probe& PCI_PROBE_MMCONF) == 0) >> return 0; >> --- > > My understanding is that PCI_PROBE_MMCONF is set even if the system > doesn't have MCFG table. So I don't think this solves the issue. I > guess this is what Yinghai pointed out on your V2 patch [6/6]. > > Additionally, I think there is a remaining issue even if we change > this check like below. > > if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF)) > return 0; > > I think this check has an assumption that system has at least one > MCFG table entry and it has been initialized before > acpi_pci_root_add() is called. I think this doesn't work on the > system that doesn't have MCFG and all the pci root bridge have > _CBA (that is, all host bridges are hot-pluggable and BIOS is > implemented in the way PCI FW spec defines). As a result, MMCONFIG > would never be enabled on such systems. Could you double check this? Good points. We underestimated the complexity here. I'm working on a new patch based on Yinghai's v3 patchset and will send it out soon. Thanks! > > Regards, > Kenji Kaneshige -- 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
On 04/10/2012 11:47 PM, Yinghai Lu wrote: > On Tue, Apr 10, 2012 at 3:32 AM, Kenji Kaneshige > <kaneshige.kenji@jp.fujitsu.com> wrote: >> Additionally, I think there is a remaining issue even if we change >> this check like below. >> >> >> if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF)) >> return 0; >> >> I think this check has an assumption that system has at least one >> MCFG table entry and it has been initialized before >> acpi_pci_root_add() is called. I think this doesn't work on the >> system that doesn't have MCFG and all the pci root bridge have >> _CBA (that is, all host bridges are hot-pluggable and BIOS is >> implemented in the way PCI FW spec defines). As a result, MMCONFIG >> would never be enabled on such systems. Could you double check this? > > You are right. > > We can just remove that checking. > > But wonder if current x86_64 system support that. > All peer root buses can be physically removed ? It's theoretically possible that BIOS doesn't provide MMCFG or _CBA for the legacy (non-removable bus 0) host bridge. > > Thanks > > Yinghai -- 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
On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu <liuj97@gmail.com> wrote: > This patch enhances pci_root driver to update MMCFG information when > hot-plugging PCI root bridges on x86 platforms. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > --- > arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/acpi/pci_root.c | 20 ++++++++++++++++ > include/acpi/acnames.h | 1 + > include/linux/pci-acpi.h | 3 ++ > 4 files changed, 82 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index da0149d..9184970 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) > return bus; > } > > +int arch_acpi_pci_root_add(struct acpi_pci_root *root) > +{ > + int result = 0; > + acpi_status status; > + unsigned long long base_addr; > + struct pci_mmcfg_region *cfg; > + > + /* > + * Try to insert MMCFG information for host bridges with _CBA method > + */ > + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, > + NULL, &base_addr); I would prefer to have _CBA evaluated in drivers/acpi/pci_root.c, not in the arch code. It's true that _CBA is probably only used by x86 today, but the spec says nothing about it being arch-dependent, and I suspect it may be used on arm soon. > + if (ACPI_SUCCESS(status)) { > + result = pci_mmconfig_insert(root->segment, > + root->secondary.start, > + root->secondary.end, > + base_addr); > + /* > + * MMCFG information for hot-pluggable host bridges may have > + * already been added by __pci_mmcfg_init(); > + */ > + if (result == -EEXIST) > + result = 0; > + } else if (status == AE_NOT_FOUND) { > + /* > + * Check whether MMCFG information has been added for > + * host bridges without _CBA method. > + */ > + rcu_read_lock(); > + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); > + if (!cfg || cfg->end_bus < root->secondary.end) > + result = -ENODEV; > + rcu_read_unlock(); > + } else > + result = -ENODEV; > + > + return result; > +} > + > +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) > +{ > + acpi_status status; > + unsigned long long base_addr; > + > + /* Remove MMCFG information for host bridges with _CBA method */ > + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, > + NULL, &base_addr); I think the arch-specific "map MMCONFIG space at this addr" should return a pointer or something that we can save and use to unmap it on remove. That way we don't have to evaluate _CBA again. > + if (ACPI_SUCCESS(status)) > + return pci_mmconfig_delete(root->segment, > + root->secondary.start, > + root->secondary.end, > + base_addr); > + else if (status != AE_NOT_FOUND) > + return -ENODEV; > + > + return 0; > +} > + > int __init pci_acpi_init(void) > { > struct pci_dev *dev = NULL; > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 4a7d575..a62bfa8 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -448,6 +448,16 @@ out: > } > EXPORT_SYMBOL(acpi_pci_osc_control_set); > > +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) > +{ > + return 0; > +} > + > +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) > +{ > + return 0; > +} > + > static int __devinit acpi_pci_root_add(struct acpi_device *device) > { > unsigned long long segment, bus; > @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) > strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); > device->driver_data = root; > > + if (arch_acpi_pci_root_add(root)) { > + printk(KERN_ERR PREFIX > + "can't add MMCFG information for Bus %04x:%02x\n", > + root->segment, (unsigned int)root->secondary.start); > + result = -ENODEV; > + goto out_free; > + } > + > /* > * All supported architectures that use ACPI have support for > * PCI domains, so we indicate this in _OSC support capabilities. > @@ -629,6 +647,7 @@ out_del_root: > list_del_rcu(&root->node); > mutex_unlock(&acpi_pci_root_lock); > synchronize_rcu(); > + arch_acpi_pci_root_remove(root); > out_free: > kfree(root); > return result; > @@ -679,6 +698,7 @@ out: > list_del_rcu(&root->node); > mutex_unlock(&acpi_pci_root_lock); > synchronize_rcu(); > + arch_acpi_pci_root_remove(root); > kfree(root); > > return 0; > diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h > index 38f5088..99bda75 100644 > --- a/include/acpi/acnames.h > +++ b/include/acpi/acnames.h > @@ -62,6 +62,7 @@ > #define METHOD_NAME__AEI "_AEI" > #define METHOD_NAME__PRW "_PRW" > #define METHOD_NAME__SRS "_SRS" > +#define METHOD_NAME__CBA "_CBA" > > /* Method names - these methods must appear at the namespace root */ > > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h > index ac93634..816b971 100644 > --- a/include/linux/pci-acpi.h > +++ b/include/linux/pci-acpi.h > @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) > > void acpi_pci_root_rescan(void); > > +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); > +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); > + > #else > > static inline void acpi_pci_root_rescan(void) { } I don't know where to put this in the conversation, but I think we should separate the question of whether we do blind probing from the question of how we set up MMCONFIG space from MCFG. We currently parse the entire MCFG in some sort of initcall, but I think we *could* change the x86 blind probing routines to attempt to set up MMCONFIG space for the buses they find, just like acpi_pci_root_add() does it for the ACPI host bridges. That way we can continue blind probing, but make MMCONFIG init more sensible. 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
On 2012-5-3 7:55, Bjorn Helgaas wrote: > On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu<liuj97@gmail.com> wrote: >> This patch enhances pci_root driver to update MMCFG information when >> hot-plugging PCI root bridges on x86 platforms. >> >> Signed-off-by: Jiang Liu<jiang.liu@huawei.com> >> --- >> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/acpi/pci_root.c | 20 ++++++++++++++++ >> include/acpi/acnames.h | 1 + >> include/linux/pci-acpi.h | 3 ++ >> 4 files changed, 82 insertions(+), 0 deletions(-) >> >> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c >> index da0149d..9184970 100644 >> --- a/arch/x86/pci/acpi.c >> +++ b/arch/x86/pci/acpi.c >> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) >> return bus; >> } >> >> +int arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + int result = 0; >> + acpi_status status; >> + unsigned long long base_addr; >> + struct pci_mmcfg_region *cfg; >> + >> + /* >> + * Try to insert MMCFG information for host bridges with _CBA method >> + */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL,&base_addr); > > I would prefer to have _CBA evaluated in drivers/acpi/pci_root.c, not > in the arch code. It's true that _CBA is probably only used by x86 > today, but the spec says nothing about it being arch-dependent, and I > suspect it may be used on arm soon. Good point. I just noticed that IA64 doesn't need MMCFG, but haven't realized there's still another candidate. > >> + if (ACPI_SUCCESS(status)) { >> + result = pci_mmconfig_insert(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + /* >> + * MMCFG information for hot-pluggable host bridges may have >> + * already been added by __pci_mmcfg_init(); >> + */ >> + if (result == -EEXIST) >> + result = 0; >> + } else if (status == AE_NOT_FOUND) { >> + /* >> + * Check whether MMCFG information has been added for >> + * host bridges without _CBA method. >> + */ >> + rcu_read_lock(); >> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); >> + if (!cfg || cfg->end_bus< root->secondary.end) >> + result = -ENODEV; >> + rcu_read_unlock(); >> + } else >> + result = -ENODEV; >> + >> + return result; >> +} >> + >> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + acpi_status status; >> + unsigned long long base_addr; >> + >> + /* Remove MMCFG information for host bridges with _CBA method */ >> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, >> + NULL,&base_addr); > > I think the arch-specific "map MMCONFIG space at this addr" should > return a pointer or something that we can save and use to unmap it on > remove. That way we don't have to evaluate _CBA again. OK, will change to follow that way. > >> + if (ACPI_SUCCESS(status)) >> + return pci_mmconfig_delete(root->segment, >> + root->secondary.start, >> + root->secondary.end, >> + base_addr); >> + else if (status != AE_NOT_FOUND) >> + return -ENODEV; >> + >> + return 0; >> +} >> + >> int __init pci_acpi_init(void) >> { >> struct pci_dev *dev = NULL; >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 4a7d575..a62bfa8 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -448,6 +448,16 @@ out: >> } >> EXPORT_SYMBOL(acpi_pci_osc_control_set); >> >> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) >> +{ >> + return 0; >> +} >> + >> static int __devinit acpi_pci_root_add(struct acpi_device *device) >> { >> unsigned long long segment, bus; >> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) >> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >> device->driver_data = root; >> >> + if (arch_acpi_pci_root_add(root)) { >> + printk(KERN_ERR PREFIX >> + "can't add MMCFG information for Bus %04x:%02x\n", >> + root->segment, (unsigned int)root->secondary.start); >> + result = -ENODEV; >> + goto out_free; >> + } >> + >> /* >> * All supported architectures that use ACPI have support for >> * PCI domains, so we indicate this in _OSC support capabilities. >> @@ -629,6 +647,7 @@ out_del_root: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> out_free: >> kfree(root); >> return result; >> @@ -679,6 +698,7 @@ out: >> list_del_rcu(&root->node); >> mutex_unlock(&acpi_pci_root_lock); >> synchronize_rcu(); >> + arch_acpi_pci_root_remove(root); >> kfree(root); >> >> return 0; >> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h >> index 38f5088..99bda75 100644 >> --- a/include/acpi/acnames.h >> +++ b/include/acpi/acnames.h >> @@ -62,6 +62,7 @@ >> #define METHOD_NAME__AEI "_AEI" >> #define METHOD_NAME__PRW "_PRW" >> #define METHOD_NAME__SRS "_SRS" >> +#define METHOD_NAME__CBA "_CBA" >> >> /* Method names - these methods must appear at the namespace root */ >> >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index ac93634..816b971 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) >> >> void acpi_pci_root_rescan(void); >> >> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); >> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); >> + >> #else >> >> static inline void acpi_pci_root_rescan(void) { } > > I don't know where to put this in the conversation, but I think we > should separate the question of whether we do blind probing from the > question of how we set up MMCONFIG space from MCFG. > > We currently parse the entire MCFG in some sort of initcall, but I > think we *could* change the x86 blind probing routines to attempt to > set up MMCONFIG space for the buses they find, just like > acpi_pci_root_add() does it for the ACPI host bridges. > > That way we can continue blind probing, but make MMCONFIG init more sensible. Good suggestion, will do more investigation about that. > > 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
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index da0149d..9184970 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root) return bus; } +int arch_acpi_pci_root_add(struct acpi_pci_root *root) +{ + int result = 0; + acpi_status status; + unsigned long long base_addr; + struct pci_mmcfg_region *cfg; + + /* + * Try to insert MMCFG information for host bridges with _CBA method + */ + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, + NULL, &base_addr); + if (ACPI_SUCCESS(status)) { + result = pci_mmconfig_insert(root->segment, + root->secondary.start, + root->secondary.end, + base_addr); + /* + * MMCFG information for hot-pluggable host bridges may have + * already been added by __pci_mmcfg_init(); + */ + if (result == -EEXIST) + result = 0; + } else if (status == AE_NOT_FOUND) { + /* + * Check whether MMCFG information has been added for + * host bridges without _CBA method. + */ + rcu_read_lock(); + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); + if (!cfg || cfg->end_bus < root->secondary.end) + result = -ENODEV; + rcu_read_unlock(); + } else + result = -ENODEV; + + return result; +} + +int arch_acpi_pci_root_remove(struct acpi_pci_root *root) +{ + acpi_status status; + unsigned long long base_addr; + + /* Remove MMCFG information for host bridges with _CBA method */ + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA, + NULL, &base_addr); + if (ACPI_SUCCESS(status)) + return pci_mmconfig_delete(root->segment, + root->secondary.start, + root->secondary.end, + base_addr); + else if (status != AE_NOT_FOUND) + return -ENODEV; + + return 0; +} + int __init pci_acpi_init(void) { struct pci_dev *dev = NULL; diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 4a7d575..a62bfa8 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -448,6 +448,16 @@ out: } EXPORT_SYMBOL(acpi_pci_osc_control_set); +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root) +{ + return 0; +} + +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root) +{ + return 0; +} + static int __devinit acpi_pci_root_add(struct acpi_device *device) { unsigned long long segment, bus; @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); device->driver_data = root; + if (arch_acpi_pci_root_add(root)) { + printk(KERN_ERR PREFIX + "can't add MMCFG information for Bus %04x:%02x\n", + root->segment, (unsigned int)root->secondary.start); + result = -ENODEV; + goto out_free; + } + /* * All supported architectures that use ACPI have support for * PCI domains, so we indicate this in _OSC support capabilities. @@ -629,6 +647,7 @@ out_del_root: list_del_rcu(&root->node); mutex_unlock(&acpi_pci_root_lock); synchronize_rcu(); + arch_acpi_pci_root_remove(root); out_free: kfree(root); return result; @@ -679,6 +698,7 @@ out: list_del_rcu(&root->node); mutex_unlock(&acpi_pci_root_lock); synchronize_rcu(); + arch_acpi_pci_root_remove(root); kfree(root); return 0; diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h index 38f5088..99bda75 100644 --- a/include/acpi/acnames.h +++ b/include/acpi/acnames.h @@ -62,6 +62,7 @@ #define METHOD_NAME__AEI "_AEI" #define METHOD_NAME__PRW "_PRW" #define METHOD_NAME__SRS "_SRS" +#define METHOD_NAME__CBA "_CBA" /* Method names - these methods must appear at the namespace root */ diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h index ac93634..816b971 100644 --- a/include/linux/pci-acpi.h +++ b/include/linux/pci-acpi.h @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) void acpi_pci_root_rescan(void); +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root); +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root); + #else static inline void acpi_pci_root_rescan(void) { }
This patch enhances pci_root driver to update MMCFG information when hot-plugging PCI root bridges on x86 platforms. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/acpi/pci_root.c | 20 ++++++++++++++++ include/acpi/acnames.h | 1 + include/linux/pci-acpi.h | 3 ++ 4 files changed, 82 insertions(+), 0 deletions(-)