On Tue, Oct 19, 2021 at 8:31 AM Andrew Jones <drjones@xxxxxxxxxx> wrote:
>
> On Tue, Oct 19, 2021 at 07:14:47AM -0700, Marc Orr wrote:
> > On Mon, Oct 18, 2021 at 9:38 PM Zixuan Wang <zxwang42@xxxxxxxxx> wrote:
> > >
> > > On Mon, Oct 18, 2021 at 4:47 AM Varad Gautam <varad.gautam@xxxxxxxx> wrote:
> > > >
> > > > Hi Zixuan,
> > > >
> > > > On 10/4/21 10:49 PM, Zixuan Wang wrote:
> > > > > From: Zixuan Wang <zixuanwang@xxxxxxxxxx>
> > > > > int main(void)
> > > > > {
> > > > > int rtn;
> > > > > rtn = test_sev_activation();
> > > > > report(rtn == EXIT_SUCCESS, "SEV activation test.");
> > > > > + rtn = test_sev_es_activation();
> > > > > + report(rtn == EXIT_SUCCESS, "SEV-ES activation test.");
> > > > > + rtn = test_sev_es_msr();
> > > >
> > > > There is nothing SEV-ES specific about this function, it only wraps
> > > > rdmsr/wrmsr, which are supposed to generate #VC exceptions on SEV-ES.
> > > > Since the same scenario can be covered by running the msr testcase
> > > > as a SEV-ES guest and observing if it crashes, does testing
> > > > rdmsr/wrmsr one more time here gain us any new information?
> > > >
> > > > Also, the function gets called from main() even if
> > > > test_sev_es_activation() failed or SEV-ES was inactive.
> > > >
> > > > Note: More broadly, what are you looking to test for here?
> > > > 1. wrmsr/rdmsr correctness (rdmsr reads what wrmsr wrote)? or,
> > > > 2. A #VC exception not causing a guest crash on SEV-ES?
> > > >
> > > > If you are looking to test 1., I suggest letting it be covered by
> > > > the generic testcases for msr.
> > > >
> > > > If you are looking to test 2., perhaps a better test is to trigger
> > > > all scenarios that would cause a #VC exception (eg. test_sev_es_vc_exit)
> > > > and check that a SEV-ES guest survives.
> > > >
> > > > Regards,
> > > > Varad
> > > >
> > >
> > > Hi Varad,
> > >
> > > This test case does not bring any SEV-related functionality testing.
> > > Instead, it is provided for development, i.e., one can check if SEV is
> > > properly set up by monitoring if this test case runs fine without
> > > crashes.
> > >
> > > Since this test case is causing some confusion and does not bring any
> > > functionality testing, I can remove it from the next version. We can
> > > still verify the SEV setup process by checking if an existing test
> > > case (e.g., x86/msr.c) runs without crashes in a SEV guest.
> > >
> > > It's hard for me to develop a meaningful SEV test case, because I just
> > > finished my Google internship and thus lost access to SEV-enabled
> > > machines.
> >
> > Removing this test case is fine. Though, it is convenient. But I
> > agree, it's redundant. Maybe we can tag any tests that are good to run
> > under SEV and/or SEV-ES via the `groups` field in the
> > x86/unittests.cfg file. The name `groups` is plural. So I assume that
> > a test can be a member of multiple groups. But I see no examples.
> >
>
> Yes, groups is specified to accept more than one group with space
> separation (see the comment block at the top of the unittests file).
> I see a couple instances where groups are comma separated, but that
> should be changed, especially since commit b373304853a0 ("scripts:
> Fix the check whether testname is in the only_tests list") was merged.
> I'll send a patch for that.
>
> Thanks,
> drew
>
Great! Marc and I are working on labeling test cases as different
groups. We are now trying to label an 'efi' group for test cases that
can run under UEFI, and if necessary, a 'sev' group for those that can
run under SEV.
Best regards,
Zixuan
