[PATCH 0/9] 32-bit domU PVH support

[PATCH 0/9] 32-bit domU PVH support

[Boris Ostrovsky]

Initial 32-bit PVH support, dom0 will need more work.

Boris Ostrovsky (9):
  x86/pvh: Don't test 64b-only vcpu_guest_context's fields
  x86/pvh: Don't copy to/from trap_ctxt for 32b PVH guests
  x86/pvh: Properly initialize PVH guest's CR3
  x86/compat: Manage argument translation area separately from l4
  x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size
  x86/pvh: Properly set HYPERVISOR_COMPAT_VIRT_START for PVH guests
  x86/pvh: Handle hypercalls for 32b PVH guests
  x86/pvh: Don't try to get l4 table for PVH guests in
    vcpu_destroy_pagetables()
  libxc/x86/pvh: Allow creation of 32b PVH guests

 tools/libxc/xc_dom_x86.c      |   32 +++++++--------
 xen/arch/x86/domain.c         |   84 ++++++++++++++++++++++++++--------------
 xen/arch/x86/domain_build.c   |    7 +++
 xen/arch/x86/domctl.c         |    9 +++-
 xen/arch/x86/hvm/hvm.c        |   54 +++++++++++++++++++++-----
 xen/arch/x86/hvm/vmx/vmcs.c   |    9 +----
 xen/arch/x86/hvm/vmx/vmx.c    |   17 ++++++++
 xen/arch/x86/mm.c             |    2 +-
 xen/include/asm-x86/hvm/hvm.h |    2 +
 9 files changed, 146 insertions(+), 70 deletions(-)

[PATCH 1/9] x86/pvh: Don't test 64b-only vcpu_guest_context's fields

[Boris Ostrovsky]

vcpu_guest_context's fs_base, gs_base_kernel and gs_base_user are not defined
for 32-bit guests.

Drop PVH 32bitfixme ASSERT.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 0363650..e396c6c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -769,16 +769,14 @@ int arch_set_info_guest(
     }
     else if ( is_pvh_domain(d) )
     {
-        /* PVH 32bitfixme */
-        ASSERT(!compat);
-
         if ( c(ctrlreg[0]) || c(ctrlreg[1]) || c(ctrlreg[2]) ||
              c(ctrlreg[4]) || c(ctrlreg[5]) || c(ctrlreg[6]) ||
              c(ctrlreg[7]) ||  c(ldt_base) || c(ldt_ents) ||
              c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
              c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
-             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel ||
-             c.nat->gdt_ents || c.nat->fs_base || c.nat->gs_base_user )
+             c(kernel_ss) || c(kernel_sp) || c(gdt_ents) ||
+             (!compat && (c.nat->gs_base_kernel ||
+              c.nat->fs_base || c.nat->gs_base_user)) )
             return -EINVAL;
     }
 
-- 
1.7.1

[PATCH 2/9] x86/pvh: Don't copy to/from trap_ctxt for 32b PVH guests

[Boris Ostrovsky]

