Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 6 Apr 2022 01:22:55 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Vasant Karasulli <vkarasulli@suse.de>
Cc:     linux-kernel@vger.kernel.org, jroedel@suse.de, kvm@vger.kernel.org,
        bp@alien8.de, x86@kernel.org, thomas.lendacky@amd.com,
        varad.gautam@suse.com
Subject: 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.
Message-ID: <Ykzrb1uyPZ2AKWos@google.com>
References: <20220318094532.7023-1-vkarasulli@suse.de>
 <20220318094532.7023-3-vkarasulli@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220318094532.7023-3-vkarasulli@suse.de>
Precedence: bulk

The shortlog and changelog are all messed up.  Ditto for the other patches in this
series.

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 :-)

> +	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.