[PATCH] nested vmx: Fix the booting of L2 PAE guest

[PATCH] nested vmx: Fix the booting of L2 PAE guest

[Dongxiao Xu]

When doing virtual VM entry and virtual VM exit, we need to
sychronize the PAE PDPTR related VMCS registers. With this fix,
we can boot 32bit PAE L2 guest (Win7 & RHEL6.4) on "Xen on Xen"
environment.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
Tested-by: Yongjie Ren <yongjie.ren@intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index bb7688f..5dfbc54 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -864,6 +864,13 @@ static const u16 vmcs_gstate_field[] = {
     GUEST_SYSENTER_EIP,
 };
 
+static const u16 gpdptr_fields[] = {
+    GUEST_PDPTR0,
+    GUEST_PDPTR1,
+    GUEST_PDPTR2,
+    GUEST_PDPTR3,
+};
+
 /*
  * Context: shadow -> virtual VMCS
  */
@@ -1053,18 +1060,6 @@ static void load_shadow_guest_state(struct vcpu *v)
                      (__get_vvmcs(vvmcs, CR4_READ_SHADOW) & cr_gh_mask);
     __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
 
-    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
-         (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
-    {
-        static const u16 gpdptr_fields[] = {
-            GUEST_PDPTR0,
-            GUEST_PDPTR1,
-            GUEST_PDPTR2,
-            GUEST_PDPTR3,
-        };
-        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
-    }
-
     /* TODO: CR3 target control */
 }
 
@@ -1159,6 +1154,10 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
     if ( lm_l1 != lm_l2 )
         paging_update_paging_modes(v);
 
+    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
+         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
+        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
+
     regs->eip = __get_vvmcs(vvmcs, GUEST_RIP);
     regs->esp = __get_vvmcs(vvmcs, GUEST_RSP);
     regs->eflags = __get_vvmcs(vvmcs, GUEST_RFLAGS);
@@ -1294,6 +1293,10 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
     sync_vvmcs_guest_state(v, regs);
     sync_exception_state(v);
 
+    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
+         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
+        shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
+
     vmx_vmcs_switch(v->arch.hvm_vmx.vmcs, nvcpu->nv_n1vmcx);
 
     nestedhvm_vcpu_exit_guestmode(v);
-- 
1.8.1.5

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Keir Fraser]

On 24/06/2013 06:55, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote:

> When doing virtual VM entry and virtual VM exit, we need to
> sychronize the PAE PDPTR related VMCS registers. With this fix,
> we can boot 32bit PAE L2 guest (Win7 & RHEL6.4) on "Xen on Xen"
> environment.
> 
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> Tested-by: Yongjie Ren <yongjie.ren@intel.com>

Acked-by: Keir Fraser <keir@xen.org>

> ---
>  xen/arch/x86/hvm/vmx/vvmx.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index bb7688f..5dfbc54 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -864,6 +864,13 @@ static const u16 vmcs_gstate_field[] = {
>      GUEST_SYSENTER_EIP,
>  };
>  
> +static const u16 gpdptr_fields[] = {
> +    GUEST_PDPTR0,
> +    GUEST_PDPTR1,
> +    GUEST_PDPTR2,
> +    GUEST_PDPTR3,
> +};
> +
>  /*
>   * Context: shadow -> virtual VMCS
>   */
> @@ -1053,18 +1060,6 @@ static void load_shadow_guest_state(struct vcpu *v)
>                       (__get_vvmcs(vvmcs, CR4_READ_SHADOW) & cr_gh_mask);
>      __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>  
> -    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> -         (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> -    {
> -        static const u16 gpdptr_fields[] = {
> -            GUEST_PDPTR0,
> -            GUEST_PDPTR1,
> -            GUEST_PDPTR2,
> -            GUEST_PDPTR3,
> -        };
> -        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
> -    }
> -
>      /* TODO: CR3 target control */
>  }
>  
> @@ -1159,6 +1154,10 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
>      if ( lm_l1 != lm_l2 )
>          paging_update_paging_modes(v);
>  
> +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> +        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
> +
>      regs->eip = __get_vvmcs(vvmcs, GUEST_RIP);
>      regs->esp = __get_vvmcs(vvmcs, GUEST_RSP);
>      regs->eflags = __get_vvmcs(vvmcs, GUEST_RFLAGS);
> @@ -1294,6 +1293,10 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
>      sync_vvmcs_guest_state(v, regs);
>      sync_exception_state(v);
>  
> +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> +        shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdptr_fields), gpdptr_fields);
> +
>      vmx_vmcs_switch(v->arch.hvm_vmx.vmcs, nvcpu->nv_n1vmcx);
>  
>      nestedhvm_vcpu_exit_guestmode(v);

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Xu, Dongxiao]

