2008-08-21 12:58:41

by Jan Beulich

[permalink] [raw]
Subject: patch "x86: MOVE PCI IO ECS code to x86/pci" breaks CPU hotplug

>Converting __cpuinit functions called out of init_amd() (and similar others)
>to __init (and making them subject of xxx_initcall() handling isn't valid, as
>they would no longer be called for hot plugged CPUs.
>
>Further, since it's likely that in virtualized environments the MSR write
>would at best be ignored, I'd also recommend using the fault-safe
>accessors here *and* check that the bit actually got set before setting
>PCI_HAS_IO_ECS (one would obviously have to BUG() when hot-plugged
>CPUs fail to set the bit when those available at boot successfully did so).

Even worse - this would even try to access the MSR on non-AMD CPUs
(currently probably prevented just by the fact that only AMD ones use
family values of 0x10 or higher).

Jan


Subject: Re: patch "x86: MOVE PCI IO ECS code to x86/pci" breaks CPU hotplug

On 21.08.08 13:59:33, Jan Beulich wrote:
> Even worse - this would even try to access the MSR on non-AMD CPUs
> (currently probably prevented just by the fact that only AMD ones use
> family values of 0x10 or higher).

Ups, really worse. Code was AMD specific before and became
generic. Will fix this.

-Robert

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

2008-08-21 16:25:47

by Yinghai Lu

[permalink] [raw]
Subject: Re: patch "x86: MOVE PCI IO ECS code to x86/pci" breaks CPU hotplug

On Thu, Aug 21, 2008 at 6:29 AM, Robert Richter <[email protected]> wrote:
> On 21.08.08 13:59:33, Jan Beulich wrote:
>> Even worse - this would even try to access the MSR on non-AMD CPUs
>> (currently probably prevented just by the fact that only AMD ones use
>> family values of 0x10 or higher).
>
> Ups, really worse. Code was AMD specific before and became
> generic. Will fix this.
>

it seems you didn't address the concern about hotplug as you promised

http://lkml.org/lkml/2008/6/13/151

YH

Subject: Re: patch "x86: MOVE PCI IO ECS code to x86/pci" breaks CPU hotplug

On 21.08.08 09:25:37, Yinghai Lu wrote:
> On Thu, Aug 21, 2008 at 6:29 AM, Robert Richter <[email protected]> wrote:
> > On 21.08.08 13:59:33, Jan Beulich wrote:
> >> Even worse - this would even try to access the MSR on non-AMD CPUs
> >> (currently probably prevented just by the fact that only AMD ones use
> >> family values of 0x10 or higher).
> >
> > Ups, really worse. Code was AMD specific before and became
> > generic. Will fix this.
> >
>
> it seems you didn't address the concern about hotplug as you promised
>
> http://lkml.org/lkml/2008/6/13/151

Yinghai, I know. You made a comment on this in reply to the patch and
this is still an open issue. Really, will fix this. :-)

-Robert

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

Subject: [PATCH] x86: fix: do not run code in amd_bus.c on non-AMD CPUs

On 21.08.08 13:59:33, Jan Beulich wrote:
> Even worse - this would even try to access the MSR on non-AMD CPUs
> (currently probably prevented just by the fact that only AMD ones use
> family values of 0x10 or higher).

This patch adds cpu vendor check to the postcore_initcalls.

Cc: Jan Beulich <[email protected]>
Cc: Yinghai Lu <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/pci/amd_bus.c | 19 ++++++++++++++++---
1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
index dbf5323..4a6f1a6 100644
--- a/arch/x86/pci/amd_bus.c
+++ b/arch/x86/pci/amd_bus.c
@@ -555,9 +555,11 @@ static int __init early_fill_mp_bus_info(void)
return 0;
}

-postcore_initcall(early_fill_mp_bus_info);
+#else /* !CONFIG_X86_64 */

-#endif
+static int __init early_fill_mp_bus_info(void) { return 0; }
+
+#endif /* !CONFIG_X86_64 */