.. as this field is not used by PVH guests

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |    9 ++++++---
 xen/arch/x86/domctl.c |    9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index e396c6c..c7ef1e6 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -805,9 +805,12 @@ int arch_set_info_guest(
     else
     {
         XLAT_cpu_user_regs(&v->arch.user_regs, &c.cmp->user_regs);
-        for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
-            XLAT_trap_info(v->arch.pv_vcpu.trap_ctxt + i,
-                           c.cmp->trap_ctxt + i);
+        if ( is_pv_domain(d) )
+        {
+            for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
+                XLAT_trap_info(v->arch.pv_vcpu.trap_ctxt + i,
+                               c.cmp->trap_ctxt + i);
+        }
     }
 
     if ( has_hvm_container_domain(d) )
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index d8ffe2b..82bd818 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1204,9 +1204,12 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
     else
     {
         XLAT_cpu_user_regs(&c.cmp->user_regs, &v->arch.user_regs);
-        for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
-            XLAT_trap_info(c.cmp->trap_ctxt + i,
-                           v->arch.pv_vcpu.trap_ctxt + i);
+        if ( is_pv_domain(d) )
+        {
+            for ( i = 0; i < ARRAY_SIZE(c.cmp->trap_ctxt); ++i )
+                XLAT_trap_info(c.cmp->trap_ctxt + i,
+                               v->arch.pv_vcpu.trap_ctxt + i);
+        }
     }
 
     for ( i = 0; i < ARRAY_SIZE(v->arch.debugreg); ++i )
-- 
1.7.1

[PATCH 3/9] x86/pvh: Properly initialize PVH guest's CR3

[Boris Ostrovsky]

.. based on whether the guest is 32- or 64-bit

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index c7ef1e6..ba28f38 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -828,7 +828,7 @@ int arch_set_info_guest(
         cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
 
         v->arch.cr3 = page_to_maddr(cr3_page);
-        v->arch.hvm_vcpu.guest_cr[3] = c.nat->ctrlreg[3];
+        v->arch.hvm_vcpu.guest_cr[3] = c(ctrlreg[3]);
         v->arch.guest_table = pagetable_from_page(cr3_page);
 
         ASSERT(paging_mode_enabled(d));
-- 
1.7.1

[PATCH 4/9] x86/compat: Manage argument translation area separately from l4

[Boris Ostrovsky]

Managing l4 page table and argument translation area are two unrelated
operations and should be handled separately

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |   36 ++++++++++++++++++++++++++----------
 1 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ba28f38..2445b8b 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -321,19 +321,11 @@ static int setup_compat_l4(struct vcpu *v)
 {
     struct page_info *pg;
     l4_pgentry_t *l4tab;
-    int rc;
 
     pg = alloc_domheap_page(v->domain, MEMF_no_owner);
     if ( pg == NULL )
         return -ENOMEM;
 
-    rc = setup_compat_arg_xlat(v);
-    if ( rc )
-    {
-        free_domheap_page(pg);
-        return rc;
-    }
-
     /* This page needs to look like a pagetable so that it can be shadowed */
     pg->u.inuse.type_info = PGT_l4_page_table|PGT_validated|1;
 
@@ -350,7 +342,6 @@ static int setup_compat_l4(struct vcpu *v)
 
 static void release_compat_l4(struct vcpu *v)
 {
-    free_compat_arg_xlat(v);
     free_domheap_page(pagetable_get_page(v->arch.guest_table));
     v->arch.guest_table = pagetable_null();
     v->arch.guest_table_user = pagetable_null();
@@ -373,7 +364,10 @@ int switch_native(struct domain *d)
     d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
 
     for_each_vcpu( d, v )
+    {
+        free_compat_arg_xlat(v);
         release_compat_l4(v);
+    }
 
     return 0;
 }
@@ -398,8 +392,13 @@ int switch_compat(struct domain *d)
     d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
 
     for_each_vcpu( d, v )
+    {
+        if ( (rc = setup_compat_arg_xlat(v)) )
+            goto undo_and_fail;
+
         if ( (rc = setup_compat_l4(v)) )
             goto undo_and_fail;
+    }
 
     domain_set_alloc_bitsize(d);
 
@@ -408,8 +407,12 @@ int switch_compat(struct domain *d)
  undo_and_fail:
     d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
     for_each_vcpu( d, v )
+    {
+        free_compat_arg_xlat(v);
+
         if ( !pagetable_is_null(v->arch.guest_table) )
             release_compat_l4(v);
+    }
 
     return rc;
 }
@@ -481,7 +484,17 @@ int vcpu_initialise(struct vcpu *v)
 
     v->arch.pv_vcpu.ctrlreg[4] = real_cr4_to_pv_guest_cr4(mmu_cr4_features);
 
-    rc = is_pv_32on64_domain(d) ? setup_compat_l4(v) : 0;
+    if ( is_pv_32on64_domain(d) )
+    {
+        if ( (rc = setup_compat_arg_xlat(v)) )
+            goto done;
+
+        if ( (rc = setup_compat_l4(v)) )
+        {
+            free_compat_arg_xlat(v);
+            goto done;
+        }
+    }
  done:
     if ( rc )
     {
@@ -497,7 +510,10 @@ int vcpu_initialise(struct vcpu *v)
 void vcpu_destroy(struct vcpu *v)
 {
     if ( is_pv_32on64_vcpu(v) )
+    {
+        free_compat_arg_xlat(v);
         release_compat_l4(v);
+    }
 
     vcpu_destroy_fpu(v);
 
-- 
1.7.1

[PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Boris Ostrovsky]

Instead of assuming that the PVH guest is 64-bit during VMCS constructions and
overriding 32/64 bit settings in VMCS later we can keep HVM's settings and
update them as needed when we know exactly what the guest is.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Eddie Dong <eddie.dong@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/domain.c         |   27 +++++++++++++++++----------
 xen/arch/x86/domain_build.c   |    7 +++++++
 xen/arch/x86/hvm/hvm.c        |   19 ++++++++++++++-----
 xen/arch/x86/hvm/vmx/vmcs.c   |    9 +--------
 xen/arch/x86/hvm/vmx/vmx.c    |   17 +++++++++++++++++
 xen/include/asm-x86/hvm/hvm.h |    2 ++
 6 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 2445b8b..d049fa8 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -358,6 +358,14 @@ int switch_native(struct domain *d)
 
     if ( !may_switch_mode(d) )
         return -EACCES;
+
+    if ( is_pvh_domain(d) )
+    {
+        for_each_vcpu( d, v )
+            if ( hvm_set_mode(v, 8) )
+                return -EACCES;
+    }
+
     if ( !is_pv_32on64_domain(d) )
         return 0;
 
@@ -377,13 +385,6 @@ int switch_compat(struct domain *d)
     struct vcpu *v;
     int rc;
 
-    if ( is_pvh_domain(d) )
-    {
-        printk(XENLOG_G_INFO
-               "Xen currently does not support 32bit PVH guests\n");
-        return -EINVAL;
-    }
-
     if ( !may_switch_mode(d) )
         return -EACCES;
     if ( is_pv_32on64_domain(d) )
@@ -396,7 +397,12 @@ int switch_compat(struct domain *d)
         if ( (rc = setup_compat_arg_xlat(v)) )
             goto undo_and_fail;
 
-        if ( (rc = setup_compat_l4(v)) )
+        if ( !is_pvh_domain(d) )
+            rc = setup_compat_l4(v);
+        else
+            rc = hvm_set_mode(v, 4);
+
+        if ( rc )
             goto undo_and_fail;
     }
 
@@ -410,7 +416,7 @@ int switch_compat(struct domain *d)
     {
         free_compat_arg_xlat(v);
 
-        if ( !pagetable_is_null(v->arch.guest_table) )
+        if ( !is_pvh_domain(d) && !pagetable_is_null(v->arch.guest_table) )
             release_compat_l4(v);
     }
 
@@ -512,7 +518,8 @@ void vcpu_destroy(struct vcpu *v)
     if ( is_pv_32on64_vcpu(v) )
     {
         free_compat_arg_xlat(v);
-        release_compat_l4(v);
+        if ( !is_pvh_vcpu(v) )
+            release_compat_l4(v);
     }
 
     vcpu_destroy_fpu(v);
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index d76707f..ca3f6d1 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -141,6 +141,13 @@ static struct vcpu *__init setup_dom0_vcpu(struct domain *d,
         if ( !d->is_pinned && !dom0_affinity_relaxed )
             cpumask_copy(v->cpu_hard_affinity, &dom0_cpus);
         cpumask_copy(v->cpu_soft_affinity, &dom0_cpus);
+
+        if ( is_pvh_vcpu(v) )
+            if ( hvm_set_mode(v, is_pv_32bit_domain(d) ? 4 : 8) )
+            {
+                vcpu_destroy(v);
+                return NULL;
+            }
     }
 
     return v;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d5e5242..c3c129d 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
     v->arch.hvm_vcpu.inject_trap.vector = -1;
 
     if ( is_pvh_domain(d) )
-    {
-        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
-        /* This is for hvm_long_mode_enabled(v). */
-        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
         return 0;
-    }
 
     rc = setup_compat_arg_xlat(v); /* teardown: free_compat_arg_xlat() */
     if ( rc != 0 )
@@ -6495,6 +6490,20 @@ enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v)
     return hvm_funcs.nhvm_intr_blocked(v);
 }
 
+int hvm_set_mode(struct vcpu *v, int mode)
+{
+    if ( mode == 8 )
+    {
+        v->arch.hvm_vcpu.guest_efer |= EFER_LMA | EFER_LME;
+        hvm_update_guest_efer(v);
+    }
+
+    if ( hvm_funcs.set_mode )
+        return hvm_funcs.set_mode(v, mode);
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 4c5ceb5..08e2097 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -984,9 +984,6 @@ static int construct_vmcs(struct vcpu *v)
         v->arch.hvm_vmx.secondary_exec_control &=
             ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
 
-        /* Start in 64-bit mode. PVH 32bitfixme. */
-        vmentry_ctl |= VM_ENTRY_IA32E_MODE;       /* GUEST_EFER.LME/LMA ignored */
-
         ASSERT(v->arch.hvm_vmx.exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS);
         ASSERT(v->arch.hvm_vmx.exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP);
         ASSERT(!(v->arch.hvm_vmx.exec_control & CPU_BASED_RDTSC_EXITING));
@@ -1124,11 +1121,7 @@ static int construct_vmcs(struct vcpu *v)
     __vmwrite(GUEST_DS_AR_BYTES, 0xc093);
     __vmwrite(GUEST_FS_AR_BYTES, 0xc093);
     __vmwrite(GUEST_GS_AR_BYTES, 0xc093);
-    if ( is_pvh_domain(d) )
-        /* CS.L == 1, exec, read/write, accessed. PVH 32bitfixme. */
-        __vmwrite(GUEST_CS_AR_BYTES, 0xa09b);
-    else
-        __vmwrite(GUEST_CS_AR_BYTES, 0xc09b); /* exec/read, accessed */
+    __vmwrite(GUEST_CS_AR_BYTES, 0xc09b); /* exec/read, accessed */
 
     /* Guest IDT. */
     __vmwrite(GUEST_IDTR_BASE, 0);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0837627..0791bfe 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1763,6 +1763,22 @@ static void vmx_enable_msr_exit_interception(struct domain *d)
                                          MSR_TYPE_W);
 }
 
+int vmx_set_mode(struct vcpu *v, int mode)
+{
+
+    if ( !is_pvh_vcpu(v) )
+        return 0;
+
+    if ( mode == 8 )
+    {
+        vmx_vmcs_enter(v);
+        __vmwrite(GUEST_CS_AR_BYTES, 0xa09b);
+        vmx_vmcs_exit(v);
+    }
+
+    return 0;
+}
+
 static struct hvm_function_table __initdata vmx_function_table = {
     .name                 = "VMX",
     .cpu_up_prepare       = vmx_cpu_up_prepare,
@@ -1822,6 +1838,7 @@ static struct hvm_function_table __initdata vmx_function_table = {
     .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
     .hypervisor_cpuid_leaf = vmx_hypervisor_cpuid_leaf,
     .enable_msr_exit_interception = vmx_enable_msr_exit_interception,
+    .set_mode = vmx_set_mode,
 };
 
 const struct hvm_function_table * __init start_vmx(void)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 77eeac5..7547007 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -207,6 +207,7 @@ struct hvm_function_table {
                                   uint32_t *ecx, uint32_t *edx);
 
     void (*enable_msr_exit_interception)(struct domain *d);
+    int (*set_mode)(struct vcpu *v, int mode);
 };
 
 extern struct hvm_function_table hvm_funcs;
