Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


liuj97 at gmail

Apr 8, 2012, 10:12 AM

Post #1 of 14 (214 views)
Permalink
[PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges

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 [at] huawei>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


yinghai at kernel

Apr 8, 2012, 12:19 PM

Post #2 of 14 (206 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On Sun, Apr 8, 2012 at 10:12 AM, Jiang Liu <liuj97 [at] gmail> 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 [at] huawei>
> ---
>  arch/x86/pci/acpi.c      |   58 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/pci_root.c  |   20 ++++++++++++++++
>  include/acpi/acnames.h   |    1 +
>  include/linux/pci-acpi.h |    3 ++
>  4 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index da0149d..9184970 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>        return bus;
>  }
>
> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       int result = 0;
> +       acpi_status status;
> +       unsigned long long base_addr;
> +       struct pci_mmcfg_region *cfg;
> +
> +       /*
> +        * Try to insert MMCFG information for host bridges with _CBA method
> +        */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);
> +       if (ACPI_SUCCESS(status)) {
> +               result = pci_mmconfig_insert(root->segment,
> +                                            root->secondary.start,
> +                                            root->secondary.end,
> +                                            base_addr);
> +               /*
> +                * MMCFG information for hot-pluggable host bridges may have
> +                * already been added by __pci_mmcfg_init();
> +                */
> +               if (result == -EEXIST)
> +                       result = 0;
> +       } else if (status == AE_NOT_FOUND) {
> +               /*
> +                * Check whether MMCFG information has been added for
> +                * host bridges without _CBA method.
> +                */
> +               rcu_read_lock();
> +               cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
> +               if (!cfg || cfg->end_bus < root->secondary.end)
> +                       result = -ENODEV;
> +               rcu_read_unlock();
> +       } else
> +               result = -ENODEV;
> +
> +       return result;
> +}
> +
> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       acpi_status status;
> +       unsigned long long base_addr;
> +
> +       /* Remove MMCFG information for host bridges with _CBA method */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);

add one flag in acpi_pci_root about we added mmconf for it before?
like mmconf_added?

