pae: tlbflush linear page table updates

Hi,

On l3 page dir updates (and related linear page table updates) we'll have to flush the tlb to make sure the linear page access goes to the correct pages ...

Gerd

@@ -737,6 +737,7 @@ static int create_pae_xen_mappings(l3_pg
             l2e_from_pfn(l3e_get_pfn(pl3e[i]), __PAGE_HYPERVISOR) :
             l2e_empty();
     unmap_domain_page(pl2e);
+    flush_tlb_all();
 
     return 1;
 }

Have you actually observed problems without the flush?

Otherwise I think a bigger audit of linear mappings may be required and I'll hold off adding flushes until we do that.

-- Keir

Yes. PAE xenlinux (UP) didn't boot up. The first make_page_readonly() faults because the first copy_from_user() in ptwr_do_page_fault() fails (which dereferences stuff using the linear page table ...).

I hoped the tlbflush also fixes the SMP issues, but it doesn't, so there are more bugs. I'm not sure the PAE/SMP crashes are ptwr related, they also happen (although a bit less likely it seems) without writable page tables.

Gerd

panic("it works"); /* avoid being flooded with debug messages */

Out of interest, can you please try moving the flush_tlb_all() into mod_l3_entry(), at the following place:

        if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e)) )
        {
            BUG_ON(!create_pae_xen_mappings(pl3e));
            flush_tlb_all(); /* <------- moved to here */
            put_page_from_l3e(nl3e, pfn);
            return 0;
        }

Hopefully that will work just as well.

-- Keir

Actually, I spotted that create_pae_xen_mappings() was being called from completely the wrong paths in mod_l3_entry(). I've now fixed that -- it's possible that will fix your problems with no need for extra flushes. :-)

-- Keir

Hi,

Machine crashes, below is the log with "printk-via-hypercall-consoleio" hack. It's not the first make_page_readonly() call though, so it's some other bug.

It is hg95d2bbf6a273d053e8876944a803b80c086e0b3e.

Gerd

__ __ _____ ___ _ _

 \ \/ /___ _ __   |___ / / _ \     __| | _____   _____| |
  \  // _ \ '_ \    |_ \| | | |__ / _` |/ _ \ \ / / _ \ |
  /  \  __/ | | |  ___) | |_| |__| (_| |  __/\ V /  __/ |
 /_/\_\___|_| |_| |____(_)___/    \__,_|\___| \_/ \___|_|

http://www.cl.cam.ac.uk/netos/xen
University of Cambridge Computer Laboratory

Xen version 3.0-devel ([email protected]) (gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)) Tue Aug 9 15:36:24 CEST 2005 Latest ChangeSet: Tue Aug 9 15:08:25 2005 95d2bbf6a273d053e8876944a803b80c086e0b3e

(XEN) Truncating memory map to 1007616kB
(XEN) Physical RAM map:

(XEN)  0000000000000000 - 000000000009fc00 (usable)
(XEN)  000000000009fc00 - 00000000000a0000 (reserved)
(XEN)  00000000000e6000 - 0000000000100000 (reserved)
(XEN)  0000000000100000 - 000000003d800000 (usable)

