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