/* common 32/64 bit code */

@@ -583,4 +585,15 @@ static int __init enable_pci_io_ecs(void)
return 0;
}

-postcore_initcall(enable_pci_io_ecs);
+static int __init amd_postcore_init(void)
+{
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+ return 0;
+
+ early_fill_mp_bus_info();
+ enable_pci_io_ecs();
+
+ return 0;
+}
+
+postcore_initcall(amd_postcore_init);
--
1.5.6.4

Subject: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable

Until now, PCI ECS setup was performed at boot time only and for cpus
that are enabled then. This patch fixes this and adds cpu hotplug.

Tests sequence (check if ECS bit is set when bringing cpu online again):

# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00404010
# ( perl -e 'sysseek(STDOUT, 0xC001001F, 0); print pack "l*", 8, 0x00400010' ) > /dev/cpu/1/msr
# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00400010
# echo 0 > /sys/devices/system/cpu/cpu1/online
# echo 1 > /sys/devices/system/cpu/cpu1/online
# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00404010

Cc: Jan Beulich <[email protected]>
Cc: Yinghai Lu <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/pci/amd_bus.c | 35 +++++++++++++++++++++++++++++++----
1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
index 4a6f1a6..6a0fca7 100644
--- a/arch/x86/pci/amd_bus.c
+++ b/arch/x86/pci/amd_bus.c
@@ -1,6 +1,7 @@
#include <linux/init.h>
#include <linux/pci.h>
#include <linux/topology.h>
+#include <linux/cpu.h>
#include "pci.h"

#ifdef CONFIG_X86_64
@@ -565,7 +566,7 @@ static int __init early_fill_mp_bus_info(void) { return 0; }

#define ENABLE_CF8_EXT_CFG (1ULL << 46)

-static void enable_pci_io_ecs_per_cpu(void *unused)
+static void enable_pci_io_ecs(void *unused)
{
u64 reg;
rdmsrl(MSR_AMD64_NB_CFG, reg);
@@ -575,13 +576,39 @@ static void enable_pci_io_ecs_per_cpu(void *unused)
}
}

-static int __init enable_pci_io_ecs(void)
+static int __cpuinit amd_cpu_notify(struct notifier_block *self,
+ unsigned long action, void *hcpu)
{
+ int cpu = (long)hcpu;
+ switch(action) {
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ smp_call_function_single(cpu, enable_pci_io_ecs, NULL, 0);
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block __cpuinitdata amd_cpu_notifier = {
+ .notifier_call = amd_cpu_notify,
+};
+
+static int __init pci_io_ecs_init(void)
+{
+ int cpu;
+
/* assume all cpus from fam10h have IO ECS */
if (boot_cpu_data.x86 < 0x10)
return 0;
- on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
+
+ register_cpu_notifier(&amd_cpu_notifier);
+ for_each_online_cpu(cpu)
+ amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
+ (void *)(long)cpu);
pci_probe |= PCI_HAS_IO_ECS;
+
return 0;
}

@@ -591,7 +618,7 @@ static int __init amd_postcore_init(void)
return 0;

early_fill_mp_bus_info();
- enable_pci_io_ecs();
+ pci_io_ecs_init();

return 0;
}
--
1.5.6.4

2008-08-22 18:48:01

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: do not run code in amd_bus.c on non-AMD CPUs

