| Message ID | 20180718202651.19802-4-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/18/2018 01:26 PM, Sean Christopherson wrote: > KVM relies on hardware to perform the "vTPR < threshold" consistency > check, i.e. the check will occur after KVM has done some amount of > guest state checking in software. As such, KVM will signal a VMExit > consistency check (due to bad guest state) instead of the expected > VMFAIL (due to bad controls). > > Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > x86/vmx_tests.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index c4803a3..031bf66 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -3726,7 +3726,7 @@ static void try_tpr_threshold_and_vtpr(unsigned threshold, unsigned vtpr) > set_vtpr(vtpr); > report_prefix_pushf("TPR threshold 0x%x, VTPR.class 0x%x", > threshold, (vtpr >> 4) & 0xf); > - test_vmx_controls(valid, false); > + test_vmx_controls(valid, !valid); > report_prefix_pop(); > } > The SDM says that vTPR should be greater than or equal to the TPR threshold. Your patch mentions the other way around.
On 07/22/2018 08:11 PM, Krish Sadhukhan wrote: > > > On 07/18/2018 01:26 PM, Sean Christopherson wrote: >> KVM relies on hardware to perform the "vTPR < threshold" consistency >> check, i.e. the check will occur after KVM has done some amount of >> guest state checking in software. As such, KVM will signal a VMExit >> consistency check (due to bad guest state) instead of the expected >> VMFAIL (due to bad controls). >> >> Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com> >> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> >> --- >> x86/vmx_tests.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >> index c4803a3..031bf66 100644 >> --- a/x86/vmx_tests.c >> +++ b/x86/vmx_tests.c >> @@ -3726,7 +3726,7 @@ static void try_tpr_threshold_and_vtpr(unsigned >> threshold, unsigned vtpr) >> set_vtpr(vtpr); >> report_prefix_pushf("TPR threshold 0x%x, VTPR.class 0x%x", >> threshold, (vtpr >> 4) & 0xf); >> - test_vmx_controls(valid, false); >> + test_vmx_controls(valid, !valid); >> report_prefix_pop(); >> } > The SDM says that vTPR should be greater than or equal to the TPR > threshold. Your patch mentions the other way around. On the code change, Reviewed-by: Krish Sadhukh <krish.sadhukhan@oracle.com>
On Wed, 2018-07-18 at 13:26 -0700, Sean Christopherson wrote: > KVM relies on hardware to perform the "vTPR < threshold" consistency > check, i.e. the check will occur after KVM has done some amount of > guest state checking in software. As such, KVM will signal a VMExit > consistency check (due to bad guest state) instead of the expected > VMFAIL (due to bad controls). This analysis is wrong, the actual issue is that L1 and L2 were sharing an APIC access page, which was causing the vTPR check to pull the value from L1 instead of L2. That bug was fixed by commit ab5df31cee7f ("kvm: nVMX: Eliminate APIC access page sharing between L1 and L2"). The vTPR tests should pass with kvm/{master,next,queue}. Lucky for me, Paolo didn't queue this patch, so nothing further needs to be done. > Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > x86/vmx_tests.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index c4803a3..031bf66 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -3726,7 +3726,7 @@ static void try_tpr_threshold_and_vtpr(unsigned threshold, unsigned vtpr) > set_vtpr(vtpr); > report_prefix_pushf("TPR threshold 0x%x, VTPR.class 0x%x", > threshold, (vtpr >> 4) & 0xf); > - test_vmx_controls(valid, false); > + test_vmx_controls(valid, !valid); > report_prefix_pop(); > } >
On 27/07/2018 19:01, Sean Christopherson wrote: > On Wed, 2018-07-18 at 13:26 -0700, Sean Christopherson wrote: >> KVM relies on hardware to perform the "vTPR < threshold" consistency >> check, i.e. the check will occur after KVM has done some amount of >> guest state checking in software. As such, KVM will signal a VMExit >> consistency check (due to bad guest state) instead of the expected >> VMFAIL (due to bad controls). > This analysis is wrong, the actual issue is that L1 and L2 were > sharing an APIC access page, which was causing the vTPR check to > pull the value from L1 instead of L2. That bug was fixed by commit > ab5df31cee7f ("kvm: nVMX: Eliminate APIC access page sharing between > L1 and L2"). The vTPR tests should pass with kvm/{master,next,queue}. > > Lucky for me, Paolo didn't queue this patch, so nothing further needs > to be done. > Yep, I didn't queue it because the tests were failing and I didn't have time to analyze it. :) Paolo
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index c4803a3..031bf66 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -3726,7 +3726,7 @@ static void try_tpr_threshold_and_vtpr(unsigned threshold, unsigned vtpr) set_vtpr(vtpr); report_prefix_pushf("TPR threshold 0x%x, VTPR.class 0x%x", threshold, (vtpr >> 4) & 0xf); - test_vmx_controls(valid, false); + test_vmx_controls(valid, !valid); report_prefix_pop(); }
KVM relies on hardware to perform the "vTPR < threshold" consistency check, i.e. the check will occur after KVM has done some amount of guest state checking in software. As such, KVM will signal a VMExit consistency check (due to bad guest state) instead of the expected VMFAIL (due to bad controls). Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- x86/vmx_tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)