RFC: x86: cap iomem_resource to addressable physical memory

RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

x86 cannot generate full 64-bit addresses; this patch clamps iomem
addresses to the accessible range.

I wanted to post it for review before committing it, however; comments
would be appreciated, especially of the kind "this is done too early/too
late/in the wrong place/incorrectly".

	-hpa

[-- Attachment #2: 0001-x86-cap-iomem_resource-to-addressable-physical-memo.patch --]
[-- Type: text/x-patch, Size: 1415 bytes --]

From 8dd8d59d1d1586b350401d6a5f8c7a9e30f142d4 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <hpa@zytor.com>
Date: Tue, 9 Jun 2009 18:20:39 -0700
Subject: [PATCH] x86: cap iomem_resource to addressable physical memory

iomem_resource is by default initialized to -1, which means 64 bits of
physical address space if 64-bit resources are enabled.  However, x86
CPUs cannot address 64 bits of physical address space.  Thus, we want
to cap the physical address space to what the union of all CPU can
actually address.

Without this patch, we may end up assigning inaccessible values to
uninitialized 64-bit PCI memory resources.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Wilcox <matthew@wil.cx>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Martin Mares <mj@ucw.cz>
Cc: stable@kernel.org
---
 arch/x86/kernel/cpu/common.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 77848d9..de8a49c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -839,6 +839,9 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
 #if defined(CONFIG_NUMA) && defined(CONFIG_X86_64)
 	numa_add_cpu(smp_processor_id());
 #endif
+
+	/* Cap the iomem address space to what is addressable on all CPUs */
+	iomem_resource.end &= (1ULL << c->x86_phys_bits) - 1;
 }
 
 #ifdef CONFIG_X86_64
-- 
1.6.0.6

RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

x86 cannot generate full 64-bit addresses; this patch clamps iomem
addresses to the accessible range.

I wanted to post it for review before committing it, however; comments
would be appreciated, especially of the kind "this is done too early/too
late/in the wrong place/incorrectly".

	-hpa

[-- Attachment #2: 0001-x86-cap-iomem_resource-to-addressable-physical-memo.patch --]
[-- Type: text/x-patch, Size: 1415 bytes --]

From 8dd8d59d1d1586b350401d6a5f8c7a9e30f142d4 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <hpa@zytor.com>
Date: Tue, 9 Jun 2009 18:20:39 -0700
Subject: [PATCH] x86: cap iomem_resource to addressable physical memory

iomem_resource is by default initialized to -1, which means 64 bits of
physical address space if 64-bit resources are enabled.  However, x86
CPUs cannot address 64 bits of physical address space.  Thus, we want
to cap the physical address space to what the union of all CPU can
actually address.

Without this patch, we may end up assigning inaccessible values to
uninitialized 64-bit PCI memory resources.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Wilcox <matthew@wil.cx>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Martin Mares <mj@ucw.cz>
Cc: stable@kernel.org
---
 arch/x86/kernel/cpu/common.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 77848d9..de8a49c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -839,6 +839,9 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
 #if defined(CONFIG_NUMA) && defined(CONFIG_X86_64)
 	numa_add_cpu(smp_processor_id());
 #endif
+
+	/* Cap the iomem address space to what is addressable on all CPUs */
+	iomem_resource.end &= (1ULL << c->x86_phys_bits) - 1;
 }
 
 #ifdef CONFIG_X86_64
-- 
1.6.0.6

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Jesse Barnes]

On Tue, 09 Jun 2009 18:32:53 -0700
"H. Peter Anvin" <hpa@linux.intel.com> wrote:

> x86 cannot generate full 64-bit addresses; this patch clamps iomem
> addresses to the accessible range.
> 
> I wanted to post it for review before committing it, however; comments
> would be appreciated, especially of the kind "this is done too
> early/too late/in the wrong place/incorrectly".
> 
> 	-hpa

Seems reasonable, since the iomem_resource describes the CPU's view of
MMIO space it should probably reflect what it can actually access.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Jesse Barnes]

On Tue, 09 Jun 2009 18:32:53 -0700
"H. Peter Anvin" <hpa@linux.intel.com> wrote:

> x86 cannot generate full 64-bit addresses; this patch clamps iomem
> addresses to the accessible range.
> 
> I wanted to post it for review before committing it, however; comments
> would be appreciated, especially of the kind "this is done too
> early/too late/in the wrong place/incorrectly".
> 
> 	-hpa

Seems reasonable, since the iomem_resource describes the CPU's view of
MMIO space it should probably reflect what it can actually access.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Yinghai Lu]

On Tue, Jun 9, 2009 at 6:32 PM, H. Peter Anvin<hpa@linux.intel.com> wrote:
> x86 cannot generate full 64-bit addresses; this patch clamps iomem
> addresses to the accessible range.
>
> I wanted to post it for review before committing it, however; comments
> would be appreciated, especially of the kind "this is done too early/too
> late/in the wrong place/incorrectly".
| --- a/arch/x86/kernel/cpu/common.c
| +++ b/arch/x86/kernel/cpu/common.c
| @@ -839,6 +839,9 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
| #if defined(CONFIG_NUMA) && defined(CONFIG_X86_64)
| 	numa_add_cpu(smp_processor_id());
| #endif
|+
|+	/* Cap the iomem address space to what is addressable on all CPUs */
|+	iomem_resource.end &= (1ULL << c->x86_phys_bits) - 1;
| }
|


do we need do that on every cpu?