(XEN) System RAM: 983MB (1007228kB)
(XEN) Xen heap: 10MB (10452kB)
(XEN) PAE enabled, limit: 16 GB
(XEN) found SMP MP-table at 000ff780
(XEN) DMI 2.3 present.
(XEN) Using APIC driver default
(XEN) ACPI: RSDP (v000 ACPIAM ) @ 0x000f55c0
(XEN) ACPI: RSDT (v001 INTEL @ÃS¸¨�š
(XEN) 0x20050420 MSFT 0x00000097) @ 0x3f630000
(XEN) ACPI: FADT (v002 INTEL @ÃS¸¨�š
(XEN) 0x20050420 MSFT 0x00000097) @ 0x3f630200
(XEN) ACPI: MADT (v001 INTEL @ÃS¸¨�š
(XEN) 0x20050420 MSFT 0x00000097) @ 0x3f630390
(XEN) ACPI: MCFG (v001 INTEL @ÃS¸¨�š
(XEN) 0x20050420 MSFT 0x00000097) @ 0x3f630400
(XEN) ACPI: MSEG (v001 INTEL @ÃS¸¨�š
(XEN) 0x20050420 MSFT 0x00000097) @ 0x3f630440
(XEN) ACPI: WDDT (v001 INTEL OEMWDDT 0x00000001 INTL 0x02002026) @ 0x3f6363c0
(XEN) ACPI: DSDT (v001 INTEL @ÃS¸¨�š
(XEN) 0x00000001 INTL 0x02002026) @ 0x00000000
(XEN) ACPI: Local APIC address 0xfee00000
(XEN) ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
(XEN) Processor #0 15:4 APIC version 20
(XEN) ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
(XEN) Processor #1 15:4 APIC version 20
(XEN) ACPI: LAPIC_NMI (acpi_id[0x01] dfl dfl lint[0x1])
(XEN) ACPI: LAPIC_NMI (acpi_id[0x02] dfl dfl lint[0x1])
(XEN) ACPI: IOAPIC (id[0x02] address[0xfec00000] gsi_base[0])
(XEN) IOAPIC[0]: apic_id 2, version 32, address 0xfec00000, GSI 0-23
(XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
(XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
(XEN) ACPI: IRQ0 used by override.
(XEN) ACPI: IRQ2 used by override.
(XEN) ACPI: IRQ9 used by override.
(XEN) Enabling APIC mode: Flat. Using 1 I/O APICs
(XEN) Using ACPI (MADT) for SMP configuration information
(XEN) Initializing CPU#0
(XEN) Detected 3793.084 MHz processor.
(XEN) Using scheduler: Simple EDF Scheduler (sedf)
(XEN) CPU: Trace cache: 12K uops, L1 D cache: 16K
(XEN) CPU: L2 cache: 2048K
(XEN) CPU: Physical Processor ID: 0
(XEN) VMXON is done
(XEN) CPU0: Intel Genuine Intel(R) CPU 3.80GHz stepping 06
(XEN) Booting processor 1/1 eip 90000
(XEN) Initializing CPU#1
(XEN) CPU: Trace cache: 12K uops, L1 D cache: 16K
(XEN) CPU: L2 cache: 2048K
(XEN) CPU: Physical Processor ID: 0
(XEN) VMXON is done
(XEN) CPU1: Intel Genuine Intel(R) CPU 3.80GHz stepping 06
(XEN) Total of 2 processors activated.
(XEN) ENABLING IO-APIC IRQs
(XEN) ..TIMER: vector=0x31 pin1=2 pin2=-1
(XEN) checking TSC synchronization across 2 CPUs: passed.
(XEN) Platform timer is 1.193MHz PIT
(XEN) Brought up 2 CPUs
(XEN) mtrr: v2.0 (20020519)
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Xen-ELF header found: 'GUEST_OS=linux,GUEST_VER=2.6,XEN_VER=3.0,VIRT_BASE=0xC0000000,PAE=yes,LOADER=generic' (XEN) PHYSICAL MEMORY ARRANGEMENT:
(XEN) Dom0 alloc.: 10000000->20000000 (131072 pages to be allocated)
(XEN) VIRTUAL MEMORY ARRANGEMENT:

(XEN)  Loaded kernel: c0100000->c051d144
(XEN)  Init. ramdisk: c051e000->c051e000
(XEN)  Phys-Mach map: c051e000->c05de000
(XEN)  Page tables:   c05de000->c05e7000
(XEN)  Start info:    c05e7000->c05e8000
(XEN)  Boot stack:    c05e8000->c05e9000
(XEN)  TOTAL:         c0000000->c0800000
(XEN)  ENTRY ADDRESS: c0100000

(XEN) Scrubbing Free RAM: ...........done.
(XEN) *** Serial input -> DOM0 (type 'CTRL-a' three times to switch input to Xen).
<5>Linux version 2.6.12.3-xen0-hg95d2bbf6a273d053e8876944a803b80c086e0b3e (kraxel@eskarina) (gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)) #2 Tue Aug 9 15:38:52 CEST 2005 <6>BIOS-provided physical RAM map:
Xen: 0000000000100000 - 0000000030000000 (usable)
<5>40MB HIGHMEM available.
<5>728MB LOWMEM available.
NX (Execute Disable) protection: active
<7>On node 0 totalpages: 196608

<7>  DMA zone: 186368 pages, LIFO batch:31
<7>  Normal zone: 0 pages, LIFO batch:1
<7>  HighMem zone: 10240 pages, LIFO batch:3

<6>found SMP MP-table at 000ff780
<6>DMI 2.3 present.
<7>ACPI: RSDP (v000 ACPIAM ) @ 0x000f55c0
<7>ACPI: RSDT (v001 INTEL @ÃS¸¨�š
0x20050420 MSFT 0x00000097) @ 0x3f630000
<7>ACPI: FADT (v002 INTEL @ÃS¸¨�š
0x20050420 MSFT 0x00000097) @ 0x3f630200
<7>ACPI: MADT (v001 INTEL @ÃS¸¨�š
0x20050420 MSFT 0x00000097) @ 0x3f630390
<7>ACPI: MCFG (v001 INTEL @ÃS¸¨�š
0x20050420 MSFT 0x00000097) @ 0x3f630400
<7>ACPI: MSEG (v001 INTEL @ÃS¸¨�š
0x20050420 MSFT 0x00000097) @ 0x3f630440
<7>ACPI: WDDT (v001 INTEL OEMWDDT 0x00000001 INTL 0x02002026) @ 0x3f6363c0 <7>ACPI: DSDT (v001 INTEL @ÃS¸¨�š
0x00000001 INTL 0x02002026) @ 0x00000000
<7>ACPI: Local APIC address 0xfee00000
<6>ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
<6>ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
<6>ACPI: LAPIC_NMI (acpi_id[0x01] dfl dfl lint[0x1])
<6>ACPI: LAPIC_NMI (acpi_id[0x02] dfl dfl lint[0x1])
<6>ACPI: IOAPIC (id[0x02] address[0xfec00000] gsi_base[0])
IOAPIC[0]: apic_id 2, version 32, address 0xfec00000, GSI 0-23
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
<7>ACPI: IRQ0 used by override.
<7>ACPI: IRQ2 used by override.
<7>ACPI: IRQ9 used by override.
Enabling APIC mode: Flat. Using 1 I/O APICs
<6>Using ACPI (MADT) for SMP configuration information
<6>IRQ lockup detection disabled
Allocating PCI resources starting at 30000000 (gap: 30000000:d0000000) Built 1 zonelists
<5>Kernel command line: console=tty0 console=ttyS0 root=/dev/sda2 ro selinux=0 sysrq=yes <6>Initializing CPU#0
(XEN) DOM0: (file=mm.c, line=1444) Bad type (saw f0000001!= exp a0000000) for pfn 10103 kernel BUG at arch/xen/i386/kernel/cpu/common.c:576 (cpu_gdt_init)!

 [<c04812a7>] cpu_gdt_init+0xa7/0xc0
 [<c021982f>] sort+0x11f/0x1f0
 [<c048137f>] cpu_init+0xbf/0x250
 [<c0213ee0>] cmp_ex+0x0/0x20
 [<c047878c>] start_kernel+0x9c/0x1e0
 [<c0478390>] unknown_bootoption+0x0/0x210

<0>Kernel panic - not syncing: BUG!
(XEN) Domain 0 shutdown: rebooting machine.

Hmm, that happens when xen is build with debug=y only, without that it crashes much earlier ...

Gerd

Weird. The calls to create_pae_xen_mappings were definitely on the error paths in mod_l3_entry(), which is obviously wrong. I'm surprised that fixing it would make things worse, unless some other patch in the meantime has screwed pae...

I'll try and find some time to look at pae myself and see if I can help out.

-- Keir

No, it's actually the changeset 6056:a1f7e01b0990a378584e718e6d48eac38824fdb9 which broke it.

The create_pae_xen_mappings() call in the error path is broken indead. That must have sneaked in somewhen, I'm pretty sure I wrote that initially as something like

         if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e)) ||
	      !create_pae_xen_mappings(pl3e) )
         {
             put_page_from_l3e(nl3e, pfn);
             return 0;
         }

