Re: 2.6.27-rc1 >4096MB issue

[Jeremy Fitzhardinge <jeremy@goop.org>]

Carl Jones wrote:
> On Sun, Aug 3, 2008 at 4:00 AM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>   
>> Carl Jones wrote:
>>     
>>> http://radium.outervoid.net/~carl/boot.log
>>> http://radium.outervoid.net/~carl/xmlist.log ('xm list test1 --long'
>>> output, in case that is helpful)
>>>
>>>       
>> Thanks.  Does this fix it?
>>
>> iff -r 25bf2d9a2e4c arch/x86/xen/setup.c
>> --- a/arch/x86/xen/setup.c      Fri Aug 01 17:12:18 2008 -0700
>> +++ b/arch/x86/xen/setup.c      Sat Aug 02 09:00:02 2008 -0700
>> @@ -42,7 +42,7 @@
>>
>>        e820.nr_map = 0;
>>
>> -       e820_add_region(0, PFN_PHYS(max_pfn), E820_RAM);
>> +       e820_add_region(0, PFN_PHYS((u64)max_pfn), E820_RAM);
>>
>>        /*
>>         * Even though this is normal, usable memory under Xen, reserve
>>
>>
>>   J
>>
>>     
>
> Yep works nicely now. I tested up to 15GB or so with with that patch
> applied and CONFIG_XEN_MAX_DOMAIN_MEMORY=32 set:
>
> testing:~# cat /proc/meminfo
> MemTotal:     15769832 kB
>   

Excellent, thanks.  That's 32-bit?  (That's a pretty silly amount of 
memory to give to a 32-bit system, but it's nice to know it works.)

Could you try this patch instead to see if it works?  It's a more 
general fix.

Thanks,
    J


Subject: make PFN_PHYS explicitly return 64-bit result

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).  This means that the
resulting address could be truncated if it doesn't fit within an
unsigned long.  This isn't generally a problem because most users end
up using it for "low" memory, but there's no reason why PFN_PHYS
couldn't be used for any possible pfn.

Unfortunately there's no univerally recognized type for holding a full
physical address, so this patch makes it always return a u64 result.
In theory this could generate worse code, but in practice it will make
no difference:
- most users end up casting the result to a pointer or unsigned long,
  so on 32-bit systems the compiler should be able to eliminate the 64
  bit part of the expression.
- most users are in init code, which is neither size or performance
  critical

This patch also introduces PFN_LOW_PHYS which explicitly operates on
an unsigned long.  It is currently unused, but it could be used to
document the fact that the caller expects that the result will fit
into an unsigned long.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 arch/m32r/mm/discontig.c   |    2 +-
 include/asm-x86/xen/page.h |    4 ++--
 include/linux/pfn.h        |    3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

===================================================================
--- a/arch/m32r/mm/discontig.c
+++ b/arch/m32r/mm/discontig.c
@@ -112,7 +112,7 @@
 				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));
 
===================================================================
--- a/include/asm-x86/xen/page.h
+++ b/include/asm-x86/xen/page.h
@@ -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);
 }
 
 /*
===================================================================
--- a/include/linux/pfn.h
+++ b/include/linux/pfn.h
@@ -4,6 +4,7 @@
 #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)	((u64)(x) << PAGE_SHIFT)
+#define PFN_LOW_PHYS(x)	((unsigned long)(x) << PAGE_SHIFT)
 
 #endif