[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2] Nested VMX: CR emulation fix up
Jan Beulich wrote on 2013-10-28: >>>> On 23.10.13 at 09:13, Yang Zhang <yang.z.zhang@xxxxxxxxx> wrote: >> From: Yang Zhang <yang.z.zhang@xxxxxxxxx> >> >> This patch fixs two issues: >> 1. The CR_READ_SHADOW should only cover the value that L2 wirtes to >> CR when L2 is running. But currently, L0 wirtes wrong value to it >> during virtual vmentry and L2's CR access emualtion. >> >> 2. L2 changed cr[0/4] in a way that did not change any of L1's >> shadowed bits, but did change L0 shadowed bits. In this case, the >> effective cr[0/4] value that L1 would like to write into the >> hardware is consist of the L2-owned bits from the new value combined >> with the L1-owned bits from L1's guest cr[0/4]. >> >> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx> >> Acked-by: "Dong, Eddie" <eddie.dong@xxxxxxxxx> > > So apart from wanting to see a proper ack on this patch (or a > description of the changes in v2 that makes clear that a previous ack did not > get invalidated) ... > >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -1096,6 +1096,29 @@ static void vmx_update_guest_cr(struct vcpu >> *v, > unsigned int cr) >> vmx_update_cpu_exec_control(v); >> } >> + if ( !nestedhvm_vcpu_in_guestmode(v) ) + >> __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]); + else >> if ( !nestedhvm_vmswitch_in_progress(v) ) + { + /* + >> * We get here when L2 changed cr0 in a way that did + not >> change + * any of L1's shadowed bits (see >> nvmx_n2_vmexit_handler), + * but did change L0 shadowed >> bits. So we first calculate the + * effective cr0 value >> that L1 would like to write into the + * hardware. It >> consists of the L2-owned bits from the new + * value >> combined with the L1-owned bits from L1's + guest cr0. + */ >> + v->arch.hvm_vcpu.guest_cr[0] &= + >> ~__get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, CR0_GUEST_HOST_MASK); + >> v->arch.hvm_vcpu.guest_cr[0] |= + >> __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, GUEST_CR0) & + >> __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, + CR0_GUEST_HOST_MASK); > > ... I'd also like to see clarified whether __get_vvmcs() really is > cheap enough to get invoked twice for the same register here instead > of the result latched into a local variable and re-used. Actually, this code is executed rarely. But you are right, using a local variable is a better choice. > >> + + /* nvcpu.guest_cr[0] is what L2 write to cr0 actually. >> */ + __vmwrite(CR0_READ_SHADOW, >> v->arch.hvm_vcpu.nvcpu.guest_cr[0]); + } else > > And should the above result in the patch to be rev'ed again, please > also correct the coding style violation here. Current code seems a little redundant. I revised it to put the redundant code into nvmx_set_cr_read_shadow(). Please review it. void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned int cr) { unsigned long cr_field, read_shadow_field, mask_field; switch ( cr ) { case 0: cr_field = GUEST_CR0; read_shadow_field = CR0_READ_SHADOW; mask_field = CR0_GUEST_HOST_MASK; break; case 4: cr_field = GUEST_CR4; read_shadow_field = CR4_READ_SHADOW; mask_field = CR4_GUEST_HOST_MASK; break; default: gdprintk(XENLOG_WARNING, "Set read shadow for CR%d.\n", cr); return; } if ( !nestedhvm_vmswitch_in_progress(v) ) { unsigned long virtual_cr_mask = __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, mask_field); /* * We get here when L2 changed cr in a way that did not change * any of L1's shadowed bits (see nvmx_n2_vmexit_handler), * but did change L0 shadowed bits. So we first calculate the * effective cr value that L1 would like to write into the * hardware. It consists of the L2-owned bits from the new * value combined with the L1-owned bits from L1's guest cr. */ v->arch.hvm_vcpu.guest_cr[cr] &= ~virtual_cr_mask; v->arch.hvm_vcpu.guest_cr[cr] |= virtual_cr_mask & __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, cr_field); } /* nvcpu.guest_cr is what L2 write to cr actually. */ __vmwrite(read_shadow_field, v->arch.hvm_vcpu.nvcpu.guest_cr[cr]); } > > Jan Best regards, Yang _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |