From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Auger Eric <eric.auger@redhat.com>
Cc: thuth@redhat.com, drjones@redhat.com, eric.auger.pro@gmail.com,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
pbonzini@redhat.com
Subject: Re: [PATCH] selftests: KVM: AMD Nested SVM test infrastructure
Date: Tue, 21 Jan 2020 12:46:57 +0100 [thread overview]
Message-ID: <877e1lf2vi.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <a288001b-56a6-363b-18c0-18a1e1876ccc@redhat.com>
Auger Eric <eric.auger@redhat.com> writes:
> Hi Vitaly,
>
> On 1/20/20 11:53 AM, Vitaly Kuznetsov wrote:
>> Eric Auger <eric.auger@redhat.com> writes:
>>
...
>>> +
>>> +static struct test tests[] = {
>>> + /* name, supported, custom setup, l2 code, exit code, custom check, finished */
>>> + {"vmmcall", NULL, NULL, l2_vmcall, SVM_EXIT_VMMCALL},
>>> + {"vmrun", NULL, NULL, l2_vmrun, SVM_EXIT_VMRUN},
>>> + {"CR3 read intercept", NULL, prepare_cr3_intercept, l2_cr3_read, SVM_EXIT_READ_CR3},
>>> +};
>>
>> selftests are usualy not that well structured :-) E.g. we don't have
>> sub-tests and a way to specify which one to run so there is a single
>> flow when everything is being executed. I'd suggest to keep things as
>> simple as possibe (especially in the basic 'svm' test).
> In this case the differences between the tests is very tiny. One line on
> L2 and one line on L1 to check the exit status. I wondered whether it
> deserves to have separate test files for that. I did not intend to run
> the subtests separately nor to add many more subtests but rather saw all
> of them as a single basic test. More complex tests would be definitively
> separate.
>
> But if the consensus is to keep each tests separate, I will do.
>
No, I wasn't asking for that, it's just that the 'tests' array looks
like we're going to add more and more here (like we do in
kvm-unit-tests). If it's not the case you can probably simplify the code
by executing these three checks consequently without defining any
'sub-test' stuctures (like we do for other selftests). But I don't have
a strong opinion on this so we can keep things the way they are.
--
Vitaly
next prev parent reply other threads:[~2020-01-21 11:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-17 17:37 [PATCH] selftests: KVM: AMD Nested SVM test infrastructure Eric Auger
2020-01-20 10:53 ` Vitaly Kuznetsov
2020-01-21 11:12 ` Auger Eric
2020-01-21 11:46 ` Vitaly Kuznetsov [this message]
2020-01-21 12:17 ` Paolo Bonzini
2020-01-21 12:50 ` Auger Eric
2020-01-21 23:02 ` Wei Huang
2020-01-22 8:45 ` Auger Eric
2020-02-03 9:26 ` Auger Eric
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=877e1lf2vi.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=drjones@redhat.com \
--cc=eric.auger.pro@gmail.com \
--cc=eric.auger@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=thuth@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.