define and use phys_addr_t
Subsystems (5)
PATCH 1/1 define and use phys_addr_t
This series:
- adds a globally defined phys_addr_t, replacing the arch-specific x86 and powerpc ones
- makes PFN_PHYS return a phys_addr_t
- redefines resource_size_t as phys_addr_t
The first two are bugfixes and should be applied to git.
Thanks,
J
PATCH 1/1 make PFN_PHYS explicitly return phys_addr_t
PFN_PHYS, as its name suggests, turns a pfn into a physical address. However, it is a macro which just operates on its argument without modifying its type. pfns are typed unsigned long, but an unsigned long may not be long enough to hold a physical address (32-bit systems with more than 32 bits of physcial address).
Make sure we cast to phys_addr_t to return a complete result.
--- arch/m32r/mm/discontig.c | 4 ++-- include/asm-x86/xen/page.h | 4 ++-- include/linux/pfn.h | 6 +++++- 3 files changed, 9 insertions(+), 5 deletions(-)
arch/m32r/mm/discontig.c +2 -2
@@ -111,9 +111,9 @@ initrd_start, INITRD_SIZE); }else {printk ("initrd extends beyond end of memory " -"(0x%08lx > 0x%08lx)\ndisabling initrd\n" , +"(0x%08lx > 0x%08llx)\ndisabling initrd\n" ,INITRD_START +INITRD_SIZE , -PFN_PHYS (max_low_pfn)); + (unsigned long long )PFN_PHYS (max_low_pfn)); initrd_start =0 ; }
include/asm-x86/xen/page.h +2 -2
@@ -76,13 +76,13 @@static inline xmaddr_t phys_to_machine (xpaddr_t phys) {unsigned offset = phys.paddr & ~PAGE_MASK ; -return XMADDR (PFN_PHYS ((u64 )pfn_to_mfn (PFN_DOWN (phys.paddr))) | offset); +return XMADDR (PFN_PHYS (pfn_to_mfn (PFN_DOWN (phys.paddr))) | offset); }static inline xpaddr_t machine_to_phys (xmaddr_t machine) {unsigned offset = machine.maddr & ~PAGE_MASK ; -return XPADDR (PFN_PHYS ((u64 )mfn_to_pfn (PFN_DOWN (machine.maddr))) | offset); +return XPADDR (PFN_PHYS (mfn_to_pfn (PFN_DOWN (machine.maddr))) | offset); }/*
include/linux/pfn.h +5 -1
@@ -1,9 +1,13 @@#ifndef _LINUX_PFN_H_#define _LINUX_PFN_H_ + +#ifndef __ASSEMBLY__ +#include <linux/types.h> +#endif #define PFN_ALIGN (x) (((unsigned long)(x) + (PAGE_SIZE - 1)) & PAGE_MASK)#define PFN_UP (x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)#define PFN_DOWN (x) ((x) >> PAGE_SHIFT) -#define PFN_PHYS (x) ((x) << PAGE_SHIFT) +#define PFN_PHYS (x) ((phys_addr_t)(x) << PAGE_SHIFT)#endif
PATCH 1/1 add phys_addr_t for holding physical addresses
In reply to Jeremy FitzhardingeAdd a kernel-wide "phys_addr_t" which is guaranteed to be able to hold any physical address. By default it equals the word size of the architecture, but a 32-bit architecture can set ARCH_PHYS_ADDR_T_64BIT if it needs a 64-bit phys_addr_t.
--- arch/powerpc/Kconfig | 3 +++ arch/powerpc/include/asm/types.h | 7 ------- arch/x86/Kconfig | 3 +++ include/asm-x86/page_32.h | 2 -- include/asm-x86/page_64.h | 1 - include/linux/types.h | 6 ++++++ mm/Kconfig | 3 +++ 7 files changed, 15 insertions(+), 10 deletions(-)
arch/powerpc/Kconfig +3 -0
@@ -21,6 +21,9 @@ config PPC_MERGE def_bool y + +config ARCH_PHYS_ADDR_T_64BIT + def_bool PPC64 || PHYS_64BIT config MMU bool
arch/powerpc/include/asm/types.h +0 -7
@@ -48,13 +48,6 @@typedef __vector128 vector128 ; -/* Physical address used by some IO functions */ -#if defined(CONFIG_PPC64 ) || defined(CONFIG_PHYS_64BIT ) -typedef u64 phys_addr_t ; -#else -typedef u32 phys_addr_t ; -#endif -#ifdef __powerpc64__typedef u64 dma_addr_t ;#else
arch/x86/Kconfig +3 -0
@@ -938,6 +938,9 @@ has the cost of more pagetable lookup overhead, and also consumes more pagetable space per process. +config ARCH_PHYS_ADDR_T_64BIT + def_bool X86_64 || X86_PAE + # Common NUMA Features config NUMA bool "Numa Memory Allocation and Scheduler Support (EXPERIMENTAL)"
include/asm-x86/page_32.h +0 -2
@@ -33,7 +33,6 @@typedef u64 pudval_t ;typedef u64 pgdval_t ;typedef u64 pgprotval_t ; -typedef u64 phys_addr_t ; typedef union { struct {
@@ -54,7 +53,6 @@typedef unsigned long pudval_t ;typedef unsigned long pgdval_t ;typedef unsigned long pgprotval_t ; -typedef unsigned long phys_addr_t ; typedef union { pteval_t pte;
include/asm-x86/page_64.h +0 -1
@@ -79,7 +79,6 @@typedef unsigned long pudval_t ;typedef unsigned long pgdval_t ;typedef unsigned long pgprotval_t ; -typedef unsigned long phys_addr_t ;typedef struct page *pgtable_t ;
include/linux/types.h +6 -0
@@ -197,6 +197,12 @@typedef u32 resource_size_t ;#endif +#ifdef CONFIG_PHYS_ADDR_T_64BIT +typedef u64 phys_addr_t ; +#else +typedef u32 phys_addr_t ; +#endif +struct ustat {__kernel_daddr_t f_tfree;__kernel_ino_t f_tinode;
mm/Kconfig +3 -0
@@ -187,6 +187,9 @@ help This option allows memory and IO resources to be 64 bit. +config PHYS_ADDR_T_64BIT + def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT + config ZONE_DMA_FLAG int default "0" if !ZONE_DMA
PATCH 1/1 redefine resource_size_t as phys_addr_t
In reply to Jeremy FitzhardingeThere's no good reason why a resource_size_t shouldn't just be a physical address, so simply redefine it in terms of phys_addr_t.
--- arch/powerpc/platforms/Kconfig.cputype | 1 - arch/powerpc/sysdev/ppc4xx_pci.c | 16 ++++++---------- arch/x86/Kconfig | 1 - arch/x86/kernel/e820.c | 4 +--- drivers/pci/setup-bus.c | 9 ++++----- include/linux/types.h | 8 ++------ 6 files changed, 13 insertions(+), 26 deletions(-)
arch/powerpc/platforms/Kconfig.cputype +0 -1
@@ -135,7 +135,6 @@ config PHYS_64BIT bool 'Large physical address support' if E500 depends on 44x || E500 - select RESOURCES_64BIT default y if 44x ---help--- This option enables kernel support for larger than 32-bit physical
arch/powerpc/sysdev/ppc4xx_pci.c +6 -10
@@ -41,13 +41,10 @@#define U64_TO_U32_LOW (val) ((u32)((val) & 0x00000000ffffffffULL))#define U64_TO_U32_HIGH (val) ((u32)((val) >> 32)) -#ifdef CONFIG_RESOURCES_64BIT -#define RES_TO_U32_LOW (val) U64_TO_U32_LOW(val) -#define RES_TO_U32_HIGH (val) U64_TO_U32_HIGH(val) -#else -#define RES_TO_U32_LOW (val) (val) -#define RES_TO_U32_HIGH (val) (0) -#endif +#define RES_TO_U32_LOW (val) \ + ((sizeof(resource_size_t) > sizeof(u32)) ? U64_TO_U32_LOW(val) : (val)) +#define RES_TO_U32_HIGH (val) \ + ((sizeof(resource_size_t) > sizeof(u32)) ? U64_TO_U32_HIGH(val) : (0))static inline int ppc440spe_revA (void ) {
@@ -145,12 +142,11 @@/* Use that */ res->start = pci_addr; -#ifndef CONFIG_RESOURCES_64BIT /* Beware of 32 bits resources */ -if ((pci_addr + size) >0x100000000ull ) +if (sizeof (resource_size_t) ==sizeof (u32) && + (pci_addr + size) >0x100000000ull ) res->end =0xffffffff ;else -#endif res->end = res->start + size -1 ;break ; }
arch/x86/Kconfig +0 -1
@@ -931,7 +931,6 @@ def_bool n prompt "PAE (Physical Address Extension) Support" depends on X86_32 && !HIGHMEM4G - select RESOURCES_64BIT help PAE is required for NX support, and furthermore enables larger swapspace support for non-overcommit purposes. It
arch/x86/kernel/e820.c +1 -3
@@ -1276,12 +1276,10 @@ res =alloc_bootmem_low (sizeof (struct resource ) * e820.nr_map);for (i =0 ; i < e820.nr_map; i++) { end = e820.map[i].addr + e820.map[i].size -1 ; -#ifndef CONFIG_RESOURCES_64BIT -if (end >0x100000000ULL ) { +if (end != (resource_size_t )end) { res++;continue ; } -#endif res->name =e820_type_to_string (e820.map[i].type); res->start = e820.map[i].addr; res->end = end;
drivers/pci/setup-bus.c +4 -5
@@ -377,11 +377,10 @@ align =0 ; min_align =0 ;for (order =0 ; order <= max_order; order++) { -#ifdef CONFIG_RESOURCES_64BIT -resource_size_t align1 =1ULL << (order +20 ); -#else -resource_size_t align1 =1U << (order +20 ); -#endif +resource_size_t align1 =1 ; + + align1 <<= (order +20 ); +if (!align) min_align = align1;else if (ALIGN (align + min_align, min_align) < align1)
include/linux/types.h +2 -6
@@ -191,17 +191,13 @@ #ifdef __KERNEL__ typedef unsigned __bitwise__ gfp_t; -#ifdef CONFIG_RESOURCES_64BIT -typedef u64 resource_size_t; -#else -typedef u32 resource_size_t; -#endif - #ifdef CONFIG_PHYS_ADDR_T_64BIT typedef u64 phys_addr_t; #else typedef u32 phys_addr_t; #endif + +typedef phys_addr_t resource_size_t; struct ustat { __kernel_daddr_t f_tfree;
Andrew Morton
maintainerRe: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
In reply to Jeremy FitzhardingeQuoted 5 lines · On Tue, 19 Aug 2008 13:02:50 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Add a kernel-wide "
phys_addr_t" which is guaranteed to be able to hold any physical address. By default it equals the word size of the architecture, but a 32-bit architecture can setARCH_PHYS_ADDR_T_64BITif it needs a 64-bitphys_addr_t.
You say this is a bugfix but you don't describe the bug. This makes it rather hard to make the 2.6.2[5678] decisions.
Ditto on [patch 2/3].
Re: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
Quoted 11 lines · Andrew Morton
Quoted 6 lines · On Tue, 19 Aug 2008 13:02:50 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Add a kernel-wide "
phys_addr_t" which is guaranteed to be able to hold any physical address. By default it equals the word size of the architecture, but a 32-bit architecture can setARCH_PHYS_ADDR_T_64BITif it needs a 64-bitphys_addr_t.
Quoted 5 lines
You say this is a bugfix but you don't describe the bug. This makes it rather hard to make the 2.6.2[5678] decisions.
Ditto on [patch 2/3].
1/3 is not a bugfix in itself, but a pre-requisite for 2/3.
2/3 replaces an ad-hoc Xen fix with a general fix to prevent address truncation when using PFN_PHYS() on any PFN above the 4G mark. The Xen crash is the only bug I know of that's directly attributable to this, and it was already addressed in older kernels with the casts in the Xen code that this patch removes.
So I don't think there's any strong need to push this to earlier kernels.
J
Andrew Morton
maintainerRe: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
Quoted 20 lines · On Fri, 22 Aug 2008 14:11:16 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Quoted 11 lines · Andrew Morton
Quoted 6 lines · On Tue, 19 Aug 2008 13:02:50 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Add a kernel-wide "
phys_addr_t" which is guaranteed to be able to hold any physical address. By default it equals the word size of the architecture, but a 32-bit architecture can setARCH_PHYS_ADDR_T_64BITif it needs a 64-bitphys_addr_t.Quoted 5 lines
You say this is a bugfix but you don't describe the bug. This makes it rather hard to make the 2.6.2[5678] decisions.
Ditto on [patch 2/3].
1/3 is not a bugfix in itself, but a pre-requisite for 2/3.
Quoted 8 lines
2/3 replaces an ad-hoc Xen fix with a general fix to prevent address truncation when using
PFN_PHYS()on any PFN above the 4G mark. The Xen crash is the only bug I know of that's directly attributable to this, and it was already addressed in older kernels with the casts in the Xen code that this patch removes.So I don't think there's any strong need to push this to earlier kernels.
Still confused. The above implies that 2.6.27 doesn't need fixing either, because the typecasts already avoid the crash.
Re: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
Quoted 32 lines · Andrew Morton
Quoted 28 lines · On Fri, 22 Aug 2008 14:11:16 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Quoted 12 lines · Andrew Morton
Quoted 7 lines · On Tue, 19 Aug 2008 13:02:50 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
Add a kernel-wide "
phys_addr_t" which is guaranteed to be able to hold any physical address. By default it equals the word size of the architecture, but a 32-bit architecture can setARCH_PHYS_ADDR_T_64BITif it needs a 64-bitphys_addr_t.Quoted 6 lines
You say this is a bugfix but you don't describe the bug. This makes it rather hard to make the 2.6.2[5678] decisions.
Ditto on [patch 2/3].
1/3 is not a bugfix in itself, but a pre-requisite for 2/3.
2/3 replaces an ad-hoc Xen fix with a general fix to prevent address truncation when using
PFN_PHYS()on any PFN above the 4G mark. The Xen crash is the only bug I know of that's directly attributable to this, and it was already addressed in older kernels with the casts in the Xen code that this patch removes.
Quoted 7 lines
Quoted 3 lines
So I don't think there's any strong need to push this to earlier kernels.
Still confused. The above implies that 2.6.27 doesn't need fixing either, because the typecasts already avoid the crash.
Yes, that's true. It's a bit more of a comprehensive and correct fix; I think it's fairly low risk at this point in the -rc series, but it could be deferred (or just defer 3/3, which really is cosmetic).
J
Re: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
In reply to Jeremy Fitzhardinge.../...
include/asm-x86/page_32.h line 36
Quoted 8 lines
include/asm-x86/page_32.h +0
-1@@ -33,7 +33,6 @@typedef u64 pudval_t ;typedef u64 pgdval_t ;typedef u64 pgprotval_t ; -typedef u64 phys_addr_t ;
.../...
Might sound a stupid question, but why have a CONFIG_ option and a global definition based on it rather than each arch defining it as part of the base types ? I don't have a firm preference for one or the other at this point, I can see pro and cons to both approach, so I'm curious to see what others think about it.
Cheers,
Ben.
Re: [PATCH 1 of 3] add phys_addr_t for holding physical addresses
Quoted 15 lines · Benjamin Herrenschmidt
.../...
include/asm-x86/page_32.h line 36
Quoted 9 lines
include/asm-x86/page_32.h +0
-1@@ -33,7 +33,6 @@typedef u64 pudval_t ;typedef u64 pgdval_t ;typedef u64 pgprotval_t ; -typedef u64 phys_addr_t ;.../...
Quoted 5 lines
Might sound a stupid question, but why have a
CONFIG_option and a global definition based on it rather than each arch defining it as part of the base types ? I don't have a firm preference for one or the other at this point, I can see pro and cons to both approach, so I'm curious to see what others think about it.
My thinking is that:
There's only two possible types it can have: u32 and u64. If we leave it to per-arch definitions, they'll come up with a variety of different ways of spelling those types (like u64 itself, but I gather that's being fixed).
Furthermore, with only a couple of exceptions, the size is the same as the bitness of the architecture, so there's no need to set a config in most cases.
So, avoiding lots of duplication, basically.
J