On Mon, Sep 26, 2022, David Matlack wrote: > On Mon, Sep 26, 2022 at 2:45 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > I would much rather this be an assert, i.e. force the test to do TEST_REQUIRE(), > > even if that means duplicate code. One of the roles of TEST_REQUIRE() is to > > document test requirements. > > This gets difficult when you consider dirty_log_perf_test. Users can > use dirty_log_perf_test in nested mode by passing "-n". But > dirty_log_perf_test is an architecture-neutral test, so adding > TEST_REQUIRE() there would require an ifdef, and then gets even more > complicated if we add support for AMD or nested on non-x86 > architectures. But you're going to have that problem regardless, e.g. perf_test_setup_nested() already has TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX)); so why not also have the EPT requirement there? IMO, it's far less confusing if the hard requirements are collected together, even if that one location isn't ideal. If someone wants to improve the memstress framework, a hook can be added for probing nested requirements. In other words, IMO this is a shortcoming of the memstress code, not a valid reason to shove the requirement down in common code. > Another option is to do nothing and let the test fail if running on > hosts without EPT. I don't like this solution though Neither do I.
