Re: [PATCH 42/70] x86/sev-es: Support nested #VC exceptions

From: Andy Lutomirski
Date: Thu Mar 19 2020 - 11:46:56 EST


On Thu, Mar 19, 2020 at 2:14 AM Joerg Roedel <joro@xxxxxxxxxx> wrote:
>
> From: Joerg Roedel <jroedel@xxxxxxx>
>
> Handle #VC exceptions that happen while the GHCB is in use. This can
> happen when an NMI happens in the #VC exception handler and the NMI
> handler causes a #VC exception itself. Save the contents of the GHCB
> when nesting is detected and restore it when the GHCB is no longer
> used.
>
> Signed-off-by: Joerg Roedel <jroedel@xxxxxxx>
> ---
> arch/x86/kernel/sev-es.c | 63 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
> index 97241d2f0f70..3b7bbc8d841e 100644
> --- a/arch/x86/kernel/sev-es.c
> +++ b/arch/x86/kernel/sev-es.c
> @@ -32,9 +32,57 @@ struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
> */
> struct ghcb __initdata *boot_ghcb;
>
> +struct ghcb_state {
> + struct ghcb *ghcb;
> +};
> +
> /* Runtime GHCB pointers */
> static struct ghcb __percpu *ghcb_page;
>
> +/*
> + * Mark the per-cpu GHCB as in-use to detect nested #VC exceptions.
> + * There is no need for it to be atomic, because nothing is written to the GHCB
> + * between the read and the write of ghcb_active. So it is safe to use it when a
> + * nested #VC exception happens before the write.
> + */
> +static DEFINE_PER_CPU(bool, ghcb_active);
> +
> +static struct ghcb *sev_es_get_ghcb(struct ghcb_state *state)
> +{
> + struct ghcb *ghcb = (struct ghcb *)this_cpu_ptr(ghcb_page);
> + bool *active = this_cpu_ptr(&ghcb_active);
> +
> + if (unlikely(*active)) {
> + /* GHCB is already in use - save its contents */
> +
> + state->ghcb = kzalloc(sizeof(struct ghcb), GFP_ATOMIC);
> + if (!state->ghcb)
> + return NULL;

This can't possibly end well. Maybe have a little percpu list of
GHCBs and make sure there are enough for any possible nesting?

Also, I admit confusion. Isn't the GHCB required to be unencrypted?
How does that work with kzalloc()?