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).
Jan
* Jan Beulich <[email protected]> wrote:
> 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).
hm, which patch is this exactly, and in what tree? It's not upstream nor
in -tip.
Ingo
On 21.08.08 15:02:59, Ingo Molnar wrote:
>
> * Jan Beulich <[email protected]> wrote:
>
> > 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).
>
> hm, which patch is this exactly, and in what tree? It's not upstream nor
> in -tip.
Probably this in the mainline kernel is meant:
commit 3a27dd1ce5de08e21e0266ddf00e6f1f843bfe8b
Author: Robert Richter <[email protected]>
Date: Thu Jun 12 20:19:23 2008 +0200
x86: Move PCI IO ECS code to x86/pci
"Form follows function". Code is now where it belongs to.
Signed-off-by: Robert Richter <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
The problem is code like this:
+static int __init enable_pci_io_ecs(void)
+{
+ /* 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, 1);
+ pci_probe |= PCI_HAS_IO_ECS;
+ return 0;
+}
It is only called during the setup and if the cpu is enabled. I intend
to rewrite the cpu setup code using cpu notifiers.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
>>> Ingo Molnar <[email protected]> 21.08.08 15:02 >>>
>
>* Jan Beulich <[email protected]> wrote:
>
>> 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).
>
>hm, which patch is this exactly, and in what tree? It's not upstream nor
>in -tip.
It is upstream:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3a27dd1ce5de08e21e0266ddf00e6f1f843bfe8b
(and I just verified it didn't change between -rc3 and -rc4).
Jan
* Robert Richter <[email protected]> wrote:
> On 21.08.08 15:02:59, Ingo Molnar wrote:
> >
> > * Jan Beulich <[email protected]> wrote:
> >
> > > 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).
> >
> > hm, which patch is this exactly, and in what tree? It's not upstream nor
> > in -tip.
>
> Probably this in the mainline kernel is meant:
>
> commit 3a27dd1ce5de08e21e0266ddf00e6f1f843bfe8b
> Author: Robert Richter <[email protected]>
> Date: Thu Jun 12 20:19:23 2008 +0200
>
> x86: Move PCI IO ECS code to x86/pci
>
> "Form follows function". Code is now where it belongs to.
>
> Signed-off-by: Robert Richter <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
> The problem is code like this:
>
> +static int __init enable_pci_io_ecs(void)
> +{
> + /* 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, 1);
> + pci_probe |= PCI_HAS_IO_ECS;
> + return 0;
> +}
>
> It is only called during the setup and if the cpu is enabled. I intend
> to rewrite the cpu setup code using cpu notifiers.
ok. Unfortunately the revert looks quite non-trivial for v2.6.27. Could
we have some _really_ minimal fix for v2.6.27?
Ingo