On 4/23/21 1:31 PM, Paolo Bonzini wrote:
On 23/04/21 17:56, Sean Christopherson wrote:
On Thu, Apr 22, 2021, Krish Sadhukhan wrote:
On 4/22/21 11:01 AM, Sean Christopherson wrote:
offset = svm->nested.ctl.msrpm_base_pa + (p * 4);
if (kvm_vcpu_read_guest(&svm->vcpu, offset, &value, 4)) <-
This reads vmcb12
return false;
svm->nested.msrpm[p] = svm->msrpm[p] | value; <- Merge
vmcb12's bitmap to KVM's bitmap for L2
...
Getting back to your concern that this patch breaks
nested_svm_vmrun_msrpm(). If L1 passes a valid address in which
some bits
in 11:0 are set, the hardware is anyway going to ignore those bits,
irrespective of whether we clear them (before my patch) or pass them
as is
(my patch) and therefore what L1 thinks as a valid address will
effectively
be an invalid address to the hardware. The only difference my patch
makes is
it enables tests to verify hardware behavior. Am missing something ?
See the above snippet where KVM reads the effectively vmcb12->msrpm
to merge L1's
desires with KVM's desires. By removing the code that ensures
svm->nested.ctl.msrpm_base_pa is page aligned, the above offset
calculation will
be wrong.
In fact the kvm-unit-test you sent was also wrong for this same reason
when it was testing addresses near the end of the physical address space.
Paolo
It seems to me that we should clear bits 11:0 in
nested_svm_vmrun_msrpm() where we are forming msrpm_base_pa address for
vmcb02. nested_svm_check_bitmap_pa() aligns the address passed to it,
before checking it.
Should I send a patch for this ?