@@ -242,6 +243,7 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc);
 u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
 #define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
 
+int hvm_set_mode(struct vcpu *v, int mode);
 void hvm_init_guest_time(struct domain *d);
 void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
 u64 hvm_get_guest_time_fixed(struct vcpu *v, u64 at_tsc);
-- 
1.7.1

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Jan Beulich]

>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -141,6 +141,13 @@ static struct vcpu *__init setup_dom0_vcpu(struct domain *d,
>          if ( !d->is_pinned && !dom0_affinity_relaxed )
>              cpumask_copy(v->cpu_hard_affinity, &dom0_cpus);
>          cpumask_copy(v->cpu_soft_affinity, &dom0_cpus);
> +
> +        if ( is_pvh_vcpu(v) )
> +            if ( hvm_set_mode(v, is_pv_32bit_domain(d) ? 4 : 8) )

This should be just one if().

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>      v->arch.hvm_vcpu.inject_trap.vector = -1;
>  
>      if ( is_pvh_domain(d) )
> -    {
> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
> -        /* This is for hvm_long_mode_enabled(v). */
> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>          return 0;
> -    }

With this removed, is there any guarantee that hvm_set_mode()
will be called for each vCPU?

Anyway, while I'll apply the previous patch as a cleanup one, I'll
defer this and later ones until a decision between pursuing PVH
vs going the "HVMlite" route was made.

Jan

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Boris Ostrovsky]

On 06/23/2015 09:22 AM, Jan Beulich wrote:
>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>       v->arch.hvm_vcpu.inject_trap.vector = -1;
>>   
>>       if ( is_pvh_domain(d) )
>> -    {
>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>> -        /* This is for hvm_long_mode_enabled(v). */
>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>           return 0;
>> -    }
> With this removed, is there any guarantee that hvm_set_mode()
> will be called for each vCPU?

IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which 
results in a call to switch_compat/native(), which loop over all VCPUs, 
calling set_mode.


-boris

>
> Anyway, while I'll apply the previous patch as a cleanup one, I'll
> defer this and later ones until a decision between pursuing PVH
> vs going the "HVMlite" route was made.
>
> Jan
>

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Jan Beulich]

