[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 08/10] nEPT: handle invept instruction from L1 VMM
Thanks, Jan! >>: handle invept instruction from L1 VMM > > >>> On 19.12.12 at 20:44, Xiantao Zhang <xiantao.zhang@xxxxxxxxx> wrote: > > --- a/xen/arch/x86/hvm/vmx/vmx.c > > +++ b/xen/arch/x86/hvm/vmx/vmx.c > > @@ -2572,11 +2572,13 @@ void vmx_vmexit_handler(struct > cpu_user_regs *regs) > > if ( nvmx_handle_vmresume(regs) == X86EMUL_OKAY ) > > update_guest_eip(); > > break; > > - > > + case EXIT_REASON_INVEPT: > > + if ( nvmx_handle_invept(regs) == X86EMUL_OKAY ) > > + update_guest_eip(); > > + break; > > In (potentially going to become) long switch statements, please don't drop > the blank lines between individual cases - instead of dropping the line here, > you wold want to insert another one below the new separately handled case. Okay. > > case EXIT_REASON_MWAIT_INSTRUCTION: > > case EXIT_REASON_MONITOR_INSTRUCTION: > > case EXIT_REASON_GETSEC: > > - case EXIT_REASON_INVEPT: > > case EXIT_REASON_INVVPID: > > /* > > * We should never exit on GETSEC because CR4.SMXE is always > > 0 when > > --- a/xen/arch/x86/hvm/vmx/vvmx.c > > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > > @@ -1356,6 +1356,45 @@ int nvmx_handle_vmwrite(struct cpu_user_regs > *regs) > > return X86EMUL_OKAY; > > } > > > > +int nvmx_handle_invept(struct cpu_user_regs *regs) { > > + struct vmx_inst_decoded decode; > > + unsigned long eptp; > > + u64 inv_type; > > + > > + if ( !cpu_has_vmx_ept ) > > + return X86EMUL_EXCEPTION; > > + > > + if ( decode_vmx_inst(regs, &decode, &eptp, 0) > > + != X86EMUL_OKAY ) > > + return X86EMUL_EXCEPTION; > > + > > + inv_type = reg_read(regs, decode.reg2); > > + gdprintk(XENLOG_DEBUG,"inv_type:%ld, eptp:%lx\n", inv_type, > > + eptp); > > An unconditional printk() on an operation potentially happening quite > frequently? Even with XENLOG_DEBUG this is not acceptable imo. Okay, I will remove it. > > + > > + switch ( inv_type ) { > > + case INVEPT_SINGLE_CONTEXT: > > + { > > + struct p2m_domain *p2m = vcpu_nestedhvm(current).nv_p2m; > > + if ( p2m ) > > + { > > + p2m_flush(current, p2m); > > Despite your comment in 00/10, there still is a whitespace issues at least > here > (didn't look that closely elsewhere). Fixed. > > + ept_sync_domain(p2m); > > + } > > + } > > + break; > > + case INVEPT_ALL_CONTEXT: > > + p2m_flush_nestedp2m(current->domain); > > + __invept(INVEPT_ALL_CONTEXT, 0, 0); > > + break; > > + default: > > + return X86EMUL_EXCEPTION; > > + } > > + vmreturn(regs, VMSUCCEED); > > + return X86EMUL_OKAY; > > +} > > + > > + > > #define __emul_value(enable1, default1) \ > > ((enable1 | default1) << 32 | (default1)) > > > > --- a/xen/arch/x86/mm/p2m.c > > +++ b/xen/arch/x86/mm/p2m.c > > @@ -1465,7 +1465,7 @@ p2m_flush_table(struct p2m_domain *p2m) void > > p2m_flush(struct vcpu *v, struct p2m_domain *p2m) { > > - ASSERT(v->domain == p2m->domain); > > + ASSERT(p2m && v->domain == p2m->domain); > > How is this change related to the rest of the patch? I will remove it, and let caller check whether p2m is NULL. Originally, this is to fix a Xen booting issue. Xiantao _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |