Mailing List Archive: [PATCH v2 08/10] nEPT: handle invept instruction from L1 VMM

[PATCH v2 08/10] nEPT: handle invept instruction from L1 VMM

Dec 19, 2012, 12:21 AM

Post #1 of 3 (413 views)

From: Zhang Xiantao <xiantao.zhang@intel.com>

Add the INVEPT instruction emulation logic.

Signed-off-by: Zhang Xiantao <xiantao.zhang@intel.com>
---
xen/arch/x86/hvm/vmx/vmx.c | 6 +++-
xen/arch/x86/hvm/vmx/vvmx.c | 39 ++++++++++++++++++++++++++++++++++++
xen/arch/x86/mm/p2m.c | 2 +-
xen/include/asm-x86/hvm/vmx/vvmx.h | 1 +
4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e5be5a2..7af92cc 100644
--- 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;
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
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index cb2c6e7..b7c3639 100644
--- 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);
+
+ switch ( inv_type ) {
+ case INVEPT_SINGLE_CONTEXT:
+ {
+ struct p2m_domain *p2m = vcpu_nestedhvm(current).nv_p2m;
+ if ( p2m )
+ {
+ p2m_flush(current, p2m);
+ 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))

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 1f59410..17903ee 100644
--- 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);
vcpu_nestedhvm(v).nv_p2m = NULL;
p2m_flush_table(p2m);
hvm_asid_flush_vcpu(v);
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 55c0ad1..cf5ed9a 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -190,6 +190,7 @@ int nvmx_handle_vmread(struct cpu_user_regs *regs);
int nvmx_handle_vmwrite(struct cpu_user_regs *regs);
int nvmx_handle_vmresume(struct cpu_user_regs *regs);
int nvmx_handle_vmlaunch(struct cpu_user_regs *regs);
+int nvmx_handle_invept(struct cpu_user_regs *regs);
int nvmx_msr_read_intercept(unsigned int msr,
u64 *msr_content);
int nvmx_msr_write_intercept(unsigned int msr,
--
1.7.1

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

Re: [PATCH v2 08/10] nEPT: handle invept instruction from L1 VMM [ In reply to ]

JBeulich at suse

Dec 19, 2012, 12:28 AM

Post #2 of 3 (399 views)

Permalink

>>> On 19.12.12 at 20:44, Xiantao Zhang <xiantao.zhang@intel.com> 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.

> 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.

> +
> + 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).

> + 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?

Jan

> vcpu_nestedhvm(v).nv_p2m = NULL;
> p2m_flush_table(p2m);
> hvm_asid_flush_vcpu(v);

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

Re: [PATCH v2 08/10] nEPT: handle invept instruction from L1 VMM [ In reply to ]

xiantao.zhang at intel

Dec 19, 2012, 6:39 PM

Post #3 of 3 (395 views)

Permalink

Thanks, Jan!
>>: handle invept instruction from L1 VMM
>
> >>> On 19.12.12 at 20:44, Xiantao Zhang <xiantao.zhang@intel.com> 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@lists.xen.org
http://lists.xen.org/xen-devel