>>> On 24.06.15 at 04:53, <boris.ostrovsky@oracle.com> wrote:
> On 06/23/2015 09:22 AM, Jan Beulich wrote:
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>       v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>   
>>>       if ( is_pvh_domain(d) )
>>> -    {
>>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>>> -        /* This is for hvm_long_mode_enabled(v). */
>>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>>           return 0;
>>> -    }
>> With this removed, is there any guarantee that hvm_set_mode()
>> will be called for each vCPU?
> 
> IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which 
> results in a call to switch_compat/native(), which loop over all VCPUs, 
> calling set_mode.

I don't recall this being a strict requirement. I think a PV 64-bit
guest would start fine without.

Jan

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Boris Ostrovsky]

On 06/24/2015 03:57 AM, Jan Beulich wrote:
>>>> On 24.06.15 at 04:53, <boris.ostrovsky@oracle.com> wrote:
>> On 06/23/2015 09:22 AM, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>>        v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>>    
>>>>        if ( is_pvh_domain(d) )
>>>> -    {
>>>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>>>> -        /* This is for hvm_long_mode_enabled(v). */
>>>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>>>            return 0;
>>>> -    }
>>> With this removed, is there any guarantee that hvm_set_mode()
>>> will be called for each vCPU?
>> IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which
>> results in a call to switch_compat/native(), which loop over all VCPUs,
>> calling set_mode.
> I don't recall this being a strict requirement. I think a PV 64-bit
> guest would start fine without.

We do call it via libxl__build_pv() -> xc_dom_boot_mem_init() -> 
arch_setup_mem_init() -> x86_compat().

-boris

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Jan Beulich]

>>> On 24.06.15 at 13:42, <boris.ostrovsky@oracle.com> wrote:
> On 06/24/2015 03:57 AM, Jan Beulich wrote:
>>>>> On 24.06.15 at 04:53, <boris.ostrovsky@oracle.com> wrote:
>>> On 06/23/2015 09:22 AM, Jan Beulich wrote:
>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>>>        v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>>>    
>>>>>        if ( is_pvh_domain(d) )
>>>>> -    {
>>>>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>>>>> -        /* This is for hvm_long_mode_enabled(v). */
>>>>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>>>>            return 0;
>>>>> -    }
>>>> With this removed, is there any guarantee that hvm_set_mode()
>>>> will be called for each vCPU?
>>> IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which
>>> results in a call to switch_compat/native(), which loop over all VCPUs,
>>> calling set_mode.
>> I don't recall this being a strict requirement. I think a PV 64-bit
>> guest would start fine without.
> 
> We do call it via libxl__build_pv() -> xc_dom_boot_mem_init() -> 
> arch_setup_mem_init() -> x86_compat().

Right, that's in our tool stack. The question though was whether it's
a requirement to be called.

Jan

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Boris Ostrovsky]

On 06/24/2015 08:10 AM, Jan Beulich wrote:
>>>> On 24.06.15 at 13:42, <boris.ostrovsky@oracle.com> wrote:
>> On 06/24/2015 03:57 AM, Jan Beulich wrote:
>>>>>> On 24.06.15 at 04:53, <boris.ostrovsky@oracle.com> wrote:
>>>> On 06/23/2015 09:22 AM, Jan Beulich wrote:
>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>>>>         v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>>>>     
>>>>>>         if ( is_pvh_domain(d) )
>>>>>> -    {
>>>>>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>>>>>> -        /* This is for hvm_long_mode_enabled(v). */
>>>>>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>>>>>             return 0;
>>>>>> -    }
>>>>> With this removed, is there any guarantee that hvm_set_mode()
>>>>> will be called for each vCPU?
>>>> IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which
>>>> results in a call to switch_compat/native(), which loop over all VCPUs,
>>>> calling set_mode.
>>> I don't recall this being a strict requirement. I think a PV 64-bit
>>> guest would start fine without.
>> We do call it via libxl__build_pv() -> xc_dom_boot_mem_init() ->
>> arch_setup_mem_init() -> x86_compat().
> Right, that's in our tool stack. The question though was whether it's
> a requirement to be called.

Since this change will assume that this domctl is called for both 32- 
and 64-bit --- yes, this becomes a requirement for 64-bit PVH guests.

boris

Re: [PATCH 5/9] x86/pvh: Set PVH guest's mode in XEN_DOMCTL_set_address_size

[Jan Beulich]