so create_pae_xen_mappings() failure (due to the guest OS trying illegal things) will *trigger* the error path. Strange it never showed up, maybe linux never ever updates l3 entries after creating them.

BUG_ON(!create_pae_xen_mappings()) is a bad idea, it _can_ fail, the failure should just be propagated up (so in the end the hypercall running into this returns some errror) or the domain should simply be killed ...

Gerd

panic("it works"); /* avoid being flooded with debug messages */

And it isn't the mod_l3 update which broke PAE, it's the __not_mapped() removal. Applying the bits below reversed makes PAE boot again.

Gerd

diff -r 663f0fb1e444 -r a1f7e01b0990 xen/arch/x86/mm.c
@@ -2902,42 +2887,13 @@
     .cmpxchg8b_emulated = ptwr_emulated_cmpxchg8b
 };
 
-#if defined(__x86_64__)
-/*
- * Returns zero on if mapped, or -1 otherwise
- */
-static int __not_mapped(l2_pgentry_t *pl2e)
-{
-    unsigned long page = read_cr3();
-
-    page &= PAGE_MASK;
-    page = ((unsigned long *) __va(page))[l4_table_offset((unsigned long)pl2e)];
-    if ( !(page & _PAGE_PRESENT) ) 
-        return -1;        
-        
-    page &= PAGE_MASK;
-    page = ((unsigned long *) __va(page))[l3_table_offset((unsigned long)pl2e)];
-    if ( !(page & _PAGE_PRESENT) ) 
-        return -1;
-
-    page &= PAGE_MASK;
-    page = ((unsigned long *) __va(page))[l2_table_offset((unsigned long)pl2e)];
-    if ( !(page & _PAGE_PRESENT) )
-        return -1;
-
-    return 0;
-}
-#else
-#define __not_mapped(p) (0)
-#endif
-
 /* Write page fault handler: check if guest is trying to modify a PTE. */
 int ptwr_do_page_fault(struct domain *d, unsigned long addr)
 {
     unsigned long    pfn;
     struct pfn_info *page;
     l1_pgentry_t     pte;
-    l2_pgentry_t    *pl2e;
+    l2_pgentry_t    *pl2e, l2e;
     int              which;
     unsigned long    l2_idx;
 

@@ -2984,10 +2940,7 @@
     pl2e = &__linear_l2_table[l2_idx];
     which = PTWR_PT_INACTIVE;
 
-    if ( unlikely(__not_mapped(pl2e)) )
-        goto inactive;
-
-    if ( (l2e_get_pfn(*pl2e)) == pfn )
+    if ( (__get_user(l2e.l2, &pl2e->l2) == 0) && (l2e_get_pfn(l2e) == pfn) )
     {
         /*
          * Check the PRESENT bit to set ACTIVE mode.

@@ -2995,13 +2948,11 @@
          * ACTIVE p.t. (it may be the same p.t. mapped at another virt addr).
          * The ptwr_flush call below will restore the PRESENT bit.
          */
-        if ( likely(l2e_get_flags(*pl2e) & _PAGE_PRESENT) ||
+        if ( likely(l2e_get_flags(l2e) & _PAGE_PRESENT) ||
              (d->arch.ptwr[PTWR_PT_ACTIVE].l1va &&
               (l2_idx == d->arch.ptwr[PTWR_PT_ACTIVE].l2_idx)) )
             which = PTWR_PT_ACTIVE;
     }
-
-  inactive:
 
     /*
      * If this is a multi-processor guest then ensure that the page is hooked

I think it preallocates all the L2's so mod_l3_entry probably isn't used. But yes: it looks like I broke it. :-)

The checks that can fail only really belong in alloc_l3_table. An already typed l3 should not need those checks on its highest l3e (mod_l3_entry explicitly disallows meddling with that entry).

-- Keir

afaics the only change for pae is use of __get_user() rather than directly reading from the pl2e. Perhaps __get_user() on 8-byte quantities is broken, although I'm sure that must be used elsewhere and so is tested....

-- Keir

Sure? The patch below fixes it for me.

The tlbflush stuff is red herring btw, the real difference is optimization. Build with "optimize=n" boot fine, others don't.

Gerd

Index: xen/arch/x86/mm.c
===================================================================
@@ -2940,7 +2940,7 @@ int ptwr_do_page_fault(struct domain *d,
     pl2e = &__linear_l2_table[l2_idx];
     which = PTWR_PT_INACTIVE;
 
-    if ( (__get_user(l2e.l2, &pl2e->l2) == 0) && (l2e_get_pfn(l2e) == pfn) )
+    if ( (__copy_from_user(&l2e, pl2e, sizeof(l2e)) == 0) && (l2e_get_pfn(l2e) == pfn) )
     {
         /*
          * Check the PRESENT bit to set ACTIVE mode.

Hi,

Uhm, well, it doesn't really fix it.

While looking into this I've noticed the bug comes and goes away with the optimization level. Building with -O1 works fine, building with -O2 breaks (both with the patch mailed applied).

There might be something wrong with the inline assembler ...

Gerd

panic("it works"); /* avoid being flooded with debug messages */

See the fix I just this minute checked in :-)

Hopefully it should fix all these weirdnesses....

-- Keir

Looks good, the odd bugs seem to be gone now.
Small change, big effect ;)

Gerd