looks like we could do that in identify_boot_cpu.

YH

Re: RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> do we need do that on every cpu?
> 
> looks like we could do that in identify_boot_cpu.
> 

What if the CPUs are heterogenous?  It's obviously a suboptimal
situation, but it doesn't seem like something we can rely on.

	-hpa

Re: RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> do we need do that on every cpu?
> 
> looks like we could do that in identify_boot_cpu.
> 

What if the CPUs are heterogenous?  It's obviously a suboptimal
situation, but it doesn't seem like something we can rely on.

	-hpa

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Pavel Machek]

On Thu 2009-06-11 13:46:52, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > do we need do that on every cpu?
> > 
> > looks like we could do that in identify_boot_cpu.
> > 
> 
> What if the CPUs are heterogenous?  It's obviously a suboptimal
> situation, but it doesn't seem like something we can rely on.

Is it ok if that changes during runtime? What if someone hotplugs
heterogenous cpu? I'd say basing it on boot cpu seems simplest.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Pavel Machek]

On Thu 2009-06-11 13:46:52, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > do we need do that on every cpu?
> > 
> > looks like we could do that in identify_boot_cpu.
> > 
> 
> What if the CPUs are heterogenous?  It's obviously a suboptimal
> situation, but it doesn't seem like something we can rely on.

Is it ok if that changes during runtime? What if someone hotplugs
heterogenous cpu? I'd say basing it on boot cpu seems simplest.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

Pavel Machek wrote:
> On Thu 2009-06-11 13:46:52, H. Peter Anvin wrote:
>> Yinghai Lu wrote:
>>> do we need do that on every cpu?
>>>
>>> looks like we could do that in identify_boot_cpu.
>>>
>> What if the CPUs are heterogenous?  It's obviously a suboptimal
>> situation, but it doesn't seem like something we can rely on.
> 
> Is it ok if that changes during runtime? What if someone hotplugs
> heterogenous cpu? I'd say basing it on boot cpu seems simplest.
> 									Pavel
> 

If someone hotplugs heterogenous CPUs it should do the right thing if 
the right thing is possible to do.  If you already have resource 
assignments that are unfulfillable you're screwed anyway.

	-hpa

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Pavel Machek]

On Sun 2009-06-21 00:40:35, H. Peter Anvin wrote:
> Pavel Machek wrote:
>> On Thu 2009-06-11 13:46:52, H. Peter Anvin wrote:
>>> Yinghai Lu wrote:
>>>> do we need do that on every cpu?
>>>>
>>>> looks like we could do that in identify_boot_cpu.
>>>>
>>> What if the CPUs are heterogenous?  It's obviously a suboptimal
>>> situation, but it doesn't seem like something we can rely on.
>>
>> Is it ok if that changes during runtime? What if someone hotplugs
>> heterogenous cpu? I'd say basing it on boot cpu seems simplest.
>
> If someone hotplugs heterogenous CPUs it should do the right thing if  
> the right thing is possible to do.  If you already have resource  
> assignments that are unfulfillable you're screwed anyway.

Ok... and is there enough locking in there so that it is actually ok
to change mask during hotplug? (Is it okay because it is single long
and all the writers are somehow serialized by hotplug mechanism?)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: RFC: x86: cap iomem_resource to addressable physical memory

[Pavel Machek]

On Sun 2009-06-21 00:40:35, H. Peter Anvin wrote:
> Pavel Machek wrote:
>> On Thu 2009-06-11 13:46:52, H. Peter Anvin wrote:
>>> Yinghai Lu wrote:
>>>> do we need do that on every cpu?
>>>>
>>>> looks like we could do that in identify_boot_cpu.
>>>>
>>> What if the CPUs are heterogenous?  It's obviously a suboptimal
>>> situation, but it doesn't seem like something we can rely on.
>>
>> Is it ok if that changes during runtime? What if someone hotplugs
>> heterogenous cpu? I'd say basing it on boot cpu seems simplest.
>
> If someone hotplugs heterogenous CPUs it should do the right thing if  
> the right thing is possible to do.  If you already have resource  
> assignments that are unfulfillable you're screwed anyway.

Ok... and is there enough locking in there so that it is actually ok
to change mask during hotplug? (Is it okay because it is single long
and all the writers are somehow serialized by hotplug mechanism?)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

Pavel Machek wrote:
> 
> Ok... and is there enough locking in there so that it is actually ok
> to change mask during hotplug? (Is it okay because it is single long
> and all the writers are somehow serialized by hotplug mechanism?)
> 									Pavel

Making it a locked reference probably would be a good idea (although I 
personally think it will never actually matter in practice).  Although 
on 32 bits (PAE) it can be more than one long, it doesn't matter because 
only the upper long can actually be modified.  It does, however, 
complicate the actual code somewhat... I'll look at it tomorrow.

	-hpa

Re: RFC: x86: cap iomem_resource to addressable physical memory

[H. Peter Anvin]

Pavel Machek wrote:
> 
> Ok... and is there enough locking in there so that it is actually ok
> to change mask during hotplug? (Is it okay because it is single long
> and all the writers are somehow serialized by hotplug mechanism?)
> 									Pavel

Making it a locked reference probably would be a good idea (although I 
personally think it will never actually matter in practice).  Although 
on 32 bits (PAE) it can be more than one long, it doesn't matter because 
only the upper long can actually be modified.  It does, however, 
complicate the actual code somewhat... I'll look at it tomorrow.

	-hpa