* [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
@ 2012-04-06 2:59 Taku Izumi
2012-04-06 6:31 ` Kenji Kaneshige
` (3 more replies)
0 siblings, 4 replies; 26+ messages in thread
From: Taku Izumi @ 2012-04-06 2:59 UTC (permalink / raw)
To: linux-pci
This patch introduces the configuration for the base address of
the memory mapped configuration space (MMCFG) for hot pluggable PCI
hostbridges on x86 and x86_64.
The MMCFG for hotplugable host bridges must be described by using
ACPI _CBA method. This patch adds implementation for _CBA method
on ACPI pci_root driver and MMCFG manipulating functons for x86 and
x86_64.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
arch/x86/pci/mmconfig-shared.c | 43 +++++++++++++++++++++++++++++++++++------
drivers/acpi/pci_root.c | 28 +++++++++++++++++++++++++-
drivers/pci/pci.c | 31 +++++++++++++++++++++++++++++
include/acpi/acnames.h | 1
include/linux/pci.h | 3 ++
5 files changed, 99 insertions(+), 7 deletions(-)
Index: linux-next-build/drivers/acpi/pci_root.c
===================================================================
--- linux-next-build.orig/drivers/acpi/pci_root.c
+++ linux-next-build/drivers/acpi/pci_root.c
@@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
static int __devinit acpi_pci_root_add(struct acpi_device *device)
{
- unsigned long long segment, bus;
+ unsigned long long segment, bus, base_addr;
acpi_status status;
int result;
struct acpi_pci_root *root;
@@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
device->driver_data = root;
/*
+ * Check _CBA for hot pluggable host bridge
+ */
+ base_addr = 0;
+ status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
+ &base_addr);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+ printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
+ result = -ENODEV;
+ goto end;
+ }
+ if (base_addr) {
+ if (pci_add_mmcfg_region(root->segment,
+ root->secondary.start,
+ root->secondary.end,
+ base_addr) != 0) {
+ printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
+ result = -ENODEV;
+ goto end;
+ }
+ }
+
+ /*
* All supported architectures that use ACPI have support for
* PCI domains, so we indicate this in _OSC support capabilities.
*/
@@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
return 0;
end:
+ if (base_addr)
+ pci_remove_mmcfg_region(root->segment, root->secondary.start);
if (!list_empty(&root->node))
list_del(&root->node);
kfree(root);
@@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
device_set_run_wake(root->bus->bridge, false);
pci_acpi_remove_bus_pm_notifier(device);
+ pci_remove_mmcfg_region(root->segment, root->secondary.start);
+
kfree(root);
return 0;
}
Index: linux-next-build/include/acpi/acnames.h
===================================================================
--- linux-next-build.orig/include/acpi/acnames.h
+++ linux-next-build/include/acpi/acnames.h
@@ -61,6 +61,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 */
Index: linux-next-build/include/linux/pci.h
===================================================================
--- linux-next-build.orig/include/linux/pci.h
+++ linux-next-build/include/linux/pci.h
@@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
void pcibios_set_master(struct pci_dev *dev);
int pcibios_set_pcie_reset_state(struct pci_dev *dev,
enum pcie_reset_state state);
+int pci_add_mmcfg_region(int segment, int start,
+ int end, u64 addr);
+void pci_remove_mmcfg_region(int segment, int bus);
#ifdef CONFIG_PCI_MMCONFIG
extern void __init pci_mmcfg_early_init(void);
Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
===================================================================
--- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
+++ linux-next-build/arch/x86/pci/mmconfig-shared.c
@@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
LIST_HEAD(pci_mmcfg_list);
-static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
+static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
{
if (cfg->res.parent)
release_resource(&cfg->res);
@@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
pci_mmconfig_remove(cfg);
}
-static __init void list_add_sorted(struct pci_mmcfg_region *new)
+static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
{
struct pci_mmcfg_region *cfg;
@@ -61,8 +61,10 @@ static __init void list_add_sorted(struc
list_add_tail(&new->list, &pci_mmcfg_list);
}
-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
- int end, u64 addr)
+static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
+ int start,
+ int end,
+ u64 addr)
{
struct pci_mmcfg_region *new;
struct resource *res;
@@ -108,6 +110,33 @@ struct pci_mmcfg_region *pci_mmconfig_lo
return NULL;
}
+int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
+{
+ struct pci_mmcfg_region *cfg;
+
+ cfg = pci_mmconfig_add(segment, start, end, addr);
+ if (!cfg) {
+ printk(KERN_WARNING PREFIX
+ "no memory for MMCFG entry\n");
+ return -ENOMEM;
+ }
+
+ insert_resource(&iomem_resource, &cfg->res);
+
+ return 0;
+}
+
+void pci_remove_mmcfg_region(int segment, int bus)
+{
+ struct pci_mmcfg_region *cfg;
+
+ cfg = pci_mmconfig_lookup(segment, bus);
+ if (!cfg)
+ return;
+
+ pci_mmconfig_remove(cfg);
+}
+
static const char __init *pci_mmcfg_e7520(void)
{
u32 win;
@@ -357,8 +386,10 @@ static void __init pci_mmcfg_insert_reso
{
struct pci_mmcfg_region *cfg;
- list_for_each_entry(cfg, &pci_mmcfg_list, list)
- insert_resource(&iomem_resource, &cfg->res);
+ list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ if (!cfg->res.parent)
+ insert_resource(&iomem_resource, &cfg->res);
+ }
/* Mark that the resources have been inserted. */
pci_mmcfg_resources_inserted = 1;
Index: linux-next-build/drivers/pci/pci.c
===================================================================
--- linux-next-build.orig/drivers/pci/pci.c
+++ linux-next-build/drivers/pci/pci.c
@@ -79,6 +79,37 @@ unsigned long pci_hotplug_mem_size = DEF
enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
+/**
+ *
+ * pci_add_mmcfg_region
+ * @segment: segment number
+ * @start: start bus number
+ * @end: end bus number
+ * @addr: base address
+ *
+ * Add MMCONFIG entry for specified segment or bus range group
+ *
+ * This is the default implementation. Architecture implementations
+ * can override this.
+ */
+int __attribute__ ((weak)) pci_add_mmcfg_region(int segment, int start,
+ int end, u64 addr)
+{
+ return -EINVAL;
+}
+
+/**
+ * pci_remove_mmcfg_region
+ * @segment: segment number
+ * @bus: bus number
+ *
+ * Remove MMCONFIG entry for specified segment or bus range group
+ *
+ * This is the default implementation. Architecture implementations
+ * can override this.
+ */
+void __attribute__ ((weak)) pci_remove_mmcfg_region(int segment, int bus) {}
+
/*
* The default CLS is used if arch didn't set CLS explicitly and not
* all pci devices agree on the same value. Arch can override either
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-06 2:59 [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64 Taku Izumi
@ 2012-04-06 6:31 ` Kenji Kaneshige
2012-04-06 11:15 ` Taku Izumi
` (2 more replies)
2012-04-08 17:12 ` [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
` (2 subsequent siblings)
3 siblings, 3 replies; 26+ messages in thread
From: Kenji Kaneshige @ 2012-04-06 6:31 UTC (permalink / raw)
To: Taku Izumi; +Cc: linux-pci
(2012/04/06 11:59), Taku Izumi wrote:
>
> This patch introduces the configuration for the base address of
> the memory mapped configuration space (MMCFG) for hot pluggable PCI
> hostbridges on x86 and x86_64.
>
> The MMCFG for hotplugable host bridges must be described by using
> ACPI _CBA method. This patch adds implementation for _CBA method
> on ACPI pci_root driver and MMCFG manipulating functons for x86 and
> x86_64.
>
> Signed-off-by: Taku Izumi<izumi.taku@jp.fujitsu.com>
> ---
> arch/x86/pci/mmconfig-shared.c | 43 +++++++++++++++++++++++++++++++++++------
> drivers/acpi/pci_root.c | 28 +++++++++++++++++++++++++-
> drivers/pci/pci.c | 31 +++++++++++++++++++++++++++++
> include/acpi/acnames.h | 1
> include/linux/pci.h | 3 ++
> 5 files changed, 99 insertions(+), 7 deletions(-)
>
> Index: linux-next-build/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-next-build.orig/drivers/acpi/pci_root.c
> +++ linux-next-build/drivers/acpi/pci_root.c
> @@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
>
> static int __devinit acpi_pci_root_add(struct acpi_device *device)
> {
> - unsigned long long segment, bus;
> + unsigned long long segment, bus, base_addr;
> acpi_status status;
> int result;
> struct acpi_pci_root *root;
> @@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
> device->driver_data = root;
>
> /*
> + * Check _CBA for hot pluggable host bridge
> + */
> + base_addr = 0;
> + status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
> + &base_addr);
> + if (ACPI_FAILURE(status)&& status != AE_NOT_FOUND) {
> + printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
> + result = -ENODEV;
> + goto end;
> + }
> + if (base_addr) {
> + if (pci_add_mmcfg_region(root->segment,
> + root->secondary.start,
> + root->secondary.end,
> + base_addr) != 0) {
> + printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
> + result = -ENODEV;
> + goto end;
> + }
how about
result = pci_add_mmcfg_region()
if (result) {
printk();
goto end;
}
?
> + }
> +
> + /*
> * All supported architectures that use ACPI have support for
> * PCI domains, so we indicate this in _OSC support capabilities.
> */
> @@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
> return 0;
>
> end:
> + if (base_addr)
> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> if (!list_empty(&root->node))
> list_del(&root->node);
> kfree(root);
> @@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
> device_set_run_wake(root->bus->bridge, false);
> pci_acpi_remove_bus_pm_notifier(device);
>
> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> +
> kfree(root);
> return 0;
> }
> Index: linux-next-build/include/acpi/acnames.h
> ===================================================================
> --- linux-next-build.orig/include/acpi/acnames.h
> +++ linux-next-build/include/acpi/acnames.h
> @@ -61,6 +61,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 */
>
> Index: linux-next-build/include/linux/pci.h
> ===================================================================
> --- linux-next-build.orig/include/linux/pci.h
> +++ linux-next-build/include/linux/pci.h
> @@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
> void pcibios_set_master(struct pci_dev *dev);
> int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> enum pcie_reset_state state);
> +int pci_add_mmcfg_region(int segment, int start,
> + int end, u64 addr);
> +void pci_remove_mmcfg_region(int segment, int bus);
>
> #ifdef CONFIG_PCI_MMCONFIG
> extern void __init pci_mmcfg_early_init(void);
> Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
> +++ linux-next-build/arch/x86/pci/mmconfig-shared.c
> @@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
>
> LIST_HEAD(pci_mmcfg_list);
>
> -static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> +static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> {
> if (cfg->res.parent)
> release_resource(&cfg->res);
> @@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
> pci_mmconfig_remove(cfg);
> }
>
> -static __init void list_add_sorted(struct pci_mmcfg_region *new)
> +static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
> {
> struct pci_mmcfg_region *cfg;
>
> @@ -61,8 +61,10 @@ static __init void list_add_sorted(struc
> list_add_tail(&new->list,&pci_mmcfg_list);
> }
>
> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> - int end, u64 addr)
> +static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
> + int start,
> + int end,
> + u64 addr)
> {
> struct pci_mmcfg_region *new;
> struct resource *res;
> @@ -108,6 +110,33 @@ struct pci_mmcfg_region *pci_mmconfig_lo
> return NULL;
> }
>
> +int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
> +{
> + struct pci_mmcfg_region *cfg;
> +
> + cfg = pci_mmconfig_add(segment, start, end, addr);
> + if (!cfg) {
> + printk(KERN_WARNING PREFIX
> + "no memory for MMCFG entry\n");
> + return -ENOMEM;
> + }
We need to check if the region is valid (if the region is
reserved in ACPI motherborad resource).
> +
> + insert_resource(&iomem_resource,&cfg->res);
> +
> + return 0;
> +}
> +
> +void pci_remove_mmcfg_region(int segment, int bus)
> +{
> + struct pci_mmcfg_region *cfg;
> +
> + cfg = pci_mmconfig_lookup(segment, bus);
> + if (!cfg)
> + return;
> +
> + pci_mmconfig_remove(cfg);
> +}
> +
> static const char __init *pci_mmcfg_e7520(void)
> {
> u32 win;
> @@ -357,8 +386,10 @@ static void __init pci_mmcfg_insert_reso
> {
> struct pci_mmcfg_region *cfg;
>
> - list_for_each_entry(cfg,&pci_mmcfg_list, list)
> - insert_resource(&iomem_resource,&cfg->res);
> + list_for_each_entry(cfg,&pci_mmcfg_list, list) {
We need to add lock for manipulating pci_mmcfg_list.
> + if (!cfg->res.parent)
Why do we need this line?
Regards,
Kenji Kaneshige
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-06 6:31 ` Kenji Kaneshige
@ 2012-04-06 11:15 ` Taku Izumi
2012-04-06 11:16 ` Taku Izumi
2012-04-07 15:09 ` Jiang Liu
2 siblings, 0 replies; 26+ messages in thread
From: Taku Izumi @ 2012-04-06 11:15 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: linux-pci
Thank you for your comment.
On Fri, 06 Apr 2012 15:31:57 +0900
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> wrote:
> (2012/04/06 11:59), Taku Izumi wrote:
> >
> > This patch introduces the configuration for the base address of
> > the memory mapped configuration space (MMCFG) for hot pluggable PCI
> > hostbridges on x86 and x86_64.
> >
> > The MMCFG for hotplugable host bridges must be described by using
> > ACPI _CBA method. This patch adds implementation for _CBA method
> > on ACPI pci_root driver and MMCFG manipulating functons for x86 and
> > x86_64.
> >
> > Signed-off-by: Taku Izumi<izumi.taku@jp.fujitsu.com>
> > ---
> > arch/x86/pci/mmconfig-shared.c | 43 +++++++++++++++++++++++++++++++++++------
> > drivers/acpi/pci_root.c | 28 +++++++++++++++++++++++++-
> > drivers/pci/pci.c | 31 +++++++++++++++++++++++++++++
> > include/acpi/acnames.h | 1
> > include/linux/pci.h | 3 ++
> > 5 files changed, 99 insertions(+), 7 deletions(-)
> >
> > Index: linux-next-build/drivers/acpi/pci_root.c
> > ===================================================================
> > --- linux-next-build.orig/drivers/acpi/pci_root.c
> > +++ linux-next-build/drivers/acpi/pci_root.c
> > @@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
> >
> > static int __devinit acpi_pci_root_add(struct acpi_device *device)
> > {
> > - unsigned long long segment, bus;
> > + unsigned long long segment, bus, base_addr;
> > acpi_status status;
> > int result;
> > struct acpi_pci_root *root;
> > @@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
> > device->driver_data = root;
> >
> > /*
> > + * Check _CBA for hot pluggable host bridge
> > + */
> > + base_addr = 0;
> > + status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
> > + &base_addr);
> > + if (ACPI_FAILURE(status)&& status != AE_NOT_FOUND) {
> > + printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
> > + result = -ENODEV;
> > + goto end;
> > + }
> > + if (base_addr) {
> > + if (pci_add_mmcfg_region(root->segment,
> > + root->secondary.start,
> > + root->secondary.end,
> > + base_addr) != 0) {
> > + printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
> > + result = -ENODEV;
> > + goto end;
> > + }
>
> how about
> result = pci_add_mmcfg_region()
> if (result) {
> printk();
> goto end;
> }
> ?
I'll take in your good idea.
> > + }
> > +
> > + /*
> > * All supported architectures that use ACPI have support for
> > * PCI domains, so we indicate this in _OSC support capabilities.
> > */
> > @@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
> > return 0;
> >
> > end:
> > + if (base_addr)
> > + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> > if (!list_empty(&root->node))
> > list_del(&root->node);
> > kfree(root);
> > @@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
> > device_set_run_wake(root->bus->bridge, false);
> > pci_acpi_remove_bus_pm_notifier(device);
> >
> > + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> > +
> > kfree(root);
> > return 0;
> > }
> > Index: linux-next-build/include/acpi/acnames.h
> > ===================================================================
> > --- linux-next-build.orig/include/acpi/acnames.h
> > +++ linux-next-build/include/acpi/acnames.h
> > @@ -61,6 +61,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 */
> >
> > Index: linux-next-build/include/linux/pci.h
> > ===================================================================
> > --- linux-next-build.orig/include/linux/pci.h
> > +++ linux-next-build/include/linux/pci.h
> > @@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
> > void pcibios_set_master(struct pci_dev *dev);
> > int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> > enum pcie_reset_state state);
> > +int pci_add_mmcfg_region(int segment, int start,
> > + int end, u64 addr);
> > +void pci_remove_mmcfg_region(int segment, int bus);
> >
> > #ifdef CONFIG_PCI_MMCONFIG
> > extern void __init pci_mmcfg_early_init(void);
> > Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
> > ===================================================================
> > --- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
> > +++ linux-next-build/arch/x86/pci/mmconfig-shared.c
> > @@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
> >
> > LIST_HEAD(pci_mmcfg_list);
> >
> > -static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> > +static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> > {
> > if (cfg->res.parent)
> > release_resource(&cfg->res);
> > @@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
> > pci_mmconfig_remove(cfg);
> > }
> >
> > -static __init void list_add_sorted(struct pci_mmcfg_region *new)
> > +static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
> > {
> > struct pci_mmcfg_region *cfg;
> >
> > @@ -61,8 +61,10 @@ static __init void list_add_sorted(struc
> > list_add_tail(&new->list,&pci_mmcfg_list);
> > }
> >
> > -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> > - int end, u64 addr)
> > +static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
> > + int start,
> > + int end,
> > + u64 addr)
> > {
> > struct pci_mmcfg_region *new;
> > struct resource *res;
> > @@ -108,6 +110,33 @@ struct pci_mmcfg_region *pci_mmconfig_lo
> > return NULL;
> > }
> >
> > +int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
> > +{
> > + struct pci_mmcfg_region *cfg;
> > +
> > + cfg = pci_mmconfig_add(segment, start, end, addr);
> > + if (!cfg) {
> > + printk(KERN_WARNING PREFIX
> > + "no memory for MMCFG entry\n");
> > + return -ENOMEM;
> > + }
>
> We need to check if the region is valid (if the region is
> reserved in ACPI motherborad resource).
I'll add the code to sanity check.
>
> > +
> > + insert_resource(&iomem_resource,&cfg->res);
> > +
> > + return 0;
> > +}
> > +
> > +void pci_remove_mmcfg_region(int segment, int bus)
> > +{
> > + struct pci_mmcfg_region *cfg;
> > +
> > + cfg = pci_mmconfig_lookup(segment, bus);
> > + if (!cfg)
> > + return;
> > +
> > + pci_mmconfig_remove(cfg);
> > +}
> > +
> > static const char __init *pci_mmcfg_e7520(void)
> > {
> > u32 win;
> > @@ -357,8 +386,10 @@ static void __init pci_mmcfg_insert_reso
> > {
> > struct pci_mmcfg_region *cfg;
> >
> > - list_for_each_entry(cfg,&pci_mmcfg_list, list)
> > - insert_resource(&iomem_resource,&cfg->res);
> > + list_for_each_entry(cfg,&pci_mmcfg_list, list) {
>
> We need to add lock for manipulating pci_mmcfg_list.
>
> > + if (!cfg->res.parent)
>
> Why do we need this line?
>
> 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
>
--
Taku Izumi <izumi.taku@jp.fujitsu.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-06 6:31 ` Kenji Kaneshige
2012-04-06 11:15 ` Taku Izumi
@ 2012-04-06 11:16 ` Taku Izumi
2012-04-07 15:20 ` Jiang Liu
2012-04-07 15:09 ` Jiang Liu
2 siblings, 1 reply; 26+ messages in thread
From: Taku Izumi @ 2012-04-06 11:16 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: linux-pci
This patch introduces the configuration for the base address of
the memory mapped configuration space (MMCFG) for hotpluggable PCI
hostbridges on x86 and x86_64.
The MMCFG for hotpluggable host bridges must be described by using
ACPI _CBA method. This patch adds implementation for _CBA method
on ACPI pci_root driver and MMCFG manipulating functons for x86 and
x86_64.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
arch/x86/pci/mmconfig-shared.c | 61 ++++++++++++++++++++++++++++++++++-------
drivers/acpi/pci_root.c | 28 ++++++++++++++++++
drivers/pci/pci.c | 31 ++++++++++++++++++++
include/acpi/acnames.h | 1
include/linux/pci.h | 3 ++
5 files changed, 113 insertions(+), 11 deletions(-)
Index: linux-next-build/drivers/acpi/pci_root.c
===================================================================
--- linux-next-build.orig/drivers/acpi/pci_root.c
+++ linux-next-build/drivers/acpi/pci_root.c
@@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
static int __devinit acpi_pci_root_add(struct acpi_device *device)
{
- unsigned long long segment, bus;
+ unsigned long long segment, bus, base_addr;
acpi_status status;
int result;
struct acpi_pci_root *root;
@@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
device->driver_data = root;
/*
+ * Check _CBA for hot pluggable host bridge
+ */
+ base_addr = 0;
+ status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
+ &base_addr);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+ printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
+ result = -ENODEV;
+ goto end;
+ }
+ if (base_addr) {
+ result = pci_add_mmcfg_region(root->segment,
+ root->secondary.start,
+ root->secondary.end,
+ base_addr);
+ if (result) {
+ printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
+ goto end;
+ }
+ }
+
+ /*
* All supported architectures that use ACPI have support for
* PCI domains, so we indicate this in _OSC support capabilities.
*/
@@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
return 0;
end:
+ if (base_addr)
+ pci_remove_mmcfg_region(root->segment, root->secondary.start);
if (!list_empty(&root->node))
list_del(&root->node);
kfree(root);
@@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
device_set_run_wake(root->bus->bridge, false);
pci_acpi_remove_bus_pm_notifier(device);
+ pci_remove_mmcfg_region(root->segment, root->secondary.start);
+
kfree(root);
return 0;
}
Index: linux-next-build/include/acpi/acnames.h
===================================================================
--- linux-next-build.orig/include/acpi/acnames.h
+++ linux-next-build/include/acpi/acnames.h
@@ -61,6 +61,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 */
Index: linux-next-build/include/linux/pci.h
===================================================================
--- linux-next-build.orig/include/linux/pci.h
+++ linux-next-build/include/linux/pci.h
@@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
void pcibios_set_master(struct pci_dev *dev);
int pcibios_set_pcie_reset_state(struct pci_dev *dev,
enum pcie_reset_state state);
+int pci_add_mmcfg_region(int segment, int start,
+ int end, u64 addr);
+void pci_remove_mmcfg_region(int segment, int bus);
#ifdef CONFIG_PCI_MMCONFIG
extern void __init pci_mmcfg_early_init(void);
Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
===================================================================
--- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
+++ linux-next-build/arch/x86/pci/mmconfig-shared.c
@@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
LIST_HEAD(pci_mmcfg_list);
-static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
+static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
{
if (cfg->res.parent)
release_resource(&cfg->res);
@@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
pci_mmconfig_remove(cfg);
}
-static __init void list_add_sorted(struct pci_mmcfg_region *new)
+static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
{
struct pci_mmcfg_region *cfg;
@@ -61,8 +61,8 @@ static __init void list_add_sorted(struc
list_add_tail(&new->list, &pci_mmcfg_list);
}
-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
- int end, u64 addr)
+static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
+ int start, int end, u64 addr)
{
struct pci_mmcfg_region *new;
struct resource *res;
@@ -357,8 +357,10 @@ static void __init pci_mmcfg_insert_reso
{
struct pci_mmcfg_region *cfg;
- list_for_each_entry(cfg, &pci_mmcfg_list, list)
- insert_resource(&iomem_resource, &cfg->res);
+ list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ if (!cfg->res.parent)
+ insert_resource(&iomem_resource, &cfg->res);
+ }
/* Mark that the resources have been inserted. */
pci_mmcfg_resources_inserted = 1;
@@ -401,7 +403,7 @@ static acpi_status __init check_mcfg_res
return AE_OK;
}
-static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
+static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
void *context, void **rv)
{
struct resource *mcfg_res = context;
@@ -415,7 +417,7 @@ static acpi_status __init find_mboard_re
return AE_OK;
}
-static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
+static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
{
struct resource mcfg_res;
@@ -434,8 +436,9 @@ static int __init is_acpi_reserved(u64 s
typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
-static int __init is_mmconf_reserved(check_reserved_t is_reserved,
- struct pci_mmcfg_region *cfg, int with_e820)
+static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
+ struct pci_mmcfg_region *cfg,
+ int with_e820)
{
u64 addr = cfg->res.start;
u64 size = resource_size(&cfg->res);
@@ -580,6 +583,44 @@ static int __init pci_parse_mcfg(struct
return 0;
}
+int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
+{
+ struct pci_mmcfg_region *cfg;
+ int valid;
+
+ cfg = pci_mmconfig_add(segment, start, end, addr);
+ if (!cfg) {
+ printk(KERN_WARNING PREFIX
+ "no memory for MMCFG entry\n");
+ return -ENOMEM;
+ }
+
+ valid = is_mmconf_reserved(is_acpi_reserved, cfg, 0);
+ if (!valid) {
+ printk(KERN_ERR FW_BUG PREFIX
+ "MMCONFIG at %pR not reserved in "
+ "ACPI motherboard resources\n",
+ &cfg->res);
+ pci_mmconfig_remove(cfg);
+ return -EINVAL;
+ }
+
+ insert_resource(&iomem_resource, &cfg->res);
+
+ return 0;
+}
+
+void pci_remove_mmcfg_region(int segment, int bus)
+{
+ struct pci_mmcfg_region *cfg;
+
+ cfg = pci_mmconfig_lookup(segment, bus);
+ if (!cfg)
+ return;
+
+ pci_mmconfig_remove(cfg);
+}
+
static void __init __pci_mmcfg_init(int early)
{
/* MMCONFIG disabled */
Index: linux-next-build/drivers/pci/pci.c
===================================================================
--- linux-next-build.orig/drivers/pci/pci.c
+++ linux-next-build/drivers/pci/pci.c
@@ -79,6 +79,37 @@ unsigned long pci_hotplug_mem_size = DEF
enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
+/**
+ *
+ * pci_add_mmcfg_region
+ * @segment: segment number
+ * @start: start bus number
+ * @end: end bus number
+ * @addr: base address
+ *
+ * Add MMCONFIG entry for specified segment or bus range group
+ *
+ * This is the default implementation. Architecture implementations
+ * can override this.
+ */
+int __attribute__ ((weak)) pci_add_mmcfg_region(int segment, int start,
+ int end, u64 addr)
+{
+ return -EINVAL;
+}
+
+/**
+ * pci_remove_mmcfg_region
+ * @segment: segment number
+ * @bus: bus number
+ *
+ * Remove MMCONFIG entry for specified segment or bus range group
+ *
+ * This is the default implementation. Architecture implementations
+ * can override this.
+ */
+void __attribute__ ((weak)) pci_remove_mmcfg_region(int segment, int bus) {}
+
/*
* The default CLS is used if arch didn't set CLS explicitly and not
* all pci devices agree on the same value. Arch can override either
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-06 6:31 ` Kenji Kaneshige
2012-04-06 11:15 ` Taku Izumi
2012-04-06 11:16 ` Taku Izumi
@ 2012-04-07 15:09 ` Jiang Liu
2 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-07 15:09 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: Taku Izumi, linux-pci
On 04/06/2012 02:31 PM, Kenji Kaneshige wrote:
> (2012/04/06 11:59), Taku Izumi wrote:
>>
>> This patch introduces the configuration for the base address of
>> the memory mapped configuration space (MMCFG) for hot pluggable PCI
>> hostbridges on x86 and x86_64.
>>
>> The MMCFG for hotplugable host bridges must be described by using
>> ACPI _CBA method. This patch adds implementation for _CBA method
>> on ACPI pci_root driver and MMCFG manipulating functons for x86 and
>> x86_64.
>>
>> Signed-off-by: Taku Izumi<izumi.taku@jp.fujitsu.com>
>> ---
>> arch/x86/pci/mmconfig-shared.c | 43 +++++++++++++++++++++++++++++++++++------
>> drivers/acpi/pci_root.c | 28 +++++++++++++++++++++++++-
>> drivers/pci/pci.c | 31 +++++++++++++++++++++++++++++
>> include/acpi/acnames.h | 1
>> include/linux/pci.h | 3 ++
>> 5 files changed, 99 insertions(+), 7 deletions(-)
>>
>> Index: linux-next-build/drivers/acpi/pci_root.c
>> ===================================================================
>> --- linux-next-build.orig/drivers/acpi/pci_root.c
>> +++ linux-next-build/drivers/acpi/pci_root.c
>> @@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
>>
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> - unsigned long long segment, bus;
>> + unsigned long long segment, bus, base_addr;
>> acpi_status status;
>> int result;
>> struct acpi_pci_root *root;
>> @@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
>> device->driver_data = root;
>>
>> /*
>> + * Check _CBA for hot pluggable host bridge
>> + */
>> + base_addr = 0;
>> + status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
>> + &base_addr);
>> + if (ACPI_FAILURE(status)&& status != AE_NOT_FOUND) {
>> + printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
>> + result = -ENODEV;
>> + goto end;
>> + }
>> + if (base_addr) {
>> + if (pci_add_mmcfg_region(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr) != 0) {
>> + printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
>> + result = -ENODEV;
>> + goto end;
>> + }
>
> how about
> result = pci_add_mmcfg_region()
> if (result) {
> printk();
> goto end;
> }
> ?
>
>> + }
>> +
>> + /*
>> * All supported architectures that use ACPI have support for
>> * PCI domains, so we indicate this in _OSC support capabilities.
>> */
>> @@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
>> return 0;
>>
>> end:
>> + if (base_addr)
>> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
>> if (!list_empty(&root->node))
>> list_del(&root->node);
>> kfree(root);
>> @@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
>> device_set_run_wake(root->bus->bridge, false);
>> pci_acpi_remove_bus_pm_notifier(device);
>>
>> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
>> +
>> kfree(root);
>> return 0;
>> }
>> Index: linux-next-build/include/acpi/acnames.h
>> ===================================================================
>> --- linux-next-build.orig/include/acpi/acnames.h
>> +++ linux-next-build/include/acpi/acnames.h
>> @@ -61,6 +61,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 */
>>
>> Index: linux-next-build/include/linux/pci.h
>> ===================================================================
>> --- linux-next-build.orig/include/linux/pci.h
>> +++ linux-next-build/include/linux/pci.h
>> @@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
>> void pcibios_set_master(struct pci_dev *dev);
>> int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>> enum pcie_reset_state state);
>> +int pci_add_mmcfg_region(int segment, int start,
>> + int end, u64 addr);
>> +void pci_remove_mmcfg_region(int segment, int bus);
>>
>> #ifdef CONFIG_PCI_MMCONFIG
>> extern void __init pci_mmcfg_early_init(void);
>> Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
>> ===================================================================
>> --- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
>> +++ linux-next-build/arch/x86/pci/mmconfig-shared.c
>> @@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
>>
>> LIST_HEAD(pci_mmcfg_list);
>>
>> -static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
>> +static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
>> {
>> if (cfg->res.parent)
>> release_resource(&cfg->res);
>> @@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
>> pci_mmconfig_remove(cfg);
>> }
>>
>> -static __init void list_add_sorted(struct pci_mmcfg_region *new)
>> +static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
>> {
>> struct pci_mmcfg_region *cfg;
>>
>> @@ -61,8 +61,10 @@ static __init void list_add_sorted(struc
>> list_add_tail(&new->list,&pci_mmcfg_list);
>> }
>>
>> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
>> - int end, u64 addr)
>> +static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
>> + int start,
>> + int end,
>> + u64 addr)
>> {
>> struct pci_mmcfg_region *new;
>> struct resource *res;
>> @@ -108,6 +110,33 @@ struct pci_mmcfg_region *pci_mmconfig_lo
>> return NULL;
>> }
>>
>> +int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
>> +{
>> + struct pci_mmcfg_region *cfg;
>> +
>> + cfg = pci_mmconfig_add(segment, start, end, addr);
>> + if (!cfg) {
>> + printk(KERN_WARNING PREFIX
>> + "no memory for MMCFG entry\n");
>> + return -ENOMEM;
>> + }
>
> We need to check if the region is valid (if the region is
> reserved in ACPI motherborad resource).
>
>
>> +
>> + insert_resource(&iomem_resource,&cfg->res);
>> +
>> + return 0;
>> +}
>> +
>> +void pci_remove_mmcfg_region(int segment, int bus)
>> +{
>> + struct pci_mmcfg_region *cfg;
>> +
>> + cfg = pci_mmconfig_lookup(segment, bus);
>> + if (!cfg)
>> + return;
>> +
>> + pci_mmconfig_remove(cfg);
>> +}
>> +
>> static const char __init *pci_mmcfg_e7520(void)
>> {
>> u32 win;
>> @@ -357,8 +386,10 @@ static void __init pci_mmcfg_insert_reso
>> {
>> struct pci_mmcfg_region *cfg;
>>
>> - list_for_each_entry(cfg,&pci_mmcfg_list, list)
>> - insert_resource(&iomem_resource,&cfg->res);
>> + list_for_each_entry(cfg,&pci_mmcfg_list, list) {
>
> We need to add lock for manipulating pci_mmcfg_list.
I have worked out a similar patch several weeks ago to address this issue,
use lock to protect the mmconfig list when pci host bridge hotplug happens.
But I can't access my dev machine from home now, and will send it out for
your reference tomorrow.
Thanks!
>
>> + if (!cfg->res.parent)
>
> Why do we need this line?
>
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-06 11:16 ` Taku Izumi
@ 2012-04-07 15:20 ` Jiang Liu
2012-04-25 17:14 ` Bjorn Helgaas
0 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-04-07 15:20 UTC (permalink / raw)
To: Taku Izumi; +Cc: Kenji Kaneshige, linux-pci
On 04/06/2012 07:16 PM, Taku Izumi wrote:
>
> This patch introduces the configuration for the base address of
> the memory mapped configuration space (MMCFG) for hotpluggable PCI
> hostbridges on x86 and x86_64.
>
> The MMCFG for hotpluggable host bridges must be described by using
> ACPI _CBA method. This patch adds implementation for _CBA method
> on ACPI pci_root driver and MMCFG manipulating functons for x86 and
> x86_64.
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
> arch/x86/pci/mmconfig-shared.c | 61 ++++++++++++++++++++++++++++++++++-------
> drivers/acpi/pci_root.c | 28 ++++++++++++++++++
> drivers/pci/pci.c | 31 ++++++++++++++++++++
> include/acpi/acnames.h | 1
> include/linux/pci.h | 3 ++
> 5 files changed, 113 insertions(+), 11 deletions(-)
>
> Index: linux-next-build/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-next-build.orig/drivers/acpi/pci_root.c
> +++ linux-next-build/drivers/acpi/pci_root.c
> @@ -451,7 +451,7 @@ EXPORT_SYMBOL(acpi_pci_osc_control_set);
>
> static int __devinit acpi_pci_root_add(struct acpi_device *device)
> {
> - unsigned long long segment, bus;
> + unsigned long long segment, bus, base_addr;
> acpi_status status;
> int result;
> struct acpi_pci_root *root;
> @@ -506,6 +506,28 @@ static int __devinit acpi_pci_root_add(s
> device->driver_data = root;
>
> /*
> + * Check _CBA for hot pluggable host bridge
> + */
> + base_addr = 0;
> + status = acpi_evaluate_integer(device->handle, METHOD_NAME__CBA, NULL,
> + &base_addr);
> + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> + printk(KERN_ERR PREFIX "can't evaluate _CBA\n");
> + result = -ENODEV;
> + goto end;
> + }
> + if (base_addr) {
> + result = pci_add_mmcfg_region(root->segment,
> + root->secondary.start,
> + root->secondary.end,
> + base_addr);
> + if (result) {
> + printk(KERN_ERR PREFIX "can't add MMCFG entry\n");
> + goto end;
> + }
The MMCFG information for host bridges present at boot time should have already
been added by __pci_mmcfg_init() when acpi_pci_root_add() is called.
So it would be better for pci_add_mmcfg_region() to return -EEXIST for host
bridges present at boot time.
> + }
> +
> + /*
> * All supported architectures that use ACPI have support for
> * PCI domains, so we indicate this in _OSC support capabilities.
> */
> @@ -625,6 +647,8 @@ static int __devinit acpi_pci_root_add(s
> return 0;
>
> end:
> + if (base_addr)
> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> if (!list_empty(&root->node))
> list_del(&root->node);
> kfree(root);
> @@ -646,6 +670,8 @@ static int acpi_pci_root_remove(struct a
> device_set_run_wake(root->bus->bridge, false);
> pci_acpi_remove_bus_pm_notifier(device);
>
> + pci_remove_mmcfg_region(root->segment, root->secondary.start);
> +
It would be better to only call pci_remove_mmcfg_region() if the host bridge has _CBA
method available, otherwise we have no way to recover this MMCFG information and
can't rebind the pci_root driver to a pci host bridge after an unbinding operation.
> kfree(root);
> return 0;
> }
> Index: linux-next-build/include/acpi/acnames.h
> ===================================================================
> --- linux-next-build.orig/include/acpi/acnames.h
> +++ linux-next-build/include/acpi/acnames.h
> @@ -61,6 +61,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 */
>
> Index: linux-next-build/include/linux/pci.h
> ===================================================================
> --- linux-next-build.orig/include/linux/pci.h
> +++ linux-next-build/include/linux/pci.h
> @@ -1460,6 +1460,9 @@ void pcibios_disable_device(struct pci_d
> void pcibios_set_master(struct pci_dev *dev);
> int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> enum pcie_reset_state state);
> +int pci_add_mmcfg_region(int segment, int start,
> + int end, u64 addr);
> +void pci_remove_mmcfg_region(int segment, int bus);
>
> #ifdef CONFIG_PCI_MMCONFIG
> extern void __init pci_mmcfg_early_init(void);
> Index: linux-next-build/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- linux-next-build.orig/arch/x86/pci/mmconfig-shared.c
> +++ linux-next-build/arch/x86/pci/mmconfig-shared.c
> @@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resource
>
> LIST_HEAD(pci_mmcfg_list);
>
> -static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> +static __devinit void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> {
> if (cfg->res.parent)
> release_resource(&cfg->res);
> @@ -45,7 +45,7 @@ static __init void free_all_mmcfg(void)
> pci_mmconfig_remove(cfg);
> }
>
> -static __init void list_add_sorted(struct pci_mmcfg_region *new)
> +static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
> {
> struct pci_mmcfg_region *cfg;
>
> @@ -61,8 +61,8 @@ static __init void list_add_sorted(struc
> list_add_tail(&new->list, &pci_mmcfg_list);
> }
>
> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> - int end, u64 addr)
> +static __devinit struct pci_mmcfg_region *pci_mmconfig_add(int segment,
> + int start, int end, u64 addr)
> {
> struct pci_mmcfg_region *new;
> struct resource *res;
> @@ -357,8 +357,10 @@ static void __init pci_mmcfg_insert_reso
> {
> struct pci_mmcfg_region *cfg;
>
> - list_for_each_entry(cfg, &pci_mmcfg_list, list)
> - insert_resource(&iomem_resource, &cfg->res);
> + list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> + if (!cfg->res.parent)
> + insert_resource(&iomem_resource, &cfg->res);
> + }
>
> /* Mark that the resources have been inserted. */
> pci_mmcfg_resources_inserted = 1;
> @@ -401,7 +403,7 @@ static acpi_status __init check_mcfg_res
> return AE_OK;
> }
>
> -static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
> +static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
> void *context, void **rv)
> {
> struct resource *mcfg_res = context;
> @@ -415,7 +417,7 @@ static acpi_status __init find_mboard_re
> return AE_OK;
> }
>
> -static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> +static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> {
> struct resource mcfg_res;
>
> @@ -434,8 +436,9 @@ static int __init is_acpi_reserved(u64 s
>
> typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
>
> -static int __init is_mmconf_reserved(check_reserved_t is_reserved,
> - struct pci_mmcfg_region *cfg, int with_e820)
> +static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
> + struct pci_mmcfg_region *cfg,
> + int with_e820)
> {
> u64 addr = cfg->res.start;
> u64 size = resource_size(&cfg->res);
> @@ -580,6 +583,44 @@ static int __init pci_parse_mcfg(struct
> return 0;
> }
>
> +int pci_add_mmcfg_region(int segment, int start, int end, u64 addr)
> +{
> + struct pci_mmcfg_region *cfg;
> + int valid;
> +
> + cfg = pci_mmconfig_add(segment, start, end, addr);
> + if (!cfg) {
> + printk(KERN_WARNING PREFIX
> + "no memory for MMCFG entry\n");
> + return -ENOMEM;
> + }
> +
> + valid = is_mmconf_reserved(is_acpi_reserved, cfg, 0);
> + if (!valid) {
> + printk(KERN_ERR FW_BUG PREFIX
> + "MMCONFIG at %pR not reserved in "
> + "ACPI motherboard resources\n",
> + &cfg->res);
> + pci_mmconfig_remove(cfg);
> + return -EINVAL;
> + }
> +
> + insert_resource(&iomem_resource, &cfg->res);
> +
> + return 0;
> +}
> +
> +void pci_remove_mmcfg_region(int segment, int bus)
> +{
> + struct pci_mmcfg_region *cfg;
> +
> + cfg = pci_mmconfig_lookup(segment, bus);
> + if (!cfg)
> + return;
> +
> + pci_mmconfig_remove(cfg);
> +}
> +
> static void __init __pci_mmcfg_init(int early)
> {
> /* MMCONFIG disabled */
> Index: linux-next-build/drivers/pci/pci.c
> ===================================================================
> --- linux-next-build.orig/drivers/pci/pci.c
> +++ linux-next-build/drivers/pci/pci.c
> @@ -79,6 +79,37 @@ unsigned long pci_hotplug_mem_size = DEF
>
> enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
>
> +/**
> + *
> + * pci_add_mmcfg_region
> + * @segment: segment number
> + * @start: start bus number
> + * @end: end bus number
> + * @addr: base address
> + *
> + * Add MMCONFIG entry for specified segment or bus range group
> + *
> + * This is the default implementation. Architecture implementations
> + * can override this.
> + */
> +int __attribute__ ((weak)) pci_add_mmcfg_region(int segment, int start,
> + int end, u64 addr)
> +{
> + return -EINVAL;
> +}
> +
> +/**
> + * pci_remove_mmcfg_region
> + * @segment: segment number
> + * @bus: bus number
> + *
> + * Remove MMCONFIG entry for specified segment or bus range group
> + *
> + * This is the default implementation. Architecture implementations
> + * can override this.
> + */
> +void __attribute__ ((weak)) pci_remove_mmcfg_region(int segment, int bus) {}
> +
> /*
> * The default CLS is used if arch didn't set CLS explicitly and not
> * all pci devices agree on the same value. Arch can override either
>
> --
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-06 2:59 [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64 Taku Izumi
2012-04-06 6:31 ` Kenji Kaneshige
@ 2012-04-08 17:12 ` Jiang Liu
2012-04-25 17:17 ` Bjorn Helgaas
2012-04-08 17:12 ` [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug Jiang Liu
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
3 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-04-08 17:12 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Bjorn Helgaas
Cc: Jiang Liu, Jiang Liu, Keping Chen, linux-kernel, linux-pci
This patchset enhance pci_root driver to update MMCFG information when
hot-plugging PCI root bridges. It applies to Yinghai's tree at
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug
The second patch is based on Taku Izumi work with some enhancements to
correctly handle PCI host bridges without _CBA method.
Jiang Liu (2):
PCI,x86: introduce new MMCFG interfaces to support PCI host bridge
hotplug
PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host
bridges
arch/x86/include/asm/pci_x86.h | 4 +
arch/x86/pci/acpi.c | 58 +++++++++++++
arch/x86/pci/mmconfig-shared.c | 186 +++++++++++++++++++++++++++++++---------
arch/x86/pci/mmconfig_32.c | 28 ++++++-
arch/x86/pci/mmconfig_64.c | 35 +++++++-
drivers/acpi/pci_root.c | 20 +++++
include/acpi/acnames.h | 1 +
include/linux/pci-acpi.h | 3 +
8 files changed, 288 insertions(+), 47 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug
2012-04-06 2:59 [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64 Taku Izumi
2012-04-06 6:31 ` Kenji Kaneshige
2012-04-08 17:12 ` [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
@ 2012-04-08 17:12 ` Jiang Liu
2012-04-08 19:12 ` Yinghai Lu
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
3 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-04-08 17:12 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Bjorn Helgaas
Cc: Jiang Liu, Jiang Liu, Keping Chen, linux-kernel, linux-pci
This patch introduces two new interfaces, pci_mmconfig_insert() and
pci_mmconfig_delete(), to support PCI host bridge hotplug on x86 platforms.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/include/asm/pci_x86.h | 4 +
arch/x86/pci/mmconfig-shared.c | 186 +++++++++++++++++++++++++++++++---------
arch/x86/pci/mmconfig_32.c | 28 ++++++-
arch/x86/pci/mmconfig_64.c | 35 +++++++-
4 files changed, 206 insertions(+), 47 deletions(-)
diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index b3a5317..ba8570c 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -135,6 +135,10 @@ struct pci_mmcfg_region {
extern int __init pci_mmcfg_arch_init(void);
extern void __init pci_mmcfg_arch_free(void);
+extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
+extern void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
+extern int __devinit pci_mmconfig_insert(int seg, int start, int end, u64 addr);
+extern int __devinit pci_mmconfig_delete(int seg, int start, int end, u64 addr);
extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
extern struct list_head pci_mmcfg_list;
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 301e325..dda9470 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -17,6 +17,8 @@
#include <linux/bitmap.h>
#include <linux/dmi.h>
#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/rculist.h>
#include <asm/e820.h>
#include <asm/pci_x86.h>
#include <asm/acpi.h>
@@ -45,24 +47,25 @@ static __init void free_all_mmcfg(void)
pci_mmconfig_remove(cfg);
}
-static __init void list_add_sorted(struct pci_mmcfg_region *new)
+static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
{
struct pci_mmcfg_region *cfg;
/* keep list sorted by segment and starting bus number */
- list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
if (cfg->segment > new->segment ||
(cfg->segment == new->segment &&
cfg->start_bus >= new->start_bus)) {
- list_add_tail(&new->list, &cfg->list);
+ list_add_tail_rcu(&new->list, &cfg->list);
return;
}
}
- list_add_tail(&new->list, &pci_mmcfg_list);
+ list_add_tail_rcu(&new->list, &pci_mmcfg_list);
}
-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
- int end, u64 addr)
+static __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
+ int start,
+ int end, u64 addr)
{
struct pci_mmcfg_region *new;
struct resource *res;
@@ -79,8 +82,6 @@ static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
new->start_bus = start;
new->end_bus = end;
- list_add_sorted(new);
-
res = &new->res;
res->start = addr + PCI_MMCFG_BUS_OFFSET(start);
res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1;
@@ -96,11 +97,23 @@ static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
return new;
}
+static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
+ int end, u64 addr)
+{
+ struct pci_mmcfg_region *new;
+
+ new = pci_mmconfig_alloc(segment, start, end, addr);
+ if (new)
+ list_add_sorted(new);
+
+ return new;
+}
+
struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
{
struct pci_mmcfg_region *cfg;
- list_for_each_entry(cfg, &pci_mmcfg_list, list)
+ list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list)
if (cfg->segment == segment &&
cfg->start_bus <= bus && bus <= cfg->end_bus)
return cfg;
@@ -364,8 +377,8 @@ static void __init pci_mmcfg_insert_resources(void)
pci_mmcfg_resources_inserted = 1;
}
-static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
- void *data)
+static acpi_status __devinit check_mcfg_resource(struct acpi_resource *res,
+ void *data)
{
struct resource *mcfg_res = data;
struct acpi_resource_address64 address;
@@ -401,8 +414,8 @@ static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
return AE_OK;
}
-static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
- void *context, void **rv)
+static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
{
struct resource *mcfg_res = context;
@@ -415,7 +428,7 @@ static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
return AE_OK;
}
-static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
+static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
{
struct resource mcfg_res;
@@ -434,8 +447,9 @@ static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
-static int __init is_mmconf_reserved(check_reserved_t is_reserved,
- struct pci_mmcfg_region *cfg, int with_e820)
+static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
+ struct pci_mmcfg_region *cfg,
+ int with_e820)
{
u64 addr = cfg->res.start;
u64 size = resource_size(&cfg->res);
@@ -474,39 +488,38 @@ static int __init is_mmconf_reserved(check_reserved_t is_reserved,
return valid;
}
+static int __devinit pci_mmcfg_check_reserved(struct pci_mmcfg_region *cfg,
+ int early)
+{
+ if (!early && !acpi_disabled) {
+ if (is_mmconf_reserved(is_acpi_reserved, cfg, 0))
+ return 1;
+ else
+ printk(KERN_ERR FW_BUG PREFIX
+ "MMCONFIG at %pR not reserved in "
+ "ACPI motherboard resources\n",
+ &cfg->res);
+ }
+
+ /* Don't try to do this check unless configuration
+ type 1 is available. how about type 2 ?*/
+ if (raw_pci_ops)
+ return is_mmconf_reserved(e820_all_mapped, cfg, 1);
+
+ return 0;
+}
+
static void __init pci_mmcfg_reject_broken(int early)
{
struct pci_mmcfg_region *cfg;
list_for_each_entry(cfg, &pci_mmcfg_list, list) {
- int valid = 0;
-
- if (!early && !acpi_disabled) {
- valid = is_mmconf_reserved(is_acpi_reserved, cfg, 0);
-
- if (valid)
- continue;
- else
- printk(KERN_ERR FW_BUG PREFIX
- "MMCONFIG at %pR not reserved in "
- "ACPI motherboard resources\n",
- &cfg->res);
+ if (pci_mmcfg_check_reserved(cfg, early) == 0) {
+ printk(KERN_INFO PREFIX "not using MMCONFIG\n");
+ free_all_mmcfg();
+ return;
}
-
- /* Don't try to do this check unless configuration
- type 1 is available. how about type 2 ?*/
- if (raw_pci_ops)
- valid = is_mmconf_reserved(e820_all_mapped, cfg, 1);
-
- if (!valid)
- goto reject;
}
-
- return;
-
-reject:
- printk(KERN_INFO PREFIX "not using MMCONFIG\n");
- free_all_mmcfg();
}
static int __initdata known_bridge;
@@ -665,3 +678,92 @@ static int __init pci_mmcfg_late_insert_resources(void)
* with other system resources.
*/
late_initcall(pci_mmcfg_late_insert_resources);
+
+static DEFINE_MUTEX(pci_mmcfg_lock);
+
+/* Add MMCFG information for hot-added host bridges at runtime */
+int __devinit pci_mmconfig_insert(int segment, int start, int end, u64 addr)
+{
+ int rc;
+ struct pci_mmcfg_region *cfg = NULL;
+
+ if (addr == 0 || segment < 0 || segment > USHRT_MAX ||
+ start < 0 || start > 255 || end < start || end > 255)
+ return -EINVAL;
+
+ mutex_lock(&pci_mmcfg_lock);
+ cfg = pci_mmconfig_lookup(segment, start);
+ if (cfg) {
+ if (cfg->start_bus == start && cfg->end_bus == end &&
+ cfg->address == addr) {
+ rc = -EEXIST;
+ } else {
+ rc = -EINVAL;
+ printk(KERN_WARNING PREFIX
+ "MMCONFIG for domain %04x [bus %02x-%02x] "
+ "conflicts with domain %04x [bus %02x-%02x]\n",
+ segment, start, end,
+ cfg->segment, cfg->start_bus, cfg->end_bus);
+ }
+ goto out;
+ }
+
+ cfg = pci_mmconfig_alloc(segment, start, end, addr);
+ if (cfg == NULL) {
+ rc = -ENOMEM;
+ } else if (!pci_mmcfg_check_reserved(cfg, 0)) {
+ rc = -EINVAL;
+ printk(KERN_WARNING PREFIX
+ "MMCONFIG for domain %04x [bus %02x-%02x] "
+ "isn't reserved\n", segment, start, end);
+ } else if (insert_resource(&iomem_resource, &cfg->res)) {
+ rc = -EBUSY;
+ printk(KERN_WARNING PREFIX
+ "failed to insert resource for domain "
+ "%04x [bus %02x-%02x]\n", segment, start, end);
+ } else if (pci_mmcfg_arch_map(cfg)) {
+ rc = -EBUSY;
+ printk(KERN_WARNING PREFIX
+ "failed to map resource for domain "
+ "%04x [bus %02x-%02x]\n", segment, start, end);
+ } else {
+ list_add_sorted(cfg);
+ cfg = NULL;
+ rc = 0;
+ }
+
+ if (cfg) {
+ if (cfg->res.parent)
+ release_resource(&cfg->res);
+ kfree(cfg);
+ }
+
+out:
+ mutex_unlock(&pci_mmcfg_lock);
+
+ return rc;
+}
+
+/* Delete MMCFG information at runtime */
+int __devinit pci_mmconfig_delete(int segment, int start, int end, u64 addr)
+{
+ struct pci_mmcfg_region *cfg;
+
+ mutex_lock(&pci_mmcfg_lock);
+ list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
+ if (cfg->segment == segment && cfg->start_bus == start &&
+ cfg->end_bus == end && cfg->address == addr) {
+ list_del_rcu(&cfg->list);
+ synchronize_rcu();
+ pci_mmcfg_arch_unmap(cfg);
+ if (cfg->res.parent)
+ release_resource(&cfg->res);
+ mutex_unlock(&pci_mmcfg_lock);
+ kfree(cfg);
+ return 0;
+ }
+ }
+ mutex_unlock(&pci_mmcfg_lock);
+
+ return -ENOENT;
+}
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 5372e86..c2756de 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -11,6 +11,7 @@
#include <linux/pci.h>
#include <linux/init.h>
+#include <linux/rcupdate.h>
#include <asm/e820.h>
#include <asm/pci_x86.h>
#include <acpi/acpi.h>
@@ -60,9 +61,12 @@ err: *value = -1;
return -EINVAL;
}
+ rcu_read_lock();
base = get_base_addr(seg, bus, devfn);
- if (!base)
+ if (!base) {
+ rcu_read_unlock();
goto err;
+ }
raw_spin_lock_irqsave(&pci_config_lock, flags);
@@ -80,6 +84,7 @@ err: *value = -1;
break;
}
raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+ rcu_read_unlock();
return 0;
}
@@ -93,9 +98,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
if ((bus > 255) || (devfn > 255) || (reg > 4095))
return -EINVAL;
+ rcu_read_lock();
base = get_base_addr(seg, bus, devfn);
- if (!base)
+ if (!base) {
+ rcu_read_unlock();
return -EINVAL;
+ }
raw_spin_lock_irqsave(&pci_config_lock, flags);
@@ -113,6 +121,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
break;
}
raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+ rcu_read_unlock();
return 0;
}
@@ -132,3 +141,18 @@ int __init pci_mmcfg_arch_init(void)
void __init pci_mmcfg_arch_free(void)
{
}
+
+int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
+{
+ return 0;
+}
+
+void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+{
+ unsigned long flags;
+
+ /* Invalidate the cached mmcfg map entry. */
+ raw_spin_lock_irqsave(&pci_config_lock, flags);
+ mmcfg_last_accessed_device = 0;
+ raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+}
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 915a493..6822dd6 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -9,6 +9,7 @@
#include <linux/init.h>
#include <linux/acpi.h>
#include <linux/bitmap.h>
+#include <linux/rcupdate.h>
#include <asm/e820.h>
#include <asm/pci_x86.h>
@@ -34,9 +35,12 @@ err: *value = -1;
return -EINVAL;
}
+ rcu_read_lock();
addr = pci_dev_base(seg, bus, devfn);
- if (!addr)
+ if (!addr) {
+ rcu_read_unlock();
goto err;
+ }
switch (len) {
case 1:
@@ -49,6 +53,7 @@ err: *value = -1;
*value = mmio_config_readl(addr + reg);
break;
}
+ rcu_read_unlock();
return 0;
}
@@ -62,9 +67,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
return -EINVAL;
+ rcu_read_lock();
addr = pci_dev_base(seg, bus, devfn);
- if (!addr)
+ if (!addr) {
+ rcu_read_unlock();
return -EINVAL;
+ }
switch (len) {
case 1:
@@ -77,6 +85,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
mmio_config_writel(addr + reg, value);
break;
}
+ rcu_read_unlock();
return 0;
}
@@ -86,7 +95,7 @@ static const struct pci_raw_ops pci_mmcfg = {
.write = pci_mmcfg_write,
};
-static void __iomem * __init mcfg_ioremap(struct pci_mmcfg_region *cfg)
+static void __iomem * __devinit mcfg_ioremap(struct pci_mmcfg_region *cfg)
{
void __iomem *addr;
u64 start, size;
@@ -129,3 +138,23 @@ void __init pci_mmcfg_arch_free(void)
}
}
}
+
+int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
+{
+ cfg->virt = mcfg_ioremap(cfg);
+ if (!cfg->virt) {
+ printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
+ &cfg->res);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+{
+ if (cfg && cfg->virt) {
+ iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus));
+ cfg->virt = NULL;
+ }
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-06 2:59 [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64 Taku Izumi
` (2 preceding siblings ...)
2012-04-08 17:12 ` [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug Jiang Liu
@ 2012-04-08 17:12 ` Jiang Liu
2012-04-08 19:19 ` Yinghai Lu
` (2 more replies)
3 siblings, 3 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-08 17:12 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Bjorn Helgaas
Cc: Jiang Liu, Jiang Liu, Keping Chen, linux-kernel, linux-pci
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);
+ 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) { }
--
1.7.5.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug
2012-04-08 17:12 ` [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug Jiang Liu
@ 2012-04-08 19:12 ` Yinghai Lu
0 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-04-08 19:12 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
On Sun, Apr 8, 2012 at 10:12 AM, Jiang Liu <liuj97@gmail.com> wrote:
> This patch introduces two new interfaces, pci_mmconfig_insert() and
> pci_mmconfig_delete(), to support PCI host bridge hotplug on x86 platforms.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
> arch/x86/include/asm/pci_x86.h | 4 +
> arch/x86/pci/mmconfig-shared.c | 186 +++++++++++++++++++++++++++++++---------
> arch/x86/pci/mmconfig_32.c | 28 ++++++-
> arch/x86/pci/mmconfig_64.c | 35 +++++++-
> 4 files changed, 206 insertions(+), 47 deletions(-)
this patch is way too big. at least could split it to five patches.
>
> diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
> index b3a5317..ba8570c 100644
> --- a/arch/x86/include/asm/pci_x86.h
> +++ b/arch/x86/include/asm/pci_x86.h
> @@ -135,6 +135,10 @@ struct pci_mmcfg_region {
>
> extern int __init pci_mmcfg_arch_init(void);
> extern void __init pci_mmcfg_arch_free(void);
> +extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
> +extern void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
> +extern int __devinit pci_mmconfig_insert(int seg, int start, int end, u64 addr);
> +extern int __devinit pci_mmconfig_delete(int seg, int start, int end, u64 addr);
> extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
>
> extern struct list_head pci_mmcfg_list;
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 301e325..dda9470 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -17,6 +17,8 @@
> #include <linux/bitmap.h>
> #include <linux/dmi.h>
> #include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/rculist.h>
> #include <asm/e820.h>
> #include <asm/pci_x86.h>
> #include <asm/acpi.h>
> @@ -45,24 +47,25 @@ static __init void free_all_mmcfg(void)
> pci_mmconfig_remove(cfg);
> }
>
> -static __init void list_add_sorted(struct pci_mmcfg_region *new)
> +static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
> {
> struct pci_mmcfg_region *cfg;
>
> /* keep list sorted by segment and starting bus number */
> - list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
> if (cfg->segment > new->segment ||
> (cfg->segment == new->segment &&
> cfg->start_bus >= new->start_bus)) {
> - list_add_tail(&new->list, &cfg->list);
> + list_add_tail_rcu(&new->list, &cfg->list);
> return;
> }
> }
> - list_add_tail(&new->list, &pci_mmcfg_list);
> + list_add_tail_rcu(&new->list, &pci_mmcfg_list);
> }
>
> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> - int end, u64 addr)
> +static __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
> + int start,
> + int end, u64 addr)
> {
> struct pci_mmcfg_region *new;
> struct resource *res;
> @@ -79,8 +82,6 @@ static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> new->start_bus = start;
> new->end_bus = end;
>
> - list_add_sorted(new);
> -
> res = &new->res;
> res->start = addr + PCI_MMCFG_BUS_OFFSET(start);
> res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1;
> @@ -96,11 +97,23 @@ static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> return new;
> }
>
> +static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> + int end, u64 addr)
> +{
> + struct pci_mmcfg_region *new;
> +
> + new = pci_mmconfig_alloc(segment, start, end, addr);
> + if (new)
> + list_add_sorted(new);
> +
> + return new;
> +}
> +
above two change could be in separated patch.
> struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
> {
> struct pci_mmcfg_region *cfg;
>
> - list_for_each_entry(cfg, &pci_mmcfg_list, list)
> + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list)
> if (cfg->segment == segment &&
> cfg->start_bus <= bus && bus <= cfg->end_bus)
> return cfg;
> @@ -364,8 +377,8 @@ static void __init pci_mmcfg_insert_resources(void)
> pci_mmcfg_resources_inserted = 1;
> }
>
> -static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
> - void *data)
> +static acpi_status __devinit check_mcfg_resource(struct acpi_resource *res,
> + void *data)
> {
> struct resource *mcfg_res = data;
> struct acpi_resource_address64 address;
> @@ -401,8 +414,8 @@ static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
> return AE_OK;
> }
>
> -static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
> - void *context, void **rv)
> +static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
> + void *context, void **rv)
> {
> struct resource *mcfg_res = context;
>
> @@ -415,7 +428,7 @@ static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
> return AE_OK;
> }
>
> -static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> +static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> {
> struct resource mcfg_res;
>
> @@ -434,8 +447,9 @@ static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
>
> typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
>
> -static int __init is_mmconf_reserved(check_reserved_t is_reserved,
> - struct pci_mmcfg_region *cfg, int with_e820)
> +static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
> + struct pci_mmcfg_region *cfg,
> + int with_e820)
> {
> u64 addr = cfg->res.start;
> u64 size = resource_size(&cfg->res);
> @@ -474,39 +488,38 @@ static int __init is_mmconf_reserved(check_reserved_t is_reserved,
> return valid;
> }
>
> +static int __devinit pci_mmcfg_check_reserved(struct pci_mmcfg_region *cfg,
> + int early)
> +{
> + if (!early && !acpi_disabled) {
> + if (is_mmconf_reserved(is_acpi_reserved, cfg, 0))
> + return 1;
> + else
> + printk(KERN_ERR FW_BUG PREFIX
> + "MMCONFIG at %pR not reserved in "
> + "ACPI motherboard resources\n",
> + &cfg->res);
> + }
> +
> + /* Don't try to do this check unless configuration
> + type 1 is available. how about type 2 ?*/
> + if (raw_pci_ops)
> + return is_mmconf_reserved(e820_all_mapped, cfg, 1);
> +
> + return 0;
> +}
> +
> static void __init pci_mmcfg_reject_broken(int early)
> {
> struct pci_mmcfg_region *cfg;
>
> list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> - int valid = 0;
> -
> - if (!early && !acpi_disabled) {
> - valid = is_mmconf_reserved(is_acpi_reserved, cfg, 0);
> -
> - if (valid)
> - continue;
> - else
> - printk(KERN_ERR FW_BUG PREFIX
> - "MMCONFIG at %pR not reserved in "
> - "ACPI motherboard resources\n",
> - &cfg->res);
> + if (pci_mmcfg_check_reserved(cfg, early) == 0) {
> + printk(KERN_INFO PREFIX "not using MMCONFIG\n");
> + free_all_mmcfg();
> + return;
> }
> -
> - /* Don't try to do this check unless configuration
> - type 1 is available. how about type 2 ?*/
> - if (raw_pci_ops)
> - valid = is_mmconf_reserved(e820_all_mapped, cfg, 1);
> -
> - if (!valid)
> - goto reject;
> }
> -
> - return;
> -
> -reject:
> - printk(KERN_INFO PREFIX "not using MMCONFIG\n");
> - free_all_mmcfg();
> }
>
> static int __initdata known_bridge;
above two changes could be in separated patch.
> @@ -665,3 +678,92 @@ static int __init pci_mmcfg_late_insert_resources(void)
> * with other system resources.
> */
> late_initcall(pci_mmcfg_late_insert_resources);
> +
> +static DEFINE_MUTEX(pci_mmcfg_lock);
> +
> +/* Add MMCFG information for hot-added host bridges at runtime */
> +int __devinit pci_mmconfig_insert(int segment, int start, int end, u64 addr)
> +{
> + int rc;
> + struct pci_mmcfg_region *cfg = NULL;
> +
> + if (addr == 0 || segment < 0 || segment > USHRT_MAX ||
> + start < 0 || start > 255 || end < start || end > 255)
> + return -EINVAL;
> +
> + mutex_lock(&pci_mmcfg_lock);
> + cfg = pci_mmconfig_lookup(segment, start);
> + if (cfg) {
> + if (cfg->start_bus == start && cfg->end_bus == end &&
> + cfg->address == addr) {
> + rc = -EEXIST;
> + } else {
> + rc = -EINVAL;
> + printk(KERN_WARNING PREFIX
> + "MMCONFIG for domain %04x [bus %02x-%02x] "
> + "conflicts with domain %04x [bus %02x-%02x]\n",
> + segment, start, end,
> + cfg->segment, cfg->start_bus, cfg->end_bus);
> + }
> + goto out;
> + }
> +
> + cfg = pci_mmconfig_alloc(segment, start, end, addr);
> + if (cfg == NULL) {
> + rc = -ENOMEM;
> + } else if (!pci_mmcfg_check_reserved(cfg, 0)) {
> + rc = -EINVAL;
> + printk(KERN_WARNING PREFIX
> + "MMCONFIG for domain %04x [bus %02x-%02x] "
> + "isn't reserved\n", segment, start, end);
> + } else if (insert_resource(&iomem_resource, &cfg->res)) {
> + rc = -EBUSY;
> + printk(KERN_WARNING PREFIX
> + "failed to insert resource for domain "
> + "%04x [bus %02x-%02x]\n", segment, start, end);
> + } else if (pci_mmcfg_arch_map(cfg)) {
> + rc = -EBUSY;
> + printk(KERN_WARNING PREFIX
> + "failed to map resource for domain "
> + "%04x [bus %02x-%02x]\n", segment, start, end);
> + } else {
> + list_add_sorted(cfg);
> + cfg = NULL;
> + rc = 0;
> + }
> +
> + if (cfg) {
> + if (cfg->res.parent)
> + release_resource(&cfg->res);
> + kfree(cfg);
> + }
> +
> +out:
> + mutex_unlock(&pci_mmcfg_lock);
> +
> + return rc;
> +}
> +
> +/* Delete MMCFG information at runtime */
> +int __devinit pci_mmconfig_delete(int segment, int start, int end, u64 addr)
> +{
> + struct pci_mmcfg_region *cfg;
> +
> + mutex_lock(&pci_mmcfg_lock);
> + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
> + if (cfg->segment == segment && cfg->start_bus == start &&
> + cfg->end_bus == end && cfg->address == addr) {
> + list_del_rcu(&cfg->list);
> + synchronize_rcu();
> + pci_mmcfg_arch_unmap(cfg);
> + if (cfg->res.parent)
> + release_resource(&cfg->res);
> + mutex_unlock(&pci_mmcfg_lock);
> + kfree(cfg);
> + return 0;
> + }
> + }
> + mutex_unlock(&pci_mmcfg_lock);
> +
> + return -ENOENT;
> +}
new added _insert/_remove could be in another patch.
> diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
> index 5372e86..c2756de 100644
> --- a/arch/x86/pci/mmconfig_32.c
> +++ b/arch/x86/pci/mmconfig_32.c
> @@ -11,6 +11,7 @@
>
> #include <linux/pci.h>
> #include <linux/init.h>
> +#include <linux/rcupdate.h>
> #include <asm/e820.h>
> #include <asm/pci_x86.h>
> #include <acpi/acpi.h>
> @@ -60,9 +61,12 @@ err: *value = -1;
> return -EINVAL;
> }
>
> + rcu_read_lock();
> base = get_base_addr(seg, bus, devfn);
> - if (!base)
> + if (!base) {
> + rcu_read_unlock();
> goto err;
> + }
>
> raw_spin_lock_irqsave(&pci_config_lock, flags);
>
> @@ -80,6 +84,7 @@ err: *value = -1;
> break;
> }
> raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> + rcu_read_unlock();
>
> return 0;
> }
> @@ -93,9 +98,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> if ((bus > 255) || (devfn > 255) || (reg > 4095))
> return -EINVAL;
>
> + rcu_read_lock();
> base = get_base_addr(seg, bus, devfn);
> - if (!base)
> + if (!base) {
> + rcu_read_unlock();
> return -EINVAL;
> + }
>
> raw_spin_lock_irqsave(&pci_config_lock, flags);
>
> @@ -113,6 +121,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> break;
> }
> raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> + rcu_read_unlock();
>
> return 0;
> }
> @@ -132,3 +141,18 @@ int __init pci_mmcfg_arch_init(void)
> void __init pci_mmcfg_arch_free(void)
> {
> }
> +
> +int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
> +{
> + return 0;
> +}
> +
> +void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
> +{
> + unsigned long flags;
> +
> + /* Invalidate the cached mmcfg map entry. */
> + raw_spin_lock_irqsave(&pci_config_lock, flags);
> + mmcfg_last_accessed_device = 0;
> + raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> +}
> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
> index 915a493..6822dd6 100644
> --- a/arch/x86/pci/mmconfig_64.c
> +++ b/arch/x86/pci/mmconfig_64.c
> @@ -9,6 +9,7 @@
> #include <linux/init.h>
> #include <linux/acpi.h>
> #include <linux/bitmap.h>
> +#include <linux/rcupdate.h>
> #include <asm/e820.h>
> #include <asm/pci_x86.h>
>
> @@ -34,9 +35,12 @@ err: *value = -1;
> return -EINVAL;
> }
>
> + rcu_read_lock();
> addr = pci_dev_base(seg, bus, devfn);
> - if (!addr)
> + if (!addr) {
> + rcu_read_unlock();
> goto err;
> + }
>
> switch (len) {
> case 1:
> @@ -49,6 +53,7 @@ err: *value = -1;
> *value = mmio_config_readl(addr + reg);
> break;
> }
> + rcu_read_unlock();
>
> return 0;
> }
> @@ -62,9 +67,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
> return -EINVAL;
>
> + rcu_read_lock();
> addr = pci_dev_base(seg, bus, devfn);
> - if (!addr)
> + if (!addr) {
> + rcu_read_unlock();
> return -EINVAL;
> + }
>
> switch (len) {
> case 1:
> @@ -77,6 +85,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> mmio_config_writel(addr + reg, value);
> break;
> }
> + rcu_read_unlock();
>
> return 0;
> }
> @@ -86,7 +95,7 @@ static const struct pci_raw_ops pci_mmcfg = {
> .write = pci_mmcfg_write,
> };
>
> -static void __iomem * __init mcfg_ioremap(struct pci_mmcfg_region *cfg)
> +static void __iomem * __devinit mcfg_ioremap(struct pci_mmcfg_region *cfg)
> {
> void __iomem *addr;
> u64 start, size;
> @@ -129,3 +138,23 @@ void __init pci_mmcfg_arch_free(void)
> }
> }
> }
> +
> +int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
> +{
> + cfg->virt = mcfg_ioremap(cfg);
> + if (!cfg->virt) {
> + printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
> + &cfg->res);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
> +{
> + if (cfg && cfg->virt) {
> + iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus));
> + cfg->virt = NULL;
> + }
> +}
another one.
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
@ 2012-04-08 19:19 ` Yinghai Lu
2012-04-09 3:43 ` Jiang Liu
2012-04-09 14:37 ` Jiang Liu
2012-04-09 11:43 ` Kenji Kaneshige
2012-05-02 23:55 ` Bjorn Helgaas
2 siblings, 2 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-04-08 19:19 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
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
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 19:19 ` Yinghai Lu
@ 2012-04-09 3:43 ` Jiang Liu
2012-04-09 14:37 ` Jiang Liu
1 sibling, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-09 3:43 UTC (permalink / raw)
To: Yinghai Lu
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Bjorn Helgaas,
Keping Chen, linux-kernel, linux-pci
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
>>
>
> .
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-08 19:19 ` Yinghai Lu
@ 2012-04-09 11:43 ` Kenji Kaneshige
2012-04-09 16:02 ` Jiang Liu
2012-05-02 23:55 ` Bjorn Helgaas
2 siblings, 1 reply; 26+ messages in thread
From: Kenji Kaneshige @ 2012-04-09 11:43 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Yinghai Lu, Bjorn Helgaas, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 19:19 ` Yinghai Lu
2012-04-09 3:43 ` Jiang Liu
@ 2012-04-09 14:37 ` Jiang Liu
2012-04-09 15:11 ` Yinghai Lu
2012-04-09 20:48 ` Yinghai Lu
1 sibling, 2 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-09 14:37 UTC (permalink / raw)
To: Yinghai Lu
Cc: Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
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
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 14:37 ` Jiang Liu
@ 2012-04-09 15:11 ` Yinghai Lu
2012-04-09 20:48 ` Yinghai Lu
1 sibling, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-04-09 15:11 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 11:43 ` Kenji Kaneshige
@ 2012-04-09 16:02 ` Jiang Liu
2012-04-10 10:32 ` Kenji Kaneshige
0 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-04-09 16:02 UTC (permalink / raw)
To: Kenji Kaneshige
Cc: Taku Izumi, Yinghai Lu, Bjorn Helgaas, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 14:37 ` Jiang Liu
2012-04-09 15:11 ` Yinghai Lu
@ 2012-04-09 20:48 ` Yinghai Lu
1 sibling, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-04-09 20:48 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]
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
[-- Attachment #2: jiang_mmconf_6_v3.patch --]
[-- Type: application/octet-stream, Size: 9448 bytes --]
From: Jiang Liu <liuj97@gmail.com>
To: Taku Izumi <izumi.taku@jp.fujitsu.com>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Yinghai Lu <yinghai@kernel.org>
Cc: Jiang Liu <jiang.liu@huawei.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Len Brown <lenb@kernel.org>,
Jiang Liu <liuj97@gmail.com>,
Keping Chen <chenkeping@huawei.com>,
x86@vger.kernel.org,
linux-pci@vger.kernel.org,
linux-acpi@vger.kernel.org
Subject: [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
Date: Tue, 10 Apr 2012 00:22:48 +0800
Message-Id: <1333988568-11282-7-git-send-email-jiang.liu@huawei.com>
This patch enhances pci_root driver to update MMCFG information when
hot-plugging PCI root bridges on x86 platforms.
-v3: add mmconf_added to simply free path, also make pci_mmconfig_insert()
to process extra exist case --- By Yinghai
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/include/asm/pci_x86.h | 4 +--
arch/x86/pci/acpi.c | 47 +++++++++++++++++++++++++++++++++++++++++
arch/x86/pci/mmconfig-shared.c | 17 ++++++--------
arch/x86/pci/mmconfig_32.c | 2 -
arch/x86/pci/mmconfig_64.c | 2 -
drivers/acpi/pci_root.c | 17 ++++++++++++++
include/acpi/acnames.h | 1
include/acpi/acpi_bus.h | 1
include/linux/pci-acpi.h | 3 ++
9 files changed, 81 insertions(+), 13 deletions(-)
Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -488,6 +488,53 @@ struct pci_bus * __devinit pci_acpi_scan
return bus;
}
+int __devinit arch_acpi_pci_root_add(struct acpi_pci_root *root)
+{
+ int result = 0;
+ unsigned long long base_addr = 0;
+
+ /* MMCONFIG disabled */
+ if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
+ return 0;
+
+ /*
+ * Try to insert MMCFG information for host bridges with _CBA method
+ */
+ acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
+ NULL, &base_addr);
+
+ result = pci_mmconfig_insert(root->segment,
+ root->secondary.start,
+ root->secondary.end,
+ base_addr);
+ if (!result)
+ root->mmconf_added = true;
+
+ if (result == -EEXIST)
+ result = 0;
+
+ if (result)
+ printk(KERN_ERR
+ "can't add MMCFG information for Bus %04x:%02x\n",
+ root->segment, (unsigned int)root->secondary.start);
+
+ /* still could use conf1 with it */
+ if (!root->segment)
+ result = 0;
+
+ return result;
+}
+
+int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
+{
+ if (!root->mmconf_added)
+ return 0;
+
+ return pci_mmconfig_delete(root->segment,
+ root->secondary.start,
+ root->secondary.end);
+}
+
int __init pci_acpi_init(void)
{
struct pci_dev *dev = NULL;
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/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,11 @@ static int __devinit acpi_pci_root_add(s
strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
device->driver_data = root;
+ if (arch_acpi_pci_root_add(root)) {
+ 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 +644,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;
@@ -686,6 +702,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;
Index: linux-2.6/include/acpi/acnames.h
===================================================================
--- linux-2.6.orig/include/acpi/acnames.h
+++ linux-2.6/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 */
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
--- linux-2.6.orig/include/linux/pci-acpi.h
+++ linux-2.6/include/linux/pci-acpi.h
@@ -18,6 +18,9 @@ extern acpi_status pci_acpi_add_pm_notif
struct pci_dev *pci_dev);
extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
+int arch_acpi_pci_root_add(struct acpi_pci_root *root);
+int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
+
static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
{
struct pci_bus *pbus = pdev->bus;
Index: linux-2.6/arch/x86/include/asm/pci_x86.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/pci_x86.h
+++ linux-2.6/arch/x86/include/asm/pci_x86.h
@@ -136,9 +136,9 @@ struct pci_mmcfg_region {
extern int __init pci_mmcfg_arch_init(void);
extern void __init pci_mmcfg_arch_free(void);
extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
-extern void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
+extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
extern int __devinit pci_mmconfig_insert(int seg, int start, int end, u64 addr);
-extern int __devinit pci_mmconfig_delete(int seg, int start, int end, u64 addr);
+extern int pci_mmconfig_delete(int seg, int start, int end);
extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
extern struct list_head pci_mmcfg_list;
Index: linux-2.6/arch/x86/pci/mmconfig-shared.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/mmconfig-shared.c
+++ linux-2.6/arch/x86/pci/mmconfig-shared.c
@@ -684,21 +684,19 @@ static DEFINE_MUTEX(pci_mmcfg_lock);
/* Add MMCFG information for hot-added host bridges at runtime */
int __devinit pci_mmconfig_insert(int segment, int start, int end, u64 addr)
{
- int rc;
+ int rc = -EINVAL;
struct pci_mmcfg_region *cfg = NULL;
- if (addr == 0 || segment < 0 || segment > USHRT_MAX ||
+ if (segment < 0 || segment > USHRT_MAX ||
start < 0 || start > 255 || end < start || end > 255)
- return -EINVAL;
+ return rc;
mutex_lock(&pci_mmcfg_lock);
cfg = pci_mmconfig_lookup(segment, start);
if (cfg) {
- if (cfg->start_bus == start && cfg->end_bus == end &&
- cfg->address == addr) {
+ if (cfg->start_bus <= start && cfg->end_bus >= end) {
rc = -EEXIST;
} else {
- rc = -EINVAL;
printk(KERN_WARNING PREFIX
"MMCONFIG for domain %04x [bus %02x-%02x] "
"conflicts with domain %04x [bus %02x-%02x]\n",
@@ -707,12 +705,13 @@ int __devinit pci_mmconfig_insert(int se
}
goto out;
}
+ if (!addr)
+ goto out;
cfg = pci_mmconfig_alloc(segment, start, end, addr);
if (cfg == NULL) {
rc = -ENOMEM;
} else if (!pci_mmcfg_check_reserved(cfg, 0)) {
- rc = -EINVAL;
printk(KERN_WARNING PREFIX
"MMCONFIG for domain %04x [bus %02x-%02x] "
"isn't reserved\n", segment, start, end);
@@ -745,14 +744,14 @@ out:
}
/* Delete MMCFG information at runtime */
-int __devinit pci_mmconfig_delete(int segment, int start, int end, u64 addr)
+int pci_mmconfig_delete(int segment, int start, int end)
{
struct pci_mmcfg_region *cfg;
mutex_lock(&pci_mmcfg_lock);
list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
if (cfg->segment == segment && cfg->start_bus == start &&
- cfg->end_bus == end && cfg->address == addr) {
+ cfg->end_bus == end) {
list_del_rcu(&cfg->list);
synchronize_rcu();
pci_mmcfg_arch_unmap(cfg);
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -371,6 +371,7 @@ struct acpi_pci_root {
u32 osc_support_set; /* _OSC state of support bits */
u32 osc_control_set; /* _OSC state of control bits */
bool hot_added;
+ bool mmconf_added;
};
/* helper */
Index: linux-2.6/arch/x86/pci/mmconfig_64.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/mmconfig_64.c
+++ linux-2.6/arch/x86/pci/mmconfig_64.c
@@ -151,7 +151,7 @@ int __devinit pci_mmcfg_arch_map(struct
return 0;
}
-void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
{
if (cfg && cfg->virt) {
iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus));
Index: linux-2.6/arch/x86/pci/mmconfig_32.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/mmconfig_32.c
+++ linux-2.6/arch/x86/pci/mmconfig_32.c
@@ -147,7 +147,7 @@ int __devinit pci_mmcfg_arch_map(struct
return 0;
}
-void __devinit pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
{
unsigned long flags;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 16:02 ` Jiang Liu
@ 2012-04-10 10:32 ` Kenji Kaneshige
2012-04-10 15:47 ` Yinghai Lu
2012-04-10 16:01 ` Jiang Liu
0 siblings, 2 replies; 26+ messages in thread
From: Kenji Kaneshige @ 2012-04-10 10:32 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Yinghai Lu, Bjorn Helgaas, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
(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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-10 10:32 ` Kenji Kaneshige
@ 2012-04-10 15:47 ` Yinghai Lu
2012-04-10 16:05 ` Jiang Liu
2012-04-10 16:01 ` Jiang Liu
1 sibling, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-04-10 15:47 UTC (permalink / raw)
To: Kenji Kaneshige
Cc: Jiang Liu, Taku Izumi, Bjorn Helgaas, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-10 10:32 ` Kenji Kaneshige
2012-04-10 15:47 ` Yinghai Lu
@ 2012-04-10 16:01 ` Jiang Liu
1 sibling, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-10 16:01 UTC (permalink / raw)
To: Kenji Kaneshige
Cc: Taku Izumi, Yinghai Lu, Bjorn Helgaas, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-10 15:47 ` Yinghai Lu
@ 2012-04-10 16:05 ` Jiang Liu
0 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-04-10 16:05 UTC (permalink / raw)
To: Yinghai Lu
Cc: Kenji Kaneshige, Taku Izumi, Bjorn Helgaas, Jiang Liu,
Keping Chen, linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-07 15:20 ` Jiang Liu
@ 2012-04-25 17:14 ` Bjorn Helgaas
2012-04-26 2:55 ` Taku Izumi
0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-04-25 17:14 UTC (permalink / raw)
To: Jiang Liu; +Cc: Taku Izumi, Kenji Kaneshige, linux-pci
On Sat, Apr 7, 2012 at 9:20 AM, Jiang Liu <liuj97@gmail.com> wrote:
> On 04/06/2012 07:16 PM, Taku Izumi wrote:
>>
>> This patch introduces the configuration for the base address of
>> the memory mapped configuration space (MMCFG) for hotpluggable PCI
>> hostbridges on x86 and x86_64.
>>
>> The MMCFG for hotpluggable host bridges must be described by using
>> ACPI _CBA method. This patch adds implementation for _CBA method
>> on ACPI pci_root driver and MMCFG manipulating functons for x86 and
>> x86_64.
>>
>> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> ---
>> arch/x86/pci/mmconfig-shared.c | 61 ++++++++++++++++++++++++++++++++++-------
>> drivers/acpi/pci_root.c | 28 ++++++++++++++++++
>> drivers/pci/pci.c | 31 ++++++++++++++++++++
>> include/acpi/acnames.h | 1
>> include/linux/pci.h | 3 ++
>> 5 files changed, 113 insertions(+), 11 deletions(-)
Taku, I'm ignoring this patch for now because I assume that Jiang
Liu's series "[PATCH V4 0/6] PCI, x86: update MMCFG information when
hot-plugging PCI host bridges" includes the same functionality.
If my assumption is wrong, please let me know.
Bjorn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 17:12 ` [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
@ 2012-04-25 17:17 ` Bjorn Helgaas
0 siblings, 0 replies; 26+ messages in thread
From: Bjorn Helgaas @ 2012-04-25 17:17 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Jiang Liu, Keping Chen,
linux-kernel, linux-pci
On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu <liuj97@gmail.com> wrote:
> This patchset enhance pci_root driver to update MMCFG information when
> hot-plugging PCI root bridges. It applies to Yinghai's tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug
>
> The second patch is based on Taku Izumi work with some enhancements to
> correctly handle PCI host bridges without _CBA method.
>
> Jiang Liu (2):
> PCI,x86: introduce new MMCFG interfaces to support PCI host bridge
> hotplug
> PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host
> bridges
>
> arch/x86/include/asm/pci_x86.h | 4 +
> arch/x86/pci/acpi.c | 58 +++++++++++++
> arch/x86/pci/mmconfig-shared.c | 186 +++++++++++++++++++++++++++++++---------
> arch/x86/pci/mmconfig_32.c | 28 ++++++-
> arch/x86/pci/mmconfig_64.c | 35 +++++++-
> drivers/acpi/pci_root.c | 20 +++++
> include/acpi/acnames.h | 1 +
> include/linux/pci-acpi.h | 3 +
> 8 files changed, 288 insertions(+), 47 deletions(-)
Jiang, I assume this series is superceded by "[PATCH V4 0/6] PCI, x86:
update MMCFG information when hot-plugging PCI host bridges". Let me
know if that's incorrect.
Bjorn
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
2012-04-25 17:14 ` Bjorn Helgaas
@ 2012-04-26 2:55 ` Taku Izumi
0 siblings, 0 replies; 26+ messages in thread
From: Taku Izumi @ 2012-04-26 2:55 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Jiang Liu'
Cc: 'Kenji Kaneshige', linux-pci
> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Thursday, April 26, 2012 2:14 AM
> To: Jiang Liu
> Cc: Taku Izumi; Kenji Kaneshige; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64
>
> On Sat, Apr 7, 2012 at 9:20 AM, Jiang Liu <liuj97@gmail.com> wrote:
> > On 04/06/2012 07:16 PM, Taku Izumi wrote:
> >>
> >> This patch introduces the configuration for the base address of
> >> the memory mapped configuration space (MMCFG) for hotpluggable PCI
> >> hostbridges on x86 and x86_64.
> >>
> >> The MMCFG for hotpluggable host bridges must be described by using
> >> ACPI _CBA method. This patch adds implementation for _CBA method
> >> on ACPI pci_root driver and MMCFG manipulating functons for x86 and
> >> x86_64.
> >>
> >> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> >> ---
> >> arch/x86/pci/mmconfig-shared.c | 61 ++++++++++++++++++++++++++++++++++-------
> >> drivers/acpi/pci_root.c | 28 ++++++++++++++++++
> >> drivers/pci/pci.c | 31 ++++++++++++++++++++
> >> include/acpi/acnames.h | 1
> >> include/linux/pci.h | 3 ++
> >> 5 files changed, 113 insertions(+), 11 deletions(-)
>
> Taku, I'm ignoring this patch for now because I assume that Jiang
> Liu's series "[PATCH V4 0/6] PCI, x86: update MMCFG information when
> hot-plugging PCI host bridges" includes the same functionality.
>
> If my assumption is wrong, please let me know.
Hi Bjorn,
Yes, you are right. Please ignore my former patch.
I'm re-creating new patchset based on Liu's series.
Best regarads,
Taku Izumi
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-08 19:19 ` Yinghai Lu
2012-04-09 11:43 ` Kenji Kaneshige
@ 2012-05-02 23:55 ` Bjorn Helgaas
2012-05-03 6:39 ` Jiang Liu
2 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-05-02 23:55 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Jiang Liu, Keping Chen,
linux-kernel, linux-pci, Pearson, Greg
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-05-02 23:55 ` Bjorn Helgaas
@ 2012-05-03 6:39 ` Jiang Liu
0 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-05-03 6:39 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Yinghai Lu, Keping Chen,
linux-kernel, linux-pci, Pearson, Greg
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
>
> .
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2012-05-03 6:42 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-06 2:59 [PATCH] [RFC] PCI, ACPI, x86: MMCFG support for hotpluggable PCI hostbridges on x86, x86_64 Taku Izumi
2012-04-06 6:31 ` Kenji Kaneshige
2012-04-06 11:15 ` Taku Izumi
2012-04-06 11:16 ` Taku Izumi
2012-04-07 15:20 ` Jiang Liu
2012-04-25 17:14 ` Bjorn Helgaas
2012-04-26 2:55 ` Taku Izumi
2012-04-07 15:09 ` Jiang Liu
2012-04-08 17:12 ` [PATCH RFC 0/2] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-25 17:17 ` Bjorn Helgaas
2012-04-08 17:12 ` [PATCH 1/2] PCI,x86: introduce new MMCFG interfaces to support PCI host bridge hotplug Jiang Liu
2012-04-08 19:12 ` Yinghai Lu
2012-04-08 17:12 ` [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-08 19:19 ` Yinghai Lu
2012-04-09 3:43 ` Jiang Liu
2012-04-09 14:37 ` Jiang Liu
2012-04-09 15:11 ` Yinghai Lu
2012-04-09 20:48 ` Yinghai Lu
2012-04-09 11:43 ` Kenji Kaneshige
2012-04-09 16:02 ` Jiang Liu
2012-04-10 10:32 ` Kenji Kaneshige
2012-04-10 15:47 ` Yinghai Lu
2012-04-10 16:05 ` Jiang Liu
2012-04-10 16:01 ` Jiang Liu
2012-05-02 23:55 ` Bjorn Helgaas
2012-05-03 6:39 ` Jiang Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.