[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC v13 06/20] pvh: vmx-specific changes



On 26/09/13 16:29, Jan Beulich wrote:
On 23.09.13 at 18:49, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
Changes:
* Enforce HAP mode for now
* Disable exits related to virtual interrupts or emulated APICs
* Disable changing paging mode
  - "unrestricted guest" (i.e., real mode for EPT) disabled
  - write guest EFER disabled
* Start in 64-bit mode
* Force TSC mode to be "none"
* Paging mode update to happen in arch_set_info_guest

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
---
v13:
  - Fix up default cr0 settings
  - Get rid of some unnecessary PVH-related changes
  - Return EOPNOTSUPP instead of ENOSYS if hardware features are not present
  - Remove an unnecessary variable from pvh_check_requirements
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
---
  xen/arch/x86/hvm/vmx/vmcs.c |  130
+++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 126 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index cf54d18..53fccdf 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -828,6 +828,60 @@ void virtual_vmcs_vmwrite(void *vvmcs, u32
vmcs_encoding, u64 val)
      virtual_vmcs_exit(vvmcs);
  }
+static int pvh_check_requirements(struct vcpu *v)
+{
+    u64 required;
+
+    /* Check for required hardware features */
+    if ( !cpu_has_vmx_ept )
+    {
+        printk(XENLOG_G_INFO "PVH: CPU does not have EPT support\n");
+        return -EOPNOTSUPP;
+    }
+    if ( !cpu_has_vmx_pat )
+    {
+        printk(XENLOG_G_INFO "PVH: CPU does not have PAT support\n");
+        return -EOPNOTSUPP;
+    }
+    if ( !cpu_has_vmx_msr_bitmap )
+    {
+        printk(XENLOG_G_INFO "PVH: CPU does not have msr bitmap\n");
+        return -EOPNOTSUPP;
+    }
+    if ( !cpu_has_vmx_secondary_exec_control )
+    {
+        printk(XENLOG_G_INFO "CPU Secondary exec is required to run PVH\n");
+        return -EOPNOTSUPP;
+    }
Up to here the checks are VMX specific, and hence belong in a VMX
specific file, ...

+    required = X86_CR4_PAE | X86_CR4_VMXE | X86_CR4_OSFXSR;
+    if ( (real_cr4_to_pv_guest_cr4(mmu_cr4_features) & required) != required )
+    {
+        printk(XENLOG_G_INFO "PVH: required CR4 features not available:%lx\n",
+               required);
+        return -EOPNOTSUPP;
+    }
+
+    /* Check for required configuration options */
+    if ( !paging_mode_hap(v->domain) )
+    {
+        printk(XENLOG_G_INFO "HAP is required for PVH guest.\n");
+        return -EINVAL;
+    }
+    /*
+     * If rdtsc exiting is turned on and it goes thru emulate_privileged_op,
+     * then pv_vcpu.ctrlreg must be added to the pvh struct.
+     */
+    if ( v->domain->arch.vtsc )
+    {
+        printk(XENLOG_G_INFO
+               "At present PVH only supports the default timer mode\n");
+        return -EINVAL;
+    }
... but all of these are pretty generic (apart from the X86_CR4_VMXE
in the CR4 mask checked above, but I wonder whether that
shouldn't be checked much earlier - for HVM guests no such check
exists here afaik).

The hap check can be removed and checked in hvm_domain_initialise(), and the vtsc one moved to tsc_set_info().

Is it really necessary to check these bits in cr4? Or is this more or less an ASSERT() to make sure people haven't accidentally disabled these in the real->guest cr4 conversion?

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.