[Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Aurelien Jarno]

Revision: 4239
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=4239
Author:   aurel32
Date:     2008-04-22 20:37:43 +0000 (Tue, 22 Apr 2008)

Log Message:
-----------
x86/x86-64 MMU PAE fixes

This patch fixes MMU emulation in PAE mode for > 4GB physical addresses:
- a20_mask should have the correct size to not clear the high part of
  the addresses.
- PHYS_ADDR_MASK should not clear the high part of the addresses.
- pdpe, pde and pte could be located anywhere in memory on x86-64, but
  only in the first 4GB on x86, define their pointer to as target_ulong.
- pml4e_addr could be located anywhere in memory, define its pointer
  as uint64_t.
- paddr represents a physical address and thus should be of type
  target_phys_addr_t.

Modified Paths:
--------------
    trunk/target-i386/cpu.h
    trunk/target-i386/helper2.c

Modified: trunk/target-i386/cpu.h
===================================================================
--- trunk/target-i386/cpu.h	2008-04-22 20:37:34 UTC (rev 4238)
+++ trunk/target-i386/cpu.h	2008-04-22 20:37:43 UTC (rev 4239)
@@ -499,7 +499,7 @@
     SegmentCache idt; /* only base and limit are used */
 
     target_ulong cr[9]; /* NOTE: cr1, cr5-7 are unused */
-    uint32_t a20_mask;
+    uint64_t a20_mask;
 
     /* FPU state */
     unsigned int fpstt; /* top of stack index */

Modified: trunk/target-i386/helper2.c
===================================================================
--- trunk/target-i386/helper2.c	2008-04-22 20:37:34 UTC (rev 4238)
+++ trunk/target-i386/helper2.c	2008-04-22 20:37:43 UTC (rev 4239)
@@ -377,7 +377,7 @@
     env->hflags |= HF_GIF_MASK;
 
     cpu_x86_update_cr0(env, 0x60000010);
-    env->a20_mask = 0xffffffff;
+    env->a20_mask = ~0x0;
     env->smbase = 0x30000;
 
     env->idt.limit = 0xffff;
@@ -695,7 +695,7 @@
         /* when a20 is changed, all the MMU mappings are invalid, so
            we must flush everything */
         tlb_flush(env, 1);
-        env->a20_mask = 0xffefffff | (a20_state << 20);
+        env->a20_mask = (~0x100000) | (a20_state << 20);
     }
 }
 
@@ -800,7 +800,7 @@
 
 #else
 
-#define PHYS_ADDR_MASK 0xfffff000
+#define PHYS_ADDR_MASK (~0xfff)
 
 /* return value:
    -1 = cannot handle fault
@@ -812,9 +812,10 @@
                              int is_write1, int mmu_idx, int is_softmmu)
 {
     uint64_t ptep, pte;
-    uint32_t pdpe_addr, pde_addr, pte_addr;
+    target_ulong pde_addr, pte_addr;
     int error_code, is_dirty, prot, page_size, ret, is_write, is_user;
-    unsigned long paddr, page_offset;
+    target_phys_addr_t paddr;
+    uint32_t page_offset;
     target_ulong vaddr, virt_addr;
 
     is_user = mmu_idx == MMU_USER_IDX;
@@ -834,12 +835,11 @@
 
     if (env->cr[4] & CR4_PAE_MASK) {
         uint64_t pde, pdpe;
+        target_ulong pdpe_addr;
 
-        /* XXX: we only use 32 bit physical addresses */
 #ifdef TARGET_X86_64
         if (env->hflags & HF_LMA_MASK) {
-            uint32_t pml4e_addr;
-            uint64_t pml4e;
+            uint64_t pml4e_addr, pml4e;
             int32_t sext;
 
             /* test virtual address sign extension */
@@ -1101,17 +1101,19 @@
 
 target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
 {
-    uint32_t pde_addr, pte_addr;
-    uint32_t pde, pte, paddr, page_offset, page_size;
+    target_ulong pde_addr, pte_addr;
+    uint64_t pte;
+    target_phys_addr_t paddr;
+    uint32_t page_offset;
+    int page_size;
 
     if (env->cr[4] & CR4_PAE_MASK) {
-        uint32_t pdpe_addr, pde_addr, pte_addr;
-        uint32_t pdpe;
+        target_ulong pdpe_addr;
+        uint64_t pde, pdpe;
 
-        /* XXX: we only use 32 bit physical addresses */
 #ifdef TARGET_X86_64
         if (env->hflags & HF_LMA_MASK) {
-            uint32_t pml4e_addr, pml4e;
+            uint64_t pml4e_addr, pml4e;
             int32_t sext;
 
             /* test virtual address sign extension */
@@ -1121,13 +1123,13 @@
 
             pml4e_addr = ((env->cr[3] & ~0xfff) + (((addr >> 39) & 0x1ff) << 3)) &
                 env->a20_mask;
-            pml4e = ldl_phys(pml4e_addr);
+            pml4e = ldq_phys(pml4e_addr);
             if (!(pml4e & PG_PRESENT_MASK))
                 return -1;
 
             pdpe_addr = ((pml4e & ~0xfff) + (((addr >> 30) & 0x1ff) << 3)) &
                 env->a20_mask;
-            pdpe = ldl_phys(pdpe_addr);
+            pdpe = ldq_phys(pdpe_addr);
             if (!(pdpe & PG_PRESENT_MASK))
                 return -1;
         } else
@@ -1135,14 +1137,14 @@
         {
             pdpe_addr = ((env->cr[3] & ~0x1f) + ((addr >> 27) & 0x18)) &
                 env->a20_mask;
-            pdpe = ldl_phys(pdpe_addr);
+            pdpe = ldq_phys(pdpe_addr);
             if (!(pdpe & PG_PRESENT_MASK))
                 return -1;
         }
 
         pde_addr = ((pdpe & ~0xfff) + (((addr >> 21) & 0x1ff) << 3)) &
             env->a20_mask;
-        pde = ldl_phys(pde_addr);
+        pde = ldq_phys(pde_addr);
         if (!(pde & PG_PRESENT_MASK)) {
             return -1;
         }
@@ -1155,9 +1157,11 @@
             pte_addr = ((pde & ~0xfff) + (((addr >> 12) & 0x1ff) << 3)) &
                 env->a20_mask;
             page_size = 4096;
-            pte = ldl_phys(pte_addr);
+            pte = ldq_phys(pte_addr);
         }
     } else {
+        uint32_t pde;
+
         if (!(env->cr[0] & CR0_PG_MASK)) {
             pte = addr;
             page_size = 4096;

Re: [Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Paul Brook]

On Tuesday 22 April 2008, Aurelien Jarno wrote:
> -#define PHYS_ADDR_MASK 0xfffff000
> +#define PHYS_ADDR_MASK (~0xfff)

I think this is wrong. According to my docs physical addresses have an 
architectural limit of 52 bits. Bits 52-62 of a PTE are reserved (must be 
zero), and bit 63 is the NX bit.

You already reverted the following, but I'm going to mention it anyway:

> -#elif defined(TARGET_X86_64) && !defined(USE_KQEMU)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 40

I think this should either be 52, or cpu specific.

Paul

Re: [Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Stuart Brady]

On Tue, Apr 22, 2008 at 09:57:12PM +0100, Paul Brook wrote:
> On Tuesday 22 April 2008, Aurelien Jarno wrote:
> > -#define PHYS_ADDR_MASK 0xfffff000
> > +#define PHYS_ADDR_MASK (~0xfff)
> 
> I think this is wrong. According to my docs physical addresses have an 
> architectural limit of 52 bits. Bits 52-62 of a PTE are reserved (must be 
> zero), and bit 63 is the NX bit.

The documentation I'm using:

"Intel 64 and IA-32 Architectures Software Development Manual,
 Volume 3A: System Programming Guide, Part 1"

"3.8 36-Bit Physical Addressing Using The PAE Paging Mechanism"

  Lists bits 36-63 as "must be zero".
  (For i386, or x86_64 running in 32-bit mode.)

"3.10 PAE-Enabled Paging in IA-32e Mode"

  Lists bits 40-51 as "must be zero".
  Lists bits 52 to 62 as "available".
  Lists bit 63 as "EXB" (i.e. the NX bit).
  (For x86_64 running in 64-bit mode.)

HTH,
-- 
Stuart Brady

Re: [Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Paul Brook]

On Tuesday 22 April 2008, Stuart Brady wrote:
> On Tue, Apr 22, 2008 at 09:57:12PM +0100, Paul Brook wrote:
> > On Tuesday 22 April 2008, Aurelien Jarno wrote:
> > > -#define PHYS_ADDR_MASK 0xfffff000
> > > +#define PHYS_ADDR_MASK (~0xfff)
> >
> > I think this is wrong. According to my docs physical addresses have an
> > architectural limit of 52 bits. Bits 52-62 of a PTE are reserved (must be
> > zero), and bit 63 is the NX bit.
>
> The documentation I'm using:
>
> "Intel 64 and IA-32 Architectures Software Development Manual,
>  Volume 3A: System Programming Guide, Part 1"
>
> "3.8 36-Bit Physical Addressing Using The PAE Paging Mechanism"
>
>   Lists bits 36-63 as "must be zero".

This disagrees with the AMD docs, which allow full 52-bit addresses in legacy 
mode. This may be a historical thing.

> "3.10 PAE-Enabled Paging in IA-32e Mode"
>
>   Lists bits 40-51 as "must be zero".

The AMD docs define a 52-bit physical address space. I'm not sure what the 
behavior is on CPUs that only implement a smaller physical address bus.

>   Lists bits 52 to 62 as "available".

The AMD docs, and some versions of the Intel docs list these as must be zero.
This may be annother case of Intel screwing up the architecture. I wouldn't be 
surprised if future CPUs define these bits to have some meaning.

Paul

Re: [Qemu-devel] [4239] x86/x86-64 MMU PAE fixes

[Stuart Brady]

On Tue, Apr 22, 2008 at 11:19:24PM +0100, Stuart Brady wrote:
> The documentation I'm using:
> 
> "Intel 64 and IA-32 Architectures Software Development Manual,
>  Volume 3A: System Programming Guide, Part 1"

...

"AMD64 Architecture Programmer's Manual, Volume 2: System Programming"
gives a different story.

Only bits 52-63 are reserved (MBZ), although for PDEs and PTEs (but not
PDPEs, it would seem), bit 63 is the NX bit.

However, "The AMD x86-64 Architecture Programmers Overview" says that
"bits 63:52 in all page-table entry formats are available for use by
system software".
-- 
Stuart Brady