On Fri, Aug 22, 2008 at 11:23 AM, Robert Richter <[email protected]> wrote:
> On 21.08.08 13:59:33, Jan Beulich wrote:
>> Even worse - this would even try to access the MSR on non-AMD CPUs
>> (currently probably prevented just by the fact that only AMD ones use
>> family values of 0x10 or higher).
>
> This patch adds cpu vendor check to the postcore_initcalls.
>
> Cc: Jan Beulich <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> arch/x86/pci/amd_bus.c | 19 ++++++++++++++++---
> 1 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
> index dbf5323..4a6f1a6 100644
> --- a/arch/x86/pci/amd_bus.c
> +++ b/arch/x86/pci/amd_bus.c
> @@ -555,9 +555,11 @@ static int __init early_fill_mp_bus_info(void)
> return 0;
> }
>
> -postcore_initcall(early_fill_mp_bus_info);
> +#else /* !CONFIG_X86_64 */
>
> -#endif
> +static int __init early_fill_mp_bus_info(void) { return 0; }
> +
> +#endif /* !CONFIG_X86_64 */
>
> /* common 32/64 bit code */
>
> @@ -583,4 +585,15 @@ static int __init enable_pci_io_ecs(void)
> return 0;
> }
>
> -postcore_initcall(enable_pci_io_ecs);
> +static int __init amd_postcore_init(void)
> +{
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> + return 0;
> +
> + early_fill_mp_bus_info();
> + enable_pci_io_ecs();
> +
> + return 0;
> +}
> +
> +postcore_initcall(amd_postcore_init);

1. early_fill_mp_bus_info has checking with vendor/deviceid in itself.

2. you didn't address: when cpu is hot plug in later,
enable_pci_to_ecs_per_cpu will not be called.
( maxcpus and addtional_cpus)

YH

Subject: Re: [PATCH] x86: fix: do not run code in amd_bus.c on non-AMD CPUs

On 22.08.08 11:47:45, Yinghai Lu wrote:
> 1. early_fill_mp_bus_info has checking with vendor/deviceid in itself.

Right, but scanning the pci bus could be skipped for non-AMD cpus. If
you don't like this check for your function I will resubmit a new
patch.

> 2. you didn't address: when cpu is hot plug in later,
> enable_pci_to_ecs_per_cpu will not be called.
> ( maxcpus and addtional_cpus)

Next patch?

-Robert

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

2008-08-22 18:56:19

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable

On Fri, Aug 22, 2008 at 11:23 AM, Robert Richter <[email protected]> wrote:
> Until now, PCI ECS setup was performed at boot time only and for cpus
> that are enabled then. This patch fixes this and adds cpu hotplug.
>
> Tests sequence (check if ECS bit is set when bringing cpu online again):
>
> # ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
> 00000008 00404010
> # ( perl -e 'sysseek(STDOUT, 0xC001001F, 0); print pack "l*", 8, 0x00400010' ) > /dev/cpu/1/msr
> # ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
> 00000008 00400010
> # echo 0 > /sys/devices/system/cpu/cpu1/online
> # echo 1 > /sys/devices/system/cpu/cpu1/online
> # ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
> 00000008 00404010
>
> Cc: Jan Beulich <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> arch/x86/pci/amd_bus.c | 35 +++++++++++++++++++++++++++++++----
> 1 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
> index 4a6f1a6..6a0fca7 100644
> --- a/arch/x86/pci/amd_bus.c
> +++ b/arch/x86/pci/amd_bus.c
> @@ -1,6 +1,7 @@
> #include <linux/init.h>
> #include <linux/pci.h>
> #include <linux/topology.h>
> +#include <linux/cpu.h>
> #include "pci.h"
>
> #ifdef CONFIG_X86_64
> @@ -565,7 +566,7 @@ static int __init early_fill_mp_bus_info(void) { return 0; }
>
> #define ENABLE_CF8_EXT_CFG (1ULL << 46)
>
> -static void enable_pci_io_ecs_per_cpu(void *unused)
> +static void enable_pci_io_ecs(void *unused)
> {
> u64 reg;
> rdmsrl(MSR_AMD64_NB_CFG, reg);
> @@ -575,13 +576,39 @@ static void enable_pci_io_ecs_per_cpu(void *unused)
> }
> }
>
> -static int __init enable_pci_io_ecs(void)
> +static int __cpuinit amd_cpu_notify(struct notifier_block *self,
> + unsigned long action, void *hcpu)
> {
> + int cpu = (long)hcpu;
> + switch(action) {
> + case CPU_ONLINE:
> + case CPU_ONLINE_FROZEN:
> + smp_call_function_single(cpu, enable_pci_io_ecs, NULL, 0);
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block __cpuinitdata amd_cpu_notifier = {
> + .notifier_call = amd_cpu_notify,
> +};
> +
> +static int __init pci_io_ecs_init(void)
> +{
> + int cpu;
> +
> /* assume all cpus from fam10h have IO ECS */
> if (boot_cpu_data.x86 < 0x10)
> return 0;
> - on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
> +
> + register_cpu_notifier(&amd_cpu_notifier);
> + for_each_online_cpu(cpu)
> + amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
> + (void *)(long)cpu);

wonder if those two lines should be reversed.

> pci_probe |= PCI_HAS_IO_ECS;
> +
> return 0;
> }
>
> @@ -591,7 +618,7 @@ static int __init amd_postcore_init(void)
> return 0;
>
> early_fill_mp_bus_info();
> - enable_pci_io_ecs();
> + pci_io_ecs_init();
>
> return 0;
> }
> --
> 1.5.6.4
>
>
>