>>> On 24.06.15 at 18:21, <boris.ostrovsky@oracle.com> wrote:
> On 06/24/2015 08:10 AM, Jan Beulich wrote:
>>>>> On 24.06.15 at 13:42, <boris.ostrovsky@oracle.com> wrote:
>>> On 06/24/2015 03:57 AM, Jan Beulich wrote:
>>>>>>> On 24.06.15 at 04:53, <boris.ostrovsky@oracle.com> wrote:
>>>>> On 06/23/2015 09:22 AM, Jan Beulich wrote:
>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>>> @@ -2320,12 +2320,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>>>>>         v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>>>>>     
>>>>>>>         if ( is_pvh_domain(d) )
>>>>>>> -    {
>>>>>>> -        v->arch.hvm_vcpu.hcall_64bit = 1;    /* PVH 32bitfixme. */
>>>>>>> -        /* This is for hvm_long_mode_enabled(v). */
>>>>>>> -        v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
>>>>>>>             return 0;
>>>>>>> -    }
>>>>>> With this removed, is there any guarantee that hvm_set_mode()
>>>>>> will be called for each vCPU?
>>>>> IIUIC, toolstack is required to call XEN_DOMCTL_set_address_size which
>>>>> results in a call to switch_compat/native(), which loop over all VCPUs,
>>>>> calling set_mode.
>>>> I don't recall this being a strict requirement. I think a PV 64-bit
>>>> guest would start fine without.
>>> We do call it via libxl__build_pv() -> xc_dom_boot_mem_init() ->
>>> arch_setup_mem_init() -> x86_compat().
>> Right, that's in our tool stack. The question though was whether it's
>> a requirement to be called.
> 
> Since this change will assume that this domctl is called for both 32- 
> and 64-bit --- yes, this becomes a requirement for 64-bit PVH guests.

But that's the whole point of my question - it isn't right now, and
hence I don't think it should become a requirement. Instead I
think state should start out to be ready for a 64-bit guest just
like it does for PV.

Jan

[PATCH 6/9] x86/pvh: Properly set HYPERVISOR_COMPAT_VIRT_START for PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d049fa8..f87da64 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -584,7 +584,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
     mapcache_domain_init(d);
 
     HYPERVISOR_COMPAT_VIRT_START(d) =
-        is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
+        is_hvm_domain(d) ? ~0u : __HYPERVISOR_COMPAT_VIRT_START;
 
     if ( !is_idle_domain(d) )
     {
-- 
1.7.1

Re: [PATCH 6/9] x86/pvh: Properly set HYPERVISOR_COMPAT_VIRT_START for PVH guests

[Jan Beulich]

>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -584,7 +584,7 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>      mapcache_domain_init(d);
>  
>      HYPERVISOR_COMPAT_VIRT_START(d) =
> -        is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
> +        is_hvm_domain(d) ? ~0u : __HYPERVISOR_COMPAT_VIRT_START;

Why would a PVH domain be different in this respect from a HVM one?

Jan

Re: [PATCH 6/9] x86/pvh: Properly set HYPERVISOR_COMPAT_VIRT_START for PVH guests

[Boris Ostrovsky]

On 06/23/2015 09:25 AM, Jan Beulich wrote:
>>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -584,7 +584,7 @@ int arch_domain_create(struct domain *d, unsigned int
>> domcr_flags,
>>       mapcache_domain_init(d);
>>   
>>       HYPERVISOR_COMPAT_VIRT_START(d) =
>> -        is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
>> +        is_hvm_domain(d) ? ~0u : __HYPERVISOR_COMPAT_VIRT_START;
> Why would a PVH domain be different in this respect from a HVM one?
>

No reason indeed. I somehow thought that layout should be similar to PV, 
which it doesn't need to be.

So this patch should be dropped.

-boris

[PATCH 7/9] x86/pvh: Handle hypercalls for 32b PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/hvm/hvm.c |   35 +++++++++++++++++++++++++++++------
 1 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c3c129d..12f6839 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4910,7 +4910,6 @@ static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };
 
-/* PVH 32bitfixme. */
 static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
     HYPERCALL(platform_op),
     HYPERCALL(memory_op),
@@ -4929,6 +4928,27 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };
 
+extern void compat_mmuext_op(void); /* XXX: need proper declaration */
+static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
+    HYPERCALL(platform_op),
+    COMPAT_CALL(memory_op),
+    HYPERCALL(xen_version),
+    HYPERCALL(console_io),
+    [ __HYPERVISOR_grant_table_op ]  =
+        (hvm_hypercall_t *)hvm_grant_table_op_compat32,
+    COMPAT_CALL(vcpu_op),
+    COMPAT_CALL(mmuext_op),
+    HYPERCALL(xsm_op),
+    COMPAT_CALL(sched_op),
+    HYPERCALL(event_channel_op),
+    [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
+    HYPERCALL(hvm_op),
+    HYPERCALL(sysctl),
+    HYPERCALL(domctl),
+    [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
+};
+
+
 extern const uint8_t hypercall_args_table[], compat_hypercall_args_table[];
 
 int hvm_do_hypercall(struct cpu_user_regs *regs)
@@ -4959,8 +4979,10 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         return viridian_hypercall(regs);
 
     if ( (eax >= NR_hypercalls) ||
-         (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
-                               : !hvm_hypercall32_table[eax]) )
+         (is_pvh_vcpu(curr) ? ((mode == 8) ?
+                               !pvh_hypercall64_table[eax] :
+                               !pvh_hypercall32_table[eax])
+                            : !hvm_hypercall32_table[eax]) )
     {
         regs->eax = -ENOSYS;
         return HVM_HCALL_completed;
@@ -5015,8 +5037,6 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         }
 #endif
     }