Hi stakeholders,

I saw the patch is not merged yet. Do you have any other comment about this patch? I think it is a critical fix for 4.3 release in nested virtualization side.

Thanks,
Dongxiao

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: Monday, June 24, 2013 2:46 PM
> To: Xu, Dongxiao; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] nested vmx: Fix the booting of L2 PAE guest
> 
> On 24/06/2013 06:55, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote:
> 
> > When doing virtual VM entry and virtual VM exit, we need to
> > sychronize the PAE PDPTR related VMCS registers. With this fix,
> > we can boot 32bit PAE L2 guest (Win7 & RHEL6.4) on "Xen on Xen"
> > environment.
> >
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > Tested-by: Yongjie Ren <yongjie.ren@intel.com>
> 
> Acked-by: Keir Fraser <keir@xen.org>
> 
> > ---
> >  xen/arch/x86/hvm/vmx/vvmx.c | 27 +++++++++++++++------------
> >  1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index bb7688f..5dfbc54 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -864,6 +864,13 @@ static const u16 vmcs_gstate_field[] = {
> >      GUEST_SYSENTER_EIP,
> >  };
> >
> > +static const u16 gpdptr_fields[] = {
> > +    GUEST_PDPTR0,
> > +    GUEST_PDPTR1,
> > +    GUEST_PDPTR2,
> > +    GUEST_PDPTR3,
> > +};
> > +
> >  /*
> >   * Context: shadow -> virtual VMCS
> >   */
> > @@ -1053,18 +1060,6 @@ static void load_shadow_guest_state(struct vcpu
> *v)
> >                       (__get_vvmcs(vvmcs, CR4_READ_SHADOW) &
> cr_gh_mask);
> >      __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
> >
> > -    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> > -         (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> > -    {
> > -        static const u16 gpdptr_fields[] = {
> > -            GUEST_PDPTR0,
> > -            GUEST_PDPTR1,
> > -            GUEST_PDPTR2,
> > -            GUEST_PDPTR3,
> > -        };
> > -        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
> gpdptr_fields);
> > -    }
> > -
> >      /* TODO: CR3 target control */
> >  }
> >
> > @@ -1159,6 +1154,10 @@ static void virtual_vmentry(struct cpu_user_regs
> *regs)
> >      if ( lm_l1 != lm_l2 )
> >          paging_update_paging_modes(v);
> >
> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> > +        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
> gpdptr_fields);
> > +
> >      regs->eip = __get_vvmcs(vvmcs, GUEST_RIP);
> >      regs->esp = __get_vvmcs(vvmcs, GUEST_RSP);
> >      regs->eflags = __get_vvmcs(vvmcs, GUEST_RFLAGS);
> > @@ -1294,6 +1293,10 @@ static void virtual_vmexit(struct cpu_user_regs
> *regs)
> >      sync_vvmcs_guest_state(v, regs);
> >      sync_exception_state(v);
> >
> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
> > +        shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdptr_fields),
> gpdptr_fields);
> > +
> >      vmx_vmcs_switch(v->arch.hvm_vmx.vmcs, nvcpu->nv_n1vmcx);
> >
> >      nestedhvm_vcpu_exit_guestmode(v);
> 

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Jan Beulich]

>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi stakeholders,
> 
> I saw the patch is not merged yet. Do you have any other comment about this 
> patch? I think it is a critical fix for 4.3 release in nested virtualization 
> side.

Irrespective of Keir's ack I was hoping for an ack from one of the
VMX maintainers. Even more so as they are, just like you, working
for Intel I think it would be appropriate for you to get in touch
with them to fulfill their maintainer task here. In fact you should
have Cc-ed them with your initial patch submission.

Independently of that, you should have also Cc-ed George if you
want this to go in for 4.3. In the absence of this, I had simply put
this on my post-4.3 queue...

Jan

