* [PATCH V2 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-04-09 16:22 ` [PATCH V2 2/6] PCI, x86: split out pci_mmconfig_alloc() " Jiang Liu
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Jiang Liu, Keping Chen, x86, linux-pci,
linux-acpi
Split out pci_mmcfg_check_reserved() for code reuse, which will be used
when supporting PCI host bridge hotplug.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/pci/mmconfig-shared.c | 51 +++++++++++++++++++--------------------
1 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 301e325..f799949 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -474,39 +474,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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH V2 2/6] PCI, x86: split out pci_mmconfig_alloc() for code reuse
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-09 16:22 ` [PATCH V2 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-04-09 16:22 ` [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list Jiang Liu
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Jiang Liu, Keping Chen, x86, linux-pci,
linux-acpi
Split out pci_mmconfig_alloc() for code reuse, which will be used
when supporting PCI root bridge hotplug.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/pci/mmconfig-shared.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index f799949..5e2cd2a 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -61,8 +61,9 @@ static __init void list_add_sorted(struct pci_mmcfg_region *new)
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_alloc(int segment,
+ int start,
+ int end, u64 addr)
{
struct pci_mmcfg_region *new;
struct resource *res;
@@ -79,8 +80,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,6 +95,18 @@ 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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
2012-04-09 16:22 ` [PATCH V2 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse Jiang Liu
2012-04-09 16:22 ` [PATCH V2 2/6] PCI, x86: split out pci_mmconfig_alloc() " Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-05-02 22:31 ` Bjorn Helgaas
2012-04-09 16:22 ` [PATCH V2 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap() Jiang Liu
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Jiang Liu, Keping Chen, x86, linux-pci,
linux-acpi
Use RCU list to protect mmconfig list from dynamic change
when supporting PCI host bridge hotplug.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/pci/mmconfig-shared.c | 11 ++++++-----
arch/x86/pci/mmconfig_32.c | 13 +++++++++++--
arch/x86/pci/mmconfig_64.c | 13 +++++++++++--
3 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 5e2cd2a..3bcc361 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -17,6 +17,7 @@
#include <linux/bitmap.h>
#include <linux/dmi.h>
#include <linux/slab.h>
+#include <linux/rculist.h>
#include <asm/e820.h>
#include <asm/pci_x86.h>
#include <asm/acpi.h>
@@ -45,20 +46,20 @@ 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 __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
@@ -111,7 +112,7 @@ 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;
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 5372e86..5dad04a 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;
}
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 915a493..acc48c5 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;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list
2012-04-09 16:22 ` [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list Jiang Liu
@ 2012-05-02 22:31 ` Bjorn Helgaas
2012-05-03 6:13 ` Jiang Liu
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2012-05-02 22:31 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Yinghai Lu, Jiang Liu, Keping Chen,
x86, linux-pci, linux-acpi
On Mon, Apr 9, 2012 at 10:22 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Use RCU list to protect mmconfig list from dynamic change
> when supporting PCI host bridge hotplug.
I'm not convinced this is safe. But I'm not an RCU expert, so maybe
you can convince me :)
Documentation/RCU/checklist.txt says the update code still requires
mutual exclusion, and I don't see any in list_add_sorted() or its
callers.
MMCONFIG accesses are already protected by the pci_config_lock
spinlock, so I don't think the performance advantages of RCU really
gain us anything in this situation.
It would definitely be simpler to just hold pci_config_lock while
updating and searching the pci_mmcfg_list, but if there are issues
with that, you can educate me about what they are and why this RCU
code is correct.
Bjorn
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
> arch/x86/pci/mmconfig-shared.c | 11 ++++++-----
> arch/x86/pci/mmconfig_32.c | 13 +++++++++++--
> arch/x86/pci/mmconfig_64.c | 13 +++++++++++--
> 3 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 5e2cd2a..3bcc361 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -17,6 +17,7 @@
> #include <linux/bitmap.h>
> #include <linux/dmi.h>
> #include <linux/slab.h>
> +#include <linux/rculist.h>
> #include <asm/e820.h>
> #include <asm/pci_x86.h>
> #include <asm/acpi.h>
> @@ -45,20 +46,20 @@ 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 __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
> @@ -111,7 +112,7 @@ 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;
> diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
> index 5372e86..5dad04a 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;
> }
> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
> index 915a493..acc48c5 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;
> }
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list
2012-05-02 22:31 ` Bjorn Helgaas
@ 2012-05-03 6:13 ` Jiang Liu
2012-05-03 16:34 ` Bjorn Helgaas
0 siblings, 1 reply; 13+ messages in thread
From: Jiang Liu @ 2012-05-03 6:13 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Yinghai Lu, Keping Chen,
x86, linux-pci, linux-acpi
Hi Bjorn,
Thanks for your comments!
I have thought about using pci_config_lock to serialize
access to the pci_mmcfg_list, but that may hurt MMCFG on x86_64
systems because mmconfig_64.c supports concurrent access to
the MMCFG space without holding the pci_config_lock. So I use
RCU to avoid possible performance penalty for x86_64 systems.
There are two mechanisms to protect list_add_sorted()
from concurrent updates. The first case is, list_add_sorted()
may be called without holding any lock for serialization at
early booting stages because the system is still in single-threaded
mode. The second case is, pci_config_lock is used to serialize
concurrent modification to the pci_mmcfg_list if list_add_sorted()
is called at runtime.
The first case is cover by this patch, and the second case
is covered by "[PATCH V2 5/6] PCI, x86: introduce
pci_mmconfig_insert()/delete() for PCI root bridge hotplug".
Thanks!
On 2012-5-3 6:31, Bjorn Helgaas wrote:
> On Mon, Apr 9, 2012 at 10:22 AM, Jiang Liu<liuj97@gmail.com> wrote:
>> Use RCU list to protect mmconfig list from dynamic change
>> when supporting PCI host bridge hotplug.
>
> I'm not convinced this is safe. But I'm not an RCU expert, so maybe
> you can convince me :)
>
> Documentation/RCU/checklist.txt says the update code still requires
> mutual exclusion, and I don't see any in list_add_sorted() or its
> callers.
>
> MMCONFIG accesses are already protected by the pci_config_lock
> spinlock, so I don't think the performance advantages of RCU really
> gain us anything in this situation.
>
> It would definitely be simpler to just hold pci_config_lock while
> updating and searching the pci_mmcfg_list, but if there are issues
> with that, you can educate me about what they are and why this RCU
> code is correct.
>
> Bjorn
>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>> arch/x86/pci/mmconfig-shared.c | 11 ++++++-----
>> arch/x86/pci/mmconfig_32.c | 13 +++++++++++--
>> arch/x86/pci/mmconfig_64.c | 13 +++++++++++--
>> 3 files changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
>> index 5e2cd2a..3bcc361 100644
>> --- a/arch/x86/pci/mmconfig-shared.c
>> +++ b/arch/x86/pci/mmconfig-shared.c
>> @@ -17,6 +17,7 @@
>> #include<linux/bitmap.h>
>> #include<linux/dmi.h>
>> #include<linux/slab.h>
>> +#include<linux/rculist.h>
>> #include<asm/e820.h>
>> #include<asm/pci_x86.h>
>> #include<asm/acpi.h>
>> @@ -45,20 +46,20 @@ 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 __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
>> @@ -111,7 +112,7 @@ 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;
>> diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
>> index 5372e86..5dad04a 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;
>> }
>> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
>> index 915a493..acc48c5 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;
>> }
>> --
>> 1.7.5.4
>>
>
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list
2012-05-03 6:13 ` Jiang Liu
@ 2012-05-03 16:34 ` Bjorn Helgaas
2012-05-04 2:40 ` Jiang Liu
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2012-05-03 16:34 UTC (permalink / raw)
To: Jiang Liu
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Yinghai Lu, Keping Chen,
x86, linux-pci, linux-acpi
On Thu, May 3, 2012 at 12:13 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
> Hi Bjorn,
> Thanks for your comments!
> I have thought about using pci_config_lock to serialize
> access to the pci_mmcfg_list, but that may hurt MMCFG on x86_64
> systems because mmconfig_64.c supports concurrent access to
> the MMCFG space without holding the pci_config_lock. So I use
> RCU to avoid possible performance penalty for x86_64 systems.
Oh, right. I only looked at mmconfig_32.c, which uses the lock, and
assumed _64.c was the same. I think you're right to avoid introducing
locking where we didn't have it before.
> There are two mechanisms to protect list_add_sorted()
> from concurrent updates. The first case is, list_add_sorted()
> may be called without holding any lock for serialization at
> early booting stages because the system is still in single-threaded
> mode. The second case is, pci_config_lock is used to serialize
> concurrent modification to the pci_mmcfg_list if list_add_sorted()
> is called at runtime.
> The first case is cover by this patch, and the second case
> is covered by "[PATCH V2 5/6] PCI, x86: introduce
> pci_mmconfig_insert()/delete() for PCI root bridge hotplug".
How hard would it be to convert the first (early boot) mechanism to
use the second mechanism? It'd be nicer if there were only one path
that we use both places, even if it means the early boot path acquires
a lock that is technically unnecessary.
> On 2012-5-3 6:31, Bjorn Helgaas wrote:
>>
>> On Mon, Apr 9, 2012 at 10:22 AM, Jiang Liu<liuj97@gmail.com> wrote:
>>>
>>> Use RCU list to protect mmconfig list from dynamic change
>>> when supporting PCI host bridge hotplug.
>>
>>
>> I'm not convinced this is safe. But I'm not an RCU expert, so maybe
>> you can convince me :)
>>
>> Documentation/RCU/checklist.txt says the update code still requires
>> mutual exclusion, and I don't see any in list_add_sorted() or its
>> callers.
>>
>> MMCONFIG accesses are already protected by the pci_config_lock
>> spinlock, so I don't think the performance advantages of RCU really
>> gain us anything in this situation.
>>
>> It would definitely be simpler to just hold pci_config_lock while
>> updating and searching the pci_mmcfg_list, but if there are issues
>> with that, you can educate me about what they are and why this RCU
>> code is correct.
>>
>> Bjorn
>>
>>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>>> ---
>>> arch/x86/pci/mmconfig-shared.c | 11 ++++++-----
>>> arch/x86/pci/mmconfig_32.c | 13 +++++++++++--
>>> arch/x86/pci/mmconfig_64.c | 13 +++++++++++--
>>> 3 files changed, 28 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/mmconfig-shared.c
>>> b/arch/x86/pci/mmconfig-shared.c
>>> index 5e2cd2a..3bcc361 100644
>>> --- a/arch/x86/pci/mmconfig-shared.c
>>> +++ b/arch/x86/pci/mmconfig-shared.c
>>> @@ -17,6 +17,7 @@
>>> #include<linux/bitmap.h>
>>> #include<linux/dmi.h>
>>> #include<linux/slab.h>
>>> +#include<linux/rculist.h>
>>> #include<asm/e820.h>
>>> #include<asm/pci_x86.h>
>>> #include<asm/acpi.h>
>>> @@ -45,20 +46,20 @@ 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 __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int
>>> segment,
>>> @@ -111,7 +112,7 @@ 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;
>>> diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
>>> index 5372e86..5dad04a 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;
>>> }
>>> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
>>> index 915a493..acc48c5 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;
>>> }
>>> --
>>> 1.7.5.4
>>>
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list
2012-05-03 16:34 ` Bjorn Helgaas
@ 2012-05-04 2:40 ` Jiang Liu
0 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-05-04 2:40 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Yinghai Lu, Keping Chen,
x86, linux-pci, linux-acpi
On 05/04/2012 12:34 AM, Bjorn Helgaas wrote:
> On Thu, May 3, 2012 at 12:13 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> Hi Bjorn,
>> Thanks for your comments!
>> I have thought about using pci_config_lock to serialize
>> access to the pci_mmcfg_list, but that may hurt MMCFG on x86_64
>> systems because mmconfig_64.c supports concurrent access to
>> the MMCFG space without holding the pci_config_lock. So I use
>> RCU to avoid possible performance penalty for x86_64 systems.
>
> Oh, right. I only looked at mmconfig_32.c, which uses the lock, and
> assumed _64.c was the same. I think you're right to avoid introducing
> locking where we didn't have it before.
>
>> There are two mechanisms to protect list_add_sorted()
>> from concurrent updates. The first case is, list_add_sorted()
>> may be called without holding any lock for serialization at
>> early booting stages because the system is still in single-threaded
>> mode. The second case is, pci_config_lock is used to serialize
>> concurrent modification to the pci_mmcfg_list if list_add_sorted()
>> is called at runtime.
>> The first case is cover by this patch, and the second case
>> is covered by "[PATCH V2 5/6] PCI, x86: introduce
>> pci_mmconfig_insert()/delete() for PCI root bridge hotplug".
>
> How hard would it be to convert the first (early boot) mechanism to
> use the second mechanism? It'd be nicer if there were only one path
> that we use both places, even if it means the early boot path acquires
> a lock that is technically unnecessary.
That should be easy, will change it.
>
>> On 2012-5-3 6:31, Bjorn Helgaas wrote:
>>>
>>> On Mon, Apr 9, 2012 at 10:22 AM, Jiang Liu<liuj97@gmail.com> wrote:
>>>>
>>>> Use RCU list to protect mmconfig list from dynamic change
>>>> when supporting PCI host bridge hotplug.
>>>
>>>
>>> I'm not convinced this is safe. But I'm not an RCU expert, so maybe
>>> you can convince me :)
>>>
>>> Documentation/RCU/checklist.txt says the update code still requires
>>> mutual exclusion, and I don't see any in list_add_sorted() or its
>>> callers.
>>>
>>> MMCONFIG accesses are already protected by the pci_config_lock
>>> spinlock, so I don't think the performance advantages of RCU really
>>> gain us anything in this situation.
>>>
>>> It would definitely be simpler to just hold pci_config_lock while
>>> updating and searching the pci_mmcfg_list, but if there are issues
>>> with that, you can educate me about what they are and why this RCU
>>> code is correct.
>>>
>>> Bjorn
>>>
>>>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>>>> ---
>>>> arch/x86/pci/mmconfig-shared.c | 11 ++++++-----
>>>> arch/x86/pci/mmconfig_32.c | 13 +++++++++++--
>>>> arch/x86/pci/mmconfig_64.c | 13 +++++++++++--
>>>> 3 files changed, 28 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/x86/pci/mmconfig-shared.c
>>>> b/arch/x86/pci/mmconfig-shared.c
>>>> index 5e2cd2a..3bcc361 100644
>>>> --- a/arch/x86/pci/mmconfig-shared.c
>>>> +++ b/arch/x86/pci/mmconfig-shared.c
>>>> @@ -17,6 +17,7 @@
>>>> #include<linux/bitmap.h>
>>>> #include<linux/dmi.h>
>>>> #include<linux/slab.h>
>>>> +#include<linux/rculist.h>
>>>> #include<asm/e820.h>
>>>> #include<asm/pci_x86.h>
>>>> #include<asm/acpi.h>
>>>> @@ -45,20 +46,20 @@ 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 __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int
>>>> segment,
>>>> @@ -111,7 +112,7 @@ 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;
>>>> diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
>>>> index 5372e86..5dad04a 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;
>>>> }
>>>> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
>>>> index 915a493..acc48c5 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;
>>>> }
>>>> --
>>>> 1.7.5.4
>>>>
>>>
>>> .
>>>
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V2 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap()
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
` (2 preceding siblings ...)
2012-04-09 16:22 ` [PATCH V2 3/6] PCI, x86: use RCU list to protect mmconfig list Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-04-09 16:22 ` [PATCH V2 5/6] PCI, x86: introduce pci_mmconfig_insert()/delete() for PCI root bridge hotplug Jiang Liu
2012-04-09 16:22 ` [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
5 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Jiang Liu, Keping Chen, x86, linux-pci,
linux-acpi
Introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap(), which will be used
when supporting PCI root bridge hotplug.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/include/asm/pci_x86.h | 2 ++
arch/x86/pci/mmconfig_32.c | 15 +++++++++++++++
arch/x86/pci/mmconfig_64.c | 22 +++++++++++++++++++++-
3 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index b3a5317..4611294 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -135,6 +135,8 @@ 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 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_32.c b/arch/x86/pci/mmconfig_32.c
index 5dad04a..c2756de 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -141,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 acc48c5..6822dd6 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -95,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;
@@ -138,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] 13+ messages in thread* [PATCH V2 5/6] PCI, x86: introduce pci_mmconfig_insert()/delete() for PCI root bridge hotplug
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
` (3 preceding siblings ...)
2012-04-09 16:22 ` [PATCH V2 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap() Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-04-09 16:22 ` [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
5 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Jiang Liu, Keping Chen, x86, linux-pci,
linux-acpi
Introduce pci_mmconfig_insert()/pci_mmconfig_delete(), which will be used to
update MMCFG information when supporting PCI root bridge hotplug.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
arch/x86/include/asm/pci_x86.h | 2 +
arch/x86/pci/mmconfig-shared.c | 105 +++++++++++++++++++++++++++++++++++++---
2 files changed, 100 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 4611294..ba8570c 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -137,6 +137,8 @@ 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 3bcc361..dda9470 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -17,6 +17,7 @@
#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>
@@ -376,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;
@@ -413,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;
@@ -427,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;
@@ -446,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);
@@ -676,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;
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 16:22 [PATCH V2 0/6] PCI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
` (4 preceding siblings ...)
2012-04-09 16:22 ` [PATCH V2 5/6] PCI, x86: introduce pci_mmconfig_insert()/delete() for PCI root bridge hotplug Jiang Liu
@ 2012-04-09 16:22 ` Jiang Liu
2012-04-09 18:00 ` Yinghai Lu
5 siblings, 1 reply; 13+ messages in thread
From: Jiang Liu @ 2012-04-09 16:22 UTC (permalink / raw)
To: Taku Izumi, Kenji Kaneshige, Yinghai Lu
Cc: Jiang Liu, Bjorn Helgaas, Len Brown, Jiang Liu, Keping Chen, x86,
linux-pci, linux-acpi
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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/pci_root.c | 20 ++++++++++++++
include/acpi/acnames.h | 1 +
include/linux/pci-acpi.h | 2 +
4 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index da0149d..e38d9b7 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -488,6 +488,72 @@ 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;
+
+ /* MMCONFIG disabled */
+ if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+ return 0;
+
+ /*
+ * 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;
+
+ /* MMCONFIG disabled */
+ if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+ return 0;
+
+ /* 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..6e8dd10 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -37,6 +37,8 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
}
void acpi_pci_root_rescan(void);
+int arch_acpi_pci_root_add(struct acpi_pci_root *root);
+int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
#else
--
1.7.5.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 16:22 ` [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges Jiang Liu
@ 2012-04-09 18:00 ` Yinghai Lu
2012-04-10 6:26 ` Jiang Liu
0 siblings, 1 reply; 13+ messages in thread
From: Yinghai Lu @ 2012-04-09 18:00 UTC (permalink / raw)
To: Jiang Liu
Cc: Taku Izumi, Kenji Kaneshige, Jiang Liu, Bjorn Helgaas, Len Brown,
Keping Chen, x86, linux-pci, linux-acpi
On Mon, Apr 9, 2012 at 9:22 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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/pci_root.c | 20 ++++++++++++++
> include/acpi/acnames.h | 1 +
> include/linux/pci-acpi.h | 2 +
> 4 files changed, 89 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index da0149d..e38d9b7 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -488,6 +488,72 @@ 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;
> +
> + /* MMCONFIG disabled */
> + if ((pci_probe & PCI_PROBE_MMCONF) == 0)
> + return 0;
should be
if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
return 0;
> +
> + /*
> + * 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;
> +
> + /* MMCONFIG disabled */
> + if ((pci_probe & PCI_PROBE_MMCONF) == 0)
> + return 0;
again should be
if (!!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
return 0;
> +
> + /* 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..6e8dd10 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -37,6 +37,8 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
> }
>
> void acpi_pci_root_rescan(void);
> +int arch_acpi_pci_root_add(struct acpi_pci_root *root);
> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
>
> #else
>
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH V2 6/6] PCI, ACPI, x86: update MMCFG information when hot-plugging PCI host bridges
2012-04-09 18:00 ` Yinghai Lu
@ 2012-04-10 6:26 ` Jiang Liu
0 siblings, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2012-04-10 6:26 UTC (permalink / raw)
To: Yinghai Lu
Cc: Jiang Liu, Taku Izumi, Kenji Kaneshige, Bjorn Helgaas, Len Brown,
Keping Chen, x86, linux-pci, linux-acpi
On 2012-4-10 2:00, Yinghai Lu wrote:
> On Mon, Apr 9, 2012 at 9:22 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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/pci_root.c | 20 ++++++++++++++
>> include/acpi/acnames.h | 1 +
>> include/linux/pci-acpi.h | 2 +
>> 4 files changed, 89 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index da0149d..e38d9b7 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -488,6 +488,72 @@ 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;
>> +
>> + /* MMCONFIG disabled */
>> + if ((pci_probe& PCI_PROBE_MMCONF) == 0)
>> + return 0;
>
> should be
>
> if (!!(pci_probe& PCI_PROBE_MASK& ~PCI_PROBE_MMCONF))
> return 0;
OK, will change to use this.
I thought it's the same because it will clear all other PCI_PROBE_XXX
flags if MMCONF is enabled. But it's more reliable to make sure all
other PCI_PROBE_XXX flags are cleared.
---
pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
---
>
>> +
>> + /*
>> + * 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;
>> +
>> + /* MMCONFIG disabled */
>> + if ((pci_probe& PCI_PROBE_MMCONF) == 0)
>> + return 0;
>
> again should be
>
> if (!!(pci_probe& PCI_PROBE_MASK& ~PCI_PROBE_MMCONF))
> return 0;
>
>
>
>> +
>> + /* 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..6e8dd10 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -37,6 +37,8 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>> }
>>
>> void acpi_pci_root_rescan(void);
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root);
>> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
>>
>> #else
>>
>> --
>> 1.7.5.4
>>
>
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread