RFC: x86: cap iomem_resource to addressable physical memory
Subsystems (2)
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
From 8dd8d59d1d1586b350401d6a5f8c7a9e30f142d4 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <[email protected]>
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.
--- arch/x86/kernel/cpu/common.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/common.c +3 -0
@@ -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
Signature · 1 line
1.6.0.6
Jesse Barnes
Reviewed-byQuoted 8 lines · On Tue, 09 Jun 2009 18:32:53 -0700 "H. Peter Anvin" <[email protected]> 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.
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Quoted 6 lines · H. Peter Anvin
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
Quoted 5 lines · Yinghai Lu
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
Quoted 9 lines · H. Peter Anvin
Quoted 5 lines · Yinghai Lu
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
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 4 lines · Pavel Machek
Quoted 3 lines · H. Peter Anvin
Quoted 2 lines · Yinghai Lu
do we need do that on every cpu?
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
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
Quoted 10 lines · H. Peter Anvin
Quoted 9 lines · Pavel Machek
Quoted 7 lines · H. Peter Anvin
Quoted 4 lines · Yinghai Lu
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.
Quoted 6 lines
Quoted 2 lines
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
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 5 lines · Pavel Machek
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