Re: [PATCH 0/3 v3] KVM: nSVM: Check MBZ bits in CR3 and CR4 on vmrun of nested guests

[Krish Sadhukhan <krish.sadhukhan@oracle.com>]

On 7/3/20 10:11 AM, Paolo Bonzini wrote:
> On 03/07/20 00:33, Krish Sadhukhan wrote:
>> Ping.
>>
>> On 5/14/20 10:36 PM, Krish Sadhukhan wrote:
>>> v2 -> v3:
>>>      In patch# 1, the mask for guest CR4 reserved bits is now cached in
>>>      'struct kvm_vcpu_arch', instead of in a global variable.
>>>
>>>
>>> [PATCH 1/3 v3] KVM: x86: Create mask for guest CR4 reserved bits in
>>> [PATCH 2/3 v3] KVM: nSVM: Check that MBZ bits in CR3 and CR4 are not
>>> set on
>>> [PATCH 3/3 v3] KVM: nSVM: Test that MBZ bits in CR3 and CR4 are not
>>> set on vmrun
>>>
>>>    arch/x86/include/asm/kvm_host.h |  2 ++
>>>    arch/x86/kvm/cpuid.c            |  2 ++
>>>    arch/x86/kvm/svm/nested.c       | 22 ++++++++++++++++++++--
>>>    arch/x86/kvm/svm/svm.h          |  5 ++++-
>>>    arch/x86/kvm/x86.c              | 27 ++++-----------------------
>>>    arch/x86/kvm/x86.h              | 21 +++++++++++++++++++++
>>>    6 files changed, 53 insertions(+), 26 deletions(-)
>>>
>>> Krish Sadhukhan (2):
>>>         KVM: x86: Create mask for guest CR4 reserved bits in
>>> kvm_update_cpuid()
>>>         nSVM: Check that MBZ bits in CR3 and CR4 are not set on vmrun
>>> of nested gu
>>>
>>>    x86/svm.h       |   6 ++++
>>>    x86/svm_tests.c | 105
>>> +++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>    2 files changed, 99 insertions(+), 12 deletions(-)
>>>
>>> Krish Sadhukhan (1):
>>>         nSVM: Test that MBZ bits in CR3 and CR4 are not set on vmrun of
>>> nested g
>>>
> Sorry, this one was not queued because there were comments (also I think
> it doesn't apply anymore).

Sorry, Paolo, I got bit lost here :-). IIRC, you had two comments on 
this set:

     1. kvm_valid_cr4() should be used for checking the reserved bits in 
guest CR4

     2. LMA shouldn't be checked via guest state

v3 has addressd your first suggestion by caching CR4 reserved bits in 
kvm_update_cpuid() and then using kvm_valid_cr4() in nested_vmcb_checks().

As for your second suggestion, v3 uses is_long_mode() which uses 
vcpu->arch.efer for checking long mode.

I will send out a rebased version.

Is there anything I missed ?


Thanks.

>
> Paolo
>