On 4/20/21 1:00 PM, Sean Christopherson wrote:
On Mon, Apr 12, 2021, Krish Sadhukhan wrote:
According to APM vol 2, hardware ignores the low 12 bits in MSRPM and IOPM
bitmaps. Therefore setting/unssetting these bits has no effect as far as
VMRUN is concerned. Also, setting/unsetting these bits prevents tests from
verifying hardware behavior.
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
---
arch/x86/kvm/svm/nested.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index ae53ae46ebca..fd42c8b7f99a 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -287,8 +287,6 @@ static void nested_load_control_from_vmcb12(struct vcpu_svm *svm,
/* Copy it here because nested_svm_check_controls will check it. */
svm->nested.ctl.asid = control->asid;
- svm->nested.ctl.msrpm_base_pa &= ~0x0fffULL;
- svm->nested.ctl.iopm_base_pa &= ~0x0fffULL;
This will break nested_svm_vmrun_msrpm() if L1 passes an unaligned address.
The shortlog is also wrong, KVM isn't setting bits, it's clearing bits.
I also don't think svm->nested.ctl.msrpm_base_pa makes its way to hardware; IIUC,
it's a copy of vmcs12->control.msrpm_base_pa. The bitmap that gets loaded into
the "real" VMCB is vmcb02->control.msrpm_base_pa.
Not sure if there's a problem with my patch as such, but upon inspecting
the code, I see something missing:
In nested_load_control_from_vmcb12(), we are not really loading
msrpm_base_pa from vmcb12 even though the name of the function
suggests so.
Then nested_vmcb_check_controls() checks msrpm_base_pa from
'nested.ctl' which doesn't have the copy from vmcb12.
Then nested_vmcb02_prepare_control() prepares the vmcb02 copy of
msrpm_base_pa from vmcb01.ptr->control.msrpm_base_pa.
Then nested_svm_vmrun_msrpm() uses msrpm_base_pa from 'nested.ctl'.
Aren't we actually using msrpm_base_pa from vmcb01 instead of vmcb02 ?
}
/*
--
2.27.0
