public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Matlack <dmatlack@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>,
	kvm list <kvm@vger.kernel.org>
Subject: Re: [PATCH] KVM: selftests: Skip tests that require EPT when it is not available
Date: Mon, 26 Sep 2022 22:28:55 +0000	[thread overview]
Message-ID: <YzInp2jRH7Bq41gV@google.com> (raw)
In-Reply-To: <CALzav=d-4a8yPxPUuHNh1884Z4Pe_0ewMwnunGK_jAAvr9L-vw@mail.gmail.com>

On Mon, Sep 26, 2022, David Matlack wrote:
> On Mon, Sep 26, 2022 at 2:45 PM Sean Christopherson <seanjc@google.com> 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.

  reply	other threads:[~2022-09-26 22:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26 17:14 [PATCH] KVM: selftests: Skip tests that require EPT when it is not available David Matlack
2022-09-26 21:45 ` Sean Christopherson
2022-09-26 22:18   ` David Matlack
2022-09-26 22:28     ` Sean Christopherson [this message]
2022-09-26 22:40       ` David Matlack
2022-09-26 23:57         ` Sean Christopherson
2022-09-27 11:58 ` Paolo Bonzini
2022-09-27 16:54   ` David Matlack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YzInp2jRH7Bq41gV@google.com \
    --to=seanjc@google.com \
    --cc=dmatlack@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox