Re: [for-6.0 v5 08/13] securable guest memory: Introduce sgm "ready" flag

[Cornelia Huck <cohuck@redhat.com>]

On Fri,  4 Dec 2020 16:44:10 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The platform specific details of mechanisms for implementing securable
> guest memory may require setup at various points during initialization.
> Thus, it's not really feasible to have a single sgm initialization hook,
> but instead each mechanism needs its own initialization calls in arch or
> machine specific code.
> 
> However, to make it harder to have a bug where a mechanism isn't properly
> initialized under some circumstances, we want to have a common place,
> relatively late in boot, where we verify that sgm has been initialized if
> it was requested.
> 
> This patch introduces a ready flag to the SecurableGuestMemory base type
> to accomplish this, which we verify just before the machine specific
> initialization function.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/core/machine.c                     | 8 ++++++++
>  include/exec/securable-guest-memory.h | 2 ++
>  target/i386/sev.c                     | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 816ea3ae3e..a67a27d03c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1155,6 +1155,14 @@ void machine_run_board_init(MachineState *machine)
>      }
>  
>      if (machine->sgm) {
> +        /*
> +         * Where securable guest memory is initialized depends on the
> +         * specific mechanism in use.  But, we need to make sure it's
> +         * ready by now.  If it isn't, that's a bug in the
> +         * implementation of that sgm mechanism.
> +         */
> +        assert(machine->sgm->ready);

Under which circumstances might we arrive here with 'ready' not set?

- programming error, setup is happening too late -> assert() seems
  appropriate
- we tried to set it up, but some error happened -> should we rely on
  the setup code to error out first? (i.e. we won't end up here, unless
  there's a programming error, in which case the assert() looks fine)
  Is there a possible use case for "we could not set it up, but we
  support an unsecured guest (as long as it is clear what happens)"?
  Likely only for guests that transition themselves, but one could
  argue that QEMU should simply be invoked a second time without the
  sgm stuff being specified in the error case.

> +
>          /*
>           * With securable guest memory, the host can't see the real
>           * contents of RAM, so there's no point in it trying to merge