>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.xen@gmail.com]
>> Sent: Monday, June 24, 2013 2:46 PM
>> To: Xu, Dongxiao; xen-devel@lists.xen.org 
>> Subject: Re: [Xen-devel] [PATCH] nested vmx: Fix the booting of L2 PAE guest
>> 
>> On 24/06/2013 06:55, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote:
>> 
>> > When doing virtual VM entry and virtual VM exit, we need to
>> > sychronize the PAE PDPTR related VMCS registers. With this fix,
>> > we can boot 32bit PAE L2 guest (Win7 & RHEL6.4) on "Xen on Xen"
>> > environment.
>> >
>> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>> > Tested-by: Yongjie Ren <yongjie.ren@intel.com>
>> 
>> Acked-by: Keir Fraser <keir@xen.org>
>> 
>> > ---
>> >  xen/arch/x86/hvm/vmx/vvmx.c | 27 +++++++++++++++------------
>> >  1 file changed, 15 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> > index bb7688f..5dfbc54 100644
>> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> > @@ -864,6 +864,13 @@ static const u16 vmcs_gstate_field[] = {
>> >      GUEST_SYSENTER_EIP,
>> >  };
>> >
>> > +static const u16 gpdptr_fields[] = {
>> > +    GUEST_PDPTR0,
>> > +    GUEST_PDPTR1,
>> > +    GUEST_PDPTR2,
>> > +    GUEST_PDPTR3,
>> > +};
>> > +
>> >  /*
>> >   * Context: shadow -> virtual VMCS
>> >   */
>> > @@ -1053,18 +1060,6 @@ static void load_shadow_guest_state(struct vcpu
>> *v)
>> >                       (__get_vvmcs(vvmcs, CR4_READ_SHADOW) &
>> cr_gh_mask);
>> >      __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>> >
>> > -    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > -         (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > -    {
>> > -        static const u16 gpdptr_fields[] = {
>> > -            GUEST_PDPTR0,
>> > -            GUEST_PDPTR1,
>> > -            GUEST_PDPTR2,
>> > -            GUEST_PDPTR3,
>> > -        };
>> > -        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > -    }
>> > -
>> >      /* TODO: CR3 target control */
>> >  }
>> >
>> > @@ -1159,6 +1154,10 @@ static void virtual_vmentry(struct cpu_user_regs
>> *regs)
>> >      if ( lm_l1 != lm_l2 )
>> >          paging_update_paging_modes(v);
>> >
>> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > +        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > +
>> >      regs->eip = __get_vvmcs(vvmcs, GUEST_RIP);
>> >      regs->esp = __get_vvmcs(vvmcs, GUEST_RSP);
>> >      regs->eflags = __get_vvmcs(vvmcs, GUEST_RFLAGS);
>> > @@ -1294,6 +1293,10 @@ static void virtual_vmexit(struct cpu_user_regs
>> *regs)
>> >      sync_vvmcs_guest_state(v, regs);
>> >      sync_exception_state(v);
>> >
>> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > +        shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > +
>> >      vmx_vmcs_switch(v->arch.hvm_vmx.vmcs, nvcpu->nv_n1vmcx);
>> >
>> >      nestedhvm_vcpu_exit_guestmode(v);
>> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[George Dunlap]

On 27/06/13 09:40, Jan Beulich wrote:
>>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>> Hi stakeholders,
>>
>> I saw the patch is not merged yet. Do you have any other comment about this
>> patch? I think it is a critical fix for 4.3 release in nested virtualization
>> side.
> Irrespective of Keir's ack I was hoping for an ack from one of the
> VMX maintainers. Even more so as they are, just like you, working
> for Intel I think it would be appropriate for you to get in touch
> with them to fulfill their maintainer task here. In fact you should
> have Cc-ed them with your initial patch submission.
>
> Independently of that, you should have also Cc-ed George if you
> want this to go in for 4.3. In the absence of this, I had simply put
> this on my post-4.3 queue...

Well normally I think I would have said "no" to this change.  You guys 
haven't done a very good job of engaging with the release process we've 
been trying to develop -- you didn't report this bug to me so that I 
could track it and make informed decisions regarding the release.

However, I still consider nested VMX as "experimental" -- the fact that 
in the current tree, Win7 won't boot on Xen-on-Xen kind of confirms that 
status to me. :-)  Since it is experimental, like the ARM port, it has a 
slightly different release criteria: basically, as long as you don't 
touch code in the non-experimental path, you can take your own risks 
regarding breaking functionality.

Given that this patch only touches code inside vvmx.c, I'm assuming that 
this code *cannot* be touched by anyone who is *not* running in nested 
virt mode.

If that's correct, I'm inclined to say we can accept it.  But I'd be 
interested in hearing other people's opinions.

  -George

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Jan Beulich]

