All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
@ 2013-01-30  9:26 dongxiao.xu
  2013-01-30 17:54 ` Tim Deegan
  0 siblings, 1 reply; 6+ messages in thread
From: dongxiao.xu @ 2013-01-30  9:26 UTC (permalink / raw)
  To: xen-devel

From: Dongxiao Xu <dongxiao.xu@intel.com>

SMEP is disabled if CPU is in non-paging mode in hardware.
However Xen always uses paging mode to emulate guest non-paging
mode with HAP. To emulate this behavior, SMEP needs to be manually
disabled when guest switches to non-paging mode.

We met an issue that, SMP Linux guest with recent kernel (enable
SMEP support, for example, 3.5.3) would crash with triple fault if
setting unrestricted_guest=0 in grub. This is because Xen uses an
identity mapping page table to emulate the non-paging mode, where
the page table is set with USER flag. If SMEP is still enabled in
this case, guest will meet unhandlable page fault and then crash.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 xen/arch/x86/hvm/vmx/vmx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4d7c93f..6347f66 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1167,6 +1167,14 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
         {
             v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
             v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
+            /*
+             * SMEP is disabled if CPU is in non-paging mode in hardware.
+             * However Xen always uses paging mode to emulate guest non-paging
+             * mode with HAP.
+             * To emulate this behavior, SMEP needs to be manually disabled when
+             * guest switches to non-paging mode.
+             */
+            v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;
         }
         __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]);
         __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[4]);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
  2013-01-30  9:26 [PATCH] VMX: disable SMEP feature when guest is in non-paging mode dongxiao.xu
@ 2013-01-30 17:54 ` Tim Deegan
  2013-01-30 18:26   ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Tim Deegan @ 2013-01-30 17:54 UTC (permalink / raw)
  To: dongxiao.xu; +Cc: xen-devel

At 17:26 +0800 on 30 Jan (1359566815), dongxiao.xu@intel.com wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 4d7c93f..6347f66 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1167,6 +1167,14 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
>          {
>              v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
>              v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
> +            /*
> +             * SMEP is disabled if CPU is in non-paging mode in hardware.
> +             * However Xen always uses paging mode to emulate guest non-paging
> +             * mode with HAP.
> +             * To emulate this behavior, SMEP needs to be manually disabled when
> +             * guest switches to non-paging mode.
> +             */
> +            v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;

Shouldn't this be handled on CR0 writes as well as CR4?  The SMEP bit of
hw_cr[4] ought to be cleared and set whenever CR0_PG is changed.

Cheers,

Tim.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
  2013-01-30 17:54 ` Tim Deegan
@ 2013-01-30 18:26   ` Keir Fraser
  2013-01-31  0:56     ` Xu, Dongxiao
  0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2013-01-30 18:26 UTC (permalink / raw)
  To: Tim Deegan, dongxiao.xu; +Cc: xen-devel

On 30/01/2013 09:54, "Tim Deegan" <tim@xen.org> wrote:

> At 17:26 +0800 on 30 Jan (1359566815), dongxiao.xu@intel.com wrote:
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 4d7c93f..6347f66 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -1167,6 +1167,14 @@ static void vmx_update_guest_cr(struct vcpu *v,
>> unsigned int cr)
>>          {
>>              v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
>>              v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
>> +            /*
>> +             * SMEP is disabled if CPU is in non-paging mode in hardware.
>> +             * However Xen always uses paging mode to emulate guest
>> non-paging
>> +             * mode with HAP.
>> +             * To emulate this behavior, SMEP needs to be manually disabled
>> when
>> +             * guest switches to non-paging mode.
>> +             */
>> +            v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;
> 
> Shouldn't this be handled on CR0 writes as well as CR4?  The SMEP bit of
> hw_cr[4] ought to be cleared and set whenever CR0_PG is changed.

That's what I would expect too.

 -- Keir

> Cheers,
> 
> Tim.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
  2013-01-30 18:26   ` Keir Fraser
@ 2013-01-31  0:56     ` Xu, Dongxiao
  2013-01-31  1:11       ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Xu, Dongxiao @ 2013-01-31  0:56 UTC (permalink / raw)
  To: Keir Fraser, Tim Deegan; +Cc: xen-devel@lists.xen.org

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: Thursday, January 31, 2013 2:27 AM
> To: Tim Deegan; Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] VMX: disable SMEP feature when guest is in
> non-paging mode
> 
> On 30/01/2013 09:54, "Tim Deegan" <tim@xen.org> wrote:
> 
> > At 17:26 +0800 on 30 Jan (1359566815), dongxiao.xu@intel.com wrote:
> >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> >> index 4d7c93f..6347f66 100644
> >> --- a/xen/arch/x86/hvm/vmx/vmx.c
> >> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >> @@ -1167,6 +1167,14 @@ static void vmx_update_guest_cr(struct vcpu
> *v,
> >> unsigned int cr)
> >>          {
> >>              v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
> >>              v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
> >> +            /*
> >> +             * SMEP is disabled if CPU is in non-paging mode in
> hardware.
> >> +             * However Xen always uses paging mode to emulate guest
> >> non-paging
> >> +             * mode with HAP.
> >> +             * To emulate this behavior, SMEP needs to be manually
> disabled
> >> when
> >> +             * guest switches to non-paging mode.
> >> +             */
> >> +            v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;
> >
> > Shouldn't this be handled on CR0 writes as well as CR4?  The SMEP bit of
> > hw_cr[4] ought to be cleared and set whenever CR0_PG is changed.
> 
> That's what I would expect too.

In the case 0 (CR0 update) in vmx_update_guest_cr(), it will call vmx_update_guest_cr(v, 4) to adjust CR4 related bits if certain bits (PG, PE) in CR0 changed.

Thanks,
Dongxiao

> 
>  -- Keir
> 
> > Cheers,
> >
> > Tim.
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
  2013-01-31  0:56     ` Xu, Dongxiao
@ 2013-01-31  1:11       ` Keir Fraser
  2013-01-31  1:28         ` Xu, Dongxiao
  0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2013-01-31  1:11 UTC (permalink / raw)
  To: Xu, Dongxiao, Tim Deegan; +Cc: xen-devel@lists.xen.org

On 30/01/2013 16:56, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:

>>> Shouldn't this be handled on CR0 writes as well as CR4?  The SMEP bit of
>>> hw_cr[4] ought to be cleared and set whenever CR0_PG is changed.
>> 
>> That's what I would expect too.
> 
> In the case 0 (CR0 update) in vmx_update_guest_cr(), it will call
> vmx_update_guest_cr(v, 4) to adjust CR4 related bits if certain bits (PG, PE)
> in CR0 changed.

Good point!

In fact we can make that cr4 update unconditional and simplify the cr0
handling a bit more.

 -- Keir

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: disable SMEP feature when guest is in non-paging mode
  2013-01-31  1:11       ` Keir Fraser
@ 2013-01-31  1:28         ` Xu, Dongxiao
  0 siblings, 0 replies; 6+ messages in thread
From: Xu, Dongxiao @ 2013-01-31  1:28 UTC (permalink / raw)
  To: Keir Fraser, Tim Deegan; +Cc: xen-devel@lists.xen.org

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: Thursday, January 31, 2013 9:12 AM
> To: Xu, Dongxiao; Tim Deegan
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] VMX: disable SMEP feature when guest is in
> non-paging mode
> 
> On 30/01/2013 16:56, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> 
> >>> Shouldn't this be handled on CR0 writes as well as CR4?  The SMEP bit of
> >>> hw_cr[4] ought to be cleared and set whenever CR0_PG is changed.
> >>
> >> That's what I would expect too.
> >
> > In the case 0 (CR0 update) in vmx_update_guest_cr(), it will call
> > vmx_update_guest_cr(v, 4) to adjust CR4 related bits if certain bits (PG, PE)
> > in CR0 changed.
> 
> Good point!
> 
> In fact we can make that cr4 update unconditional and simplify the cr0
> handling a bit more.

Yes, I saw your changes, which makes the code more neat.

Thanks,
Dongxiao

> 
>  -- Keir
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-31  1:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-30  9:26 [PATCH] VMX: disable SMEP feature when guest is in non-paging mode dongxiao.xu
2013-01-30 17:54 ` Tim Deegan
2013-01-30 18:26   ` Keir Fraser
2013-01-31  0:56     ` Xu, Dongxiao
2013-01-31  1:11       ` Keir Fraser
2013-01-31  1:28         ` Xu, Dongxiao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.