2008-08-22 18:57:48

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: do not run code in amd_bus.c on non-AMD CPUs

On Fri, Aug 22, 2008 at 11:51 AM, Robert Richter <[email protected]> wrote:
> On 22.08.08 11:47:45, Yinghai Lu wrote:
>> 1. early_fill_mp_bus_info has checking with vendor/deviceid in itself.
>
> Right, but scanning the pci bus could be skipped for non-AMD cpus. If
> you don't like this check for your function I will resubmit a new
> patch.

just fine, if you check vendor before that, we should remove check
vendor again in later function.

YH

Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable

On 22.08.08 11:56:08, Yinghai Lu wrote:

[...]

> > +static int __init pci_io_ecs_init(void)
> > +{
> > + int cpu;
> > +
> > /* assume all cpus from fam10h have IO ECS */
> > if (boot_cpu_data.x86 < 0x10)
> > return 0;
> > - on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
> > +
> > + register_cpu_notifier(&amd_cpu_notifier);
> > + for_each_online_cpu(cpu)
> > + amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
> > + (void *)(long)cpu);
>
> wonder if those two lines should be reversed.

Do you mean setting PCI_HAS_IO_ECS before for_each_online_cpu(...)?
PCI_HAS_IO_ECS will be used only in pci_direct_init(). PCI is
initialized in a later init stage, so it doesn't matter. My intention
was to set the bit after the setup of all online cpus is finished.

-Robert

>
> > pci_probe |= PCI_HAS_IO_ECS;
> > +
> > return 0;
> > }
> >
> > @@ -591,7 +618,7 @@ static int __init amd_postcore_init(void)
> > return 0;
> >
> > early_fill_mp_bus_info();
> > - enable_pci_io_ecs();
> > + pci_io_ecs_init();
> >
> > return 0;
> > }
> > --
> > 1.5.6.4
> >
> >
> >
>

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

2008-08-22 19:10:51

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable

On Fri, Aug 22, 2008 at 12:07 PM, Robert Richter <[email protected]> wrote:
> On 22.08.08 11:56:08, Yinghai Lu wrote:
>
> [...]
>
>> > +static int __init pci_io_ecs_init(void)
>> > +{
>> > + int cpu;
>> > +
>> > /* assume all cpus from fam10h have IO ECS */
>> > if (boot_cpu_data.x86 < 0x10)
>> > return 0;
>> > - on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
>> > +
>> > + register_cpu_notifier(&amd_cpu_notifier);
>> > + for_each_online_cpu(cpu)
>> > + amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
>> > + (void *)(long)cpu);
>>
>> wonder if those two lines should be reversed.
>
> Do you mean setting PCI_HAS_IO_ECS before for_each_online_cpu(...)?
> PCI_HAS_IO_ECS will be used only in pci_direct_init(). PCI is
> initialized in a later init stage, so it doesn't matter. My intention
> was to set the bit after the setup of all online cpus is finished.


==>
+ for_each_online_cpu(cpu)
+ amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
+ (void *)(long)cpu);
+ register_cpu_notifier(&amd_cpu_notifier);

YH

Subject: Re: [PATCH] x86: fix: do not run code in amd_bus.c on non-AMD CPUs

On 22.08.08 11:57:39, Yinghai Lu wrote:
> On Fri, Aug 22, 2008 at 11:51 AM, Robert Richter <[email protected]> wrote:
> > On 22.08.08 11:47:45, Yinghai Lu wrote:
> >> 1. early_fill_mp_bus_info has checking with vendor/deviceid in itself.
> >
> > Right, but scanning the pci bus could be skipped for non-AMD cpus. If
> > you don't like this check for your function I will resubmit a new
> > patch.
>
> just fine, if you check vendor before that, we should remove check
> vendor again in later function.

Right. There are no more checks in, only the PCI vendor check that can
not be removed.

-Robert

>
> YH
>

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable

On 22.08.08 12:10:04, Yinghai Lu wrote:
> On Fri, Aug 22, 2008 at 12:07 PM, Robert Richter <[email protected]> wrote:
> > On 22.08.08 11:56:08, Yinghai Lu wrote:
> >
> > [...]
> >
> >> > +static int __init pci_io_ecs_init(void)
> >> > +{
> >> > + int cpu;
> >> > +
> >> > /* assume all cpus from fam10h have IO ECS */
> >> > if (boot_cpu_data.x86 < 0x10)
> >> > return 0;
> >> > - on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
> >> > +
> >> > + register_cpu_notifier(&amd_cpu_notifier);
> >> > + for_each_online_cpu(cpu)
> >> > + amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
> >> > + (void *)(long)cpu);
> >>
> >> wonder if those two lines should be reversed.
> >
> > Do you mean setting PCI_HAS_IO_ECS before for_each_online_cpu(...)?
> > PCI_HAS_IO_ECS will be used only in pci_direct_init(). PCI is
> > initialized in a later init stage, so it doesn't matter. My intention
> > was to set the bit after the setup of all online cpus is finished.
>
>
> ==>
> + for_each_online_cpu(cpu)
> + amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
> + (void *)(long)cpu);
> + register_cpu_notifier(&amd_cpu_notifier);

Hmm, most code in the kernel registers first. Theoretical, with this
code, if a cpu would go online in between, it wouldn't be initialized.

-Robert

>
> YH
>

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]

2008-08-23 15:37:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable


* Robert Richter <[email protected]> wrote:

> Until now, PCI ECS setup was performed at boot time only and for cpus
> that are enabled then. This patch fixes this and adds cpu hotplug.

this patch does not apply cleanly to -git.

Ingo

2008-08-23 15:39:59

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable


* Ingo Molnar <[email protected]> wrote:

> * Robert Richter <[email protected]> wrote:
>
> > Until now, PCI ECS setup was performed at boot time only and for
> > cpus that are enabled then. This patch fixes this and adds cpu
> > hotplug.
>
> this patch does not apply cleanly to -git.

i see, it depends on the other patch. Applied both to tip/x86/urgent.

Ingo

2008-08-25 09:13:15

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable (-v2)


* Robert Richter <[email protected]> wrote:

> Until now, PCI ECS setup was performed at boot time only and for cpus
> that are enabled then. This patch fixes this and adds cpu hotplug.

i already had v1 applied to tip/x86/urgent, so i took the -v2 delta and
queued it up in tip/x86/cleanups - see the commit below.

Ingo

--------------->
>From ed21763e7b0b3fb50e4efd9d4bc17ef5b035d304 Mon Sep 17 00:00:00 2001
From: Robert Richter <[email protected]>
Date: Fri, 22 Aug 2008 20:23:38 +0200
Subject: [PATCH] x86: cleanup in amd_cpu_notify()