-    else if ( unlikely(is_pvh_domain(currd)) )
-        regs->_eax = -ENOSYS; /* PVH 32bitfixme. */
     else
     {
         unsigned int ebx = regs->_ebx;
@@ -5042,7 +5062,10 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         }
 #endif
 
-        regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp);
+        regs->_eax = (is_pvh_vcpu(curr)
+                      ? pvh_hypercall32_table
+                      : hvm_hypercall32_table)[eax](ebx, ecx, edx,
+                                                    esi, edi, ebp);
 
 #ifndef NDEBUG
         if ( !curr->arch.hvm_vcpu.hcall_preempted )
-- 
1.7.1

Re: [PATCH 7/9] x86/pvh: Handle hypercalls for 32b PVH guests

[Jan Beulich]

>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
> @@ -4959,8 +4979,10 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>          return viridian_hypercall(regs);
>  
>      if ( (eax >= NR_hypercalls) ||
> -         (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
> -                               : !hvm_hypercall32_table[eax]) )
> +         (is_pvh_vcpu(curr) ? ((mode == 8) ?

Please no switch back to the more expensive is_pvh_vcpu().

> +                               !pvh_hypercall64_table[eax] :
> +                               !pvh_hypercall32_table[eax])
> +                            : !hvm_hypercall32_table[eax]) )

This should be done consistently - either all four variants get
checked, or pvh_hypercall64_table[] simply get replaced by
pvh_hypercall32_table[].

Jan

[PATCH 8/9] x86/pvh: Don't try to get l4 table for PVH guests in vcpu_destroy_pagetables()

[Boris Ostrovsky]

.. since it doesn't exist for PVH.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/mm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 9e08c9b..eb369bd 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2652,7 +2652,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
     if ( rc )
         return rc;
 
-    if ( is_pv_32on64_vcpu(v) )
+    if ( is_pv_32on64_vcpu(v) && !is_pvh_vcpu(v) )
     {
         l4tab = map_domain_page(mfn);
         mfn = l4e_get_pfn(*l4tab);
-- 
1.7.1

Re: [PATCH 8/9] x86/pvh: Don't try to get l4 table for PVH guests in vcpu_destroy_pagetables()

[Jan Beulich]

>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2652,7 +2652,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
>      if ( rc )
>          return rc;
>  
> -    if ( is_pv_32on64_vcpu(v) )
> +    if ( is_pv_32on64_vcpu(v) && !is_pvh_vcpu(v) )

This looks wrong - is_pv_32on64_vcpu() should imply is_pv_vcpu()
and hence !is_pvh_vcpu().

Jan

Re: [PATCH 8/9] x86/pvh: Don't try to get l4 table for PVH guests in vcpu_destroy_pagetables()

[Boris Ostrovsky]

On 06/23/2015 09:38 AM, Jan Beulich wrote:
>>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -2652,7 +2652,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
>>       if ( rc )
>>           return rc;
>>   
>> -    if ( is_pv_32on64_vcpu(v) )
>> +    if ( is_pv_32on64_vcpu(v) && !is_pvh_vcpu(v) )
> This looks wrong - is_pv_32on64_vcpu() should imply is_pv_vcpu()
> and hence !is_pvh_vcpu().


That's because I kept
     d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
in switch_compat() for both PV and PVH. I should probably only set 
has_32bit_shinfo for PVH.


-boris

Re: [PATCH 8/9] x86/pvh: Don't try to get l4 table for PVH guests in vcpu_destroy_pagetables()

[Jan Beulich]

>>> On 24.06.15 at 05:05, <boris.ostrovsky@oracle.com> wrote:
> On 06/23/2015 09:38 AM, Jan Beulich wrote:
>>>>> On 20.06.15 at 05:09, <boris.ostrovsky@oracle.com> wrote:
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -2652,7 +2652,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
>>>       if ( rc )
>>>           return rc;
>>>   
>>> -    if ( is_pv_32on64_vcpu(v) )
>>> +    if ( is_pv_32on64_vcpu(v) && !is_pvh_vcpu(v) )
>> This looks wrong - is_pv_32on64_vcpu() should imply is_pv_vcpu()
>> and hence !is_pvh_vcpu().
> 
> 
> That's because I kept
>      d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
> in switch_compat() for both PV and PVH. I should probably only set 
> has_32bit_shinfo for PVH.

Right.

Jan

[PATCH 9/9] libxc/x86/pvh: Allow creation of 32b PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_dom_x86.c |   32 +++++++++++++++-----------------
 1 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 920fe42..43e79f4 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -301,7 +301,8 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
         pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
         l1tab[l1off] =
             pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) | L1_PROT;
