[Xen-devel] [PATCH] x86/pv: Fix construction of 32bit dom0's

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

dom0_construct_pv() has logic to transition dom0 into a compat domain when
booting an ELF32 image.

One aspect which is missing is the CPUID policy recalculation, meaning that a
32bit dom0 sees a 64bit policy, which differ by the Long Mode feature flag in
particular.  Another missing item is the x87_fip_width initialisation.

Update dom0_construct_pv() to use switch_compat(), rather than retaining the
opencoding.  Position the call to switch_compat() such that the compat32 local
variable can disappear entirely.

The 32bit monitor table is now created by setup_compat_l4(), avoiding the need
to for manual creation later.  Furthermore, the L3 table creation is redundant
with the logic inside the main mapping loop, so can be dropped as well.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Slightly RFC:

1) I've not worked out exactly what the

     v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];

   line is supposed to be doing and whether it is needed, but it doesn't
   appear to matter.  It is perhaps another redundant opencoding.

2) The reported

     Dom0 alloc.:   000000003e800000->000000003ec00000 (240470 pages to be 
allocated)

   line changes by 1 page because of the alloc_domheap_page() moving ahead of
   the printk(), but I'm fairly sure this is benign.  There is a matching
   reduction in the length of the constructed m2p which is perhaps less
   benign.
---
 xen/arch/x86/pv/dom0_build.c | 43 +++++++++++++------------------------------
 xen/arch/x86/pv/domain.c     |  2 +-
 2 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 837ef7b..c3d8ee7 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -285,7 +285,7 @@ int __init dom0_construct_pv(struct domain *d,
                              module_t *initrd,
                              char *cmdline)
 {
-    int i, cpu, rc, compatible, compat32, order, machine;
+    int i, cpu, rc, compatible, order, machine;
     struct cpu_user_regs *regs;
     unsigned long pfn, mfn;
     unsigned long nr_pages;
@@ -354,14 +354,18 @@ int __init dom0_construct_pv(struct domain *d,
 
     /* compatibility check */
     compatible = 0;
-    compat32   = 0;
     machine = elf_uval(&elf, elf.ehdr, e_machine);
     printk(" Xen  kernel: 64-bit, lsb, compat32\n");
     if ( elf_32bit(&elf) && parms.pae == XEN_PAE_BIMODAL )
         parms.pae = XEN_PAE_EXTCR3;
     if ( elf_32bit(&elf) && parms.pae && machine == EM_386 )
     {
-        compat32 = 1;
+        if ( unlikely(rc = switch_compat(d)) )
+        {
+            printk("Dom0 failed to switch to compat: %d\n", rc);
+            return rc;
+        }
+
         compatible = 1;
     }
     if (elf_64bit(&elf) && machine == EM_X86_64)
@@ -392,16 +396,6 @@ int __init dom0_construct_pv(struct domain *d,
         }
     }
 
-    if ( compat32 )
-    {
-        d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
-        d->arch.pv.xpti = false;
-        d->arch.pv.pcid = false;
-        v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];
-        if ( setup_compat_arg_xlat(v) != 0 )
-            BUG();
-    }
-
     nr_pages = dom0_compute_nr_pages(d, &parms, initrd_len);
 
     if ( parms.pae == XEN_PAE_EXTCR3 )
@@ -425,8 +419,6 @@ int __init dom0_construct_pv(struct domain *d,
         parms.p2m_base = UNSET_ADDR;
     }
 
-    domain_set_alloc_bitsize(d);
-
     /*
      * Why do we need this? The number of page-table frames depends on the
      * size of the bootstrap address space. But the size of the address space
@@ -606,23 +598,14 @@ int __init dom0_construct_pv(struct domain *d,
     {
         maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l4_page_table;
         l4start = l4tab = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
+        clear_page(l4tab);
+        init_xen_l4_slots(l4tab, _mfn(virt_to_mfn(l4start)),
+                          d, INVALID_MFN, true);
+        v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
     }
     else
-    {
-        page = alloc_domheap_page(d, MEMF_no_owner | MEMF_no_scrub);
-        if ( !page )
-            panic("Not enough RAM for domain 0 PML4\n");
-        page->u.inuse.type_info = PGT_l4_page_table|PGT_validated|1;
-        l4start = l4tab = page_to_virt(page);
-        maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l3_page_table;
-        l3start = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
-    }
-    clear_page(l4tab);
-    init_xen_l4_slots(l4tab, _mfn(virt_to_mfn(l4start)),
-                      d, INVALID_MFN, true);
-    v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
-    if ( is_pv_32bit_domain(d) )
-        v->arch.guest_table_user = v->arch.guest_table;
+        /* Monitor table already created by switch_compat(). */
+        l4start = l4tab = __va(pagetable_get_paddr(v->arch.guest_table));
 
     l4tab += l4_table_offset(v_start);
     pfn = alloc_spfn;
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 7e84b04..3457586 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -70,7 +70,7 @@ static int setup_compat_l4(struct vcpu *v)
     l4_pgentry_t *l4tab;
     mfn_t mfn;
 
-    pg = alloc_domheap_page(v->domain, MEMF_no_owner);
+    pg = alloc_domheap_page(v->domain, MEMF_no_owner | MEMF_no_scrub);
     if ( pg == NULL )
         return -ENOMEM;
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel