Re: [for-6.0 v5 11/13] spapr: PEF: prevent migration

[Cornelia Huck]

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

> We haven't yet implemented the fairly involved handshaking that will be
> needed to migrate PEF protected guests.  For now, just use a migration
> blocker so we get a meaningful error if someone attempts this (this is the
> same approach used by AMD SEV).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/ppc/pef.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
> index 3ae3059cfe..edc3e744ba 100644
> --- a/hw/ppc/pef.c
> +++ b/hw/ppc/pef.c
> @@ -38,7 +38,11 @@ struct PefGuestState {
>  };
>  
>  #ifdef CONFIG_KVM
> +static Error *pef_mig_blocker;
> +
>  static int kvmppc_svm_init(Error **errp)

This looks weird?

> +
> +int kvmppc_svm_init(SecurableGuestMemory *sgm, Error **errp)
>  {
>      if (!kvm_check_extension(kvm_state, KVM_CAP_PPC_SECURABLE_GUEST)) {
>          error_setg(errp,
> @@ -54,6 +58,11 @@ static int kvmppc_svm_init(Error **errp)
>          }
>      }
>  
> +    /* add migration blocker */
> +    error_setg(&pef_mig_blocker, "PEF: Migration is not implemented");
> +    /* NB: This can fail if --only-migratable is used */
> +    migrate_add_blocker(pef_mig_blocker, &error_fatal);

Just so that I understand: is PEF something that is enabled by the host
(and the guest is either secured or doesn't start), or is it using a
model like s390x PV where the guest initiates the transition into
secured mode?

Asking because s390x adds the migration blocker only when the
transition is actually happening (i.e. guests that do not transition
into secure mode remain migratable.) This has the side effect that you
might be able to start a machine with --only-migratable that
transitions into a non-migratable machine via a guest action, if I'm
not mistaken. Without the new object, I don't see a way to block with
--only-migratable; with it, we should be able to do that. Not sure what
the desirable behaviour is here.

> +
>      return 0;
>  }
>