small coding style fix.

Signed-off-by: Robert Richter <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/pci/amd_bus.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
index 6a0fca7..22e0576 100644
--- a/arch/x86/pci/amd_bus.c
+++ b/arch/x86/pci/amd_bus.c
@@ -580,7 +580,7 @@ static int __cpuinit amd_cpu_notify(struct notifier_block *self,
unsigned long action, void *hcpu)
{
int cpu = (long)hcpu;
- switch(action) {
+ switch (action) {
case CPU_ONLINE:
case CPU_ONLINE_FROZEN:
smp_call_function_single(cpu, enable_pci_io_ecs, NULL, 0);

Subject: [PATCH] x86: fix: make PCI ECS for AMD CPUs hotplug capable (-v2)

Until now, PCI ECS setup was performed at boot time only and for cpus
that are enabled then. This patch fixes this and adds cpu hotplug.

Tests sequence (check if ECS bit is set when bringing cpu online again):

# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00404010
# ( perl -e 'sysseek(STDOUT, 0xC001001F, 0); print pack "l*", 8, 0x00400010' ) > /dev/cpu/1/msr
# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00400010
# echo 0 > /sys/devices/system/cpu/cpu1/online
# echo 1 > /sys/devices/system/cpu/cpu1/online
# ( perl -e 'sysseek(STDIN, 0xC001001F, 0)'; hexdump -n 8 -e '2/4 "%08x " "\n"' ) < /dev/cpu/1/msr
00000008 00404010

Cc: Jan Beulich <[email protected]>
Cc: Yinghai Lu <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/pci/amd_bus.c | 35 +++++++++++++++++++++++++++++++----
1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
index 4a6f1a6..22e0576 100644
--- a/arch/x86/pci/amd_bus.c
+++ b/arch/x86/pci/amd_bus.c
@@ -1,6 +1,7 @@
#include <linux/init.h>
#include <linux/pci.h>
#include <linux/topology.h>
+#include <linux/cpu.h>
#include "pci.h"

#ifdef CONFIG_X86_64
@@ -565,7 +566,7 @@ static int __init early_fill_mp_bus_info(void) { return 0; }

#define ENABLE_CF8_EXT_CFG (1ULL << 46)

-static void enable_pci_io_ecs_per_cpu(void *unused)
+static void enable_pci_io_ecs(void *unused)
{
u64 reg;
rdmsrl(MSR_AMD64_NB_CFG, reg);
@@ -575,13 +576,39 @@ static void enable_pci_io_ecs_per_cpu(void *unused)
}
}

-static int __init enable_pci_io_ecs(void)
+static int __cpuinit amd_cpu_notify(struct notifier_block *self,
+ unsigned long action, void *hcpu)
{
+ int cpu = (long)hcpu;
+ switch (action) {
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ smp_call_function_single(cpu, enable_pci_io_ecs, NULL, 0);
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block __cpuinitdata amd_cpu_notifier = {
+ .notifier_call = amd_cpu_notify,
+};
+
+static int __init pci_io_ecs_init(void)
+{
+ int cpu;
+
/* assume all cpus from fam10h have IO ECS */
if (boot_cpu_data.x86 < 0x10)
return 0;
- on_each_cpu(enable_pci_io_ecs_per_cpu, NULL, 1);
+
+ register_cpu_notifier(&amd_cpu_notifier);
+ for_each_online_cpu(cpu)
+ amd_cpu_notify(&amd_cpu_notifier, (unsigned long)CPU_ONLINE,
+ (void *)(long)cpu);
pci_probe |= PCI_HAS_IO_ECS;
+
return 0;
}

@@ -591,7 +618,7 @@ static int __init amd_postcore_init(void)
return 0;

early_fill_mp_bus_info();
- enable_pci_io_ecs();
+ pci_io_ecs_init();

return 0;
}
--
1.5.6.4