Re: [PATCH 1/3] Nested VMX: update nested paging mode when vmswitch is in progress

["Egger, Christoph" <chegger@amazon.de>]

On 18.12.13 11:24, Zhang, Yang Z wrote:
> Jan Beulich wrote on 2013-12-18:
>>>>> On 18.12.13 at 09:58, "Dong, Eddie" <eddie.dong@intel.com> wrote:
>>> Acked by Eddie Dong <eddie.dong@intel.com>
>>
>> As long as Christoph's reservations wrt SVM aren't being addressed/
>> eliminated, I don't think we can apply this patch.
>>
>> Furthermore, while you ack-ed this patch (which isn't really VMX
>> specific) and patch 3, you didn't ack patch 2, but you also didn't
>> indicate anything that's possibly wrong with it.
> 
> Actually, I asked him help to review the first patch. Since Christoph thought the first patch may break AMD. So I hope he can help to review the first patch to see whether I am wrong.
> 
>>
>> And finally, with patch 1 needing to be left out for the moment, I'd
>> like to have confirmation that all three patches can be applied
>> independently (i.e. with the current state of things only patch 3 is ready to go in).
> 
> Yes, the three patches are independent.

I have looked through code.

vcpu is in guestmode till the vmentry/vmexit emulation is done.
In SVM the vcpu guestmode changes right before setting
nv_vmswitch_in_progress to 0 when the vmentry/vmexit
emulation was successfull (there is a bunch of error-checking).

This patch breaks both vmentry and vmexit emulation for SVM.
The vmentry breakage comes with l1-hypervisor using shadow-paging.

During vmexit emulation hvm_set_cr0 and hvm_set_cr4 are called
to restore cr0 and cr4 for the l1 guest.
With this patch the paging mode for the l2 guest is updated
rather for the l1 guest.

I think this patch also breaks the case where l2 guest wants to
set cr0 or cr4 and l1-hypervisor does not intercept cr0/cr4
and l1-hypervisor uses shadow-paging. This may also count
for VMX.

This is just from reading the code. As I said, I do not have
a setup to verify this, unfortunately.

Christoph


>>
>> Jan
>>
>> Zhang, Yang Z wrote on 2013-12-12:
>>> vmswitch is in progress
>>>
>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>
>>> virtual vmentry will change paging related stucture, so
>>> corrensponding nested mode need to be updated which is missing currently.
>>>
>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>> ---
>>>  xen/arch/x86/hvm/hvm.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>> This patch fixed RHEL6 guest installation problem with L1 hyper-v.
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index
>>> 69f7e74..1f62e00 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1925,7 +1925,7 @@ int hvm_set_cr0(unsigned long value)
>>>      hvm_update_cr(v, 0, value);
>>>      
>>>      if ( (value ^ old_value) & X86_CR0_PG ) {
>>> -        if ( !nestedhvm_vmswitch_in_progress(v) &&
>>> nestedhvm_vcpu_in_guestmode(v) )
>>> +        if ( nestedhvm_vcpu_in_guestmode(v) )
>>>              paging_update_nestedmode(v); else
>>>              paging_update_paging_modes(v); @@ -2014,7 +2014,7
>> @@ int
>>> hvm_set_cr4(unsigned long value)
>>>            (X86_CR4_PSE | X86_CR4_PGE | X86_CR4_PAE |
>> X86_CR4_SMEP)) ||
>>>           (!(value & X86_CR4_PCIDE) && (old_cr & X86_CR4_PCIDE)) )
>>>      {
>>> -        if ( !nestedhvm_vmswitch_in_progress(v) &&
>>> nestedhvm_vcpu_in_guestmode(v) )
>>> +        if ( nestedhvm_vcpu_in_guestmode(v) )
>>>              paging_update_nestedmode(v); else
>>>              paging_update_paging_modes(v);
>>> --
>>> 1.7.1
>>
>>
> 
> 
> Best regards,
> Yang
> 
>