Re: [PATCH v6 2/4] x86/tests: Add tests for AMD SEV-ES #VC handling Add KUnit based tests to validate Linux's VC handling for instructions cpuid and wbinvd. These tests: 1. install a kretprobe on the #VC handler (sev_es_ghcb_hv_call, to access GHCB before/after the resulting VMGEXIT). 2. trigger an NAE by executing either cpuid or wbinvd. 3. check that the kretprobe was hit with the right exit_code available in GHCB.

Vasant Karasulli
2022-04-20T07:40:40Z

On Mi 06-04-22 01:22:55, Sean Christopherson wrote:
> The shortlog and changelog are all messed up. Ditto for the other patches in this
> series.

I am really sorry about that. I had sent another mail with the same patch version
with subject line corrected.
https://lore.kernel.org/kvm/20220318104646.8313-1-vkarasulli@suse.de/T/#t
>
> On Fri, Mar 18, 2022, Vasant Karasulli wrote:
> > Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>
> > ---
> > arch/x86/tests/Makefile | 2 +
> > arch/x86/tests/sev-test-vc.c | 114 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 116 insertions(+)
> > create mode 100644 arch/x86/tests/sev-test-vc.c
>
> ...
>
> > +int sev_es_test_vc_init(struct kunit *test)
> > +{
> > + int ret;
> > +
> > + if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
> > + kunit_info(test, "Not a SEV-ES guest. Skipping.");
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + memset(&hv_call_krp, 0, sizeof(hv_call_krp));
> > + hv_call_krp.entry_handler = hv_call_krp_entry;
> > + hv_call_krp.handler = hv_call_krp_ret;
> > + hv_call_krp.maxactive = 100;
> > + hv_call_krp.data_size = sizeof(unsigned long);
> > + hv_call_krp.kp.symbol_name = "sev_es_ghcb_hv_call";
> > + hv_call_krp.kp.addr = 0;
> > +
> > + ret = register_kretprobe(&hv_call_krp);
> > + if (ret) {
> > + kunit_info(test, "Could not register kretprobe. Skipping.");
> > + goto out;
> > + }
> > +
> > + test->priv = kunit_kzalloc(test, sizeof(u64), GFP_KERNEL);
>
> Allocating 8 bytes and storing the pointer an 8-byte field is rather pointless :-)

Yes, I will remove this in the next version.
>
> > + if (!test->priv) {
> > + ret = -ENOMEM;
> > + kunit_info(test, "Could not allocate. Skipping.");
> > + goto out;
> > + }
> > +
> > +out:
> > + return ret;
> > +}
> > +
> > +void sev_es_test_vc_exit(struct kunit *test)
> > +{
> > + if (test->priv)
> > + kunit_kfree(test, test->priv);
> > +
> > + if (hv_call_krp.kp.addr)
> > + unregister_kretprobe(&hv_call_krp);
> > +}
> > +
> > +#define check_op(kt, ec, op) \
> > +do { \
> > + struct kunit *t = (struct kunit *) kt; \
> > + op; \
> > + KUNIT_EXPECT_EQ(t, (typeof(ec)) ec, \
> > + *((typeof(ec) *)(t->priv))); \
> > +} while (0)
> > +
> > +static void sev_es_nae_cpuid(struct kunit *test)
> > +{
> > + unsigned int cpuid_fn = 0x8000001f;
> > +
> > + check_op(test, SVM_EXIT_CPUID, native_cpuid_eax(cpuid_fn));
>
> Are there plans to go beyond basic checks? Neat idea, but it seems like it will
> be prone to bitrot since it requires a somewhat esoteric setup and an opt-in config.
> And odds are very good that if the kernel can make it this far as an SEV-ES guest,
> it's gotten the basics right.

I will definitely think about adding more checks and performing these checks
early enough in the guest run.

Thanks for your feedback.

Thanks,
Vasant Karasulli
Kernel generalist
www.suse.com<http://www.suse.com>
[https://www.suse.com/assets/img/social-platforms-suse-logo.png]<http://www.suse.com/>
SUSE - Open Source Solutions for Enterprise Servers & Cloud<http://www.suse.com/>
Modernize your infrastructure with SUSE Linux Enterprise servers, cloud technology for IaaS, and SUSE's software-defined storage.