Re: [Xen-devel] [PATCH 3/3] x86/vmx: Better description of CR4 settings outside of paged mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/10/17 15:04, Roger Pau Monné wrote:
> On Fri, Sep 29, 2017 at 06:31:03PM +0000, Andrew Cooper wrote:
>> This rearanges the logic to avoid the double !hvm_paging_enabled(v) check, 
>> but
>> is otherwise identical.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
>> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
>> ---
>>  xen/arch/x86/hvm/vmx/vmx.c | 37 ++++++++++++++++++++++++++++---------
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 5b943d4..5b9b074 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -1642,21 +1642,40 @@ static void vmx_update_guest_cr(struct vcpu *v, 
>> unsigned int cr)
>>          v->arch.hvm_vcpu.hw_cr[4] |= v->arch.hvm_vcpu.guest_cr[4];
>>          if ( v->arch.hvm_vmx.vmx_realmode )
>>              v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_VME;
>> -        if ( paging_mode_hap(v->domain) && !hvm_paging_enabled(v) )
>> -        {
>> -            v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
>> -            v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
>> -        }
>> +
>>          if ( !hvm_paging_enabled(v) )
>>          {
>>              /*
>> -             * SMEP/SMAP is disabled if CPU is in non-paging mode in 
>> hardware.
>> -             * However Xen always uses paging mode to emulate guest 
>> non-paging
>> -             * mode. To emulate this behavior, SMEP/SMAP needs to be 
>> manually
>> -             * disabled when guest VCPU is in non-paging mode.
>> +             * When the guest thinks paging is disabled, Xen may need to 
>> hide
>> +             * the effects of running with CR0.PG actually enabled.  There 
>> are
>> +             * two subtly complicated cases.
>> +             */
>> +
>> +            if ( paging_mode_hap(v->domain) )
>> +            {
>> +                /*
>> +                 * On hardware lacking the Unrestricted Guest feature (or 
>> with
>> +                 * it disabled in the VMCS), we may not enter the guest with
> Shouldn't this be paging_mode_hap && vmx_unrestricted_guest?

ITYM paging_mode_hap && !vmx_unrestricted_guest

>
> From the code below I think it's harmless what we do with CR4 if
> CR0.PG is disabled, but in any case it would be good to mention it in
> the comment IMHO.

Indeed it is harmless, which is why I didn't include the extra
conditional.  Including it would invalidate my statement of "otherwise
identical".

~Andrew

>
>> +                 * CR0.PG actually disabled.  When EPT is enabled, we run 
>> with
>> +                 * guest paging settings, but with CR3 pointing at
>> +                 * HVM_PARAM_IDENT_PT which is a 32bit pagetable using 4M
>> +                 * superpages.  Override the guests paging settings to 
>> match.
>> +                 */
>> +                v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_PSE;
>> +                v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
> Thanks, Roger.


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