Re: [PATCH] Make PFN_PHYS return a properly-formed physical address
From: Jeremy Fitzhardinge
Date: Thu Aug 07 2008 - 19:45:38 EST
Andrew Morton wrote:
Yes, but resource_size_t is for IO addressing, not for memory addressing.
Lots of X86_32 machines can happily support 32-bit physical addresses
for IO while needing >32 bit addresses for physical memory.
Really? The resource tree treats normal memory as just another
resource. Is it expected that you could have usable memory not
represented by /proc/iomem?
Hm, looks like memory hotplug assumes that resource_size_t is always
64-bits, but the e820->resource conversion simply skips over-large
addresses.
#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) ((resource_size_t)(x) << PAGE_SHIFT)
Busted on PAE with CONFIG_RESOURCES_64BIT=n, surely?
Not an option:
config X86_PAE
def_bool n
prompt "PAE (Physical Address Extension) Support"
depends on X86_32 && !HIGHMEM4G
select RESOURCES_64BIT
err, OK, that was a bit arbitrary of us.
Oh well, scrub the above assertion.
Then again, do all architectures disallow 32-bit resource_size_t on
64-bit? And there's ppc32's CONFIG_HIGHMEM option to think about.
x86 and ppc were the only archs to touch it; they otherwise use the
default of "default 64BIT".
I didn't look at the ppc use case. I wasn't terribly concerned about
current users of PFN_PHYS, because it presumably works OK for them.
"Properly" would be to define a phys_addr_t which can always represent a
physical address. We have one in x86-land, but I hesitate to add it for
everyone else.
hm. It is a distinct and singular concept - it makes sense to have a
specific type to represet "a physical address for memory".
Yes. We had to be particularly careful with it on x86 because of all
the problems it's caused, but it is a generally useful thing to be able
to talk about.
Shall we go with just using plain u64 (or unsigned long long if we want
a really consistent type) in the meantime, and then waffle about
introducing a new type everywhere?
Or we could redefine resource_size_t to be big enough to refer to any
resource, including all memory. It's close to being that anyway.
nope ;) We don't know what type u64 has - some architectures use
`unsigned long' (we might fix this soon).
For now, a full cast to `unsigned long long' is needed.
Yep.
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/