* [PATCH v2 0/3] nested vmx bug fixes
@ 2013-01-07 6:41 Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 1/3] nested vmx: emulate IA32_VMX_MISC MSR Dongxiao Xu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Dongxiao Xu @ 2013-01-07 6:41 UTC (permalink / raw)
To: xen-devel
Changes from v1 to v2:
- Use a macro to replace the hardcode in patch 1/3.
This patchset fixes issues about IA32_VMX_MISC MSR emulation, VMCS guest area
synchronization about PAGE_FAULT_ERROR_CODE_MASK/PAGE_FAULT_ERROR_CODE_MATCH,
and CR0/CR4 emulation.
Please help to review and pull.
Thanks,
Dongxiao
Dongxiao Xu (3):
nested vmx: emulate IA32_VMX_MISC MSR
nested vmx: synchronize page fault error code match and mask
nested vmx: fix CR0/CR4 emulation
xen/arch/x86/hvm/vmx/vvmx.c | 136 +++++++++++++++++++++++++++++-------
xen/include/asm-x86/hvm/vmx/vmcs.h | 2 +
2 files changed, 112 insertions(+), 26 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] nested vmx: emulate IA32_VMX_MISC MSR
2013-01-07 6:41 [PATCH v2 0/3] nested vmx bug fixes Dongxiao Xu
@ 2013-01-07 6:41 ` Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 2/3] nested vmx: synchronize page fault error code match and mask Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation Dongxiao Xu
2 siblings, 0 replies; 10+ messages in thread
From: Dongxiao Xu @ 2013-01-07 6:41 UTC (permalink / raw)
To: xen-devel
Use the host value to emulate IA32_VMX_MISC MSR for L1 VMM.
For CR3-target value, we don't support this feature currently and
set the number to zero.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
xen/arch/x86/hvm/vmx/vvmx.c | 3 ++-
xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 7b27d2d..16fb3fd 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1462,7 +1462,8 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
data = 0x267ff & ~X86_CR4_SMXE;
break;
case MSR_IA32_VMX_MISC:
- gdprintk(XENLOG_WARNING, "VMX MSR %x not fully supported yet.\n", msr);
+ /* Do not support CR3-target feature now */
+ data = host_data & ~VMX_MISC_CR3_TARGET;
break;
default:
r = 0;
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index ef2c9c9..3adffcc 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -209,6 +209,8 @@ extern bool_t cpu_has_vmx_ins_outs_instr_info;
#define VMX_VPID_INVVPID_ALL_CONTEXT 0x40000000000ULL
#define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 0x80000000000ULL
+#define VMX_MISC_CR3_TARGET 0x1ff0000
+
#define cpu_has_wbinvd_exiting \
(vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
#define cpu_has_vmx_virtualize_apic_accesses \
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] nested vmx: synchronize page fault error code match and mask
2013-01-07 6:41 [PATCH v2 0/3] nested vmx bug fixes Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 1/3] nested vmx: emulate IA32_VMX_MISC MSR Dongxiao Xu
@ 2013-01-07 6:41 ` Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation Dongxiao Xu
2 siblings, 0 replies; 10+ messages in thread
From: Dongxiao Xu @ 2013-01-07 6:41 UTC (permalink / raw)
To: xen-devel
Page fault is specially handled not only with exception bitmaps,
but also with consideration of page fault error code mask/match
values. Therefore in nested virtualization case, the two values
need to be synchronized from virtual VMCS to shadow VMCS.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 16fb3fd..0f13884 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -603,6 +603,17 @@ static void nvmx_update_tpr_threshold(struct vcpu *v)
__vmwrite(TPR_THRESHOLD, 0);
}
+static void nvmx_update_pfec(struct vcpu *v)
+{
+ struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
+ void *vvmcs = nvcpu->nv_vvmcx;
+
+ __vmwrite(PAGE_FAULT_ERROR_CODE_MASK,
+ __get_vvmcs(vvmcs, PAGE_FAULT_ERROR_CODE_MASK));
+ __vmwrite(PAGE_FAULT_ERROR_CODE_MATCH,
+ __get_vvmcs(vvmcs, PAGE_FAULT_ERROR_CODE_MATCH));
+}
+
static void __clear_current_vvmcs(struct vcpu *v)
{
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
@@ -813,6 +824,7 @@ static void load_shadow_control(struct vcpu *v)
nvmx_update_apic_access_address(v);
nvmx_update_virtual_apic_address(v);
nvmx_update_tpr_threshold(v);
+ nvmx_update_pfec(v);
}
static void load_shadow_guest_state(struct vcpu *v)
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-07 6:41 [PATCH v2 0/3] nested vmx bug fixes Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 1/3] nested vmx: emulate IA32_VMX_MISC MSR Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 2/3] nested vmx: synchronize page fault error code match and mask Dongxiao Xu
@ 2013-01-07 6:41 ` Dongxiao Xu
2013-01-11 0:45 ` Xu, Dongxiao
2013-01-11 0:58 ` Dong, Eddie
2 siblings, 2 replies; 10+ messages in thread
From: Dongxiao Xu @ 2013-01-07 6:41 UTC (permalink / raw)
To: xen-devel
While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
guest host mask to 0xffffffff in shadow VMCS, then calculate the
corresponding read shadow separately for CR0 and CR4. While getting
the VM exit for CR0/CR4 access, check if L1 VMM owns the bit. If so,
inject the VM exit to L1 VMM. Otherwise, L0 will handle it and sync
the value to L1 virtual VMCS.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
xen/arch/x86/hvm/vmx/vvmx.c | 121 ++++++++++++++++++++++++++++++++++---------
1 files changed, 96 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 0f13884..d7de286 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -833,6 +833,7 @@ static void load_shadow_guest_state(struct vcpu *v)
void *vvmcs = nvcpu->nv_vvmcx;
int i;
u32 control;
+ u64 cr_gh_mask, cr_read_shadow;
/* vvmcs.gstate to shadow vmcs.gstate */
for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
@@ -854,10 +855,20 @@ static void load_shadow_guest_state(struct vcpu *v)
vvmcs_to_shadow(vvmcs, VM_ENTRY_EXCEPTION_ERROR_CODE);
vvmcs_to_shadow(vvmcs, VM_ENTRY_INSTRUCTION_LEN);
- vvmcs_to_shadow(vvmcs, CR0_READ_SHADOW);
- vvmcs_to_shadow(vvmcs, CR4_READ_SHADOW);
- vvmcs_to_shadow(vvmcs, CR0_GUEST_HOST_MASK);
- vvmcs_to_shadow(vvmcs, CR4_GUEST_HOST_MASK);
+ /*
+ * While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
+ * guest host mask to 0xffffffff in shadow VMCS (follow the host L1 VMCS),
+ * then calculate the corresponding read shadow separately for CR0 and CR4.
+ */
+ cr_gh_mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
+ cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR0) & ~cr_gh_mask) |
+ (__get_vvmcs(vvmcs, CR0_READ_SHADOW) & cr_gh_mask);
+ __vmwrite(CR0_READ_SHADOW, cr_read_shadow);
+
+ cr_gh_mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
+ cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR4) & ~cr_gh_mask) |
+ (__get_vvmcs(vvmcs, CR4_READ_SHADOW) & cr_gh_mask);
+ __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
/* TODO: PDPTRs for nested ept */
/* TODO: CR3 target control */
@@ -913,8 +924,6 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs *regs)
{
int i;
- unsigned long mask;
- unsigned long cr;
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
void *vvmcs = nvcpu->nv_vvmcx;
@@ -925,23 +934,6 @@ static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs *regs)
__set_vvmcs(vvmcs, GUEST_RIP, regs->eip);
__set_vvmcs(vvmcs, GUEST_RSP, regs->esp);
- /* SDM 20.6.6: L2 guest execution may change GUEST CR0/CR4 */
- mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
- if ( ~mask )
- {
- cr = __get_vvmcs(vvmcs, GUEST_CR0);
- cr = (cr & mask) | (__vmread(GUEST_CR0) & ~mask);
- __set_vvmcs(vvmcs, GUEST_CR0, cr);
- }
-
- mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
- if ( ~mask )
- {
- cr = __get_vvmcs(vvmcs, GUEST_CR4);
- cr = (cr & mask) | (__vmread(GUEST_CR4) & ~mask);
- __set_vvmcs(vvmcs, GUEST_CR4, cr);
- }
-
/* CR3 sync if exec doesn't want cr3 load exiting: i.e. nested EPT */
if ( !(__n2_exec_control(v) & CPU_BASED_CR3_LOAD_EXITING) )
shadow_to_vvmcs(vvmcs, GUEST_CR3);
@@ -1745,8 +1737,87 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
nvcpu->nv_vmexit_pending = 1;
}
else /* CR0, CR4, CLTS, LMSW */
- nvcpu->nv_vmexit_pending = 1;
-
+ {
+ /*
+ * While getting the VM exit for CR0/CR4 access, check if L1 VMM owns
+ * the bit.
+ * If so, inject the VM exit to L1 VMM.
+ * Otherwise, L0 will handle it and sync the value to L1 virtual VMCS.
+ */
+ unsigned long old_val, val, changed_bits;
+ switch ( VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification) )
+ {
+ case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
+ {
+ unsigned long gp = VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
+ unsigned long *reg;
+ if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0)) == NULL )
+ {
+ gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
+ break;
+ }
+ val = *reg;
+ if ( cr == 0 )
+ {
+ u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK);
+ old_val = __vmread(CR0_READ_SHADOW);
+ changed_bits = old_val ^ val;
+ if ( changed_bits & cr0_gh_mask )
+ nvcpu->nv_vmexit_pending = 1;
+ else
+ {
+ u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0);
+ __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0, (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
+ }
+ }
+ else if ( cr == 4 )
+ {
+ u64 cr4_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR4_GUEST_HOST_MASK);
+ old_val = __vmread(CR4_READ_SHADOW);
+ changed_bits = old_val ^ val;
+ if ( changed_bits & cr4_gh_mask )
+ nvcpu->nv_vmexit_pending = 1;
+ else
+ {
+ u64 guest_cr4 = __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4);
+ __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4, (guest_cr4 & cr4_gh_mask) | (val & ~cr4_gh_mask));
+ }
+ }
+ else
+ nvcpu->nv_vmexit_pending = 1;
+ break;
+ }
+ case VMX_CONTROL_REG_ACCESS_TYPE_CLTS:
+ {
+ u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK);
+ if ( cr0_gh_mask & X86_CR0_TS )
+ nvcpu->nv_vmexit_pending = 1;
+ else
+ {
+ u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0);
+ __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0, (guest_cr0 & ~X86_CR0_TS));
+ }
+ break;
+ }
+ case VMX_CONTROL_REG_ACCESS_TYPE_LMSW:
+ {
+ u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK);
+ old_val = __vmread(CR0_READ_SHADOW) & 0xf;
+ val = (exit_qualification >> 16) & 0xf;
+ changed_bits = old_val ^ val;
+ if ( changed_bits & cr0_gh_mask )
+ nvcpu->nv_vmexit_pending = 1;
+ else
+ {
+ u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0);
+ __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0, (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
+ }
+ break;
+ }
+ default:
+ break;
+ }
+ }
break;
}
case EXIT_REASON_APIC_ACCESS:
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-07 6:41 ` [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation Dongxiao Xu
@ 2013-01-11 0:45 ` Xu, Dongxiao
2013-01-11 8:08 ` Jan Beulich
2013-01-11 0:58 ` Dong, Eddie
1 sibling, 1 reply; 10+ messages in thread
From: Xu, Dongxiao @ 2013-01-11 0:45 UTC (permalink / raw)
To: xen-devel@lists.xen.org,
Jan Beulich <JBeulich@suse.com> (JBeulich@suse.com)
Cc: Dong, Eddie, Nakajima, Jun
> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org
> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Dongxiao Xu
> Sent: Monday, January 07, 2013 2:42 PM
> To: xen-devel@lists.xen.org
> Subject: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
I saw this patch is not checked in yet, do you have comments on this one?
Thanks,
Dongxiao
>
> While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
> guest host mask to 0xffffffff in shadow VMCS, then calculate the
> corresponding read shadow separately for CR0 and CR4. While getting
> the VM exit for CR0/CR4 access, check if L1 VMM owns the bit. If so,
> inject the VM exit to L1 VMM. Otherwise, L0 will handle it and sync
> the value to L1 virtual VMCS.
>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
> xen/arch/x86/hvm/vmx/vvmx.c | 121
> ++++++++++++++++++++++++++++++++++---------
> 1 files changed, 96 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 0f13884..d7de286 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -833,6 +833,7 @@ static void load_shadow_guest_state(struct vcpu *v)
> void *vvmcs = nvcpu->nv_vvmcx;
> int i;
> u32 control;
> + u64 cr_gh_mask, cr_read_shadow;
>
> /* vvmcs.gstate to shadow vmcs.gstate */
> for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
> @@ -854,10 +855,20 @@ static void load_shadow_guest_state(struct vcpu
> *v)
> vvmcs_to_shadow(vvmcs, VM_ENTRY_EXCEPTION_ERROR_CODE);
> vvmcs_to_shadow(vvmcs, VM_ENTRY_INSTRUCTION_LEN);
>
> - vvmcs_to_shadow(vvmcs, CR0_READ_SHADOW);
> - vvmcs_to_shadow(vvmcs, CR4_READ_SHADOW);
> - vvmcs_to_shadow(vvmcs, CR0_GUEST_HOST_MASK);
> - vvmcs_to_shadow(vvmcs, CR4_GUEST_HOST_MASK);
> + /*
> + * While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
> + * guest host mask to 0xffffffff in shadow VMCS (follow the host L1
> VMCS),
> + * then calculate the corresponding read shadow separately for CR0 and
> CR4.
> + */
> + cr_gh_mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR0) & ~cr_gh_mask) |
> + (__get_vvmcs(vvmcs, CR0_READ_SHADOW)
> & cr_gh_mask);
> + __vmwrite(CR0_READ_SHADOW, cr_read_shadow);
> +
> + cr_gh_mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR4) & ~cr_gh_mask) |
> + (__get_vvmcs(vvmcs, CR4_READ_SHADOW)
> & cr_gh_mask);
> + __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>
> /* TODO: PDPTRs for nested ept */
> /* TODO: CR3 target control */
> @@ -913,8 +924,6 @@ static void virtual_vmentry(struct cpu_user_regs
> *regs)
> static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs
> *regs)
> {
> int i;
> - unsigned long mask;
> - unsigned long cr;
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> void *vvmcs = nvcpu->nv_vvmcx;
>
> @@ -925,23 +934,6 @@ static void sync_vvmcs_guest_state(struct vcpu *v,
> struct cpu_user_regs *regs)
> __set_vvmcs(vvmcs, GUEST_RIP, regs->eip);
> __set_vvmcs(vvmcs, GUEST_RSP, regs->esp);
>
> - /* SDM 20.6.6: L2 guest execution may change GUEST CR0/CR4 */
> - mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> - if ( ~mask )
> - {
> - cr = __get_vvmcs(vvmcs, GUEST_CR0);
> - cr = (cr & mask) | (__vmread(GUEST_CR0) & ~mask);
> - __set_vvmcs(vvmcs, GUEST_CR0, cr);
> - }
> -
> - mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> - if ( ~mask )
> - {
> - cr = __get_vvmcs(vvmcs, GUEST_CR4);
> - cr = (cr & mask) | (__vmread(GUEST_CR4) & ~mask);
> - __set_vvmcs(vvmcs, GUEST_CR4, cr);
> - }
> -
> /* CR3 sync if exec doesn't want cr3 load exiting: i.e. nested EPT */
> if ( !(__n2_exec_control(v) & CPU_BASED_CR3_LOAD_EXITING) )
> shadow_to_vvmcs(vvmcs, GUEST_CR3);
> @@ -1745,8 +1737,87 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs
> *regs,
> nvcpu->nv_vmexit_pending = 1;
> }
> else /* CR0, CR4, CLTS, LMSW */
> - nvcpu->nv_vmexit_pending = 1;
> -
> + {
> + /*
> + * While getting the VM exit for CR0/CR4 access, check if L1
> VMM owns
> + * the bit.
> + * If so, inject the VM exit to L1 VMM.
> + * Otherwise, L0 will handle it and sync the value to L1 virtual
> VMCS.
> + */
> + unsigned long old_val, val, changed_bits;
> + switch
> ( VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification) )
> + {
> + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> + {
> + unsigned long gp =
> VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> + unsigned long *reg;
> + if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0))
> == NULL )
> + {
> + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
> + break;
> + }
> + val = *reg;
> + if ( cr == 0 )
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + old_val = __vmread(CR0_READ_SHADOW);
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr0_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> + }
> + }
> + else if ( cr == 4 )
> + {
> + u64 cr4_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR4_GUEST_HOST_MASK);
> + old_val = __vmread(CR4_READ_SHADOW);
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr4_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr4 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR4);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4,
> (guest_cr4 & cr4_gh_mask) | (val & ~cr4_gh_mask));
> + }
> + }
> + else
> + nvcpu->nv_vmexit_pending = 1;
> + break;
> + }
> + case VMX_CONTROL_REG_ACCESS_TYPE_CLTS:
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + if ( cr0_gh_mask & X86_CR0_TS )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & ~X86_CR0_TS));
> + }
> + break;
> + }
> + case VMX_CONTROL_REG_ACCESS_TYPE_LMSW:
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + old_val = __vmread(CR0_READ_SHADOW) & 0xf;
> + val = (exit_qualification >> 16) & 0xf;
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr0_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> + }
> + break;
> + }
> + default:
> + break;
> + }
> + }
> break;
> }
> case EXIT_REASON_APIC_ACCESS:
> --
> 1.7.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-07 6:41 ` [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation Dongxiao Xu
2013-01-11 0:45 ` Xu, Dongxiao
@ 2013-01-11 0:58 ` Dong, Eddie
1 sibling, 0 replies; 10+ messages in thread
From: Dong, Eddie @ 2013-01-11 0:58 UTC (permalink / raw)
To: Xu, Dongxiao, xen-devel@lists.xen.org; +Cc: Dong, Eddie
Acked by: Eddie Dong <eddie.dong@intel.com>
Thx, Eddie
> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org
> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Dongxiao Xu
> Sent: Monday, January 07, 2013 2:42 PM
> To: xen-devel@lists.xen.org
> Subject: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
>
> While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
> guest host mask to 0xffffffff in shadow VMCS, then calculate the
> corresponding read shadow separately for CR0 and CR4. While getting
> the VM exit for CR0/CR4 access, check if L1 VMM owns the bit. If so,
> inject the VM exit to L1 VMM. Otherwise, L0 will handle it and sync
> the value to L1 virtual VMCS.
>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
> xen/arch/x86/hvm/vmx/vvmx.c | 121
> ++++++++++++++++++++++++++++++++++---------
> 1 files changed, 96 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 0f13884..d7de286 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -833,6 +833,7 @@ static void load_shadow_guest_state(struct vcpu *v)
> void *vvmcs = nvcpu->nv_vvmcx;
> int i;
> u32 control;
> + u64 cr_gh_mask, cr_read_shadow;
>
> /* vvmcs.gstate to shadow vmcs.gstate */
> for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
> @@ -854,10 +855,20 @@ static void load_shadow_guest_state(struct vcpu
> *v)
> vvmcs_to_shadow(vvmcs, VM_ENTRY_EXCEPTION_ERROR_CODE);
> vvmcs_to_shadow(vvmcs, VM_ENTRY_INSTRUCTION_LEN);
>
> - vvmcs_to_shadow(vvmcs, CR0_READ_SHADOW);
> - vvmcs_to_shadow(vvmcs, CR4_READ_SHADOW);
> - vvmcs_to_shadow(vvmcs, CR0_GUEST_HOST_MASK);
> - vvmcs_to_shadow(vvmcs, CR4_GUEST_HOST_MASK);
> + /*
> + * While emulate CR0 and CR4 for nested virtualization, set the
> CR0/CR4
> + * guest host mask to 0xffffffff in shadow VMCS (follow the host L1
> VMCS),
> + * then calculate the corresponding read shadow separately for CR0
> and CR4.
> + */
> + cr_gh_mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR0) & ~cr_gh_mask)
> |
> + (__get_vvmcs(vvmcs, CR0_READ_SHADOW)
> & cr_gh_mask);
> + __vmwrite(CR0_READ_SHADOW, cr_read_shadow);
> +
> + cr_gh_mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR4) & ~cr_gh_mask)
> |
> + (__get_vvmcs(vvmcs, CR4_READ_SHADOW)
> & cr_gh_mask);
> + __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>
> /* TODO: PDPTRs for nested ept */
> /* TODO: CR3 target control */
> @@ -913,8 +924,6 @@ static void virtual_vmentry(struct cpu_user_regs
> *regs)
> static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs
> *regs)
> {
> int i;
> - unsigned long mask;
> - unsigned long cr;
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> void *vvmcs = nvcpu->nv_vvmcx;
>
> @@ -925,23 +934,6 @@ static void sync_vvmcs_guest_state(struct vcpu *v,
> struct cpu_user_regs *regs)
> __set_vvmcs(vvmcs, GUEST_RIP, regs->eip);
> __set_vvmcs(vvmcs, GUEST_RSP, regs->esp);
>
> - /* SDM 20.6.6: L2 guest execution may change GUEST CR0/CR4 */
> - mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> - if ( ~mask )
> - {
> - cr = __get_vvmcs(vvmcs, GUEST_CR0);
> - cr = (cr & mask) | (__vmread(GUEST_CR0) & ~mask);
> - __set_vvmcs(vvmcs, GUEST_CR0, cr);
> - }
> -
> - mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> - if ( ~mask )
> - {
> - cr = __get_vvmcs(vvmcs, GUEST_CR4);
> - cr = (cr & mask) | (__vmread(GUEST_CR4) & ~mask);
> - __set_vvmcs(vvmcs, GUEST_CR4, cr);
> - }
> -
> /* CR3 sync if exec doesn't want cr3 load exiting: i.e. nested EPT */
> if ( !(__n2_exec_control(v) & CPU_BASED_CR3_LOAD_EXITING) )
> shadow_to_vvmcs(vvmcs, GUEST_CR3);
> @@ -1745,8 +1737,87 @@ int nvmx_n2_vmexit_handler(struct
> cpu_user_regs *regs,
> nvcpu->nv_vmexit_pending = 1;
> }
> else /* CR0, CR4, CLTS, LMSW */
> - nvcpu->nv_vmexit_pending = 1;
> -
> + {
> + /*
> + * While getting the VM exit for CR0/CR4 access, check if L1
> VMM owns
> + * the bit.
> + * If so, inject the VM exit to L1 VMM.
> + * Otherwise, L0 will handle it and sync the value to L1
> virtual VMCS.
> + */
> + unsigned long old_val, val, changed_bits;
> + switch
> ( VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification) )
> + {
> + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> + {
> + unsigned long gp =
> VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> + unsigned long *reg;
> + if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0))
> == NULL )
> + {
> + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
> + break;
> + }
> + val = *reg;
> + if ( cr == 0 )
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + old_val = __vmread(CR0_READ_SHADOW);
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr0_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 =
> __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> + }
> + }
> + else if ( cr == 4 )
> + {
> + u64 cr4_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR4_GUEST_HOST_MASK);
> + old_val = __vmread(CR4_READ_SHADOW);
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr4_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr4 =
> __get_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4,
> (guest_cr4 & cr4_gh_mask) | (val & ~cr4_gh_mask));
> + }
> + }
> + else
> + nvcpu->nv_vmexit_pending = 1;
> + break;
> + }
> + case VMX_CONTROL_REG_ACCESS_TYPE_CLTS:
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + if ( cr0_gh_mask & X86_CR0_TS )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & ~X86_CR0_TS));
> + }
> + break;
> + }
> + case VMX_CONTROL_REG_ACCESS_TYPE_LMSW:
> + {
> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> CR0_GUEST_HOST_MASK);
> + old_val = __vmread(CR0_READ_SHADOW) & 0xf;
> + val = (exit_qualification >> 16) & 0xf;
> + changed_bits = old_val ^ val;
> + if ( changed_bits & cr0_gh_mask )
> + nvcpu->nv_vmexit_pending = 1;
> + else
> + {
> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> GUEST_CR0);
> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> + }
> + break;
> + }
> + default:
> + break;
> + }
> + }
> break;
> }
> case EXIT_REASON_APIC_ACCESS:
> --
> 1.7.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-11 0:45 ` Xu, Dongxiao
@ 2013-01-11 8:08 ` Jan Beulich
2013-01-11 15:47 ` Xu, Dongxiao
0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2013-01-11 8:08 UTC (permalink / raw)
To: Dongxiao Xu; +Cc: Eddie Dong, Jun Nakajima, xen-devel@lists.xen.org
>>> On 11.01.13 at 01:45, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org
>> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Dongxiao Xu
>> Sent: Monday, January 07, 2013 2:42 PM
>> To: xen-devel@lists.xen.org
>> Subject: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
>
> I saw this patch is not checked in yet, do you have comments on this one?
No, but other than for the two other ones which I could review
myself easily enough, I'd expect an ack from Jun or Eddie
(or one of the other x86 maintainers) on this one.
In general, with VMX patches coming from people at Intel it would
certainly help if you asked your colleagues to do the first review
round before you even submit, indicating it happened by attaching
the respective tag from the beginning.
Jan
>> While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
>> guest host mask to 0xffffffff in shadow VMCS, then calculate the
>> corresponding read shadow separately for CR0 and CR4. While getting
>> the VM exit for CR0/CR4 access, check if L1 VMM owns the bit. If so,
>> inject the VM exit to L1 VMM. Otherwise, L0 will handle it and sync
>> the value to L1 virtual VMCS.
>>
>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>> ---
>> xen/arch/x86/hvm/vmx/vvmx.c | 121
>> ++++++++++++++++++++++++++++++++++---------
>> 1 files changed, 96 insertions(+), 25 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> index 0f13884..d7de286 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -833,6 +833,7 @@ static void load_shadow_guest_state(struct vcpu *v)
>> void *vvmcs = nvcpu->nv_vvmcx;
>> int i;
>> u32 control;
>> + u64 cr_gh_mask, cr_read_shadow;
>>
>> /* vvmcs.gstate to shadow vmcs.gstate */
>> for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
>> @@ -854,10 +855,20 @@ static void load_shadow_guest_state(struct vcpu
>> *v)
>> vvmcs_to_shadow(vvmcs, VM_ENTRY_EXCEPTION_ERROR_CODE);
>> vvmcs_to_shadow(vvmcs, VM_ENTRY_INSTRUCTION_LEN);
>>
>> - vvmcs_to_shadow(vvmcs, CR0_READ_SHADOW);
>> - vvmcs_to_shadow(vvmcs, CR4_READ_SHADOW);
>> - vvmcs_to_shadow(vvmcs, CR0_GUEST_HOST_MASK);
>> - vvmcs_to_shadow(vvmcs, CR4_GUEST_HOST_MASK);
>> + /*
>> + * While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
>> + * guest host mask to 0xffffffff in shadow VMCS (follow the host L1
>> VMCS),
>> + * then calculate the corresponding read shadow separately for CR0 and
>> CR4.
>> + */
>> + cr_gh_mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
>> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR0) & ~cr_gh_mask) |
>> + (__get_vvmcs(vvmcs, CR0_READ_SHADOW)
>> & cr_gh_mask);
>> + __vmwrite(CR0_READ_SHADOW, cr_read_shadow);
>> +
>> + cr_gh_mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
>> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR4) & ~cr_gh_mask) |
>> + (__get_vvmcs(vvmcs, CR4_READ_SHADOW)
>> & cr_gh_mask);
>> + __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>>
>> /* TODO: PDPTRs for nested ept */
>> /* TODO: CR3 target control */
>> @@ -913,8 +924,6 @@ static void virtual_vmentry(struct cpu_user_regs
>> *regs)
>> static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs
>> *regs)
>> {
>> int i;
>> - unsigned long mask;
>> - unsigned long cr;
>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>> void *vvmcs = nvcpu->nv_vvmcx;
>>
>> @@ -925,23 +934,6 @@ static void sync_vvmcs_guest_state(struct vcpu *v,
>> struct cpu_user_regs *regs)
>> __set_vvmcs(vvmcs, GUEST_RIP, regs->eip);
>> __set_vvmcs(vvmcs, GUEST_RSP, regs->esp);
>>
>> - /* SDM 20.6.6: L2 guest execution may change GUEST CR0/CR4 */
>> - mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
>> - if ( ~mask )
>> - {
>> - cr = __get_vvmcs(vvmcs, GUEST_CR0);
>> - cr = (cr & mask) | (__vmread(GUEST_CR0) & ~mask);
>> - __set_vvmcs(vvmcs, GUEST_CR0, cr);
>> - }
>> -
>> - mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
>> - if ( ~mask )
>> - {
>> - cr = __get_vvmcs(vvmcs, GUEST_CR4);
>> - cr = (cr & mask) | (__vmread(GUEST_CR4) & ~mask);
>> - __set_vvmcs(vvmcs, GUEST_CR4, cr);
>> - }
>> -
>> /* CR3 sync if exec doesn't want cr3 load exiting: i.e. nested EPT */
>> if ( !(__n2_exec_control(v) & CPU_BASED_CR3_LOAD_EXITING) )
>> shadow_to_vvmcs(vvmcs, GUEST_CR3);
>> @@ -1745,8 +1737,87 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs
>> *regs,
>> nvcpu->nv_vmexit_pending = 1;
>> }
>> else /* CR0, CR4, CLTS, LMSW */
>> - nvcpu->nv_vmexit_pending = 1;
>> -
>> + {
>> + /*
>> + * While getting the VM exit for CR0/CR4 access, check if L1
>> VMM owns
>> + * the bit.
>> + * If so, inject the VM exit to L1 VMM.
>> + * Otherwise, L0 will handle it and sync the value to L1
> virtual
>> VMCS.
>> + */
>> + unsigned long old_val, val, changed_bits;
>> + switch
>> ( VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification) )
>> + {
>> + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
>> + {
>> + unsigned long gp =
>> VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
>> + unsigned long *reg;
>> + if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0))
>> == NULL )
>> + {
>> + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
>> + break;
>> + }
>> + val = *reg;
>> + if ( cr == 0 )
>> + {
>> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
>> CR0_GUEST_HOST_MASK);
>> + old_val = __vmread(CR0_READ_SHADOW);
>> + changed_bits = old_val ^ val;
>> + if ( changed_bits & cr0_gh_mask )
>> + nvcpu->nv_vmexit_pending = 1;
>> + else
>> + {
>> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
>> GUEST_CR0);
>> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
>> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
>> + }
>> + }
>> + else if ( cr == 4 )
>> + {
>> + u64 cr4_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
>> CR4_GUEST_HOST_MASK);
>> + old_val = __vmread(CR4_READ_SHADOW);
>> + changed_bits = old_val ^ val;
>> + if ( changed_bits & cr4_gh_mask )
>> + nvcpu->nv_vmexit_pending = 1;
>> + else
>> + {
>> + u64 guest_cr4 = __get_vvmcs(nvcpu->nv_vvmcx,
>> GUEST_CR4);
>> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4,
>> (guest_cr4 & cr4_gh_mask) | (val & ~cr4_gh_mask));
>> + }
>> + }
>> + else
>> + nvcpu->nv_vmexit_pending = 1;
>> + break;
>> + }
>> + case VMX_CONTROL_REG_ACCESS_TYPE_CLTS:
>> + {
>> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
>> CR0_GUEST_HOST_MASK);
>> + if ( cr0_gh_mask & X86_CR0_TS )
>> + nvcpu->nv_vmexit_pending = 1;
>> + else
>> + {
>> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
>> GUEST_CR0);
>> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
>> (guest_cr0 & ~X86_CR0_TS));
>> + }
>> + break;
>> + }
>> + case VMX_CONTROL_REG_ACCESS_TYPE_LMSW:
>> + {
>> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
>> CR0_GUEST_HOST_MASK);
>> + old_val = __vmread(CR0_READ_SHADOW) & 0xf;
>> + val = (exit_qualification >> 16) & 0xf;
>> + changed_bits = old_val ^ val;
>> + if ( changed_bits & cr0_gh_mask )
>> + nvcpu->nv_vmexit_pending = 1;
>> + else
>> + {
>> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
>> GUEST_CR0);
>> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
>> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
>> + }
>> + break;
>> + }
>> + default:
>> + break;
>> + }
>> + }
>> break;
>> }
>> case EXIT_REASON_APIC_ACCESS:
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-11 8:08 ` Jan Beulich
@ 2013-01-11 15:47 ` Xu, Dongxiao
2013-01-11 16:46 ` Nakajima, Jun
0 siblings, 1 reply; 10+ messages in thread
From: Xu, Dongxiao @ 2013-01-11 15:47 UTC (permalink / raw)
To: Jan Beulich; +Cc: Dong, Eddie, Nakajima, Jun, xen-devel@lists.xen.org
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, January 11, 2013 4:09 PM
> To: Xu, Dongxiao
> Cc: Dong, Eddie; Nakajima, Jun; xen-devel@lists.xen.org
> Subject: RE: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
>
> >>> On 11.01.13 at 01:45, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> >> -----Original Message-----
> >> From: xen-devel-bounces@lists.xen.org
> >> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Dongxiao Xu
> >> Sent: Monday, January 07, 2013 2:42 PM
> >> To: xen-devel@lists.xen.org
> >> Subject: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
> >
> > I saw this patch is not checked in yet, do you have comments on this one?
>
> No, but other than for the two other ones which I could review
> myself easily enough, I'd expect an ack from Jun or Eddie
> (or one of the other x86 maintainers) on this one.
>
> In general, with VMX patches coming from people at Intel it would
> certainly help if you asked your colleagues to do the first review
> round before you even submit, indicating it happened by attaching
> the respective tag from the beginning.
OK, I will CC Jun and Eddie by default if sending later patches.
And for this patch, Eddie acked it in another mail.
Thanks,
Dongxiao
>
> Jan
>
> >> While emulate CR0 and CR4 for nested virtualization, set the CR0/CR4
> >> guest host mask to 0xffffffff in shadow VMCS, then calculate the
> >> corresponding read shadow separately for CR0 and CR4. While getting
> >> the VM exit for CR0/CR4 access, check if L1 VMM owns the bit. If so,
> >> inject the VM exit to L1 VMM. Otherwise, L0 will handle it and sync
> >> the value to L1 virtual VMCS.
> >>
> >> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> >> ---
> >> xen/arch/x86/hvm/vmx/vvmx.c | 121
> >> ++++++++++++++++++++++++++++++++++---------
> >> 1 files changed, 96 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> >> index 0f13884..d7de286 100644
> >> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> >> @@ -833,6 +833,7 @@ static void load_shadow_guest_state(struct vcpu
> *v)
> >> void *vvmcs = nvcpu->nv_vvmcx;
> >> int i;
> >> u32 control;
> >> + u64 cr_gh_mask, cr_read_shadow;
> >>
> >> /* vvmcs.gstate to shadow vmcs.gstate */
> >> for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
> >> @@ -854,10 +855,20 @@ static void load_shadow_guest_state(struct vcpu
> >> *v)
> >> vvmcs_to_shadow(vvmcs, VM_ENTRY_EXCEPTION_ERROR_CODE);
> >> vvmcs_to_shadow(vvmcs, VM_ENTRY_INSTRUCTION_LEN);
> >>
> >> - vvmcs_to_shadow(vvmcs, CR0_READ_SHADOW);
> >> - vvmcs_to_shadow(vvmcs, CR4_READ_SHADOW);
> >> - vvmcs_to_shadow(vvmcs, CR0_GUEST_HOST_MASK);
> >> - vvmcs_to_shadow(vvmcs, CR4_GUEST_HOST_MASK);
> >> + /*
> >> + * While emulate CR0 and CR4 for nested virtualization, set the
> CR0/CR4
> >> + * guest host mask to 0xffffffff in shadow VMCS (follow the host L1
> >> VMCS),
> >> + * then calculate the corresponding read shadow separately for CR0
> and
> >> CR4.
> >> + */
> >> + cr_gh_mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> >> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR0) & ~cr_gh_mask)
> |
> >> + (__get_vvmcs(vvmcs,
> CR0_READ_SHADOW)
> >> & cr_gh_mask);
> >> + __vmwrite(CR0_READ_SHADOW, cr_read_shadow);
> >> +
> >> + cr_gh_mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> >> + cr_read_shadow = (__get_vvmcs(vvmcs, GUEST_CR4) & ~cr_gh_mask)
> |
> >> + (__get_vvmcs(vvmcs,
> CR4_READ_SHADOW)
> >> & cr_gh_mask);
> >> + __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
> >>
> >> /* TODO: PDPTRs for nested ept */
> >> /* TODO: CR3 target control */
> >> @@ -913,8 +924,6 @@ static void virtual_vmentry(struct cpu_user_regs
> >> *regs)
> >> static void sync_vvmcs_guest_state(struct vcpu *v, struct cpu_user_regs
> >> *regs)
> >> {
> >> int i;
> >> - unsigned long mask;
> >> - unsigned long cr;
> >> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> >> void *vvmcs = nvcpu->nv_vvmcx;
> >>
> >> @@ -925,23 +934,6 @@ static void sync_vvmcs_guest_state(struct vcpu
> *v,
> >> struct cpu_user_regs *regs)
> >> __set_vvmcs(vvmcs, GUEST_RIP, regs->eip);
> >> __set_vvmcs(vvmcs, GUEST_RSP, regs->esp);
> >>
> >> - /* SDM 20.6.6: L2 guest execution may change GUEST CR0/CR4 */
> >> - mask = __get_vvmcs(vvmcs, CR0_GUEST_HOST_MASK);
> >> - if ( ~mask )
> >> - {
> >> - cr = __get_vvmcs(vvmcs, GUEST_CR0);
> >> - cr = (cr & mask) | (__vmread(GUEST_CR0) & ~mask);
> >> - __set_vvmcs(vvmcs, GUEST_CR0, cr);
> >> - }
> >> -
> >> - mask = __get_vvmcs(vvmcs, CR4_GUEST_HOST_MASK);
> >> - if ( ~mask )
> >> - {
> >> - cr = __get_vvmcs(vvmcs, GUEST_CR4);
> >> - cr = (cr & mask) | (__vmread(GUEST_CR4) & ~mask);
> >> - __set_vvmcs(vvmcs, GUEST_CR4, cr);
> >> - }
> >> -
> >> /* CR3 sync if exec doesn't want cr3 load exiting: i.e. nested EPT */
> >> if ( !(__n2_exec_control(v) & CPU_BASED_CR3_LOAD_EXITING) )
> >> shadow_to_vvmcs(vvmcs, GUEST_CR3);
> >> @@ -1745,8 +1737,87 @@ int nvmx_n2_vmexit_handler(struct
> cpu_user_regs
> >> *regs,
> >> nvcpu->nv_vmexit_pending = 1;
> >> }
> >> else /* CR0, CR4, CLTS, LMSW */
> >> - nvcpu->nv_vmexit_pending = 1;
> >> -
> >> + {
> >> + /*
> >> + * While getting the VM exit for CR0/CR4 access, check if L1
> >> VMM owns
> >> + * the bit.
> >> + * If so, inject the VM exit to L1 VMM.
> >> + * Otherwise, L0 will handle it and sync the value to L1
> > virtual
> >> VMCS.
> >> + */
> >> + unsigned long old_val, val, changed_bits;
> >> + switch
> >> ( VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification) )
> >> + {
> >> + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> >> + {
> >> + unsigned long gp =
> >> VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> >> + unsigned long *reg;
> >> + if ( (reg = decode_register(gp, guest_cpu_user_regs(),
> 0))
> >> == NULL )
> >> + {
> >> + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
> >> + break;
> >> + }
> >> + val = *reg;
> >> + if ( cr == 0 )
> >> + {
> >> + u64 cr0_gh_mask =
> __get_vvmcs(nvcpu->nv_vvmcx,
> >> CR0_GUEST_HOST_MASK);
> >> + old_val = __vmread(CR0_READ_SHADOW);
> >> + changed_bits = old_val ^ val;
> >> + if ( changed_bits & cr0_gh_mask )
> >> + nvcpu->nv_vmexit_pending = 1;
> >> + else
> >> + {
> >> + u64 guest_cr0 =
> __get_vvmcs(nvcpu->nv_vvmcx,
> >> GUEST_CR0);
> >> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> >> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> >> + }
> >> + }
> >> + else if ( cr == 4 )
> >> + {
> >> + u64 cr4_gh_mask =
> __get_vvmcs(nvcpu->nv_vvmcx,
> >> CR4_GUEST_HOST_MASK);
> >> + old_val = __vmread(CR4_READ_SHADOW);
> >> + changed_bits = old_val ^ val;
> >> + if ( changed_bits & cr4_gh_mask )
> >> + nvcpu->nv_vmexit_pending = 1;
> >> + else
> >> + {
> >> + u64 guest_cr4 =
> __get_vvmcs(nvcpu->nv_vvmcx,
> >> GUEST_CR4);
> >> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR4,
> >> (guest_cr4 & cr4_gh_mask) | (val & ~cr4_gh_mask));
> >> + }
> >> + }
> >> + else
> >> + nvcpu->nv_vmexit_pending = 1;
> >> + break;
> >> + }
> >> + case VMX_CONTROL_REG_ACCESS_TYPE_CLTS:
> >> + {
> >> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> >> CR0_GUEST_HOST_MASK);
> >> + if ( cr0_gh_mask & X86_CR0_TS )
> >> + nvcpu->nv_vmexit_pending = 1;
> >> + else
> >> + {
> >> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> >> GUEST_CR0);
> >> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> >> (guest_cr0 & ~X86_CR0_TS));
> >> + }
> >> + break;
> >> + }
> >> + case VMX_CONTROL_REG_ACCESS_TYPE_LMSW:
> >> + {
> >> + u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx,
> >> CR0_GUEST_HOST_MASK);
> >> + old_val = __vmread(CR0_READ_SHADOW) & 0xf;
> >> + val = (exit_qualification >> 16) & 0xf;
> >> + changed_bits = old_val ^ val;
> >> + if ( changed_bits & cr0_gh_mask )
> >> + nvcpu->nv_vmexit_pending = 1;
> >> + else
> >> + {
> >> + u64 guest_cr0 = __get_vvmcs(nvcpu->nv_vvmcx,
> >> GUEST_CR0);
> >> + __set_vvmcs(nvcpu->nv_vvmcx, GUEST_CR0,
> >> (guest_cr0 & cr0_gh_mask) | (val & ~cr0_gh_mask));
> >> + }
> >> + break;
> >> + }
> >> + default:
> >> + break;
> >> + }
> >> + }
> >> break;
> >> }
> >> case EXIT_REASON_APIC_ACCESS:
> >> --
> >> 1.7.1
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-11 15:47 ` Xu, Dongxiao
@ 2013-01-11 16:46 ` Nakajima, Jun
2013-01-14 9:00 ` Xu, Dongxiao
0 siblings, 1 reply; 10+ messages in thread
From: Nakajima, Jun @ 2013-01-11 16:46 UTC (permalink / raw)
To: Xu, Dongxiao; +Cc: Dong, Eddie, Jan Beulich, xen-devel@lists.xen.org
On Fri, Jan 11, 2013 at 7:47 AM, Xu, Dongxiao <dongxiao.xu@intel.com> wrote:
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@suse.com]
> > Sent: Friday, January 11, 2013 4:09 PM
> > To: Xu, Dongxiao
> > Cc: Dong, Eddie; Nakajima, Jun; xen-devel@lists.xen.org
> > Subject: RE: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4
> > emulation
> >
> > >>> On 11.01.13 at 01:45, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> > >> -----Original Message-----
> > >> From: xen-devel-bounces@lists.xen.org
> > >> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Dongxiao Xu
> > >> Sent: Monday, January 07, 2013 2:42 PM
> > >> To: xen-devel@lists.xen.org
> > >> Subject: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
> > >
> > > I saw this patch is not checked in yet, do you have comments on this
> > > one?
> >
> > No, but other than for the two other ones which I could review
> > myself easily enough, I'd expect an ack from Jun or Eddie
> > (or one of the other x86 maintainers) on this one.
> >
> > In general, with VMX patches coming from people at Intel it would
> > certainly help if you asked your colleagues to do the first review
> > round before you even submit, indicating it happened by attaching
> > the respective tag from the beginning.
>
> OK, I will CC Jun and Eddie by default if sending later patches.
> And for this patch, Eddie acked it in another mail.
>
> Thanks,
> Dongxiao
>
> >
> > Jan
One comment:
> + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> + {
> + unsigned long gp = VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> + unsigned long *reg;
> + if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0)) == NULL )
> + {
> + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
> + break;
> + }
> >
What happens after the "break" (if it really happens), or how the
error is handled?
--
Jun
Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
2013-01-11 16:46 ` Nakajima, Jun
@ 2013-01-14 9:00 ` Xu, Dongxiao
0 siblings, 0 replies; 10+ messages in thread
From: Xu, Dongxiao @ 2013-01-14 9:00 UTC (permalink / raw)
To: Nakajima, Jun; +Cc: Dong, Eddie, Jan Beulich, xen-devel@lists.xen.org
> -----Original Message-----
> From: Nakajima, Jun [mailto:jun.nakajima@intel.com]
> Sent: Saturday, January 12, 2013 12:46 AM
> To: Xu, Dongxiao
> Cc: Jan Beulich; Dong, Eddie; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation
>
>
> One comment:
>
> > + case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> > + {
> > + unsigned long gp =
> VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> > + unsigned long *reg;
> > + if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0))
> == NULL )
> > + {
> > + gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
> > + break;
> > + }
> > >
>
> What happens after the "break" (if it really happens), or how the
> error is handled?
Hi Jun,
Thanks for the comment.
In current nvmx_n2_vmexit_handler() function, there is no error code in return value. Therefore if an exception happens, the current solution just ignored the error and step forward...
There are two places that may trigger error, one is my above code, another is the "default" item in switch/case selection logic.
I will write a patch to address this issue, adding a fault indicator in the return value.
Thanks,
Dongxiao
>
>
> --
> Jun
> Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-01-14 9:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-07 6:41 [PATCH v2 0/3] nested vmx bug fixes Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 1/3] nested vmx: emulate IA32_VMX_MISC MSR Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 2/3] nested vmx: synchronize page fault error code match and mask Dongxiao Xu
2013-01-07 6:41 ` [PATCH v2 3/3] nested vmx: fix CR0/CR4 emulation Dongxiao Xu
2013-01-11 0:45 ` Xu, Dongxiao
2013-01-11 8:08 ` Jan Beulich
2013-01-11 15:47 ` Xu, Dongxiao
2013-01-11 16:46 ` Nakajima, Jun
2013-01-14 9:00 ` Xu, Dongxiao
2013-01-11 0:58 ` Dong, Eddie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).