On 10/19/21 4:14 PM, 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>
>>>> +static int test_sev_es_msr(void)
>>>> +{
>>>> + /*
>>>> + * With SEV-ES, rdmsr/wrmsr trigger #VC exception. If #VC is handled
>>>> + * correctly, rdmsr/wrmsr should work like without SEV-ES and not crash
>>>> + * the guest VM.
>>>> + */
>>>> + u64 val = 0x1234;
>>>> + wrmsr(MSR_TSC_AUX, val);
>>>> + if(val != rdmsr(MSR_TSC_AUX)) {
>>>> + return EXIT_FAILURE;
>>>
>>> See note below.
>>>
>>>> + }
>>>> +
>>>> + return EXIT_SUCCESS;
>>>> +}
>>>> +
>>>> 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.
>
Right, from a fleet owner perspective I can imagine the following scenarios
being relevant to test for a SEV* offering (and I guess hence make sense to
have a special test in kvm-unit-tests for):
- CPUID shows the right SEV level
- C-bit discovery
- GHCB validity (protocol version etc.)
Generic kvm behavior is better tested via the other dedicated tests, which,
after the EFI-fication should be no problem to fit into a test plan. The
SEV* implementation can then go through the whole battery of kvm-unit-tests
plus the SEV* ones.
Regards,
Varad