> +       if (ACPI_SUCCESS(status))
> +               return pci_mmconfig_delete(root->segment,
> +                                          root->secondary.start,
> +                                          root->secondary.end,
> +                                          base_addr);
> +       else if (status != AE_NOT_FOUND)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
>  int __init pci_acpi_init(void)
>  {
>        struct pci_dev *dev = NULL;
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4a7d575..a62bfa8 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -448,6 +448,16 @@ out:
>  }
>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>
> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
>  static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  {
>        unsigned long long segment, bus;
> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>        strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>        device->driver_data = root;
>
> +       if (arch_acpi_pci_root_add(root)) {
> +               printk(KERN_ERR PREFIX
> +                       "can't add MMCFG information for Bus %04x:%02x\n",
> +                       root->segment, (unsigned int)root->secondary.start);
> +               result = -ENODEV;
> +               goto out_free;
> +       }
> +
>        /*
>         * All supported architectures that use ACPI have support for
>         * PCI domains, so we indicate this in _OSC support capabilities.
> @@ -629,6 +647,7 @@ out_del_root:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>  out_free:
>        kfree(root);
>        return result;
> @@ -679,6 +698,7 @@ out:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>        kfree(root);
>
>        return 0;
> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
> index 38f5088..99bda75 100644
> --- a/include/acpi/acnames.h
> +++ b/include/acpi/acnames.h
> @@ -62,6 +62,7 @@
>  #define METHOD_NAME__AEI        "_AEI"
>  #define METHOD_NAME__PRW        "_PRW"
>  #define METHOD_NAME__SRS        "_SRS"
> +#define METHOD_NAME__CBA       "_CBA"
>
>  /* Method names - these methods must appear at the namespace root */
>
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index ac93634..816b971 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>
>  void acpi_pci_root_rescan(void);
>
> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
> +

don't need extern here.

>  #else
>
>  static inline void acpi_pci_root_rescan(void) { }
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jiang.liu at huawei

Apr 8, 2012, 8:43 PM

Post #3 of 14 (202 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

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 [at] gmail> 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 [at] huawei>
>> ---
>> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/pci_root.c | 20 ++++++++++++++++
>> include/acpi/acnames.h | 1 +
>> include/linux/pci-acpi.h | 3 ++
>> 4 files changed, 82 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index da0149d..9184970 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>> return bus;
>> }
>>
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + int result = 0;
>> + acpi_status status;
>> + unsigned long long base_addr;
>> + struct pci_mmcfg_region *cfg;
>> +
>> + /*
>> + * Try to insert MMCFG information for host bridges with _CBA method
>> + */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>> + if (ACPI_SUCCESS(status)) {
>> + result = pci_mmconfig_insert(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + /*
>> + * MMCFG information for hot-pluggable host bridges may have
>> + * already been added by __pci_mmcfg_init();
>> + */
>> + if (result == -EEXIST)
>> + result = 0;
>> + } else if (status == AE_NOT_FOUND) {
>> + /*
>> + * Check whether MMCFG information has been added for
>> + * host bridges without _CBA method.
>> + */
>> + rcu_read_lock();
>> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
>> + if (!cfg || cfg->end_bus< root->secondary.end)
>> + result = -ENODEV;
>> + rcu_read_unlock();
>> + } else
>> + result = -ENODEV;
>> +
>> + return result;
>> +}
>> +
>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + acpi_status status;
>> + unsigned long long base_addr;
>> +
>> + /* Remove MMCFG information for host bridges with _CBA method */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>
> add one flag in acpi_pci_root about we added mmconf for it before?
> like mmconf_added?
>
>> + if (ACPI_SUCCESS(status))
>> + return pci_mmconfig_delete(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + else if (status != AE_NOT_FOUND)
>> + return -ENODEV;
>> +
>> + return 0;
>> +}
>> +
>> int __init pci_acpi_init(void)
>> {
>> struct pci_dev *dev = NULL;
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 4a7d575..a62bfa8 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -448,6 +448,16 @@ out:
>> }
>> EXPORT_SYMBOL(acpi_pci_osc_control_set);
>>
>> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> unsigned long long segment, bus;
>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>> device->driver_data = root;
>>
>> + if (arch_acpi_pci_root_add(root)) {
>> + printk(KERN_ERR PREFIX
>> + "can't add MMCFG information for Bus %04x:%02x\n",
>> + root->segment, (unsigned int)root->secondary.start);
>> + result = -ENODEV;
>> + goto out_free;
>> + }
>> +
>> /*
>> * All supported architectures that use ACPI have support for
>> * PCI domains, so we indicate this in _OSC support capabilities.
>> @@ -629,6 +647,7 @@ out_del_root:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> out_free:
>> kfree(root);
>> return result;
>> @@ -679,6 +698,7 @@ out:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> kfree(root);
>>
>> return 0;
>> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
>> index 38f5088..99bda75 100644
>> --- a/include/acpi/acnames.h
>> +++ b/include/acpi/acnames.h
>> @@ -62,6 +62,7 @@
>> #define METHOD_NAME__AEI "_AEI"
>> #define METHOD_NAME__PRW "_PRW"
>> #define METHOD_NAME__SRS "_SRS"
>> +#define METHOD_NAME__CBA "_CBA"
>>
>> /* Method names - these methods must appear at the namespace root */
>>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index ac93634..816b971 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>>
>> void acpi_pci_root_rescan(void);
>>
>> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
>> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
>> +
>
> don't need extern here.
>
>> #else
>>
>> static inline void acpi_pci_root_rescan(void) { }
>> --
>> 1.7.5.4
>>
>
> .
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


kaneshige.kenji at jp

Apr 9, 2012, 4:43 AM

Post #4 of 14 (207 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

Your patch looks good to me.

I have some comments.

(2012/04/09 2:12), Jiang Liu wrote:
> This patch enhances pci_root driver to update MMCFG information when
> hot-plugging PCI root bridges on x86 platforms.
>

Do you have the patch that can be applied to Bjorn's pci tree?

<snip.>

> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> + int result = 0;
> + acpi_status status;
> + unsigned long long base_addr;
> + struct pci_mmcfg_region *cfg;
> +
> + /*
> + * Try to insert MMCFG information for host bridges with _CBA method
> + */
> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> + NULL,&base_addr);
> + if (ACPI_SUCCESS(status)) {
> + result = pci_mmconfig_insert(root->segment,
> + root->secondary.start,
> + root->secondary.end,
> + base_addr);
> + /*
> + * MMCFG information for hot-pluggable host bridges may have
> + * already been added by __pci_mmcfg_init();
> + */
> + if (result == -EEXIST)
> + result = 0;

Just for confirmation.
From my interpretation of PCI firmware spec, MCFG doesn't have any entry
for hot-pluggable hostbridge. So I assume this is for the machine that
is not compliant to the spec. Is my understanding same as yours?

<snip.>

> static int __devinit acpi_pci_root_add(struct acpi_device *device)
> {
> unsigned long long segment, bus;
> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
> device->driver_data = root;
>
> + if (arch_acpi_pci_root_add(root)) {
> + printk(KERN_ERR PREFIX
> + "can't add MMCFG information for Bus %04x:%02x\n",
> + root->segment, (unsigned int)root->secondary.start);
> + result = -ENODEV;
> + goto out_free;
> + }

Desn't this break the system that doesn't support MMCONFIG?

In my understanding, arch_acpi_pci_root_add() returns -ENODEV if
mmconfig information is found neither in MCFG table nor _CBA. And
pci root bridge initialization seems to fail arch_acpi_pci_root_add()
returns non-zero value.

Regards,
Kenji Kaneshige
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


liuj97 at gmail

Apr 9, 2012, 7:37 AM

Post #5 of 14 (204 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

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 [at] gmail> 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 [at] huawei>
>> ---
>> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/pci_root.c | 20 ++++++++++++++++
>> include/acpi/acnames.h | 1 +
>> include/linux/pci-acpi.h | 3 ++
>> 4 files changed, 82 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index da0149d..9184970 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>> return bus;
>> }
>>
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + int result = 0;
>> + acpi_status status;
>> + unsigned long long base_addr;
>> + struct pci_mmcfg_region *cfg;
>> +
>> + /*
>> + * Try to insert MMCFG information for host bridges with _CBA method
>> + */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL, &base_addr);
>> + if (ACPI_SUCCESS(status)) {
>> + result = pci_mmconfig_insert(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + /*
>> + * MMCFG information for hot-pluggable host bridges may have
>> + * already been added by __pci_mmcfg_init();
>> + */
>> + if (result == -EEXIST)
>> + result = 0;
>> + } else if (status == AE_NOT_FOUND) {
>> + /*
>> + * Check whether MMCFG information has been added for
>> + * host bridges without _CBA method.
>> + */
>> + rcu_read_lock();
>> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
>> + if (!cfg || cfg->end_bus < root->secondary.end)
>> + result = -ENODEV;
>> + rcu_read_unlock();
>> + } else
>> + result = -ENODEV;
>> +
>> + return result;
>> +}
>> +
>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + acpi_status status;
>> + unsigned long long base_addr;
>> +
>> + /* Remove MMCFG information for host bridges with _CBA method */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL, &base_addr);
>
> add one flag in acpi_pci_root about we added mmconf for it before?
> like mmconf_added?
>
>> + if (ACPI_SUCCESS(status))
>> + return pci_mmconfig_delete(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + else if (status != AE_NOT_FOUND)
>> + return -ENODEV;
>> +
>> + return 0;
>> +}
>> +
>> int __init pci_acpi_init(void)
>> {
>> struct pci_dev *dev = NULL;
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 4a7d575..a62bfa8 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -448,6 +448,16 @@ out:
>> }
>> EXPORT_SYMBOL(acpi_pci_osc_control_set);
>>
>> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> unsigned long long segment, bus;
>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>> device->driver_data = root;
>>
>> + if (arch_acpi_pci_root_add(root)) {
>> + printk(KERN_ERR PREFIX
>> + "can't add MMCFG information for Bus %04x:%02x\n",
>> + root->segment, (unsigned int)root->secondary.start);
>> + result = -ENODEV;
>> + goto out_free;
>> + }
>> +
>> /*
>> * All supported architectures that use ACPI have support for
>> * PCI domains, so we indicate this in _OSC support capabilities.
>> @@ -629,6 +647,7 @@ out_del_root:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> out_free:
>> kfree(root);
>> return result;
>> @@ -679,6 +698,7 @@ out:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> kfree(root);
>>
>> return 0;
>> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
>> index 38f5088..99bda75 100644
>> --- a/include/acpi/acnames.h
>> +++ b/include/acpi/acnames.h
>> @@ -62,6 +62,7 @@
>> #define METHOD_NAME__AEI "_AEI"
>> #define METHOD_NAME__PRW "_PRW"
>> #define METHOD_NAME__SRS "_SRS"
>> +#define METHOD_NAME__CBA "_CBA"
>>
>> /* Method names - these methods must appear at the namespace root */
>>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index ac93634..816b971 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>>
>> void acpi_pci_root_rescan(void);
>>
>> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
>> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
>> +
>
> don't need extern here.
>
>> #else
>>
>> static inline void acpi_pci_root_rescan(void) { }
>> --
>> 1.7.5.4
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


yinghai at kernel

Apr 9, 2012, 8:11 AM

Post #6 of 14 (203 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On Mon, Apr 9, 2012 at 7:37 AM, Jiang Liu <liuj97 [at] gmail> wrote:
> Hi Yinghai,
>
>>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>>> +{
>>> +       acpi_status status;
>>> +       unsigned long long base_addr;
>>> +
>>> +       /* Remove MMCFG information for host bridges with _CBA method */
>>> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>>> +                                      NULL, &base_addr);
>>
>> add one flag in acpi_pci_root about we added mmconf for it before?
>> like mmconf_added?
>>
>
> I feel adding a flag here doesn't help here. For any hot-pluggable PCI host bridge
> with _CBA method available, arch_acpi_pci_root_remove() should delete the MMCFG
> information, no matter it's added by __pci_mmcfg_init() or arch_acpi_pci_root_add().
> So check existence of _CBA seems simpler here.

_remove should not care about _CBA, it should only care about if it
gets added on _add path.

Also like Kenji said, We should let it go through even no MMCONF is
added at that time.
like somehow _CBA does not exis for some root bus or evaluating failing etc.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


liuj97 at gmail

Apr 9, 2012, 9:02 AM

Post #7 of 14 (201 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

Hi Kenji,
Thanks for your careful review and comments.

On 04/09/2012 07:43 PM, Kenji Kaneshige wrote:
> Your patch looks good to me.
>
> I have some comments.
>
> (2012/04/09 2:12), Jiang Liu wrote:
>> This patch enhances pci_root driver to update MMCFG information when
>> hot-plugging PCI root bridges on x86 platforms.
>>
>
> Do you have the patch that can be applied to Bjorn's pci tree?
>
> <snip.>
Will try to generate a version against Bjorn's version. Could you please tell
me the exact git link for that? I haven't pull from Bjorn's tree yet.

>
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + int result = 0;
>> + acpi_status status;
>> + unsigned long long base_addr;
>> + struct pci_mmcfg_region *cfg;
>> +
>> + /*
>> + * Try to insert MMCFG information for host bridges with _CBA method
>> + */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>> + if (ACPI_SUCCESS(status)) {
>> + result = pci_mmconfig_insert(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + /*
>> + * MMCFG information for hot-pluggable host bridges may have
>> + * already been added by __pci_mmcfg_init();
>> + */
>> + if (result == -EEXIST)
>> + result = 0;
>
> Just for confirmation.
> From my interpretation of PCI firmware spec, MCFG doesn't have any entry
> for hot-pluggable hostbridge. So I assume this is for the machine that
> is not compliant to the spec. Is my understanding same as yours?
>
> <snip.>
You are right, it's defined to that way in PCI FW Spec 3.1.
Here I have some concerns about the PCI buses to host all Ubox components
on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare
MMCFG information for those host bridges by MCFG table instead of _CBA method,
though those host bridge will disappear after hot-removing a physical processor.

>
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> unsigned long long segment, bus;
>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>> device->driver_data = root;
>>
>> + if (arch_acpi_pci_root_add(root)) {
>> + printk(KERN_ERR PREFIX
>> + "can't add MMCFG information for Bus %04x:%02x\n",
>> + root->segment, (unsigned int)root->secondary.start);
>> + result = -ENODEV;
>> + goto out_free;
>> + }
>
> Desn't this break the system that doesn't support MMCONFIG?
>
> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if
> mmconfig information is found neither in MCFG table nor _CBA. And
> pci root bridge initialization seems to fail arch_acpi_pci_root_add()
> returns non-zero value.
Good catch, will add following code into arch_acpi_pci_root_add() and
arch_acpi_pci_root_remove() to solve this issue.
---
/* MMCONFIG disabled */
if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;
---


>
> Regards,
> Kenji Kaneshige

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


yinghai at kernel

Apr 9, 2012, 1:48 PM

Post #8 of 14 (209 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On Mon, Apr 9, 2012 at 7:37 AM, Jiang Liu <liuj97 [at] gmail> 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
Attachments: jiang_mmconf_6_v3.patch (9.23 KB)


kaneshige.kenji at jp

Apr 10, 2012, 3:32 AM

Post #9 of 14 (203 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

(2012/04/10 1:02), Jiang Liu wrote:
> Hi Kenji,
> Thanks for your careful review and comments.
>
> On 04/09/2012 07:43 PM, Kenji Kaneshige wrote:
>> Your patch looks good to me.
>>
>> I have some comments.
>>
>> (2012/04/09 2:12), Jiang Liu wrote:
>>> This patch enhances pci_root driver to update MMCFG information when
>>> hot-plugging PCI root bridges on x86 platforms.
>>>
>>
>> Do you have the patch that can be applied to Bjorn's pci tree?
>>
>> <snip.>
> Will try to generate a version against Bjorn's version. Could you please tell
> me the exact git link for that? I haven't pull from Bjorn's tree yet.

As you may know, it was announced _today_ (sorry:).

http://www.spinics.net/lists/linux-pci/msg14626.html

>
>>
>>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>>> +{
>>> + int result = 0;
>>> + acpi_status status;
>>> + unsigned long long base_addr;
>>> + struct pci_mmcfg_region *cfg;
>>> +
>>> + /*
>>> + * Try to insert MMCFG information for host bridges with _CBA method
>>> + */
>>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>>> + NULL,&base_addr);
>>> + if (ACPI_SUCCESS(status)) {
>>> + result = pci_mmconfig_insert(root->segment,
>>> + root->secondary.start,
>>> + root->secondary.end,
>>> + base_addr);
>>> + /*
>>> + * MMCFG information for hot-pluggable host bridges may have
>>> + * already been added by __pci_mmcfg_init();
>>> + */
>>> + if (result == -EEXIST)
>>> + result = 0;
>>
>> Just for confirmation.
>> From my interpretation of PCI firmware spec, MCFG doesn't have any entry
>> for hot-pluggable hostbridge. So I assume this is for the machine that
>> is not compliant to the spec. Is my understanding same as yours?
>>
>> <snip.>
> You are right, it's defined to that way in PCI FW Spec 3.1.
> Here I have some concerns about the PCI buses to host all Ubox components
> on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare
> MMCFG information for those host bridges by MCFG table instead of _CBA method,
> though those host bridge will disappear after hot-removing a physical processor.

Ok, thank you for clarification.

>
>>
>>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>>> {
>>> unsigned long long segment, bus;
>>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>>> device->driver_data = root;
>>>
>>> + if (arch_acpi_pci_root_add(root)) {
>>> + printk(KERN_ERR PREFIX
>>> + "can't add MMCFG information for Bus %04x:%02x\n",
>>> + root->segment, (unsigned int)root->secondary.start);

Additional comment.

This printk message looks strange because arch_acpi_pci_root_add()
is not a mmconfig specific function. So I think this message
should be moved to arch specific code (arch_acpi_pci_root_add()).

>>> + result = -ENODEV;
>>> + goto out_free;
>>> + }
>>
>> Desn't this break the system that doesn't support MMCONFIG?
>>
>> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if
>> mmconfig information is found neither in MCFG table nor _CBA. And
>> pci root bridge initialization seems to fail arch_acpi_pci_root_add()
>> returns non-zero value.
> Good catch, will add following code into arch_acpi_pci_root_add() and
> arch_acpi_pci_root_remove() to solve this issue.
> ---
> /* MMCONFIG disabled */
> if ((pci_probe& PCI_PROBE_MMCONF) == 0)
> return 0;
> ---

My understanding is that PCI_PROBE_MMCONF is set even if the system
doesn't have MCFG table. So I don't think this solves the issue. I
guess this is what Yinghai pointed out on your V2 patch [6/6].

Additionally, I think there is a remaining issue even if we change
this check like below.

if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
return 0;

I think this check has an assumption that system has at least one
MCFG table entry and it has been initialized before
acpi_pci_root_add() is called. I think this doesn't work on the
system that doesn't have MCFG and all the pci root bridge have
_CBA (that is, all host bridges are hot-pluggable and BIOS is
implemented in the way PCI FW spec defines). As a result, MMCONFIG
would never be enabled on such systems. Could you double check this?

Regards,
Kenji Kaneshige
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


yinghai at kernel

Apr 10, 2012, 8:47 AM

Post #10 of 14 (201 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On Tue, Apr 10, 2012 at 3:32 AM, Kenji Kaneshige
<kaneshige.kenji [at] jp> wrote:
> Additionally, I think there is a remaining issue even if we change
> this check like below.
>
>
>        if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
>                return 0;
>
> I think this check has an assumption that system has at least one
> MCFG table entry and it has been initialized before
> acpi_pci_root_add() is called. I think this doesn't work on the
> system that doesn't have MCFG and all the pci root bridge have
> _CBA (that is, all host bridges are hot-pluggable and BIOS is
> implemented in the way PCI FW spec defines). As a result, MMCONFIG
> would never be enabled on such systems. Could you double check this?

You are right.

We can just remove that checking.

But wonder if current x86_64 system support that.
All peer root buses can be physically removed ?

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


liuj97 at gmail

Apr 10, 2012, 9:01 AM

Post #11 of 14 (200 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On 04/10/2012 06:32 PM, Kenji Kaneshige wrote:
> (2012/04/10 1:02), Jiang Liu wrote:
>> Hi Kenji,
>> Thanks for your careful review and comments.
>>
>> On 04/09/2012 07:43 PM, Kenji Kaneshige wrote:
>>> Your patch looks good to me.
>>>
>>> I have some comments.
>>>
>>> (2012/04/09 2:12), Jiang Liu wrote:
>>>> This patch enhances pci_root driver to update MMCFG information when
>>>> hot-plugging PCI root bridges on x86 platforms.
>>>>
>>>
>>> Do you have the patch that can be applied to Bjorn's pci tree?
>>>
>>> <snip.>
>> Will try to generate a version against Bjorn's version. Could you please tell
>> me the exact git link for that? I haven't pull from Bjorn's tree yet.
>
> As you may know, it was announced _today_ (sorry:).
>
> http://www.spinics.net/lists/linux-pci/msg14626.html
>
>>
>>>
>>>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>>>> +{
>>>> + int result = 0;
>>>> + acpi_status status;
>>>> + unsigned long long base_addr;
>>>> + struct pci_mmcfg_region *cfg;
>>>> +
>>>> + /*
>>>> + * Try to insert MMCFG information for host bridges with _CBA method
>>>> + */
>>>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>>>> + NULL,&base_addr);
>>>> + if (ACPI_SUCCESS(status)) {
>>>> + result = pci_mmconfig_insert(root->segment,
>>>> + root->secondary.start,
>>>> + root->secondary.end,
>>>> + base_addr);
>>>> + /*
>>>> + * MMCFG information for hot-pluggable host bridges may have
>>>> + * already been added by __pci_mmcfg_init();
>>>> + */
>>>> + if (result == -EEXIST)
>>>> + result = 0;
>>>
>>> Just for confirmation.
>>> From my interpretation of PCI firmware spec, MCFG doesn't have any entry
>>> for hot-pluggable hostbridge. So I assume this is for the machine that
>>> is not compliant to the spec. Is my understanding same as yours?
>>>
>>> <snip.>
>> You are right, it's defined to that way in PCI FW Spec 3.1.
>> Here I have some concerns about the PCI buses to host all Ubox components
>> on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare
>> MMCFG information for those host bridges by MCFG table instead of _CBA method,
>> though those host bridge will disappear after hot-removing a physical processor.
>
> Ok, thank you for clarification.
>
>>
>>>
>>>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>>>> {
>>>> unsigned long long segment, bus;
>>>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>>>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>>>> device->driver_data = root;
>>>>
>>>> + if (arch_acpi_pci_root_add(root)) {
>>>> + printk(KERN_ERR PREFIX
>>>> + "can't add MMCFG information for Bus %04x:%02x\n",
>>>> + root->segment, (unsigned int)root->secondary.start);
>
> Additional comment.
>
> This printk message looks strange because arch_acpi_pci_root_add()
> is not a mmconfig specific function. So I think this message
> should be moved to arch specific code (arch_acpi_pci_root_add()).
>
>>>> + result = -ENODEV;
>>>> + goto out_free;
>>>> + }
>>>
>>> Desn't this break the system that doesn't support MMCONFIG?
>>>
>>> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if
>>> mmconfig information is found neither in MCFG table nor _CBA. And
>>> pci root bridge initialization seems to fail arch_acpi_pci_root_add()
>>> returns non-zero value.
>> Good catch, will add following code into arch_acpi_pci_root_add() and
>> arch_acpi_pci_root_remove() to solve this issue.
>> ---
>> /* MMCONFIG disabled */
>> if ((pci_probe& PCI_PROBE_MMCONF) == 0)
>> return 0;
>> ---
>
> My understanding is that PCI_PROBE_MMCONF is set even if the system
> doesn't have MCFG table. So I don't think this solves the issue. I
> guess this is what Yinghai pointed out on your V2 patch [6/6].
>
> Additionally, I think there is a remaining issue even if we change
> this check like below.
>
> if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
> return 0;
>
> I think this check has an assumption that system has at least one
> MCFG table entry and it has been initialized before
> acpi_pci_root_add() is called. I think this doesn't work on the
> system that doesn't have MCFG and all the pci root bridge have
> _CBA (that is, all host bridges are hot-pluggable and BIOS is
> implemented in the way PCI FW spec defines). As a result, MMCONFIG
> would never be enabled on such systems. Could you double check this?
Good points. We underestimated the complexity here.
I'm working on a new patch based on Yinghai's v3 patchset and will send
it out soon.
Thanks!

>
> Regards,
> Kenji Kaneshige

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


liuj97 at gmail

Apr 10, 2012, 9:05 AM

Post #12 of 14 (206 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On 04/10/2012 11:47 PM, Yinghai Lu wrote:
> On Tue, Apr 10, 2012 at 3:32 AM, Kenji Kaneshige
> <kaneshige.kenji [at] jp> wrote:
>> Additionally, I think there is a remaining issue even if we change
>> this check like below.
>>
>>
>> if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
>> return 0;
>>
>> I think this check has an assumption that system has at least one
>> MCFG table entry and it has been initialized before
>> acpi_pci_root_add() is called. I think this doesn't work on the
>> system that doesn't have MCFG and all the pci root bridge have
>> _CBA (that is, all host bridges are hot-pluggable and BIOS is
>> implemented in the way PCI FW spec defines). As a result, MMCONFIG
>> would never be enabled on such systems. Could you double check this?
>
> You are right.
>
> We can just remove that checking.
>
> But wonder if current x86_64 system support that.
> All peer root buses can be physically removed ?
It's theoretically possible that BIOS doesn't provide MMCFG or _CBA
for the legacy (non-removable bus 0) host bridge.

>
> Thanks
>
> Yinghai

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bhelgaas at google

May 2, 2012, 4:55 PM

Post #13 of 14 (175 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu <liuj97 [at] gmail> 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 [at] huawei>
> ---
>  arch/x86/pci/acpi.c      |   58 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/pci_root.c  |   20 ++++++++++++++++
>  include/acpi/acnames.h   |    1 +
>  include/linux/pci-acpi.h |    3 ++
>  4 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index da0149d..9184970 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>        return bus;
>  }
>
> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       int result = 0;
> +       acpi_status status;
> +       unsigned long long base_addr;
> +       struct pci_mmcfg_region *cfg;
> +
> +       /*
> +        * Try to insert MMCFG information for host bridges with _CBA method
> +        */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);

I would prefer to have _CBA evaluated in drivers/acpi/pci_root.c, not
in the arch code. It's true that _CBA is probably only used by x86
today, but the spec says nothing about it being arch-dependent, and I
suspect it may be used on arm soon.

> +       if (ACPI_SUCCESS(status)) {
> +               result = pci_mmconfig_insert(root->segment,
> +                                            root->secondary.start,
> +                                            root->secondary.end,
> +                                            base_addr);
> +               /*
> +                * MMCFG information for hot-pluggable host bridges may have
> +                * already been added by __pci_mmcfg_init();
> +                */
> +               if (result == -EEXIST)
> +                       result = 0;
> +       } else if (status == AE_NOT_FOUND) {
> +               /*
> +                * Check whether MMCFG information has been added for
> +                * host bridges without _CBA method.
> +                */
> +               rcu_read_lock();
> +               cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
> +               if (!cfg || cfg->end_bus < root->secondary.end)
> +                       result = -ENODEV;
> +               rcu_read_unlock();
> +       } else
> +               result = -ENODEV;
> +
> +       return result;
> +}
> +
> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       acpi_status status;
> +       unsigned long long base_addr;
> +
> +       /* Remove MMCFG information for host bridges with _CBA method */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);

I think the arch-specific "map MMCONFIG space at this addr" should
return a pointer or something that we can save and use to unmap it on
remove. That way we don't have to evaluate _CBA again.

> +       if (ACPI_SUCCESS(status))
> +               return pci_mmconfig_delete(root->segment,
> +                                          root->secondary.start,
> +                                          root->secondary.end,
> +                                          base_addr);
> +       else if (status != AE_NOT_FOUND)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
>  int __init pci_acpi_init(void)
>  {
>        struct pci_dev *dev = NULL;
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4a7d575..a62bfa8 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -448,6 +448,16 @@ out:
>  }
>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>
> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
>  static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  {
>        unsigned long long segment, bus;
> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>        strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>        device->driver_data = root;
>
> +       if (arch_acpi_pci_root_add(root)) {
> +               printk(KERN_ERR PREFIX
> +                       "can't add MMCFG information for Bus %04x:%02x\n",
> +                       root->segment, (unsigned int)root->secondary.start);
> +               result = -ENODEV;
> +               goto out_free;
> +       }
> +
>        /*
>         * All supported architectures that use ACPI have support for
>         * PCI domains, so we indicate this in _OSC support capabilities.
> @@ -629,6 +647,7 @@ out_del_root:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>  out_free:
>        kfree(root);
>        return result;
> @@ -679,6 +698,7 @@ out:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>        kfree(root);
>
>        return 0;
> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
> index 38f5088..99bda75 100644
> --- a/include/acpi/acnames.h
> +++ b/include/acpi/acnames.h
> @@ -62,6 +62,7 @@
>  #define METHOD_NAME__AEI        "_AEI"
>  #define METHOD_NAME__PRW        "_PRW"
>  #define METHOD_NAME__SRS        "_SRS"
> +#define METHOD_NAME__CBA       "_CBA"
>
>  /* Method names - these methods must appear at the namespace root */
>
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index ac93634..816b971 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>
>  void acpi_pci_root_rescan(void);
>
> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
> +
>  #else
>
>  static inline void acpi_pci_root_rescan(void) { }

I don't know where to put this in the conversation, but I think we
should separate the question of whether we do blind probing from the
question of how we set up MMCONFIG space from MCFG.

We currently parse the entire MCFG in some sort of initcall, but I
think we *could* change the x86 blind probing routines to attempt to
set up MMCONFIG space for the buses they find, just like
acpi_pci_root_add() does it for the ACPI host bridges.

That way we can continue blind probing, but make MMCONFIG init more sensible.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jiang.liu at huawei

May 2, 2012, 11:39 PM

Post #14 of 14 (173 views)
Permalink
Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges [In reply to]

On 2012-5-3 7:55, Bjorn Helgaas wrote:
> On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu<liuj97 [at] gmail> 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 [at] huawei>
>> ---
>> arch/x86/pci/acpi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/pci_root.c | 20 ++++++++++++++++
>> include/acpi/acnames.h | 1 +
>> include/linux/pci-acpi.h | 3 ++
>> 4 files changed, 82 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index da0149d..9184970 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>> return bus;
>> }
>>
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + int result = 0;
>> + acpi_status status;
>> + unsigned long long base_addr;
>> + struct pci_mmcfg_region *cfg;
>> +
>> + /*
>> + * Try to insert MMCFG information for host bridges with _CBA method
>> + */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>
> I would prefer to have _CBA evaluated in drivers/acpi/pci_root.c, not
> in the arch code. It's true that _CBA is probably only used by x86
> today, but the spec says nothing about it being arch-dependent, and I
> suspect it may be used on arm soon.
Good point. I just noticed that IA64 doesn't need MMCFG, but haven't
realized there's still another candidate.

>
>> + if (ACPI_SUCCESS(status)) {
>> + result = pci_mmconfig_insert(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + /*
>> + * MMCFG information for hot-pluggable host bridges may have
>> + * already been added by __pci_mmcfg_init();
>> + */
>> + if (result == -EEXIST)
>> + result = 0;
>> + } else if (status == AE_NOT_FOUND) {
>> + /*
>> + * Check whether MMCFG information has been added for
>> + * host bridges without _CBA method.
>> + */
>> + rcu_read_lock();
>> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
>> + if (!cfg || cfg->end_bus< root->secondary.end)
>> + result = -ENODEV;
>> + rcu_read_unlock();
>> + } else
>> + result = -ENODEV;
>> +
>> + return result;
>> +}
>> +
>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + acpi_status status;
>> + unsigned long long base_addr;
>> +
>> + /* Remove MMCFG information for host bridges with _CBA method */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>
> I think the arch-specific "map MMCONFIG space at this addr" should
> return a pointer or something that we can save and use to unmap it on
> remove. That way we don't have to evaluate _CBA again.
OK, will change to follow that way.

>
>> + if (ACPI_SUCCESS(status))
>> + return pci_mmconfig_delete(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + else if (status != AE_NOT_FOUND)
>> + return -ENODEV;
>> +
>> + return 0;
>> +}
>> +
>> int __init pci_acpi_init(void)
>> {
>> struct pci_dev *dev = NULL;
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 4a7d575..a62bfa8 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -448,6 +448,16 @@ out:
>> }
>> EXPORT_SYMBOL(acpi_pci_osc_control_set);
>>
>> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
>> +{
>> + return 0;
>> +}
>> +
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> unsigned long long segment, bus;
>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>> device->driver_data = root;
>>
>> + if (arch_acpi_pci_root_add(root)) {
>> + printk(KERN_ERR PREFIX
>> + "can't add MMCFG information for Bus %04x:%02x\n",
>> + root->segment, (unsigned int)root->secondary.start);
>> + result = -ENODEV;
>> + goto out_free;
>> + }
>> +
>> /*
>> * All supported architectures that use ACPI have support for
>> * PCI domains, so we indicate this in _OSC support capabilities.
>> @@ -629,6 +647,7 @@ out_del_root:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> out_free:
>> kfree(root);
>> return result;
>> @@ -679,6 +698,7 @@ out:
>> list_del_rcu(&root->node);
>> mutex_unlock(&acpi_pci_root_lock);
>> synchronize_rcu();
>> + arch_acpi_pci_root_remove(root);
>> kfree(root);
>>
>> return 0;
>> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
>> index 38f5088..99bda75 100644
>> --- a/include/acpi/acnames.h
>> +++ b/include/acpi/acnames.h
>> @@ -62,6 +62,7 @@
>> #define METHOD_NAME__AEI "_AEI"
>> #define METHOD_NAME__PRW "_PRW"
>> #define METHOD_NAME__SRS "_SRS"
>> +#define METHOD_NAME__CBA "_CBA"
>>
>> /* Method names - these methods must appear at the namespace root */
>>
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index ac93634..816b971 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>>
>> void acpi_pci_root_rescan(void);
>>
>> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
>> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
>> +
>> #else
>>
>> static inline void acpi_pci_root_rescan(void) { }
>
> I don't know where to put this in the conversation, but I think we
> should separate the question of whether we do blind probing from the
> question of how we set up MMCONFIG space from MCFG.
>
> We currently parse the entire MCFG in some sort of initcall, but I
> think we *could* change the x86 blind probing routines to attempt to
> set up MMCONFIG space for the buses they find, just like
> acpi_pci_root_add() does it for the ACPI host bridges.
>
> That way we can continue blind probing, but make MMCONFIG init more sensible.
Good suggestion, will do more investigation about that.

>
> Bjorn
>
> .
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.