-        if ( (addr >= dom->pgtables_seg.vstart) &&
+        if ( (!dom->pvh_enabled)                &&
+             (addr >= dom->pgtables_seg.vstart) &&
              (addr < dom->pgtables_seg.vend) )
             l1tab[l1off] &= ~_PAGE_RW; /* page tables are r/o */
 
@@ -591,22 +592,9 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
 
     DOMPRINTF_CALLED(dom->xch);
 
-    if ( dom->pvh_enabled )
-    {
-        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
-                     "%s: PVH not supported for 32bit guests.", __FUNCTION__);
-        return -1;
-    }
-
     /* clear everything */
     memset(ctxt, 0, sizeof(*ctxt));
 
-    ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_32;
-    ctxt->user_regs.es = FLAT_KERNEL_DS_X86_32;
-    ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_32;
-    ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_32;
-    ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_32;
-    ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_32;
     ctxt->user_regs.eip = dom->parms.virt_entry;
     ctxt->user_regs.esp =
         dom->parms.virt_base + (dom->bootstack_pfn + 1) * PAGE_SIZE_X86;
@@ -614,9 +602,6 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
         dom->parms.virt_base + (dom->start_info_pfn) * PAGE_SIZE_X86;
     ctxt->user_regs.eflags = 1 << 9; /* Interrupt Enable */
 
-    ctxt->kernel_ss = ctxt->user_regs.ss;
-    ctxt->kernel_sp = ctxt->user_regs.esp;
-
     ctxt->flags = VGCF_in_kernel_X86_32 | VGCF_online_X86_32;
     if ( dom->parms.pae == 2 /* extended_cr3 */ ||
          dom->parms.pae == 3 /* bimodal */ )
@@ -627,6 +612,19 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
     DOMPRINTF("%s: cr3: pfn 0x%" PRIpfn " mfn 0x%" PRIpfn "",
               __FUNCTION__, dom->pgtables_seg.pfn, cr3_pfn);
 
+    if ( dom->pvh_enabled )
+        return 0;
+
+    ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_32;
+    ctxt->user_regs.es = FLAT_KERNEL_DS_X86_32;
+    ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_32;
+    ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_32;
+    ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_32;
+    ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_32;
+
+    ctxt->kernel_ss = ctxt->user_regs.ss;
+    ctxt->kernel_sp = ctxt->user_regs.esp;
+
     return 0;
 }
 
-- 
1.7.1

Re: [PATCH 9/9] libxc/x86/pvh: Allow creation of 32b PVH guests

[Ian Campbell]

On Fri, 2015-06-19 at 23:09 -0400, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[PATCH 1/9] x86/pvh: Don't test 64b-only vcpu_guest_context's fields

[Boris Ostrovsky]

vcpu_guest_context's fs_base, gs_base_kernel and gs_base_user are not defined
for 32-bit guests.

Drop PVH 32bitfixme ASSERT.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
 xen/arch/x86/domain.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 0363650..e396c6c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -769,16 +769,14 @@ int arch_set_info_guest(
     }
     else if ( is_pvh_domain(d) )
     {
-        /* PVH 32bitfixme */
-        ASSERT(!compat);
-
         if ( c(ctrlreg[0]) || c(ctrlreg[1]) || c(ctrlreg[2]) ||
              c(ctrlreg[4]) || c(ctrlreg[5]) || c(ctrlreg[6]) ||
              c(ctrlreg[7]) ||  c(ldt_base) || c(ldt_ents) ||
              c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
              c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
-             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel ||
-             c.nat->gdt_ents || c.nat->fs_base || c.nat->gs_base_user )
+             c(kernel_ss) || c(kernel_sp) || c(gdt_ents) ||
+             (!compat && (c.nat->gs_base_kernel ||
+              c.nat->fs_base || c.nat->gs_base_user)) )
             return -EINVAL;
     }
 
-- 
1.7.1