mbox series

[0/4] KVM: selftests: Fix and clean up emulator_error_test

Message ID 20220929204708.2548375-1-dmatlack@google.com (mailing list archive)
Headers show
Series KVM: selftests: Fix and clean up emulator_error_test | expand

Message

David Matlack Sept. 29, 2022, 8:47 p.m. UTC
Miscellaneous fixes and cleanups to emulator_error_test. The reason I
started looking at this test is because it fails when TDP is disabled,
which pollutes my test results when I am testing a new series for
upstream.

David Matlack (4):
  KVM: selftests: Use MMIO to trigger emulation in emulator_error_test
  KVM: selftests: Delete dead ucall code from emulator_error_test
  KVM: selftests: Skip emulator_error_test if
    KVM_CAP_EXIT_ON_EMULATION_FAILURE not available
  KVM: selftests: Explicitly require instructions bytes in
    emulator_error_test

 .../kvm/x86_64/emulator_error_test.c          | 130 +++++-------------
 1 file changed, 35 insertions(+), 95 deletions(-)


base-commit: 69604fe76e58c9d195e48b41d019b07fc27ce9d7

Comments

Sean Christopherson Oct. 3, 2022, 11:31 p.m. UTC | #1
On Thu, Sep 29, 2022, David Matlack wrote:
> Miscellaneous fixes and cleanups to emulator_error_test. The reason I
> started looking at this test is because it fails when TDP is disabled,
> which pollutes my test results when I am testing a new series for
> upstream.

This series defeats the (not well documented) purpose of emulator_error_test.  The
test exists specifically to verify that KVM emulates in response to EPT violations
when "allow_smaller_maxphyaddr && guest.MAXPHYADDR < host.MAXPHADDR".

That said, the test could use some upgrades:

  1. Verify that a well-emulated instruction takes #PF(RSVD)
  2. Verify that FLDS takes #PF(RSVD) when EPT is disabled
David Matlack Oct. 4, 2022, 4:33 p.m. UTC | #2
On Mon, Oct 3, 2022 at 4:31 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Sep 29, 2022, David Matlack wrote:
> > Miscellaneous fixes and cleanups to emulator_error_test. The reason I
> > started looking at this test is because it fails when TDP is disabled,
> > which pollutes my test results when I am testing a new series for
> > upstream.
>
> This series defeats the (not well documented) purpose of emulator_error_test.  The
> test exists specifically to verify that KVM emulates in response to EPT violations
> when "allow_smaller_maxphyaddr && guest.MAXPHYADDR < host.MAXPHADDR".

I thought that might have been the case before I sent this series, but
I could not (and still can't) find any evidence to support it. The
commit message and comment at the top of the test all indicate this is
a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE. What did I miss?

Either way, I think we want both types of tests.  In v2 how about I split out:

 (1) exit_on_emulation_failure_test.c: Test that emulation failures
exit to userspace when the cap is enabled, i.e. what
emulator_error_test does by the end of this series.
 (2) smaller_maxphyaddr_emulation_test.c: Test the interaction of
allow_smaller_maxphyaddr and instruction emulation. i.e. what
emulator_error_does at the start of this series plus your suggestions.

The main benefit of splitting out (1) is to test
KVM_CAP_EXIT_ON_EMULATION_FAILURE independent of
allow_smaller_maxphyaddr, since allow_smaller_maxphyaddr is not
enabled by default.


>
> That said, the test could use some upgrades:
>
>   1. Verify that a well-emulated instruction takes #PF(RSVD)
>   2. Verify that FLDS takes #PF(RSVD) when EPT is disabled
Sean Christopherson Oct. 4, 2022, 8:30 p.m. UTC | #3
On Tue, Oct 04, 2022, David Matlack wrote:
> On Mon, Oct 3, 2022 at 4:31 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Sep 29, 2022, David Matlack wrote:
> > > Miscellaneous fixes and cleanups to emulator_error_test. The reason I
> > > started looking at this test is because it fails when TDP is disabled,
> > > which pollutes my test results when I am testing a new series for
> > > upstream.
> >
> > This series defeats the (not well documented) purpose of emulator_error_test.  The
> > test exists specifically to verify that KVM emulates in response to EPT violations
> > when "allow_smaller_maxphyaddr && guest.MAXPHYADDR < host.MAXPHADDR".
> 
> I thought that might have been the case before I sent this series, but
> I could not (and still can't) find any evidence to support it. The
> commit message and comment at the top of the test all indicate this is
> a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE. What did I miss?

Being in the general vicinity when the test was written?  :-)

> Either way, I think we want both types of tests.  In v2 how about I split out:
> 
>  (1) exit_on_emulation_failure_test.c: Test that emulation failures
> exit to userspace when the cap is enabled, i.e. what
> emulator_error_test does by the end of this series.
>  (2) smaller_maxphyaddr_emulation_test.c: Test the interaction of
> allow_smaller_maxphyaddr and instruction emulation. i.e. what
> emulator_error_does at the start of this series plus your suggestions.
> 
> The main benefit of splitting out (1) is to test
> KVM_CAP_EXIT_ON_EMULATION_FAILURE independent of
> allow_smaller_maxphyaddr, since allow_smaller_maxphyaddr is not
> enabled by default.

Works for me.