2023-02-08 16:32:46

by Liam Merwick

[permalink] [raw]
Subject: Re: [PATCH RFC v7 16/64] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction

On 14/12/2022 19:40, Michael Roth wrote:
> From: Brijesh Singh <[email protected]>
>
> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
> hypervisor will use the instruction to add pages to the RMP table. See
> APM3 for details on the instruction operations.
>
> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
> of contiguous 4KB-Page RMP entries. The hypervisor will use this
> instruction to adjust the RMP entry without invalidating the previous
> RMP entry.
>
> Add the following external interface API functions:
>
> int psmash(u64 pfn);
> psmash is used to smash a 2MB aligned page into 4K
> pages while preserving the Validated bit in the RMP.
>
> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
> Used to assign a page to guest using the RMPUPDATE instruction.
>
> int rmp_make_shared(u64 pfn, enum pg_level level);
> Used to transition a page to hypervisor/shared state using the RMPUPDATE instruction.
>
> Signed-off-by: Ashish Kalra <[email protected]>
> Signed-off-by: Brijesh Singh <[email protected]>
> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
> Signed-off-by: Michael Roth <[email protected]>
> ---
> arch/x86/include/asm/sev.h | 24 ++++++++++
> arch/x86/kernel/sev.c | 95 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 119 insertions(+)
>

...

> +
> +static int rmpupdate(u64 pfn, struct rmp_state *val)
> +{
> + unsigned long paddr = pfn << PAGE_SHIFT;
> + int retries = 0;
> + int ret;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return -ENXIO;
> +
> +retry:
> + /* Binutils version 2.36 supports the RMPUPDATE mnemonic. */
> + asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
> + : "=a"(ret)
> + : "a"(paddr), "c"((unsigned long)val)
> + : "memory", "cc");
> +
> + if (ret) {
> + if (!retries) {
> + pr_err("RMPUPDATE failed, ret: %d, pfn: %llx, npages: %d, level: %d, retrying (max: %d)...\n",
> + ret, pfn, npages, level, 2 * num_present_cpus());

This patch isn't bisectable - 'npages' isn't defined in this patch -
it's defined later in Patch18

otherwise LGTM

Regards,
Liam


> + dump_stack();
> + }
> + retries++;
> + if (retries < 2 * num_present_cpus())
> + goto retry;
> + } else if (retries > 0) {
> + pr_err("RMPUPDATE for pfn %llx succeeded after %d retries\n", pfn, retries);
> + }
> +
> + return ret;
> +}