>>> On 27.06.13 at 12:57, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> On 27/06/13 09:40, Jan Beulich wrote:
>>>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>>> Hi stakeholders,
>>>
>>> I saw the patch is not merged yet. Do you have any other comment about this
>>> patch? I think it is a critical fix for 4.3 release in nested virtualization
>>> side.
>> Irrespective of Keir's ack I was hoping for an ack from one of the
>> VMX maintainers. Even more so as they are, just like you, working
>> for Intel I think it would be appropriate for you to get in touch
>> with them to fulfill their maintainer task here. In fact you should
>> have Cc-ed them with your initial patch submission.
>>
>> Independently of that, you should have also Cc-ed George if you
>> want this to go in for 4.3. In the absence of this, I had simply put
>> this on my post-4.3 queue...
> 
> Well normally I think I would have said "no" to this change.  You guys 
> haven't done a very good job of engaging with the release process we've 
> been trying to develop -- you didn't report this bug to me so that I 
> could track it and make informed decisions regarding the release.

So you probably meant this to be sent To: ...@intel.com, and
Cc: <me>?

> However, I still consider nested VMX as "experimental" -- the fact that 
> in the current tree, Win7 won't boot on Xen-on-Xen kind of confirms that 
> status to me. :-)  Since it is experimental, like the ARM port, it has a 
> slightly different release criteria: basically, as long as you don't 
> touch code in the non-experimental path, you can take your own risks 
> regarding breaking functionality.
> 
> Given that this patch only touches code inside vvmx.c, I'm assuming that 
> this code *cannot* be touched by anyone who is *not* running in nested 
> virt mode.
> 
> If that's correct, I'm inclined to say we can accept it.  But I'd be 
> interested in hearing other people's opinions.

I agree with that. And I may apply the patch perhaps tomorrow
even if we don't hear anything from the VMX maintainers.

Jan

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[George Dunlap]

On 27/06/13 13:05, Jan Beulich wrote:
>>>> On 27.06.13 at 12:57, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> On 27/06/13 09:40, Jan Beulich wrote:
>>>>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>>>> Hi stakeholders,
>>>>
>>>> I saw the patch is not merged yet. Do you have any other comment about this
>>>> patch? I think it is a critical fix for 4.3 release in nested virtualization
>>>> side.
>>> Irrespective of Keir's ack I was hoping for an ack from one of the
>>> VMX maintainers. Even more so as they are, just like you, working
>>> for Intel I think it would be appropriate for you to get in touch
>>> with them to fulfill their maintainer task here. In fact you should
>>> have Cc-ed them with your initial patch submission.
>>>
>>> Independently of that, you should have also Cc-ed George if you
>>> want this to go in for 4.3. In the absence of this, I had simply put
>>> this on my post-4.3 queue...
>> Well normally I think I would have said "no" to this change.  You guys
>> haven't done a very good job of engaging with the release process we've
>> been trying to develop -- you didn't report this bug to me so that I
>> could track it and make informed decisions regarding the release.
> So you probably meant this to be sent To: ...@intel.com, and
> Cc: <me>?

Yes, sorry -- I meant the "you guys" to be the developers working on 
nested vmx.  You (Jan) have been very engaged with the process, and I 
appreciate it. :-)

  -George

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Jan Beulich]

>>> On 27.06.13 at 12:57, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> On 27/06/13 09:40, Jan Beulich wrote:
>>>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>>> Hi stakeholders,
>>>
>>> I saw the patch is not merged yet. Do you have any other comment about this
>>> patch? I think it is a critical fix for 4.3 release in nested virtualization
>>> side.
>> Irrespective of Keir's ack I was hoping for an ack from one of the
>> VMX maintainers. Even more so as they are, just like you, working
>> for Intel I think it would be appropriate for you to get in touch
>> with them to fulfill their maintainer task here. In fact you should
>> have Cc-ed them with your initial patch submission.
>>
>> Independently of that, you should have also Cc-ed George if you
>> want this to go in for 4.3. In the absence of this, I had simply put
>> this on my post-4.3 queue...
> 
> Well normally I think I would have said "no" to this change.  You guys 
> haven't done a very good job of engaging with the release process we've 
> been trying to develop -- you didn't report this bug to me so that I 
> could track it and make informed decisions regarding the release.
> 
> However, I still consider nested VMX as "experimental" -- the fact that 
> in the current tree, Win7 won't boot on Xen-on-Xen kind of confirms that 
> status to me. :-)  Since it is experimental, like the ARM port, it has a 
> slightly different release criteria: basically, as long as you don't 
> touch code in the non-experimental path, you can take your own risks 
> regarding breaking functionality.
> 
> Given that this patch only touches code inside vvmx.c, I'm assuming that 
> this code *cannot* be touched by anyone who is *not* running in nested 
> virt mode.
> 
> If that's correct, I'm inclined to say we can accept it.  But I'd be 
> interested in hearing other people's opinions.

Committed now that we have Eddie's ack.

Jan

Re: [PATCH] nested vmx: Fix the booting of L2 PAE guest

[Dong, Eddie]

My mailbox has problem :(
Anyway, acked.

Eddie

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com] 
Sent: Thursday, June 27, 2013 4:40 PM
To: Xu, Dongxiao
Cc: George Dunlap; Keir Fraser; Dong, Eddie; Nakajima, Jun; xen-devel@lists.xen.org
Subject: Re: [Xen-devel] [PATCH] nested vmx: Fix the booting of L2 PAE guest

>>> On 27.06.13 at 03:14, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi stakeholders,
> 
> I saw the patch is not merged yet. Do you have any other comment about this 
> patch? I think it is a critical fix for 4.3 release in nested virtualization 
> side.

Irrespective of Keir's ack I was hoping for an ack from one of the
VMX maintainers. Even more so as they are, just like you, working
for Intel I think it would be appropriate for you to get in touch
with them to fulfill their maintainer task here. In fact you should
have Cc-ed them with your initial patch submission.

Independently of that, you should have also Cc-ed George if you
want this to go in for 4.3. In the absence of this, I had simply put
this on my post-4.3 queue...

Jan

>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.xen@gmail.com]
>> Sent: Monday, June 24, 2013 2:46 PM
>> To: Xu, Dongxiao; xen-devel@lists.xen.org 
>> Subject: Re: [Xen-devel] [PATCH] nested vmx: Fix the booting of L2 PAE guest
>> 
>> On 24/06/2013 06:55, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote:
>> 
>> > When doing virtual VM entry and virtual VM exit, we need to
>> > sychronize the PAE PDPTR related VMCS registers. With this fix,
>> > we can boot 32bit PAE L2 guest (Win7 & RHEL6.4) on "Xen on Xen"
>> > environment.
>> >
>> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>> > Tested-by: Yongjie Ren <yongjie.ren@intel.com>
>> 
>> Acked-by: Keir Fraser <keir@xen.org>
>> 
>> > ---
>> >  xen/arch/x86/hvm/vmx/vvmx.c | 27 +++++++++++++++------------
>> >  1 file changed, 15 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> > index bb7688f..5dfbc54 100644
>> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> > @@ -864,6 +864,13 @@ static const u16 vmcs_gstate_field[] = {
>> >      GUEST_SYSENTER_EIP,
>> >  };
>> >
>> > +static const u16 gpdptr_fields[] = {
>> > +    GUEST_PDPTR0,
>> > +    GUEST_PDPTR1,
>> > +    GUEST_PDPTR2,
>> > +    GUEST_PDPTR3,
>> > +};
>> > +
>> >  /*
>> >   * Context: shadow -> virtual VMCS
>> >   */
>> > @@ -1053,18 +1060,6 @@ static void load_shadow_guest_state(struct vcpu
>> *v)
>> >                       (__get_vvmcs(vvmcs, CR4_READ_SHADOW) &
>> cr_gh_mask);
>> >      __vmwrite(CR4_READ_SHADOW, cr_read_shadow);
>> >
>> > -    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > -         (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > -    {
>> > -        static const u16 gpdptr_fields[] = {
>> > -            GUEST_PDPTR0,
>> > -            GUEST_PDPTR1,
>> > -            GUEST_PDPTR2,
>> > -            GUEST_PDPTR3,
>> > -        };
>> > -        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > -    }
>> > -
>> >      /* TODO: CR3 target control */
>> >  }
>> >
>> > @@ -1159,6 +1154,10 @@ static void virtual_vmentry(struct cpu_user_regs
>> *regs)
>> >      if ( lm_l1 != lm_l2 )
>> >          paging_update_paging_modes(v);
>> >
>> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > +        vvmcs_to_shadow_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > +
>> >      regs->eip = __get_vvmcs(vvmcs, GUEST_RIP);
>> >      regs->esp = __get_vvmcs(vvmcs, GUEST_RSP);
>> >      regs->eflags = __get_vvmcs(vvmcs, GUEST_RFLAGS);
>> > @@ -1294,6 +1293,10 @@ static void virtual_vmexit(struct cpu_user_regs
>> *regs)
>> >      sync_vvmcs_guest_state(v, regs);
>> >      sync_exception_state(v);
>> >
>> > +    if ( nvmx_ept_enabled(v) && hvm_pae_enabled(v) &&
>> > +         !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>> > +        shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdptr_fields),
>> gpdptr_fields);
>> > +
>> >      vmx_vmcs_switch(v->arch.hvm_vmx.vmcs, nvcpu->nv_n1vmcx);
>> >
>> >      nestedhvm_vcpu_exit_guestmode(v);
>> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel