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

Commit Message

David Gibson Dec. 4, 2020, 5:44 a.m. UTC

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 <[e-mail]>
Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
---
 hw/ppc/pef.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Cornelia Huck Dec. 14, 2020, 5:22 p.m. UTC | #1

On Fri,  4 Dec 2020 16:44:13 +1100
David Gibson <[e-mail]> 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 <[e-mail]>
> Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> ---
>  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;
>  }
>

David Gibson Dec. 17, 2020, 5:47 a.m. UTC | #2

On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:
> On Fri,  4 Dec 2020 16:44:13 +1100
> David Gibson <[e-mail]> 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 <[e-mail]>
> > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > ---
> >  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?

Oops.  Not sure how that made it past even my rudimentary compile
testing.

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

Like s390x PV it's initiated by the guest.

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

Hm, I'm not sure what the best option is here either.

> 
> > +
> >      return 0;
> >  }
> >  
>

Cornelia Huck Dec. 17, 2020, 11:38 a.m. UTC | #3

On Thu, 17 Dec 2020 16:47:36 +1100
David Gibson <[e-mail]> wrote:

> On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:
> > On Fri,  4 Dec 2020 16:44:13 +1100
> > David Gibson <[e-mail]> 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 <[e-mail]>
> > > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > > ---
> > >  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?  
> 
> Oops.  Not sure how that made it past even my rudimentary compile
> testing.
> 
> > > +
> > > +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?  
> 
> Like s390x PV it's initiated by the guest.
> 
> > 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.  
> 
> Hm, I'm not sure what the best option is here either.

If we agree on anything, it should be as consistent across
architectures as possible :)

If we want to add the migration blocker to s390x even before the guest
transitions, it needs to be tied to the new object; if we'd make it
dependent on the cpu feature bit, we'd block migration of all machines
on hardware with SE and a recent kernel.

Is there a convenient point in time when PEF guests transition where
QEMU can add a blocker?

> 
> >   
> > > +
> > >      return 0;
> > >  }
> > >    
> >   
>

Greg Kurz Dec. 17, 2020, 2:15 p.m. UTC | #4

On Thu, 17 Dec 2020 12:38:42 +0100
Cornelia Huck <[e-mail]> wrote:

> On Thu, 17 Dec 2020 16:47:36 +1100
> David Gibson <[e-mail]> wrote:
> 
> > On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:
> > > On Fri,  4 Dec 2020 16:44:13 +1100
> > > David Gibson <[e-mail]> 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 <[e-mail]>
> > > > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > > > ---
> > > >  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?  
> > 
> > Oops.  Not sure how that made it past even my rudimentary compile
> > testing.
> > 
> > > > +
> > > > +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?  
> > 
> > Like s390x PV it's initiated by the guest.
> > 
> > > 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.  
> > 

The purpose of --only-migratable is specifically to prevent the machine
to transition to a non-migrate state IIUC. The guest transition to
secure mode should be nacked in this case.

> > Hm, I'm not sure what the best option is here either.
> 
> If we agree on anything, it should be as consistent across
> architectures as possible :)
> 
> If we want to add the migration blocker to s390x even before the guest
> transitions, it needs to be tied to the new object; if we'd make it
> dependent on the cpu feature bit, we'd block migration of all machines
> on hardware with SE and a recent kernel.
> 
> Is there a convenient point in time when PEF guests transition where
> QEMU can add a blocker?
> 
> > 
> > >   
> > > > +
> > > >      return 0;
> > > >  }
> > > >    
> > >   
> > 
>

Cornelia Huck Dec. 18, 2020, 11:41 a.m. UTC | #5

On Thu, 17 Dec 2020 15:15:30 +0100
Greg Kurz <[e-mail]> wrote:

> On Thu, 17 Dec 2020 12:38:42 +0100
> Cornelia Huck <[e-mail]> wrote:
> 
> > On Thu, 17 Dec 2020 16:47:36 +1100
> > David Gibson <[e-mail]> wrote:
> >   
> > > On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:  
> > > > On Fri,  4 Dec 2020 16:44:13 +1100
> > > > David Gibson <[e-mail]> 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 <[e-mail]>
> > > > > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > > > > ---
> > > > >  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?    
> > > 
> > > Oops.  Not sure how that made it past even my rudimentary compile
> > > testing.
> > >   
> > > > > +
> > > > > +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?    
> > > 
> > > Like s390x PV it's initiated by the guest.
> > >   
> > > > 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.    
> > >   
> 
> The purpose of --only-migratable is specifically to prevent the machine
> to transition to a non-migrate state IIUC. The guest transition to
> secure mode should be nacked in this case.

Yes, that's what happens for s390x: The guest tries to transition, QEMU
can't add a migration blocker and fails the instruction used for
transitioning, the guest sees the error.

The drawback is that we see the failure only when we already launched
the machine and the guest tries to transition. If I start QEMU with
--only-migratable, it will refuse to start when non-migratable devices
are configured in the command line, so I see the issue right from the
start. (For s390x, that would possibly mean that we should not even
present the cpu feature bit when only_migratable is set?)

> 
> > > Hm, I'm not sure what the best option is here either.  
> > 
> > If we agree on anything, it should be as consistent across
> > architectures as possible :)
> > 
> > If we want to add the migration blocker to s390x even before the guest
> > transitions, it needs to be tied to the new object; if we'd make it
> > dependent on the cpu feature bit, we'd block migration of all machines
> > on hardware with SE and a recent kernel.
> > 
> > Is there a convenient point in time when PEF guests transition where
> > QEMU can add a blocker?
> >   
> > >   
> > > >     
> > > > > +
> > > > >      return 0;
> > > > >  }
> > > > >      
> > > >     
> > >   
> >   
>

Dr. David Alan Gilbert Dec. 18, 2020, 12:08 p.m. UTC | #6

* Cornelia Huck ([e-mail]) wrote:
> On Thu, 17 Dec 2020 15:15:30 +0100
> Greg Kurz <[e-mail]> wrote:
> 
> > On Thu, 17 Dec 2020 12:38:42 +0100
> > Cornelia Huck <[e-mail]> wrote:
> > 
> > > On Thu, 17 Dec 2020 16:47:36 +1100
> > > David Gibson <[e-mail]> wrote:
> > >   
> > > > On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:  
> > > > > On Fri,  4 Dec 2020 16:44:13 +1100
> > > > > David Gibson <[e-mail]> 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 <[e-mail]>
> > > > > > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > > > > > ---
> > > > > >  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?    
> > > > 
> > > > Oops.  Not sure how that made it past even my rudimentary compile
> > > > testing.
> > > >   
> > > > > > +
> > > > > > +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?    
> > > > 
> > > > Like s390x PV it's initiated by the guest.
> > > >   
> > > > > 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.    
> > > >   
> > 
> > The purpose of --only-migratable is specifically to prevent the machine
> > to transition to a non-migrate state IIUC. The guest transition to
> > secure mode should be nacked in this case.
> 
> Yes, that's what happens for s390x: The guest tries to transition, QEMU
> can't add a migration blocker and fails the instruction used for
> transitioning, the guest sees the error.
> 
> The drawback is that we see the failure only when we already launched
> the machine and the guest tries to transition. If I start QEMU with
> --only-migratable, it will refuse to start when non-migratable devices
> are configured in the command line, so I see the issue right from the
> start. (For s390x, that would possibly mean that we should not even
> present the cpu feature bit when only_migratable is set?)

I see --only-migratable as refusing to start if you've enabled anything
that would stop migration.
So I'd expect:
  a) Allow the cpu flag to be turned on/off somehow
     
  b) If you ask for it (-cpu ...,_confidentialcomp or whatever) and
you've got --only-migratable then you'd fail before startup.

Dave

> > 
> > > > Hm, I'm not sure what the best option is here either.  
> > > 
> > > If we agree on anything, it should be as consistent across
> > > architectures as possible :)
> > > 
> > > If we want to add the migration blocker to s390x even before the guest
> > > transitions, it needs to be tied to the new object; if we'd make it
> > > dependent on the cpu feature bit, we'd block migration of all machines
> > > on hardware with SE and a recent kernel.
> > > 
> > > Is there a convenient point in time when PEF guests transition where
> > > QEMU can add a blocker?
> > >   
> > > >   
> > > > >     
> > > > > > +
> > > > > >      return 0;
> > > > > >  }
> > > > > >      
> > > > >     
> > > >   
> > >   
> > 
>

Ram Pai Jan. 4, 2021, 7:15 a.m. UTC | #7

On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:
> On Thu, 17 Dec 2020 15:15:30 +0100
> Greg Kurz <[e-mail]> wrote:
> 
> > On Thu, 17 Dec 2020 12:38:42 +0100
> > Cornelia Huck <[e-mail]> wrote:
> > 
> > > On Thu, 17 Dec 2020 16:47:36 +1100
> > > David Gibson <[e-mail]> wrote:
> > >   
> > > > On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:  
> > > > > On Fri,  4 Dec 2020 16:44:13 +1100
> > > > > David Gibson <[e-mail]> 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 <[e-mail]>
> > > > > > Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
> > > > > > ---
> > > > > >  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?    
> > > > 
> > > > Oops.  Not sure how that made it past even my rudimentary compile
> > > > testing.
> > > >   
> > > > > > +
> > > > > > +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?    
> > > > 
> > > > Like s390x PV it's initiated by the guest.
> > > >   
> > > > > 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.    
> > > >   
> > 
> > The purpose of --only-migratable is specifically to prevent the machine
> > to transition to a non-migrate state IIUC. The guest transition to
> > secure mode should be nacked in this case.
> 
> Yes, that's what happens for s390x: The guest tries to transition, QEMU
> can't add a migration blocker and fails the instruction used for
> transitioning, the guest sees the error.
> 
> The drawback is that we see the failure only when we already launched
> the machine and the guest tries to transition. If I start QEMU with
> --only-migratable, it will refuse to start when non-migratable devices
> are configured in the command line, so I see the issue right from the
> start. (For s390x, that would possibly mean that we should not even
> present the cpu feature bit when only_migratable is set?)

What happens in s390x,  if the guest tries to transition to secure, when
the secure object is NOT configured on the machine?

On PEF systems, the transition fails and the guest is terminated.

My point is -- QEMU will not be able to predict in advance, what the
guest might or might not do, regardless of what devices and objects are
configured in the machine.   If the guest does something unexpected, it
has to be terminated. 

So one possible design choice is to let the guest know that migration
must be facilitated. It can then decide if it wants to continue as a
normal VM or terminate itself, or take the plunge and switch to secure.
A well behaving guest will not switch to secure.

RP

Halil Pasic Jan. 4, 2021, 12:46 p.m. UTC | #8

On Sun, 3 Jan 2021 23:15:50 -0800
Ram Pai <[e-mail]> wrote:

> On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:
> > On Thu, 17 Dec 2020 15:15:30 +0100
[..]
> > > > > > > +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?      
> > > > > 
> > > > > Like s390x PV it's initiated by the guest.
> > > > >     
> > > > > > 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.      
> > > > >     
> > > 
> > > The purpose of --only-migratable is specifically to prevent the machine
> > > to transition to a non-migrate state IIUC. The guest transition to
> > > secure mode should be nacked in this case.  
> > 
> > Yes, that's what happens for s390x: The guest tries to transition, QEMU
> > can't add a migration blocker and fails the instruction used for
> > transitioning, the guest sees the error.
> > 
> > The drawback is that we see the failure only when we already launched
> > the machine and the guest tries to transition. If I start QEMU with
> > --only-migratable, it will refuse to start when non-migratable devices
> > are configured in the command line, so I see the issue right from the
> > start. (For s390x, that would possibly mean that we should not even
> > present the cpu feature bit when only_migratable is set?)  
> 
> What happens in s390x,  if the guest tries to transition to secure, when
> the secure object is NOT configured on the machine?
> 

Nothing in particular.

> On PEF systems, the transition fails and the guest is terminated.
> 
> My point is -- QEMU will not be able to predict in advance, what the
> guest might or might not do, regardless of what devices and objects are
> configured in the machine.   If the guest does something unexpected, it
> has to be terminated.

We can't fail transition to secure when the secure object is not
configured on the machine, because that would break pre-existing
setups. This feature is still to be shipped, but secure execution has
already been shipped, but without migration support.

That's why when you have both the secure object configured, and mandate
migratability, the we can fail. Actually we should fail now, because the
two options are not compatible: you can't have a qemu that is guaranteed
to be migratable, and guaranteed to be able to operate in secure
execution mode today. Failing early, and not on the guests opt-in would
be preferable.

After migration support is added, the combo should be fine, and probably
also the default for secure execution machines.
 
> 
> So one possible design choice is to let the guest know that migration
> must be facilitated. It can then decide if it wants to continue as a
> normal VM or terminate itself, or take the plunge and switch to secure.
> A well behaving guest will not switch to secure.
> 

I don't understand this point. Sorry.

Regards,
Halil

[..]

Ram Pai Jan. 4, 2021, 6:40 p.m. UTC | #9

On Mon, Jan 04, 2021 at 01:46:29PM +0100, Halil Pasic wrote:
> On Sun, 3 Jan 2021 23:15:50 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:
> > > On Thu, 17 Dec 2020 15:15:30 +0100
> [..]
> > > > > > > > +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?      
> > > > > > 
> > > > > > Like s390x PV it's initiated by the guest.
> > > > > >     
> > > > > > > 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.      
> > > > > >     
> > > > 
> > > > The purpose of --only-migratable is specifically to prevent the machine
> > > > to transition to a non-migrate state IIUC. The guest transition to
> > > > secure mode should be nacked in this case.  
> > > 
> > > Yes, that's what happens for s390x: The guest tries to transition, QEMU
> > > can't add a migration blocker and fails the instruction used for
> > > transitioning, the guest sees the error.
> > > 
> > > The drawback is that we see the failure only when we already launched
> > > the machine and the guest tries to transition. If I start QEMU with
> > > --only-migratable, it will refuse to start when non-migratable devices
> > > are configured in the command line, so I see the issue right from the
> > > start. (For s390x, that would possibly mean that we should not even
> > > present the cpu feature bit when only_migratable is set?)  
> > 
> > What happens in s390x,  if the guest tries to transition to secure, when
> > the secure object is NOT configured on the machine?
> > 
> 
> Nothing in particular.
> 
> > On PEF systems, the transition fails and the guest is terminated.
> > 
> > My point is -- QEMU will not be able to predict in advance, what the
> > guest might or might not do, regardless of what devices and objects are
> > configured in the machine.   If the guest does something unexpected, it
> > has to be terminated.
> 
> We can't fail transition to secure when the secure object is not
> configured on the machine, because that would break pre-existing
> setups.

So the instruction to switch-to-secure; which I believe is a ultracall
on S390,  will return success even though the switch-to-secure has failed?
Will the guest continue as a normal guest or as a secure guest?

> This feature is still to be shipped, but secure execution has
> already been shipped, but without migration support.
> 
> That's why when you have both the secure object configured, and mandate
> migratability, the we can fail. Actually we should fail now, because the
> two options are not compatible: you can't have a qemu that is guaranteed
> to be migratable, and guaranteed to be able to operate in secure
> execution mode today. Failing early, and not on the guests opt-in would
> be preferable.
> 
> After migration support is added, the combo should be fine, and probably
> also the default for secure execution machines.
> 
> > 
> > So one possible design choice is to let the guest know that migration
> > must be facilitated. It can then decide if it wants to continue as a
> > normal VM or terminate itself, or take the plunge and switch to secure.
> > A well behaving guest will not switch to secure.
> > 
> 
> I don't understand this point. Sorry.

Qemu will present the 'must-support-migrate' and the 'secure-object' capability
to the guest.

The secure-aware guest, has three choices
   (a) terminate itself. OR
   (b) not call the switch-to-secure ucall, and continue as normal guest. OR
   (c) call the switch-to-secure ucall.

Legacy guests which are not aware of secure-object, will continue to do
(b).   New Guests which are secure-object aware, will observe that 
'must-support-migrate' and 'secure-object' capabilities are
incompatible.  Hence will choose (a) or (b), but will never choose
(c).



The main difference between my proposal and the other proposal is...

  In my proposal the guest makes the compatibility decision and acts
  accordingly.  In the other proposal QEMU makes the compatibility
  decision and acts accordingly. I argue that QEMU cannot make a good
  compatibility decision, because it wont know in advance, if the guest
  will or will-not switch-to-secure.


RP

Halil Pasic Jan. 5, 2021, 10:56 a.m. UTC | #10

On Mon, 4 Jan 2021 10:40:26 -0800
Ram Pai <[e-mail]> wrote:

> On Mon, Jan 04, 2021 at 01:46:29PM +0100, Halil Pasic wrote:
> > On Sun, 3 Jan 2021 23:15:50 -0800
> > Ram Pai <[e-mail]> wrote:
> >   
> > > On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:  
> > > > On Thu, 17 Dec 2020 15:15:30 +0100  
> > [..]  
> > > > > > > > > +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?        
> > > > > > > 
> > > > > > > Like s390x PV it's initiated by the guest.
> > > > > > >       
> > > > > > > > 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.        
> > > > > > >       
> > > > > 
> > > > > The purpose of --only-migratable is specifically to prevent the machine
> > > > > to transition to a non-migrate state IIUC. The guest transition to
> > > > > secure mode should be nacked in this case.    
> > > > 
> > > > Yes, that's what happens for s390x: The guest tries to transition, QEMU
> > > > can't add a migration blocker and fails the instruction used for
> > > > transitioning, the guest sees the error.
> > > > 
> > > > The drawback is that we see the failure only when we already launched
> > > > the machine and the guest tries to transition. If I start QEMU with
> > > > --only-migratable, it will refuse to start when non-migratable devices
> > > > are configured in the command line, so I see the issue right from the
> > > > start. (For s390x, that would possibly mean that we should not even
> > > > present the cpu feature bit when only_migratable is set?)    
> > > 
> > > What happens in s390x,  if the guest tries to transition to secure, when
> > > the secure object is NOT configured on the machine?
> > >   
> > 
> > Nothing in particular.
> >   
> > > On PEF systems, the transition fails and the guest is terminated.
> > > 
> > > My point is -- QEMU will not be able to predict in advance, what the
> > > guest might or might not do, regardless of what devices and objects are
> > > configured in the machine.   If the guest does something unexpected, it
> > > has to be terminated.  
> > 
> > We can't fail transition to secure when the secure object is not
> > configured on the machine, because that would break pre-existing
> > setups.  
> 
> So the instruction to switch-to-secure; which I believe is a ultracall
> on S390,  

Yes it is an ultravisor call. 

> will return success even though the switch-to-secure has failed?

No, I don't think so.

> Will the guest continue as a normal guest or as a secure guest?
> 

I think the guest will give up. It definitely can't continue as secure
because the conversion to secure failed. And it should not continue as
non-secure because that's not what the user asked for.

I'm not sure you got my point. My point is: we may not break existing
setups when adding new features. Secure execution can work without secure
object today, and what works today shall keep working tomorrow and
beyond.

> > This feature is still to be shipped, but secure execution has
> > already been shipped, but without migration support.
> > 
> > That's why when you have both the secure object configured, and mandate
> > migratability, the we can fail. Actually we should fail now, because the
> > two options are not compatible: you can't have a qemu that is guaranteed
> > to be migratable, and guaranteed to be able to operate in secure
> > execution mode today. Failing early, and not on the guests opt-in would
> > be preferable.
> > 
> > After migration support is added, the combo should be fine, and probably
> > also the default for secure execution machines.
> >   
> > > 
> > > So one possible design choice is to let the guest know that migration
> > > must be facilitated. It can then decide if it wants to continue as a
> > > normal VM or terminate itself, or take the plunge and switch to secure.
> > > A well behaving guest will not switch to secure.
> > >   
> > 
> > I don't understand this point. Sorry.  
> 
> Qemu will present the 'must-support-migrate' and the 'secure-object' capability
> to the guest.

How does the qemu preset the 'must-support-migrate' and the
'secure-object' capability to the guest on (PPC and especially on s390)? And
please clarify what do you mean by 'secure-object'. I used to believe I
understood, but now I have the feeling I don't understand.

> 
> The secure-aware guest, has three choices
>    (a) terminate itself. OR
>    (b) not call the switch-to-secure ucall, and continue as normal guest. OR
>    (c) call the switch-to-secure ucall.
> 
> Legacy guests which are not aware of secure-object, will continue to do
> (b).   
> New Guests which are secure-object aware, will observe that 
> 'must-support-migrate' and 'secure-object' capabilities are
> incompatible.  Hence will choose (a) or (b), but will never choose
> (c).
> 

The first problem is, IMHO, that you want to expose QEMU internals to the
guest. For the guest, there is no such thing as 'must-support-migrate'
(AFAIK).

The other problem is, that migration and secure are not inherently
incompatible. On s390x it is the property of the current host
implementation, that we can't do migration for secure. But this can
change in the future. 

> 
> 
> The main difference between my proposal and the other proposal is...
> 
>   In my proposal the guest makes the compatibility decision and acts
>   accordingly.  In the other proposal QEMU makes the compatibility
>   decision and acts accordingly. I argue that QEMU cannot make a good
>   compatibility decision, because it wont know in advance, if the guest
>   will or will-not switch-to-secure.
> 

You have a point there when you say that QEMU does not know in advance,
if the guest will or will-not switch-to-secure. I made that argument
regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
was to flip that property on demand when the conversion occurs. David
explained to me that this is not possible for ppc, and that having the
"securable-guest-memory" property (or whatever the name will be)
specified is a strong indication, that the VM is intended to be used as
a secure VM (thus it is OK to hurt the case where the guest does not
try to transition). That argument applies here as well.

But more importantly, as I explained above, the guest does not know if
migration and secure are incompatible or not. So the guest can't make a
good decision.

Regards,
Halil

Ram Pai Jan. 5, 2021, 8:41 p.m. UTC | #11

On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> On Mon, 4 Jan 2021 10:40:26 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Mon, Jan 04, 2021 at 01:46:29PM +0100, Halil Pasic wrote:
> > > On Sun, 3 Jan 2021 23:15:50 -0800
> > > Ram Pai <[e-mail]> wrote:
> > >   
> > > > On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:  
> > > > > On Thu, 17 Dec 2020 15:15:30 +0100  
> > > [..]  
> > > > > > > > > > +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?        
> > > > > > > > 
> > > > > > > > Like s390x PV it's initiated by the guest.
> > > > > > > >       
> > > > > > > > > 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.        
> > > > > > > >       
> > > > > > 
> > > > > > The purpose of --only-migratable is specifically to prevent the machine
> > > > > > to transition to a non-migrate state IIUC. The guest transition to
> > > > > > secure mode should be nacked in this case.    
> > > > > 
> > > > > Yes, that's what happens for s390x: The guest tries to transition, QEMU
> > > > > can't add a migration blocker and fails the instruction used for
> > > > > transitioning, the guest sees the error.
> > > > > 
> > > > > The drawback is that we see the failure only when we already launched
> > > > > the machine and the guest tries to transition. If I start QEMU with
> > > > > --only-migratable, it will refuse to start when non-migratable devices
> > > > > are configured in the command line, so I see the issue right from the
> > > > > start. (For s390x, that would possibly mean that we should not even
> > > > > present the cpu feature bit when only_migratable is set?)    
> > > > 
> > > > What happens in s390x,  if the guest tries to transition to secure, when
> > > > the secure object is NOT configured on the machine?
> > > >   
> > > 
> > > Nothing in particular.
> > >   
> > > > On PEF systems, the transition fails and the guest is terminated.
> > > > 
> > > > My point is -- QEMU will not be able to predict in advance, what the
> > > > guest might or might not do, regardless of what devices and objects are
> > > > configured in the machine.   If the guest does something unexpected, it
> > > > has to be terminated.  
> > > 
> > > We can't fail transition to secure when the secure object is not
> > > configured on the machine, because that would break pre-existing
> > > setups.  
> > 
> > So the instruction to switch-to-secure; which I believe is a ultracall
> > on S390,  
> 
> Yes it is an ultravisor call. 
> 
> > will return success even though the switch-to-secure has failed?
> 
> No, I don't think so.
> 
> > Will the guest continue as a normal guest or as a secure guest?
> > 
> 
> I think the guest will give up. It definitely can't continue as secure
> because the conversion to secure failed. And it should not continue as
> non-secure because that's not what the user asked for.
> 
> I'm not sure you got my point. My point is: we may not break existing
> setups when adding new features. Secure execution can work without secure
> object today, and what works today shall keep working tomorrow and
> beyond.
> 
> > > This feature is still to be shipped, but secure execution has
> > > already been shipped, but without migration support.
> > > 
> > > That's why when you have both the secure object configured, and mandate
> > > migratability, the we can fail. Actually we should fail now, because the
> > > two options are not compatible: you can't have a qemu that is guaranteed
> > > to be migratable, and guaranteed to be able to operate in secure
> > > execution mode today. Failing early, and not on the guests opt-in would
> > > be preferable.
> > > 
> > > After migration support is added, the combo should be fine, and probably
> > > also the default for secure execution machines.
> > >   
> > > > 
> > > > So one possible design choice is to let the guest know that migration
> > > > must be facilitated. It can then decide if it wants to continue as a
> > > > normal VM or terminate itself, or take the plunge and switch to secure.
> > > > A well behaving guest will not switch to secure.
> > > >   
> > > 
> > > I don't understand this point. Sorry.  
> > 
> > Qemu will present the 'must-support-migrate' and the 'secure-object' capability
> > to the guest.
> 
> How does the qemu preset the 'must-support-migrate' and the
> 'secure-object' capability to the guest on (PPC and especially on s390)? 

This can be modeled with device tree properties on PPC. However, I
figure, my proposal has its own flaws; as admitted below.


> And
> please clarify what do you mean by 'secure-object'. I used to believe I
> understood, but now I have the feeling I don't understand.

Its the feature that enables the machine to be capable of running secure
guests.


> 
> > 
> > The secure-aware guest, has three choices
> >    (a) terminate itself. OR
> >    (b) not call the switch-to-secure ucall, and continue as normal guest. OR
> >    (c) call the switch-to-secure ucall.
> > 
> > Legacy guests which are not aware of secure-object, will continue to do
> > (b).   
> > New Guests which are secure-object aware, will observe that 
> > 'must-support-migrate' and 'secure-object' capabilities are
> > incompatible.  Hence will choose (a) or (b), but will never choose
> > (c).
> > 
> 
> The first problem is, IMHO, that you want to expose QEMU internals to the
> guest. For the guest, there is no such thing as 'must-support-migrate'
> (AFAIK).

right. good point.  The key point is, migration must be
transparent to the guest. And that is where; I realize, my proposal falters.

> 
> The other problem is, that migration and secure are not inherently
> incompatible. On s390x it is the property of the current host
> implementation, that we can't do migration for secure. But this can
> change in the future. 

> 
> > 
> > 
> > The main difference between my proposal and the other proposal is...
> > 
> >   In my proposal the guest makes the compatibility decision and acts
> >   accordingly.  In the other proposal QEMU makes the compatibility
> >   decision and acts accordingly. I argue that QEMU cannot make a good
> >   compatibility decision, because it wont know in advance, if the guest
> >   will or will-not switch-to-secure.
> > 
> 
> You have a point there when you say that QEMU does not know in advance,
> if the guest will or will-not switch-to-secure. I made that argument
> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> was to flip that property on demand when the conversion occurs. David
> explained to me that this is not possible for ppc, and that having the
> "securable-guest-memory" property (or whatever the name will be)
> specified is a strong indication, that the VM is intended to be used as
> a secure VM (thus it is OK to hurt the case where the guest does not
> try to transition). That argument applies here as well.

As suggested by Cornelia Huck, what if QEMU disabled the
"securable-guest-memory" property if 'must-support-migrate' is enabled?
Offcourse; this has to be done with a big fat warning stating
"secure-guest-memory" feature is disabled on the machine.
Doing so, will continue to support guest that do not try to transition.
Guest that try to transition will fail and terminate themselves.

> 
> But more importantly, as I explained above, the guest does not know if
> migration and secure are incompatible or not. So the guest can't make a
> good decision.

Agree.

RP

Cornelia Huck Jan. 11, 2021, 4:59 p.m. UTC | #12

On Tue, 5 Jan 2021 12:41:25 -0800
Ram Pai <[e-mail]> wrote:

> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > On Mon, 4 Jan 2021 10:40:26 -0800
> > Ram Pai <[e-mail]> wrote:

> > > The main difference between my proposal and the other proposal is...
> > > 
> > >   In my proposal the guest makes the compatibility decision and acts
> > >   accordingly.  In the other proposal QEMU makes the compatibility
> > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > >   compatibility decision, because it wont know in advance, if the guest
> > >   will or will-not switch-to-secure.
> > >   
> > 
> > You have a point there when you say that QEMU does not know in advance,
> > if the guest will or will-not switch-to-secure. I made that argument
> > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > was to flip that property on demand when the conversion occurs. David
> > explained to me that this is not possible for ppc, and that having the
> > "securable-guest-memory" property (or whatever the name will be)
> > specified is a strong indication, that the VM is intended to be used as
> > a secure VM (thus it is OK to hurt the case where the guest does not
> > try to transition). That argument applies here as well.  
> 
> As suggested by Cornelia Huck, what if QEMU disabled the
> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> Offcourse; this has to be done with a big fat warning stating
> "secure-guest-memory" feature is disabled on the machine.
> Doing so, will continue to support guest that do not try to transition.
> Guest that try to transition will fail and terminate themselves.

Just to recap the s390x situation:

- We currently offer a cpu feature that indicates secure execution to
  be available to the guest if the host supports it.
- When we introduce the secure object, we still need to support
  previous configurations and continue to offer the cpu feature, even
  if the secure object is not specified.
- As migration is currently not supported for secured guests, we add a
  blocker once the guest actually transitions. That means that
  transition fails if --only-migratable was specified on the command
  line. (Guests not transitioning will obviously not notice anything.)
- With the secure object, we will already fail starting QEMU if
  --only-migratable was specified.

My suggestion is now that we don't even offer the cpu feature if
--only-migratable has been specified. For a guest that does not want to
transition to secure mode, nothing changes; a guest that wants to
transition to secure mode will notice that the feature is not available
and fail appropriately (or ultimately, when the ultravisor call fails).
We'd still fail starting QEMU for the secure object + --only-migratable
combination.

Does that make sense?

Ram Pai Jan. 11, 2021, 7:58 p.m. UTC | #13

On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:
> On Tue, 5 Jan 2021 12:41:25 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > Ram Pai <[e-mail]> wrote:
> 
> > > > The main difference between my proposal and the other proposal is...
> > > > 
> > > >   In my proposal the guest makes the compatibility decision and acts
> > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > >   compatibility decision, because it wont know in advance, if the guest
> > > >   will or will-not switch-to-secure.
> > > >   
> > > 
> > > You have a point there when you say that QEMU does not know in advance,
> > > if the guest will or will-not switch-to-secure. I made that argument
> > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > was to flip that property on demand when the conversion occurs. David
> > > explained to me that this is not possible for ppc, and that having the
> > > "securable-guest-memory" property (or whatever the name will be)
> > > specified is a strong indication, that the VM is intended to be used as
> > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > try to transition). That argument applies here as well.  
> > 
> > As suggested by Cornelia Huck, what if QEMU disabled the
> > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > Offcourse; this has to be done with a big fat warning stating
> > "secure-guest-memory" feature is disabled on the machine.
> > Doing so, will continue to support guest that do not try to transition.
> > Guest that try to transition will fail and terminate themselves.
> 
> Just to recap the s390x situation:
> 
> - We currently offer a cpu feature that indicates secure execution to
>   be available to the guest if the host supports it.
> - When we introduce the secure object, we still need to support
>   previous configurations and continue to offer the cpu feature, even
>   if the secure object is not specified.
> - As migration is currently not supported for secured guests, we add a
>   blocker once the guest actually transitions. That means that
>   transition fails if --only-migratable was specified on the command
>   line. (Guests not transitioning will obviously not notice anything.)
> - With the secure object, we will already fail starting QEMU if
>   --only-migratable was specified.
> 
> My suggestion is now that we don't even offer the cpu feature if
> --only-migratable has been specified. For a guest that does not want to
> transition to secure mode, nothing changes; a guest that wants to
> transition to secure mode will notice that the feature is not available
> and fail appropriately (or ultimately, when the ultravisor call fails).


On POWER, secure-execution is not **automatically** enabled even when
the host supports it.  The feature is enabled only if the secure-object
is configured, and the host supports it.

However the behavior proposed above will be consistent on POWER and
on s390x,  when '--only-migratable' is specified and 'secure-object'
is NOT specified.

So I am in agreement till now. 


> We'd still fail starting QEMU for the secure object + --only-migratable
> combination.

Why fail? 

Instead, print a warning and  disable the secure-object; which will
disable your cpu-feature. Guests that do not transition to secure, will
continue to operate, and guests that transition to secure, will fail.

RP

Cornelia Huck Jan. 12, 2021, 8:19 a.m. UTC | #14

On Mon, 11 Jan 2021 11:58:30 -0800
Ram Pai <[e-mail]> wrote:

> On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:
> > On Tue, 5 Jan 2021 12:41:25 -0800
> > Ram Pai <[e-mail]> wrote:
> >   
> > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > Ram Pai <[e-mail]> wrote:  
> >   
> > > > > The main difference between my proposal and the other proposal is...
> > > > > 
> > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > >   will or will-not switch-to-secure.
> > > > >     
> > > > 
> > > > You have a point there when you say that QEMU does not know in advance,
> > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > was to flip that property on demand when the conversion occurs. David
> > > > explained to me that this is not possible for ppc, and that having the
> > > > "securable-guest-memory" property (or whatever the name will be)
> > > > specified is a strong indication, that the VM is intended to be used as
> > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > try to transition). That argument applies here as well.    
> > > 
> > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > Offcourse; this has to be done with a big fat warning stating
> > > "secure-guest-memory" feature is disabled on the machine.
> > > Doing so, will continue to support guest that do not try to transition.
> > > Guest that try to transition will fail and terminate themselves.  
> > 
> > Just to recap the s390x situation:
> > 
> > - We currently offer a cpu feature that indicates secure execution to
> >   be available to the guest if the host supports it.
> > - When we introduce the secure object, we still need to support
> >   previous configurations and continue to offer the cpu feature, even
> >   if the secure object is not specified.
> > - As migration is currently not supported for secured guests, we add a
> >   blocker once the guest actually transitions. That means that
> >   transition fails if --only-migratable was specified on the command
> >   line. (Guests not transitioning will obviously not notice anything.)
> > - With the secure object, we will already fail starting QEMU if
> >   --only-migratable was specified.
> > 
> > My suggestion is now that we don't even offer the cpu feature if
> > --only-migratable has been specified. For a guest that does not want to
> > transition to secure mode, nothing changes; a guest that wants to
> > transition to secure mode will notice that the feature is not available
> > and fail appropriately (or ultimately, when the ultravisor call fails).  
> 
> 
> On POWER, secure-execution is not **automatically** enabled even when
> the host supports it.  The feature is enabled only if the secure-object
> is configured, and the host supports it.

Yes, the cpu feature on s390x is simply pre-existing.

> 
> However the behavior proposed above will be consistent on POWER and
> on s390x,  when '--only-migratable' is specified and 'secure-object'
> is NOT specified.
> 
> So I am in agreement till now. 
> 
> 
> > We'd still fail starting QEMU for the secure object + --only-migratable
> > combination.  
> 
> Why fail? 
> 
> Instead, print a warning and  disable the secure-object; which will
> disable your cpu-feature. Guests that do not transition to secure, will
> continue to operate, and guests that transition to secure, will fail.

But that would be consistent with how other non-migratable objects are
handled, no? It's simply a case of incompatible options on the command
line.

Ram Pai Jan. 12, 2021, 6:55 p.m. UTC | #15

On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:
> On Mon, 11 Jan 2021 11:58:30 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:
> > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > Ram Pai <[e-mail]> wrote:
> > >   
> > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > Ram Pai <[e-mail]> wrote:  
> > >   
> > > > > > The main difference between my proposal and the other proposal is...
> > > > > > 
> > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > >   will or will-not switch-to-secure.
> > > > > >     
> > > > > 
> > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > was to flip that property on demand when the conversion occurs. David
> > > > > explained to me that this is not possible for ppc, and that having the
> > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > try to transition). That argument applies here as well.    
> > > > 
> > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > Offcourse; this has to be done with a big fat warning stating
> > > > "secure-guest-memory" feature is disabled on the machine.
> > > > Doing so, will continue to support guest that do not try to transition.
> > > > Guest that try to transition will fail and terminate themselves.  
> > > 
> > > Just to recap the s390x situation:
> > > 
> > > - We currently offer a cpu feature that indicates secure execution to
> > >   be available to the guest if the host supports it.
> > > - When we introduce the secure object, we still need to support
> > >   previous configurations and continue to offer the cpu feature, even
> > >   if the secure object is not specified.
> > > - As migration is currently not supported for secured guests, we add a
> > >   blocker once the guest actually transitions. That means that
> > >   transition fails if --only-migratable was specified on the command
> > >   line. (Guests not transitioning will obviously not notice anything.)
> > > - With the secure object, we will already fail starting QEMU if
> > >   --only-migratable was specified.
> > > 
> > > My suggestion is now that we don't even offer the cpu feature if
> > > --only-migratable has been specified. For a guest that does not want to
> > > transition to secure mode, nothing changes; a guest that wants to
> > > transition to secure mode will notice that the feature is not available
> > > and fail appropriately (or ultimately, when the ultravisor call fails).  
> > 
> > 
> > On POWER, secure-execution is not **automatically** enabled even when
> > the host supports it.  The feature is enabled only if the secure-object
> > is configured, and the host supports it.
> 
> Yes, the cpu feature on s390x is simply pre-existing.
> 
> > 
> > However the behavior proposed above will be consistent on POWER and
> > on s390x,  when '--only-migratable' is specified and 'secure-object'
> > is NOT specified.
> > 
> > So I am in agreement till now. 
> > 
> > 
> > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > combination.  
> > 
> > Why fail? 
> > 
> > Instead, print a warning and  disable the secure-object; which will
> > disable your cpu-feature. Guests that do not transition to secure, will
> > continue to operate, and guests that transition to secure, will fail.
> 
> But that would be consistent with how other non-migratable objects are
> handled, no? It's simply a case of incompatible options on the command
> line.

Actually the two options are inherently NOT incompatible.  Halil also
mentioned this in one of his replies.

Its just that the current implementation is lacking, which will be fixed
in the near future. 

We can design it upfront, with the assumption that they both are compatible.
In the short term  disable one; preferrably the secure-object, if both 
options are specified. In the long term, remove the restriction, when
the implemetation is complete.

Cornelia Huck Jan. 13, 2021, 8:06 a.m. UTC | #16

On Tue, 12 Jan 2021 10:55:11 -0800
Ram Pai <[e-mail]> wrote:

> On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:
> > On Mon, 11 Jan 2021 11:58:30 -0800
> > Ram Pai <[e-mail]> wrote:
> >   
> > > On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:  
> > > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > > Ram Pai <[e-mail]> wrote:
> > > >     
> > > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
> > > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > > Ram Pai <[e-mail]> wrote:    
> > > >     
> > > > > > > The main difference between my proposal and the other proposal is...
> > > > > > > 
> > > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > > >   will or will-not switch-to-secure.
> > > > > > >       
> > > > > > 
> > > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > > was to flip that property on demand when the conversion occurs. David
> > > > > > explained to me that this is not possible for ppc, and that having the
> > > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > > try to transition). That argument applies here as well.      
> > > > > 
> > > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > > Offcourse; this has to be done with a big fat warning stating
> > > > > "secure-guest-memory" feature is disabled on the machine.
> > > > > Doing so, will continue to support guest that do not try to transition.
> > > > > Guest that try to transition will fail and terminate themselves.    
> > > > 
> > > > Just to recap the s390x situation:
> > > > 
> > > > - We currently offer a cpu feature that indicates secure execution to
> > > >   be available to the guest if the host supports it.
> > > > - When we introduce the secure object, we still need to support
> > > >   previous configurations and continue to offer the cpu feature, even
> > > >   if the secure object is not specified.
> > > > - As migration is currently not supported for secured guests, we add a
> > > >   blocker once the guest actually transitions. That means that
> > > >   transition fails if --only-migratable was specified on the command
> > > >   line. (Guests not transitioning will obviously not notice anything.)
> > > > - With the secure object, we will already fail starting QEMU if
> > > >   --only-migratable was specified.
> > > > 
> > > > My suggestion is now that we don't even offer the cpu feature if
> > > > --only-migratable has been specified. For a guest that does not want to
> > > > transition to secure mode, nothing changes; a guest that wants to
> > > > transition to secure mode will notice that the feature is not available
> > > > and fail appropriately (or ultimately, when the ultravisor call fails).    
> > > 
> > > 
> > > On POWER, secure-execution is not **automatically** enabled even when
> > > the host supports it.  The feature is enabled only if the secure-object
> > > is configured, and the host supports it.  
> > 
> > Yes, the cpu feature on s390x is simply pre-existing.
> >   
> > > 
> > > However the behavior proposed above will be consistent on POWER and
> > > on s390x,  when '--only-migratable' is specified and 'secure-object'
> > > is NOT specified.
> > > 
> > > So I am in agreement till now. 
> > > 
> > >   
> > > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > > combination.    
> > > 
> > > Why fail? 
> > > 
> > > Instead, print a warning and  disable the secure-object; which will
> > > disable your cpu-feature. Guests that do not transition to secure, will
> > > continue to operate, and guests that transition to secure, will fail.  
> > 
> > But that would be consistent with how other non-migratable objects are
> > handled, no? It's simply a case of incompatible options on the command
> > line.  
> 
> Actually the two options are inherently NOT incompatible.  Halil also
> mentioned this in one of his replies.
> 
> Its just that the current implementation is lacking, which will be fixed
> in the near future. 
> 
> We can design it upfront, with the assumption that they both are compatible.
> In the short term  disable one; preferrably the secure-object, if both 
> options are specified. In the long term, remove the restriction, when
> the implemetation is complete.

Can't we simply mark the object as non-migratable now, and then remove
that later? I don't see what is so special about it.

Dr. David Alan Gilbert Jan. 13, 2021, 12:42 p.m. UTC | #17

* Cornelia Huck ([e-mail]) wrote:
> On Tue, 5 Jan 2021 12:41:25 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > Ram Pai <[e-mail]> wrote:
> 
> > > > The main difference between my proposal and the other proposal is...
> > > > 
> > > >   In my proposal the guest makes the compatibility decision and acts
> > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > >   compatibility decision, because it wont know in advance, if the guest
> > > >   will or will-not switch-to-secure.
> > > >   
> > > 
> > > You have a point there when you say that QEMU does not know in advance,
> > > if the guest will or will-not switch-to-secure. I made that argument
> > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > was to flip that property on demand when the conversion occurs. David
> > > explained to me that this is not possible for ppc, and that having the
> > > "securable-guest-memory" property (or whatever the name will be)
> > > specified is a strong indication, that the VM is intended to be used as
> > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > try to transition). That argument applies here as well.  
> > 
> > As suggested by Cornelia Huck, what if QEMU disabled the
> > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > Offcourse; this has to be done with a big fat warning stating
> > "secure-guest-memory" feature is disabled on the machine.
> > Doing so, will continue to support guest that do not try to transition.
> > Guest that try to transition will fail and terminate themselves.
> 
> Just to recap the s390x situation:
> 
> - We currently offer a cpu feature that indicates secure execution to
>   be available to the guest if the host supports it.
> - When we introduce the secure object, we still need to support
>   previous configurations and continue to offer the cpu feature, even
>   if the secure object is not specified.
> - As migration is currently not supported for secured guests, we add a
>   blocker once the guest actually transitions. That means that
>   transition fails if --only-migratable was specified on the command
>   line. (Guests not transitioning will obviously not notice anything.)
> - With the secure object, we will already fail starting QEMU if
>   --only-migratable was specified.
> 
> My suggestion is now that we don't even offer the cpu feature if
> --only-migratable has been specified. For a guest that does not want to
> transition to secure mode, nothing changes; a guest that wants to
> transition to secure mode will notice that the feature is not available
> and fail appropriately (or ultimately, when the ultravisor call fails).
> We'd still fail starting QEMU for the secure object + --only-migratable
> combination.
> 
> Does that make sense?

It's a little unusual; I don't think we have any other cases where
--only-migratable changes the behaviour; I think it normally only stops
you doing something that would have made it unmigratable or causes
an operation that would make it unmigratable to fail.

Dave

Christian Borntraeger Jan. 14, 2021, 10:28 a.m. UTC | #18

On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> * Cornelia Huck ([e-mail]) wrote:
>> On Tue, 5 Jan 2021 12:41:25 -0800
>> Ram Pai <[e-mail]> wrote:
>>
>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>> Ram Pai <[e-mail]> wrote:
>>
>>>>> The main difference between my proposal and the other proposal is...
>>>>>
>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>   will or will-not switch-to-secure.
>>>>>   
>>>>
>>>> You have a point there when you say that QEMU does not know in advance,
>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>> was to flip that property on demand when the conversion occurs. David
>>>> explained to me that this is not possible for ppc, and that having the
>>>> "securable-guest-memory" property (or whatever the name will be)
>>>> specified is a strong indication, that the VM is intended to be used as
>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>> try to transition). That argument applies here as well.  
>>>
>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>> Offcourse; this has to be done with a big fat warning stating
>>> "secure-guest-memory" feature is disabled on the machine.
>>> Doing so, will continue to support guest that do not try to transition.
>>> Guest that try to transition will fail and terminate themselves.
>>
>> Just to recap the s390x situation:
>>
>> - We currently offer a cpu feature that indicates secure execution to
>>   be available to the guest if the host supports it.
>> - When we introduce the secure object, we still need to support
>>   previous configurations and continue to offer the cpu feature, even
>>   if the secure object is not specified.
>> - As migration is currently not supported for secured guests, we add a
>>   blocker once the guest actually transitions. That means that
>>   transition fails if --only-migratable was specified on the command
>>   line. (Guests not transitioning will obviously not notice anything.)
>> - With the secure object, we will already fail starting QEMU if
>>   --only-migratable was specified.
>>
>> My suggestion is now that we don't even offer the cpu feature if
>> --only-migratable has been specified. For a guest that does not want to
>> transition to secure mode, nothing changes; a guest that wants to
>> transition to secure mode will notice that the feature is not available
>> and fail appropriately (or ultimately, when the ultravisor call fails).
>> We'd still fail starting QEMU for the secure object + --only-migratable
>> combination.
>>
>> Does that make sense?
> 
> It's a little unusual; I don't think we have any other cases where
> --only-migratable changes the behaviour; I think it normally only stops
> you doing something that would have made it unmigratable or causes
> an operation that would make it unmigratable to fail.

I would like to NOT block this feature with --only-migrateable. A guest
can startup unprotected (and then is is migrateable). the migration blocker
is really a dynamic aspect during runtime.

Dr. David Alan Gilbert Jan. 14, 2021, 10:36 a.m. UTC | #19

* Christian Borntraeger ([e-mail]) wrote:
> 
> 
> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck ([e-mail]) wrote:
> >> On Tue, 5 Jan 2021 12:41:25 -0800
> >> Ram Pai <[e-mail]> wrote:
> >>
> >>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> >>>> On Mon, 4 Jan 2021 10:40:26 -0800
> >>>> Ram Pai <[e-mail]> wrote:
> >>
> >>>>> The main difference between my proposal and the other proposal is...
> >>>>>
> >>>>>   In my proposal the guest makes the compatibility decision and acts
> >>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> >>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> >>>>>   compatibility decision, because it wont know in advance, if the guest
> >>>>>   will or will-not switch-to-secure.
> >>>>>   
> >>>>
> >>>> You have a point there when you say that QEMU does not know in advance,
> >>>> if the guest will or will-not switch-to-secure. I made that argument
> >>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >>>> was to flip that property on demand when the conversion occurs. David
> >>>> explained to me that this is not possible for ppc, and that having the
> >>>> "securable-guest-memory" property (or whatever the name will be)
> >>>> specified is a strong indication, that the VM is intended to be used as
> >>>> a secure VM (thus it is OK to hurt the case where the guest does not
> >>>> try to transition). That argument applies here as well.  
> >>>
> >>> As suggested by Cornelia Huck, what if QEMU disabled the
> >>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> >>> Offcourse; this has to be done with a big fat warning stating
> >>> "secure-guest-memory" feature is disabled on the machine.
> >>> Doing so, will continue to support guest that do not try to transition.
> >>> Guest that try to transition will fail and terminate themselves.
> >>
> >> Just to recap the s390x situation:
> >>
> >> - We currently offer a cpu feature that indicates secure execution to
> >>   be available to the guest if the host supports it.
> >> - When we introduce the secure object, we still need to support
> >>   previous configurations and continue to offer the cpu feature, even
> >>   if the secure object is not specified.
> >> - As migration is currently not supported for secured guests, we add a
> >>   blocker once the guest actually transitions. That means that
> >>   transition fails if --only-migratable was specified on the command
> >>   line. (Guests not transitioning will obviously not notice anything.)
> >> - With the secure object, we will already fail starting QEMU if
> >>   --only-migratable was specified.
> >>
> >> My suggestion is now that we don't even offer the cpu feature if
> >> --only-migratable has been specified. For a guest that does not want to
> >> transition to secure mode, nothing changes; a guest that wants to
> >> transition to secure mode will notice that the feature is not available
> >> and fail appropriately (or ultimately, when the ultravisor call fails).
> >> We'd still fail starting QEMU for the secure object + --only-migratable
> >> combination.
> >>
> >> Does that make sense?
> > 
> > It's a little unusual; I don't think we have any other cases where
> > --only-migratable changes the behaviour; I think it normally only stops
> > you doing something that would have made it unmigratable or causes
> > an operation that would make it unmigratable to fail.
> 
> I would like to NOT block this feature with --only-migrateable. A guest
> can startup unprotected (and then is is migrateable). the migration blocker
> is really a dynamic aspect during runtime. 

But the point of --only-migratable is to turn things that would have
blocked migration into failures, so that a VM started with
--only-migratable is *always* migratable.


Dave

Christian Borntraeger Jan. 14, 2021, 10:52 a.m. UTC | #20

On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger ([e-mail]) wrote:
>>
>>
>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
>>> * Cornelia Huck ([e-mail]) wrote:
>>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>>> Ram Pai <[e-mail]> wrote:
>>>>
>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>>>> Ram Pai <[e-mail]> wrote:
>>>>
>>>>>>> The main difference between my proposal and the other proposal is...
>>>>>>>
>>>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>>>   will or will-not switch-to-secure.
>>>>>>>   
>>>>>>
>>>>>> You have a point there when you say that QEMU does not know in advance,
>>>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>>>> was to flip that property on demand when the conversion occurs. David
>>>>>> explained to me that this is not possible for ppc, and that having the
>>>>>> "securable-guest-memory" property (or whatever the name will be)
>>>>>> specified is a strong indication, that the VM is intended to be used as
>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>>>> try to transition). That argument applies here as well.  
>>>>>
>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>>>> Offcourse; this has to be done with a big fat warning stating
>>>>> "secure-guest-memory" feature is disabled on the machine.
>>>>> Doing so, will continue to support guest that do not try to transition.
>>>>> Guest that try to transition will fail and terminate themselves.
>>>>
>>>> Just to recap the s390x situation:
>>>>
>>>> - We currently offer a cpu feature that indicates secure execution to
>>>>   be available to the guest if the host supports it.
>>>> - When we introduce the secure object, we still need to support
>>>>   previous configurations and continue to offer the cpu feature, even
>>>>   if the secure object is not specified.
>>>> - As migration is currently not supported for secured guests, we add a
>>>>   blocker once the guest actually transitions. That means that
>>>>   transition fails if --only-migratable was specified on the command
>>>>   line. (Guests not transitioning will obviously not notice anything.)
>>>> - With the secure object, we will already fail starting QEMU if
>>>>   --only-migratable was specified.
>>>>
>>>> My suggestion is now that we don't even offer the cpu feature if
>>>> --only-migratable has been specified. For a guest that does not want to
>>>> transition to secure mode, nothing changes; a guest that wants to
>>>> transition to secure mode will notice that the feature is not available
>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
>>>> We'd still fail starting QEMU for the secure object + --only-migratable
>>>> combination.
>>>>
>>>> Does that make sense?
>>>
>>> It's a little unusual; I don't think we have any other cases where
>>> --only-migratable changes the behaviour; I think it normally only stops
>>> you doing something that would have made it unmigratable or causes
>>> an operation that would make it unmigratable to fail.
>>
>> I would like to NOT block this feature with --only-migrateable. A guest
>> can startup unprotected (and then is is migrateable). the migration blocker
>> is really a dynamic aspect during runtime. 
> 
> But the point of --only-migratable is to turn things that would have
> blocked migration into failures, so that a VM started with
> --only-migratable is *always* migratable.

Hmmm, fair enough. How do we do this with host-model? The constructed model
would contain unpack, but then it will fail to startup? Or do we silently 
drop unpack in that case? Both variants do not feel completely right.

Cornelia Huck Jan. 14, 2021, 11:05 a.m. UTC | #21

On Thu, 14 Jan 2021 11:52:11 +0100
Christian Borntraeger <[e-mail]> wrote:

> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> > * Christian Borntraeger ([e-mail]) wrote:  
> >>
> >>
> >> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> >>> * Cornelia Huck ([e-mail]) wrote:  
> >>>> On Tue, 5 Jan 2021 12:41:25 -0800
> >>>> Ram Pai <[e-mail]> wrote:
> >>>>  
> >>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> >>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> >>>>>> Ram Pai <[e-mail]> wrote:  
> >>>>  
> >>>>>>> The main difference between my proposal and the other proposal is...
> >>>>>>>
> >>>>>>>   In my proposal the guest makes the compatibility decision and acts
> >>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> >>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> >>>>>>>   compatibility decision, because it wont know in advance, if the guest
> >>>>>>>   will or will-not switch-to-secure.
> >>>>>>>     
> >>>>>>
> >>>>>> You have a point there when you say that QEMU does not know in advance,
> >>>>>> if the guest will or will-not switch-to-secure. I made that argument
> >>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >>>>>> was to flip that property on demand when the conversion occurs. David
> >>>>>> explained to me that this is not possible for ppc, and that having the
> >>>>>> "securable-guest-memory" property (or whatever the name will be)
> >>>>>> specified is a strong indication, that the VM is intended to be used as
> >>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> >>>>>> try to transition). That argument applies here as well.    
> >>>>>
> >>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> >>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> >>>>> Offcourse; this has to be done with a big fat warning stating
> >>>>> "secure-guest-memory" feature is disabled on the machine.
> >>>>> Doing so, will continue to support guest that do not try to transition.
> >>>>> Guest that try to transition will fail and terminate themselves.  
> >>>>
> >>>> Just to recap the s390x situation:
> >>>>
> >>>> - We currently offer a cpu feature that indicates secure execution to
> >>>>   be available to the guest if the host supports it.
> >>>> - When we introduce the secure object, we still need to support
> >>>>   previous configurations and continue to offer the cpu feature, even
> >>>>   if the secure object is not specified.
> >>>> - As migration is currently not supported for secured guests, we add a
> >>>>   blocker once the guest actually transitions. That means that
> >>>>   transition fails if --only-migratable was specified on the command
> >>>>   line. (Guests not transitioning will obviously not notice anything.)
> >>>> - With the secure object, we will already fail starting QEMU if
> >>>>   --only-migratable was specified.
> >>>>
> >>>> My suggestion is now that we don't even offer the cpu feature if
> >>>> --only-migratable has been specified. For a guest that does not want to
> >>>> transition to secure mode, nothing changes; a guest that wants to
> >>>> transition to secure mode will notice that the feature is not available
> >>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> >>>> We'd still fail starting QEMU for the secure object + --only-migratable
> >>>> combination.
> >>>>
> >>>> Does that make sense?  
> >>>
> >>> It's a little unusual; I don't think we have any other cases where
> >>> --only-migratable changes the behaviour; I think it normally only stops
> >>> you doing something that would have made it unmigratable or causes
> >>> an operation that would make it unmigratable to fail.  
> >>
> >> I would like to NOT block this feature with --only-migrateable. A guest
> >> can startup unprotected (and then is is migrateable). the migration blocker
> >> is really a dynamic aspect during runtime.   
> > 
> > But the point of --only-migratable is to turn things that would have
> > blocked migration into failures, so that a VM started with
> > --only-migratable is *always* migratable.  
> 
> Hmmm, fair enough. How do we do this with host-model? The constructed model
> would contain unpack, but then it will fail to startup? Or do we silently 
> drop unpack in that case? Both variants do not feel completely right. 

Failing if you explicitly specified unpacked feels right, but failing
if you just used the host model feels odd. Removing unpack also is a
bit odd, but I think the better option if we want to do anything about
it at all.

Daniel P. Berrangé Jan. 14, 2021, 11:23 a.m. UTC | #22

On Mon, Jan 11, 2021 at 11:58:30AM -0800, Ram Pai wrote:
> On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:
> > On Tue, 5 Jan 2021 12:41:25 -0800
> > Ram Pai <[e-mail]> wrote:
> > 
> > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > Ram Pai <[e-mail]> wrote:
> > 
> > > > > The main difference between my proposal and the other proposal is...
> > > > > 
> > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > >   will or will-not switch-to-secure.
> > > > >   
> > > > 
> > > > You have a point there when you say that QEMU does not know in advance,
> > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > was to flip that property on demand when the conversion occurs. David
> > > > explained to me that this is not possible for ppc, and that having the
> > > > "securable-guest-memory" property (or whatever the name will be)
> > > > specified is a strong indication, that the VM is intended to be used as
> > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > try to transition). That argument applies here as well.  
> > > 
> > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > Offcourse; this has to be done with a big fat warning stating
> > > "secure-guest-memory" feature is disabled on the machine.
> > > Doing so, will continue to support guest that do not try to transition.
> > > Guest that try to transition will fail and terminate themselves.
> > 
> > Just to recap the s390x situation:
> > 
> > - We currently offer a cpu feature that indicates secure execution to
> >   be available to the guest if the host supports it.
> > - When we introduce the secure object, we still need to support
> >   previous configurations and continue to offer the cpu feature, even
> >   if the secure object is not specified.
> > - As migration is currently not supported for secured guests, we add a
> >   blocker once the guest actually transitions. That means that
> >   transition fails if --only-migratable was specified on the command
> >   line. (Guests not transitioning will obviously not notice anything.)
> > - With the secure object, we will already fail starting QEMU if
> >   --only-migratable was specified.
> > 
> > My suggestion is now that we don't even offer the cpu feature if
> > --only-migratable has been specified. For a guest that does not want to
> > transition to secure mode, nothing changes; a guest that wants to
> > transition to secure mode will notice that the feature is not available
> > and fail appropriately (or ultimately, when the ultravisor call fails).
> 
> 
> On POWER, secure-execution is not **automatically** enabled even when
> the host supports it.  The feature is enabled only if the secure-object
> is configured, and the host supports it.
> 
> However the behavior proposed above will be consistent on POWER and
> on s390x,  when '--only-migratable' is specified and 'secure-object'
> is NOT specified.
> 
> So I am in agreement till now. 
> 
> 
> > We'd still fail starting QEMU for the secure object + --only-migratable
> > combination.
> 
> Why fail? 
> 
> Instead, print a warning and  disable the secure-object; which will
> disable your cpu-feature. Guests that do not transition to secure, will
> continue to operate, and guests that transition to secure, will fail.

Ignoring a configuration option that was explicitly requested by the
user/mgmt app is bad practice. If a request feature combination cannot
be honoured, QEMU must treat that as a fatal error and exit, so that
the mgmt app knows their config is unsupported.

Regards,
Daniel

Daniel P. Berrangé Jan. 14, 2021, 11:25 a.m. UTC | #23

On Wed, Jan 13, 2021 at 12:42:26PM +0000, Dr. David Alan Gilbert wrote:
> * Cornelia Huck ([e-mail]) wrote:
> > On Tue, 5 Jan 2021 12:41:25 -0800
> > Ram Pai <[e-mail]> wrote:
> > 
> > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > Ram Pai <[e-mail]> wrote:
> > 
> > > > > The main difference between my proposal and the other proposal is...
> > > > > 
> > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > >   will or will-not switch-to-secure.
> > > > >   
> > > > 
> > > > You have a point there when you say that QEMU does not know in advance,
> > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > was to flip that property on demand when the conversion occurs. David
> > > > explained to me that this is not possible for ppc, and that having the
> > > > "securable-guest-memory" property (or whatever the name will be)
> > > > specified is a strong indication, that the VM is intended to be used as
> > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > try to transition). That argument applies here as well.  
> > > 
> > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > Offcourse; this has to be done with a big fat warning stating
> > > "secure-guest-memory" feature is disabled on the machine.
> > > Doing so, will continue to support guest that do not try to transition.
> > > Guest that try to transition will fail and terminate themselves.
> > 
> > Just to recap the s390x situation:
> > 
> > - We currently offer a cpu feature that indicates secure execution to
> >   be available to the guest if the host supports it.
> > - When we introduce the secure object, we still need to support
> >   previous configurations and continue to offer the cpu feature, even
> >   if the secure object is not specified.
> > - As migration is currently not supported for secured guests, we add a
> >   blocker once the guest actually transitions. That means that
> >   transition fails if --only-migratable was specified on the command
> >   line. (Guests not transitioning will obviously not notice anything.)
> > - With the secure object, we will already fail starting QEMU if
> >   --only-migratable was specified.
> > 
> > My suggestion is now that we don't even offer the cpu feature if
> > --only-migratable has been specified. For a guest that does not want to
> > transition to secure mode, nothing changes; a guest that wants to
> > transition to secure mode will notice that the feature is not available
> > and fail appropriately (or ultimately, when the ultravisor call fails).
> > We'd still fail starting QEMU for the secure object + --only-migratable
> > combination.
> > 
> > Does that make sense?
> 
> It's a little unusual; I don't think we have any other cases where
> --only-migratable changes the behaviour; I think it normally only stops
> you doing something that would have made it unmigratable or causes
> an operation that would make it unmigratable to fail.

I agree,  --only-migratable is supposed to be a *behavioural* toggle
for QEMU. It must /not/ have any impact on the guest ABI.

A management application needs to be able to add/remove --only-migratable
at will without changing the exposing guest ABI.

Regards,
Daniel

Dr. David Alan Gilbert Jan. 14, 2021, 11:45 a.m. UTC | #24

* Cornelia Huck ([e-mail]) wrote:
> On Thu, 14 Jan 2021 11:52:11 +0100
> Christian Borntraeger <[e-mail]> wrote:
> 
> > On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> > > * Christian Borntraeger ([e-mail]) wrote:  
> > >>
> > >>
> > >> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> > >>> * Cornelia Huck ([e-mail]) wrote:  
> > >>>> On Tue, 5 Jan 2021 12:41:25 -0800
> > >>>> Ram Pai <[e-mail]> wrote:
> > >>>>  
> > >>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> > >>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> > >>>>>> Ram Pai <[e-mail]> wrote:  
> > >>>>  
> > >>>>>>> The main difference between my proposal and the other proposal is...
> > >>>>>>>
> > >>>>>>>   In my proposal the guest makes the compatibility decision and acts
> > >>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> > >>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> > >>>>>>>   compatibility decision, because it wont know in advance, if the guest
> > >>>>>>>   will or will-not switch-to-secure.
> > >>>>>>>     
> > >>>>>>
> > >>>>>> You have a point there when you say that QEMU does not know in advance,
> > >>>>>> if the guest will or will-not switch-to-secure. I made that argument
> > >>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > >>>>>> was to flip that property on demand when the conversion occurs. David
> > >>>>>> explained to me that this is not possible for ppc, and that having the
> > >>>>>> "securable-guest-memory" property (or whatever the name will be)
> > >>>>>> specified is a strong indication, that the VM is intended to be used as
> > >>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> > >>>>>> try to transition). That argument applies here as well.    
> > >>>>>
> > >>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> > >>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > >>>>> Offcourse; this has to be done with a big fat warning stating
> > >>>>> "secure-guest-memory" feature is disabled on the machine.
> > >>>>> Doing so, will continue to support guest that do not try to transition.
> > >>>>> Guest that try to transition will fail and terminate themselves.  
> > >>>>
> > >>>> Just to recap the s390x situation:
> > >>>>
> > >>>> - We currently offer a cpu feature that indicates secure execution to
> > >>>>   be available to the guest if the host supports it.
> > >>>> - When we introduce the secure object, we still need to support
> > >>>>   previous configurations and continue to offer the cpu feature, even
> > >>>>   if the secure object is not specified.
> > >>>> - As migration is currently not supported for secured guests, we add a
> > >>>>   blocker once the guest actually transitions. That means that
> > >>>>   transition fails if --only-migratable was specified on the command
> > >>>>   line. (Guests not transitioning will obviously not notice anything.)
> > >>>> - With the secure object, we will already fail starting QEMU if
> > >>>>   --only-migratable was specified.
> > >>>>
> > >>>> My suggestion is now that we don't even offer the cpu feature if
> > >>>> --only-migratable has been specified. For a guest that does not want to
> > >>>> transition to secure mode, nothing changes; a guest that wants to
> > >>>> transition to secure mode will notice that the feature is not available
> > >>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> > >>>> We'd still fail starting QEMU for the secure object + --only-migratable
> > >>>> combination.
> > >>>>
> > >>>> Does that make sense?  
> > >>>
> > >>> It's a little unusual; I don't think we have any other cases where
> > >>> --only-migratable changes the behaviour; I think it normally only stops
> > >>> you doing something that would have made it unmigratable or causes
> > >>> an operation that would make it unmigratable to fail.  
> > >>
> > >> I would like to NOT block this feature with --only-migrateable. A guest
> > >> can startup unprotected (and then is is migrateable). the migration blocker
> > >> is really a dynamic aspect during runtime.   
> > > 
> > > But the point of --only-migratable is to turn things that would have
> > > blocked migration into failures, so that a VM started with
> > > --only-migratable is *always* migratable.  
> > 
> > Hmmm, fair enough. How do we do this with host-model? The constructed model
> > would contain unpack, but then it will fail to startup? Or do we silently 
> > drop unpack in that case? Both variants do not feel completely right. 
> 
> Failing if you explicitly specified unpacked feels right, but failing
> if you just used the host model feels odd. Removing unpack also is a
> bit odd, but I think the better option if we want to do anything about
> it at all.

'host-model' feels a bit special; but breaking the rule that
only-migratable doesn't change behaviour is weird.
Can you do host,-unpack   to make that work explicitly?

But hang on; why is 'unpack' the name of a secure guest facility - is
it really a feature for secure guest or something else?

Dave

Christian Borntraeger Jan. 14, 2021, 11:50 a.m. UTC | #25

On 14.01.21 12:45, Dr. David Alan Gilbert wrote:
> * Cornelia Huck ([e-mail]) wrote:
>> On Thu, 14 Jan 2021 11:52:11 +0100
>> Christian Borntraeger <[e-mail]> wrote:
>>
>>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
>>>> * Christian Borntraeger ([e-mail]) wrote:  
>>>>>
>>>>>
>>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
>>>>>> * Cornelia Huck ([e-mail]) wrote:  
>>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>>>>>> Ram Pai <[e-mail]> wrote:
>>>>>>>  
>>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
>>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>>>>>>> Ram Pai <[e-mail]> wrote:  
>>>>>>>  
>>>>>>>>>> The main difference between my proposal and the other proposal is...
>>>>>>>>>>
>>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>>>>>>   will or will-not switch-to-secure.
>>>>>>>>>>     
>>>>>>>>>
>>>>>>>>> You have a point there when you say that QEMU does not know in advance,
>>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>>>>>>> was to flip that property on demand when the conversion occurs. David
>>>>>>>>> explained to me that this is not possible for ppc, and that having the
>>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
>>>>>>>>> specified is a strong indication, that the VM is intended to be used as
>>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>>>>>>> try to transition). That argument applies here as well.    
>>>>>>>>
>>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>>>>>>> Offcourse; this has to be done with a big fat warning stating
>>>>>>>> "secure-guest-memory" feature is disabled on the machine.
>>>>>>>> Doing so, will continue to support guest that do not try to transition.
>>>>>>>> Guest that try to transition will fail and terminate themselves.  
>>>>>>>
>>>>>>> Just to recap the s390x situation:
>>>>>>>
>>>>>>> - We currently offer a cpu feature that indicates secure execution to
>>>>>>>   be available to the guest if the host supports it.
>>>>>>> - When we introduce the secure object, we still need to support
>>>>>>>   previous configurations and continue to offer the cpu feature, even
>>>>>>>   if the secure object is not specified.
>>>>>>> - As migration is currently not supported for secured guests, we add a
>>>>>>>   blocker once the guest actually transitions. That means that
>>>>>>>   transition fails if --only-migratable was specified on the command
>>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
>>>>>>> - With the secure object, we will already fail starting QEMU if
>>>>>>>   --only-migratable was specified.
>>>>>>>
>>>>>>> My suggestion is now that we don't even offer the cpu feature if
>>>>>>> --only-migratable has been specified. For a guest that does not want to
>>>>>>> transition to secure mode, nothing changes; a guest that wants to
>>>>>>> transition to secure mode will notice that the feature is not available
>>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
>>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
>>>>>>> combination.
>>>>>>>
>>>>>>> Does that make sense?  
>>>>>>
>>>>>> It's a little unusual; I don't think we have any other cases where
>>>>>> --only-migratable changes the behaviour; I think it normally only stops
>>>>>> you doing something that would have made it unmigratable or causes
>>>>>> an operation that would make it unmigratable to fail.  
>>>>>
>>>>> I would like to NOT block this feature with --only-migrateable. A guest
>>>>> can startup unprotected (and then is is migrateable). the migration blocker
>>>>> is really a dynamic aspect during runtime.   
>>>>
>>>> But the point of --only-migratable is to turn things that would have
>>>> blocked migration into failures, so that a VM started with
>>>> --only-migratable is *always* migratable.  
>>>
>>> Hmmm, fair enough. How do we do this with host-model? The constructed model
>>> would contain unpack, but then it will fail to startup? Or do we silently 
>>> drop unpack in that case? Both variants do not feel completely right. 
>>
>> Failing if you explicitly specified unpacked feels right, but failing
>> if you just used the host model feels odd. Removing unpack also is a
>> bit odd, but I think the better option if we want to do anything about
>> it at all.
> 
> 'host-model' feels a bit special; but breaking the rule that
> only-migratable doesn't change behaviour is weird
> Can you do host,-unpack   to make that work explicitly?

I guess that should work. But it means that we need to add logic in libvirt
to disable unpack for host-passthru and host-model. Next problem is then,
that a future version might implement migration of such guests, which means
that libvirt must then stop fencing unpack.

> 
> But hang on; why is 'unpack' the name of a secure guest facility - is
> it really a feature for secure guest or something else?

unpack is the name of the function that unpacks and decrypts the encrypted image.
If if is there, then you can switch into the securable guest mode.

Daniel P. Berrangé Jan. 14, 2021, 12:20 p.m. UTC | #26

On Thu, Jan 14, 2021 at 12:50:12PM +0100, Christian Borntraeger wrote:
> 
> 
> On 14.01.21 12:45, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck ([e-mail]) wrote:
> >> On Thu, 14 Jan 2021 11:52:11 +0100
> >> Christian Borntraeger <[e-mail]> wrote:
> >>
> >>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> >>>> * Christian Borntraeger ([e-mail]) wrote:  
> >>>>>
> >>>>>
> >>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> >>>>>> * Cornelia Huck ([e-mail]) wrote:  
> >>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
> >>>>>>> Ram Pai <[e-mail]> wrote:
> >>>>>>>  
> >>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> >>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> >>>>>>>>> Ram Pai <[e-mail]> wrote:  
> >>>>>>>  
> >>>>>>>>>> The main difference between my proposal and the other proposal is...
> >>>>>>>>>>
> >>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
> >>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> >>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> >>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
> >>>>>>>>>>   will or will-not switch-to-secure.
> >>>>>>>>>>     
> >>>>>>>>>
> >>>>>>>>> You have a point there when you say that QEMU does not know in advance,
> >>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
> >>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >>>>>>>>> was to flip that property on demand when the conversion occurs. David
> >>>>>>>>> explained to me that this is not possible for ppc, and that having the
> >>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
> >>>>>>>>> specified is a strong indication, that the VM is intended to be used as
> >>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> >>>>>>>>> try to transition). That argument applies here as well.    
> >>>>>>>>
> >>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> >>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> >>>>>>>> Offcourse; this has to be done with a big fat warning stating
> >>>>>>>> "secure-guest-memory" feature is disabled on the machine.
> >>>>>>>> Doing so, will continue to support guest that do not try to transition.
> >>>>>>>> Guest that try to transition will fail and terminate themselves.  
> >>>>>>>
> >>>>>>> Just to recap the s390x situation:
> >>>>>>>
> >>>>>>> - We currently offer a cpu feature that indicates secure execution to
> >>>>>>>   be available to the guest if the host supports it.
> >>>>>>> - When we introduce the secure object, we still need to support
> >>>>>>>   previous configurations and continue to offer the cpu feature, even
> >>>>>>>   if the secure object is not specified.
> >>>>>>> - As migration is currently not supported for secured guests, we add a
> >>>>>>>   blocker once the guest actually transitions. That means that
> >>>>>>>   transition fails if --only-migratable was specified on the command
> >>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
> >>>>>>> - With the secure object, we will already fail starting QEMU if
> >>>>>>>   --only-migratable was specified.
> >>>>>>>
> >>>>>>> My suggestion is now that we don't even offer the cpu feature if
> >>>>>>> --only-migratable has been specified. For a guest that does not want to
> >>>>>>> transition to secure mode, nothing changes; a guest that wants to
> >>>>>>> transition to secure mode will notice that the feature is not available
> >>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> >>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
> >>>>>>> combination.
> >>>>>>>
> >>>>>>> Does that make sense?  
> >>>>>>
> >>>>>> It's a little unusual; I don't think we have any other cases where
> >>>>>> --only-migratable changes the behaviour; I think it normally only stops
> >>>>>> you doing something that would have made it unmigratable or causes
> >>>>>> an operation that would make it unmigratable to fail.  
> >>>>>
> >>>>> I would like to NOT block this feature with --only-migrateable. A guest
> >>>>> can startup unprotected (and then is is migrateable). the migration blocker
> >>>>> is really a dynamic aspect during runtime.   
> >>>>
> >>>> But the point of --only-migratable is to turn things that would have
> >>>> blocked migration into failures, so that a VM started with
> >>>> --only-migratable is *always* migratable.  
> >>>
> >>> Hmmm, fair enough. How do we do this with host-model? The constructed model
> >>> would contain unpack, but then it will fail to startup? Or do we silently 
> >>> drop unpack in that case? Both variants do not feel completely right. 
> >>
> >> Failing if you explicitly specified unpacked feels right, but failing
> >> if you just used the host model feels odd. Removing unpack also is a
> >> bit odd, but I think the better option if we want to do anything about
> >> it at all.
> > 
> > 'host-model' feels a bit special; but breaking the rule that
> > only-migratable doesn't change behaviour is weird
> > Can you do host,-unpack   to make that work explicitly?
> 
> I guess that should work. But it means that we need to add logic in libvirt
> to disable unpack for host-passthru and host-model. Next problem is then,
> that a future version might implement migration of such guests, which means
> that libvirt must then stop fencing unpack.

The "host-model" is supposed to always be migratable, so we should
fence the feature there.

host-passthrough is "undefined" whether it is migratable - it may or may
not work, no guarantees made by libvirt.

Ultimately I think the problem is that there ought to be an explicit
config to enable the feature for s390, as there is for SEV, and will
also presumably be needed for ppc. 

Regards,
Daniel

Cornelia Huck Jan. 14, 2021, 2:04 p.m. UTC | #27

On Thu, 14 Jan 2021 12:20:48 +0000
Daniel P. Berrangé <[e-mail]> wrote:

> On Thu, Jan 14, 2021 at 12:50:12PM +0100, Christian Borntraeger wrote:
> > 
> > 
> > On 14.01.21 12:45, Dr. David Alan Gilbert wrote:  
> > > * Cornelia Huck ([e-mail]) wrote:  
> > >> On Thu, 14 Jan 2021 11:52:11 +0100
> > >> Christian Borntraeger <[e-mail]> wrote:
> > >>  
> > >>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
> > >>>> * Christian Borntraeger ([e-mail]) wrote:    
> > >>>>>
> > >>>>>
> > >>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:    
> > >>>>>> * Cornelia Huck ([e-mail]) wrote:    
> > >>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
> > >>>>>>> Ram Pai <[e-mail]> wrote:
> > >>>>>>>    
> > >>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
> > >>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> > >>>>>>>>> Ram Pai <[e-mail]> wrote:    
> > >>>>>>>    
> > >>>>>>>>>> The main difference between my proposal and the other proposal is...
> > >>>>>>>>>>
> > >>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
> > >>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> > >>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> > >>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
> > >>>>>>>>>>   will or will-not switch-to-secure.
> > >>>>>>>>>>       
> > >>>>>>>>>
> > >>>>>>>>> You have a point there when you say that QEMU does not know in advance,
> > >>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
> > >>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > >>>>>>>>> was to flip that property on demand when the conversion occurs. David
> > >>>>>>>>> explained to me that this is not possible for ppc, and that having the
> > >>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
> > >>>>>>>>> specified is a strong indication, that the VM is intended to be used as
> > >>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> > >>>>>>>>> try to transition). That argument applies here as well.      
> > >>>>>>>>
> > >>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> > >>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > >>>>>>>> Offcourse; this has to be done with a big fat warning stating
> > >>>>>>>> "secure-guest-memory" feature is disabled on the machine.
> > >>>>>>>> Doing so, will continue to support guest that do not try to transition.
> > >>>>>>>> Guest that try to transition will fail and terminate themselves.    
> > >>>>>>>
> > >>>>>>> Just to recap the s390x situation:
> > >>>>>>>
> > >>>>>>> - We currently offer a cpu feature that indicates secure execution to
> > >>>>>>>   be available to the guest if the host supports it.
> > >>>>>>> - When we introduce the secure object, we still need to support
> > >>>>>>>   previous configurations and continue to offer the cpu feature, even
> > >>>>>>>   if the secure object is not specified.
> > >>>>>>> - As migration is currently not supported for secured guests, we add a
> > >>>>>>>   blocker once the guest actually transitions. That means that
> > >>>>>>>   transition fails if --only-migratable was specified on the command
> > >>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
> > >>>>>>> - With the secure object, we will already fail starting QEMU if
> > >>>>>>>   --only-migratable was specified.
> > >>>>>>>
> > >>>>>>> My suggestion is now that we don't even offer the cpu feature if
> > >>>>>>> --only-migratable has been specified. For a guest that does not want to
> > >>>>>>> transition to secure mode, nothing changes; a guest that wants to
> > >>>>>>> transition to secure mode will notice that the feature is not available
> > >>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> > >>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
> > >>>>>>> combination.
> > >>>>>>>
> > >>>>>>> Does that make sense?    
> > >>>>>>
> > >>>>>> It's a little unusual; I don't think we have any other cases where
> > >>>>>> --only-migratable changes the behaviour; I think it normally only stops
> > >>>>>> you doing something that would have made it unmigratable or causes
> > >>>>>> an operation that would make it unmigratable to fail.    
> > >>>>>
> > >>>>> I would like to NOT block this feature with --only-migrateable. A guest
> > >>>>> can startup unprotected (and then is is migrateable). the migration blocker
> > >>>>> is really a dynamic aspect during runtime.     
> > >>>>
> > >>>> But the point of --only-migratable is to turn things that would have
> > >>>> blocked migration into failures, so that a VM started with
> > >>>> --only-migratable is *always* migratable.    
> > >>>
> > >>> Hmmm, fair enough. How do we do this with host-model? The constructed model
> > >>> would contain unpack, but then it will fail to startup? Or do we silently 
> > >>> drop unpack in that case? Both variants do not feel completely right.   
> > >>
> > >> Failing if you explicitly specified unpacked feels right, but failing
> > >> if you just used the host model feels odd. Removing unpack also is a
> > >> bit odd, but I think the better option if we want to do anything about
> > >> it at all.  
> > > 
> > > 'host-model' feels a bit special; but breaking the rule that
> > > only-migratable doesn't change behaviour is weird
> > > Can you do host,-unpack   to make that work explicitly?  
> > 
> > I guess that should work. But it means that we need to add logic in libvirt
> > to disable unpack for host-passthru and host-model. Next problem is then,
> > that a future version might implement migration of such guests, which means
> > that libvirt must then stop fencing unpack.  
> 
> The "host-model" is supposed to always be migratable, so we should
> fence the feature there.
> 
> host-passthrough is "undefined" whether it is migratable - it may or may
> not work, no guarantees made by libvirt.
> 
> Ultimately I think the problem is that there ought to be an explicit
> config to enable the feature for s390, as there is for SEV, and will
> also presumably be needed for ppc. 

Yes, an explicit config is what we want; unfortunately, we have to deal
with existing setups as well...

The options I see are
- leave things for existing setups as they are now (i.e. might become
  unmigratable when the guest transitions), and make sure we're doing
  the right thing with the new object
- always make the unpack feature conflict with migration requirements;
  this is a guest-visible change

The first option might be less hairy, all considered?

Christian Borntraeger Jan. 14, 2021, 2:09 p.m. UTC | #28

On 14.01.21 15:04, Cornelia Huck wrote:
> On Thu, 14 Jan 2021 12:20:48 +0000
> Daniel P. Berrangé <[e-mail]> wrote:
> 
>> On Thu, Jan 14, 2021 at 12:50:12PM +0100, Christian Borntraeger wrote:
>>>
>>>
>>> On 14.01.21 12:45, Dr. David Alan Gilbert wrote:  
>>>> * Cornelia Huck ([e-mail]) wrote:  
>>>>> On Thu, 14 Jan 2021 11:52:11 +0100
>>>>> Christian Borntraeger <[e-mail]> wrote:
>>>>>  
>>>>>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
>>>>>>> * Christian Borntraeger ([e-mail]) wrote:    
>>>>>>>>
>>>>>>>>
>>>>>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:    
>>>>>>>>> * Cornelia Huck ([e-mail]) wrote:    
>>>>>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>>>>>>>>> Ram Pai <[e-mail]> wrote:
>>>>>>>>>>    
>>>>>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
>>>>>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>>>>>>>>>> Ram Pai <[e-mail]> wrote:    
>>>>>>>>>>    
>>>>>>>>>>>>> The main difference between my proposal and the other proposal is...
>>>>>>>>>>>>>
>>>>>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>>>>>>>>>   will or will-not switch-to-secure.
>>>>>>>>>>>>>       
>>>>>>>>>>>>
>>>>>>>>>>>> You have a point there when you say that QEMU does not know in advance,
>>>>>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>>>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>>>>>>>>>> was to flip that property on demand when the conversion occurs. David
>>>>>>>>>>>> explained to me that this is not possible for ppc, and that having the
>>>>>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
>>>>>>>>>>>> specified is a strong indication, that the VM is intended to be used as
>>>>>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>>>>>>>>>> try to transition). That argument applies here as well.      
>>>>>>>>>>>
>>>>>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>>>>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>>>>>>>>>> Offcourse; this has to be done with a big fat warning stating
>>>>>>>>>>> "secure-guest-memory" feature is disabled on the machine.
>>>>>>>>>>> Doing so, will continue to support guest that do not try to transition.
>>>>>>>>>>> Guest that try to transition will fail and terminate themselves.    
>>>>>>>>>>
>>>>>>>>>> Just to recap the s390x situation:
>>>>>>>>>>
>>>>>>>>>> - We currently offer a cpu feature that indicates secure execution to
>>>>>>>>>>   be available to the guest if the host supports it.
>>>>>>>>>> - When we introduce the secure object, we still need to support
>>>>>>>>>>   previous configurations and continue to offer the cpu feature, even
>>>>>>>>>>   if the secure object is not specified.
>>>>>>>>>> - As migration is currently not supported for secured guests, we add a
>>>>>>>>>>   blocker once the guest actually transitions. That means that
>>>>>>>>>>   transition fails if --only-migratable was specified on the command
>>>>>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
>>>>>>>>>> - With the secure object, we will already fail starting QEMU if
>>>>>>>>>>   --only-migratable was specified.
>>>>>>>>>>
>>>>>>>>>> My suggestion is now that we don't even offer the cpu feature if
>>>>>>>>>> --only-migratable has been specified. For a guest that does not want to
>>>>>>>>>> transition to secure mode, nothing changes; a guest that wants to
>>>>>>>>>> transition to secure mode will notice that the feature is not available
>>>>>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
>>>>>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
>>>>>>>>>> combination.
>>>>>>>>>>
>>>>>>>>>> Does that make sense?    
>>>>>>>>>
>>>>>>>>> It's a little unusual; I don't think we have any other cases where
>>>>>>>>> --only-migratable changes the behaviour; I think it normally only stops
>>>>>>>>> you doing something that would have made it unmigratable or causes
>>>>>>>>> an operation that would make it unmigratable to fail.    
>>>>>>>>
>>>>>>>> I would like to NOT block this feature with --only-migrateable. A guest
>>>>>>>> can startup unprotected (and then is is migrateable). the migration blocker
>>>>>>>> is really a dynamic aspect during runtime.     
>>>>>>>
>>>>>>> But the point of --only-migratable is to turn things that would have
>>>>>>> blocked migration into failures, so that a VM started with
>>>>>>> --only-migratable is *always* migratable.    
>>>>>>
>>>>>> Hmmm, fair enough. How do we do this with host-model? The constructed model
>>>>>> would contain unpack, but then it will fail to startup? Or do we silently 
>>>>>> drop unpack in that case? Both variants do not feel completely right.   
>>>>>
>>>>> Failing if you explicitly specified unpacked feels right, but failing
>>>>> if you just used the host model feels odd. Removing unpack also is a
>>>>> bit odd, but I think the better option if we want to do anything about
>>>>> it at all.  
>>>>
>>>> 'host-model' feels a bit special; but breaking the rule that
>>>> only-migratable doesn't change behaviour is weird
>>>> Can you do host,-unpack   to make that work explicitly?  
>>>
>>> I guess that should work. But it means that we need to add logic in libvirt
>>> to disable unpack for host-passthru and host-model. Next problem is then,
>>> that a future version might implement migration of such guests, which means
>>> that libvirt must then stop fencing unpack.  
>>
>> The "host-model" is supposed to always be migratable, so we should
>> fence the feature there.
>>
>> host-passthrough is "undefined" whether it is migratable - it may or may
>> not work, no guarantees made by libvirt.
>>
>> Ultimately I think the problem is that there ought to be an explicit
>> config to enable the feature for s390, as there is for SEV, and will
>> also presumably be needed for ppc. 
> 
> Yes, an explicit config is what we want; unfortunately, we have to deal
> with existing setups as well...
> 
> The options I see are
> - leave things for existing setups as they are now (i.e. might become
>   unmigratable when the guest transitions), and make sure we're doing
>   the right thing with the new object
> - always make the unpack feature conflict with migration requirements;
>   this is a guest-visible change
> 
> The first option might be less hairy, all considered?

What about a libvirt change that removes the unpack from the host-model as 
soon as  only-migrateable is used. When that is in place, QEMU can reject
the combination of only-migrateable + unpack.

Daniel P. Berrangé Jan. 14, 2021, 2:15 p.m. UTC | #29

On Thu, Jan 14, 2021 at 03:09:01PM +0100, Christian Borntraeger wrote:
> 
> 
> On 14.01.21 15:04, Cornelia Huck wrote:
> > On Thu, 14 Jan 2021 12:20:48 +0000
> > Daniel P. Berrangé <[e-mail]> wrote:
> > 
> >> On Thu, Jan 14, 2021 at 12:50:12PM +0100, Christian Borntraeger wrote:
> >>>
> >>>
> >>> On 14.01.21 12:45, Dr. David Alan Gilbert wrote:  
> >>>> * Cornelia Huck ([e-mail]) wrote:  
> >>>>> On Thu, 14 Jan 2021 11:52:11 +0100
> >>>>> Christian Borntraeger <[e-mail]> wrote:
> >>>>>  
> >>>>>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
> >>>>>>> * Christian Borntraeger ([e-mail]) wrote:    
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:    
> >>>>>>>>> * Cornelia Huck ([e-mail]) wrote:    
> >>>>>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
> >>>>>>>>>> Ram Pai <[e-mail]> wrote:
> >>>>>>>>>>    
> >>>>>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
> >>>>>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> >>>>>>>>>>>> Ram Pai <[e-mail]> wrote:    
> >>>>>>>>>>    
> >>>>>>>>>>>>> The main difference between my proposal and the other proposal is...
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
> >>>>>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> >>>>>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> >>>>>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
> >>>>>>>>>>>>>   will or will-not switch-to-secure.
> >>>>>>>>>>>>>       
> >>>>>>>>>>>>
> >>>>>>>>>>>> You have a point there when you say that QEMU does not know in advance,
> >>>>>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
> >>>>>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >>>>>>>>>>>> was to flip that property on demand when the conversion occurs. David
> >>>>>>>>>>>> explained to me that this is not possible for ppc, and that having the
> >>>>>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
> >>>>>>>>>>>> specified is a strong indication, that the VM is intended to be used as
> >>>>>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> >>>>>>>>>>>> try to transition). That argument applies here as well.      
> >>>>>>>>>>>
> >>>>>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> >>>>>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> >>>>>>>>>>> Offcourse; this has to be done with a big fat warning stating
> >>>>>>>>>>> "secure-guest-memory" feature is disabled on the machine.
> >>>>>>>>>>> Doing so, will continue to support guest that do not try to transition.
> >>>>>>>>>>> Guest that try to transition will fail and terminate themselves.    
> >>>>>>>>>>
> >>>>>>>>>> Just to recap the s390x situation:
> >>>>>>>>>>
> >>>>>>>>>> - We currently offer a cpu feature that indicates secure execution to
> >>>>>>>>>>   be available to the guest if the host supports it.
> >>>>>>>>>> - When we introduce the secure object, we still need to support
> >>>>>>>>>>   previous configurations and continue to offer the cpu feature, even
> >>>>>>>>>>   if the secure object is not specified.
> >>>>>>>>>> - As migration is currently not supported for secured guests, we add a
> >>>>>>>>>>   blocker once the guest actually transitions. That means that
> >>>>>>>>>>   transition fails if --only-migratable was specified on the command
> >>>>>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
> >>>>>>>>>> - With the secure object, we will already fail starting QEMU if
> >>>>>>>>>>   --only-migratable was specified.
> >>>>>>>>>>
> >>>>>>>>>> My suggestion is now that we don't even offer the cpu feature if
> >>>>>>>>>> --only-migratable has been specified. For a guest that does not want to
> >>>>>>>>>> transition to secure mode, nothing changes; a guest that wants to
> >>>>>>>>>> transition to secure mode will notice that the feature is not available
> >>>>>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> >>>>>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
> >>>>>>>>>> combination.
> >>>>>>>>>>
> >>>>>>>>>> Does that make sense?    
> >>>>>>>>>
> >>>>>>>>> It's a little unusual; I don't think we have any other cases where
> >>>>>>>>> --only-migratable changes the behaviour; I think it normally only stops
> >>>>>>>>> you doing something that would have made it unmigratable or causes
> >>>>>>>>> an operation that would make it unmigratable to fail.    
> >>>>>>>>
> >>>>>>>> I would like to NOT block this feature with --only-migrateable. A guest
> >>>>>>>> can startup unprotected (and then is is migrateable). the migration blocker
> >>>>>>>> is really a dynamic aspect during runtime.     
> >>>>>>>
> >>>>>>> But the point of --only-migratable is to turn things that would have
> >>>>>>> blocked migration into failures, so that a VM started with
> >>>>>>> --only-migratable is *always* migratable.    
> >>>>>>
> >>>>>> Hmmm, fair enough. How do we do this with host-model? The constructed model
> >>>>>> would contain unpack, but then it will fail to startup? Or do we silently 
> >>>>>> drop unpack in that case? Both variants do not feel completely right.   
> >>>>>
> >>>>> Failing if you explicitly specified unpacked feels right, but failing
> >>>>> if you just used the host model feels odd. Removing unpack also is a
> >>>>> bit odd, but I think the better option if we want to do anything about
> >>>>> it at all.  
> >>>>
> >>>> 'host-model' feels a bit special; but breaking the rule that
> >>>> only-migratable doesn't change behaviour is weird
> >>>> Can you do host,-unpack   to make that work explicitly?  
> >>>
> >>> I guess that should work. But it means that we need to add logic in libvirt
> >>> to disable unpack for host-passthru and host-model. Next problem is then,
> >>> that a future version might implement migration of such guests, which means
> >>> that libvirt must then stop fencing unpack.  
> >>
> >> The "host-model" is supposed to always be migratable, so we should
> >> fence the feature there.
> >>
> >> host-passthrough is "undefined" whether it is migratable - it may or may
> >> not work, no guarantees made by libvirt.
> >>
> >> Ultimately I think the problem is that there ought to be an explicit
> >> config to enable the feature for s390, as there is for SEV, and will
> >> also presumably be needed for ppc. 
> > 
> > Yes, an explicit config is what we want; unfortunately, we have to deal
> > with existing setups as well...
> > 
> > The options I see are
> > - leave things for existing setups as they are now (i.e. might become
> >   unmigratable when the guest transitions), and make sure we're doing
> >   the right thing with the new object
> > - always make the unpack feature conflict with migration requirements;
> >   this is a guest-visible change
> > 
> > The first option might be less hairy, all considered?
> 
> What about a libvirt change that removes the unpack from the host-model as 
> soon as  only-migrateable is used. When that is in place, QEMU can reject
> the combination of only-migrateable + unpack.

I think libvirt needs to just unconditionally remove unpack from host-model
regardless, and require an explicit opt in. We can do that in libvirt
without compat problems, because we track the expansion of "host-model"
for existing running guests.

QEMU could introduce a deprecation warning right now, and then turn it into
an error after the deprecation cycle is complete.

Regards,
Daniel

Christian Borntraeger Jan. 14, 2021, 3:25 p.m. UTC | #30

On 14.01.21 15:15, Daniel P. Berrangé wrote:
> On Thu, Jan 14, 2021 at 03:09:01PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 14.01.21 15:04, Cornelia Huck wrote:
>>> On Thu, 14 Jan 2021 12:20:48 +0000
>>> Daniel P. Berrangé <[e-mail]> wrote:
>>>
>>>> On Thu, Jan 14, 2021 at 12:50:12PM +0100, Christian Borntraeger wrote:
>>>>>
>>>>>
>>>>> On 14.01.21 12:45, Dr. David Alan Gilbert wrote:  
>>>>>> * Cornelia Huck ([e-mail]) wrote:  
>>>>>>> On Thu, 14 Jan 2021 11:52:11 +0100
>>>>>>> Christian Borntraeger <[e-mail]> wrote:
>>>>>>>  
>>>>>>>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
>>>>>>>>> * Christian Borntraeger ([e-mail]) wrote:    
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:    
>>>>>>>>>>> * Cornelia Huck ([e-mail]) wrote:    
>>>>>>>>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>>>>>>>>>>> Ram Pai <[e-mail]> wrote:
>>>>>>>>>>>>    
>>>>>>>>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
>>>>>>>>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>>>>>>>>>>>> Ram Pai <[e-mail]> wrote:    
>>>>>>>>>>>>    
>>>>>>>>>>>>>>> The main difference between my proposal and the other proposal is...
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>>>>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>>>>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>>>>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>>>>>>>>>>>   will or will-not switch-to-secure.
>>>>>>>>>>>>>>>       
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> You have a point there when you say that QEMU does not know in advance,
>>>>>>>>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>>>>>>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>>>>>>>>>>>> was to flip that property on demand when the conversion occurs. David
>>>>>>>>>>>>>> explained to me that this is not possible for ppc, and that having the
>>>>>>>>>>>>>> "securable-guest-memory" property (or whatever the name will be)
>>>>>>>>>>>>>> specified is a strong indication, that the VM is intended to be used as
>>>>>>>>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>>>>>>>>>>>> try to transition). That argument applies here as well.      
>>>>>>>>>>>>>
>>>>>>>>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>>>>>>>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>>>>>>>>>>>> Offcourse; this has to be done with a big fat warning stating
>>>>>>>>>>>>> "secure-guest-memory" feature is disabled on the machine.
>>>>>>>>>>>>> Doing so, will continue to support guest that do not try to transition.
>>>>>>>>>>>>> Guest that try to transition will fail and terminate themselves.    
>>>>>>>>>>>>
>>>>>>>>>>>> Just to recap the s390x situation:
>>>>>>>>>>>>
>>>>>>>>>>>> - We currently offer a cpu feature that indicates secure execution to
>>>>>>>>>>>>   be available to the guest if the host supports it.
>>>>>>>>>>>> - When we introduce the secure object, we still need to support
>>>>>>>>>>>>   previous configurations and continue to offer the cpu feature, even
>>>>>>>>>>>>   if the secure object is not specified.
>>>>>>>>>>>> - As migration is currently not supported for secured guests, we add a
>>>>>>>>>>>>   blocker once the guest actually transitions. That means that
>>>>>>>>>>>>   transition fails if --only-migratable was specified on the command
>>>>>>>>>>>>   line. (Guests not transitioning will obviously not notice anything.)
>>>>>>>>>>>> - With the secure object, we will already fail starting QEMU if
>>>>>>>>>>>>   --only-migratable was specified.
>>>>>>>>>>>>
>>>>>>>>>>>> My suggestion is now that we don't even offer the cpu feature if
>>>>>>>>>>>> --only-migratable has been specified. For a guest that does not want to
>>>>>>>>>>>> transition to secure mode, nothing changes; a guest that wants to
>>>>>>>>>>>> transition to secure mode will notice that the feature is not available
>>>>>>>>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
>>>>>>>>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
>>>>>>>>>>>> combination.
>>>>>>>>>>>>
>>>>>>>>>>>> Does that make sense?    
>>>>>>>>>>>
>>>>>>>>>>> It's a little unusual; I don't think we have any other cases where
>>>>>>>>>>> --only-migratable changes the behaviour; I think it normally only stops
>>>>>>>>>>> you doing something that would have made it unmigratable or causes
>>>>>>>>>>> an operation that would make it unmigratable to fail.    
>>>>>>>>>>
>>>>>>>>>> I would like to NOT block this feature with --only-migrateable. A guest
>>>>>>>>>> can startup unprotected (and then is is migrateable). the migration blocker
>>>>>>>>>> is really a dynamic aspect during runtime.     
>>>>>>>>>
>>>>>>>>> But the point of --only-migratable is to turn things that would have
>>>>>>>>> blocked migration into failures, so that a VM started with
>>>>>>>>> --only-migratable is *always* migratable.    
>>>>>>>>
>>>>>>>> Hmmm, fair enough. How do we do this with host-model? The constructed model
>>>>>>>> would contain unpack, but then it will fail to startup? Or do we silently 
>>>>>>>> drop unpack in that case? Both variants do not feel completely right.   
>>>>>>>
>>>>>>> Failing if you explicitly specified unpacked feels right, but failing
>>>>>>> if you just used the host model feels odd. Removing unpack also is a
>>>>>>> bit odd, but I think the better option if we want to do anything about
>>>>>>> it at all.  
>>>>>>
>>>>>> 'host-model' feels a bit special; but breaking the rule that
>>>>>> only-migratable doesn't change behaviour is weird
>>>>>> Can you do host,-unpack   to make that work explicitly?  
>>>>>
>>>>> I guess that should work. But it means that we need to add logic in libvirt
>>>>> to disable unpack for host-passthru and host-model. Next problem is then,
>>>>> that a future version might implement migration of such guests, which means
>>>>> that libvirt must then stop fencing unpack.  
>>>>
>>>> The "host-model" is supposed to always be migratable, so we should
>>>> fence the feature there.
>>>>
>>>> host-passthrough is "undefined" whether it is migratable - it may or may
>>>> not work, no guarantees made by libvirt.
>>>>
>>>> Ultimately I think the problem is that there ought to be an explicit
>>>> config to enable the feature for s390, as there is for SEV, and will
>>>> also presumably be needed for ppc. 
>>>
>>> Yes, an explicit config is what we want; unfortunately, we have to deal
>>> with existing setups as well...
>>>
>>> The options I see are
>>> - leave things for existing setups as they are now (i.e. might become
>>>   unmigratable when the guest transitions), and make sure we're doing
>>>   the right thing with the new object
>>> - always make the unpack feature conflict with migration requirements;
>>>   this is a guest-visible change
>>>
>>> The first option might be less hairy, all considered?
>>
>> What about a libvirt change that removes the unpack from the host-model as 
>> soon as  only-migrateable is used. When that is in place, QEMU can reject
>> the combination of only-migrateable + unpack.
> 
> I think libvirt needs to just unconditionally remove unpack from host-model
> regardless, and require an explicit opt in. We can do that in libvirt
> without compat problems, because we track the expansion of "host-model"
> for existing running guests.

This is true for running guests, but not for shutdown and restart.

I would really like to avoid bad (and hard to debug) surprises that a guest boots
fine with libvirt version x and then fail with x+1. So at the beginning
I am fine with libvirt removing "unpack" from the default host model expansion
if the --only-migrateable parameter is used. Now I look into libvirt and I 
cannot actually find code that uses this parameter. Are there some patches
posted somewhere?
> 
> QEMU could introduce a deprecation warning right now, and then turn it into
> an error after the deprecation cycle is complete.

Daniel P. Berrangé Jan. 14, 2021, 3:33 p.m. UTC | #31

On Thu, Jan 14, 2021 at 04:25:21PM +0100, Christian Borntraeger wrote:
> On 14.01.21 15:15, Daniel P. Berrangé wrote:
> > On Thu, Jan 14, 2021 at 03:09:01PM +0100, Christian Borntraeger wrote:
> >>
> >>
> >> On 14.01.21 15:04, Cornelia Huck wrote:
> >>
> >> What about a libvirt change that removes the unpack from the host-model as 
> >> soon as  only-migrateable is used. When that is in place, QEMU can reject
> >> the combination of only-migrateable + unpack.
> > 
> > I think libvirt needs to just unconditionally remove unpack from host-model
> > regardless, and require an explicit opt in. We can do that in libvirt
> > without compat problems, because we track the expansion of "host-model"
> > for existing running guests.
> 
> This is true for running guests, but not for shutdown and restart.
> 
> I would really like to avoid bad (and hard to debug) surprises that a guest boots
> fine with libvirt version x and then fail with x+1. So at the beginning
> I am fine with libvirt removing "unpack" from the default host model expansion
> if the --only-migrateable parameter is used. Now I look into libvirt and I 
> cannot actually find code that uses this parameter. Are there some patches
> posted somewhere?

Sorryy, I should have been clearer that we don't currently use
--only-migrateable.  I've been talking from the pov of the effects
if we were to introduce it into libvirt.

The way it would work would be for  'virsh start FOO' to start the guest
unconditionally, while  'virsh start --migratable FOO' would start the
same guest config but fail if it used a non-migratable feature. We need
the guest ABI to be the same in both cases.

Regards,
Daniel

David Gibson Jan. 14, 2021, 11:51 p.m. UTC | #32

On Thu, Jan 14, 2021 at 11:25:17AM +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 13, 2021 at 12:42:26PM +0000, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck ([e-mail]) wrote:
> > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > Ram Pai <[e-mail]> wrote:
> > > 
> > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > Ram Pai <[e-mail]> wrote:
> > > 
> > > > > > The main difference between my proposal and the other proposal is...
> > > > > > 
> > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > >   will or will-not switch-to-secure.
> > > > > >   
> > > > > 
> > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > was to flip that property on demand when the conversion occurs. David
> > > > > explained to me that this is not possible for ppc, and that having the
> > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > try to transition). That argument applies here as well.  
> > > > 
> > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > Offcourse; this has to be done with a big fat warning stating
> > > > "secure-guest-memory" feature is disabled on the machine.
> > > > Doing so, will continue to support guest that do not try to transition.
> > > > Guest that try to transition will fail and terminate themselves.
> > > 
> > > Just to recap the s390x situation:
> > > 
> > > - We currently offer a cpu feature that indicates secure execution to
> > >   be available to the guest if the host supports it.
> > > - When we introduce the secure object, we still need to support
> > >   previous configurations and continue to offer the cpu feature, even
> > >   if the secure object is not specified.
> > > - As migration is currently not supported for secured guests, we add a
> > >   blocker once the guest actually transitions. That means that
> > >   transition fails if --only-migratable was specified on the command
> > >   line. (Guests not transitioning will obviously not notice anything.)
> > > - With the secure object, we will already fail starting QEMU if
> > >   --only-migratable was specified.
> > > 
> > > My suggestion is now that we don't even offer the cpu feature if
> > > --only-migratable has been specified. For a guest that does not want to
> > > transition to secure mode, nothing changes; a guest that wants to
> > > transition to secure mode will notice that the feature is not available
> > > and fail appropriately (or ultimately, when the ultravisor call fails).
> > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > combination.
> > > 
> > > Does that make sense?
> > 
> > It's a little unusual; I don't think we have any other cases where
> > --only-migratable changes the behaviour; I think it normally only stops
> > you doing something that would have made it unmigratable or causes
> > an operation that would make it unmigratable to fail.
> 
> I agree,  --only-migratable is supposed to be a *behavioural* toggle
> for QEMU. It must /not/ have any impact on the guest ABI.
> 
> A management application needs to be able to add/remove --only-migratable
> at will without changing the exposing guest ABI.

At the qemu level, it sounds like the right thing to do is to fail
outright if all of the below are true:
 1. --only-migratable is specified
 2. -cpu host is specified
 3. unpack isn't explicitly disabled
 4. the host CPU actually does have the unpack facility

That can be changed if & when migration support is added for PV.

Ram Pai Jan. 15, 2021, 6:24 p.m. UTC | #33

On Thu, Jan 14, 2021 at 10:36:43AM +0000, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger ([e-mail]) wrote:
> > 
> > 
> > On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > > * Cornelia Huck ([e-mail]) wrote:
> > >> On Tue, 5 Jan 2021 12:41:25 -0800
> > >> Ram Pai <[e-mail]> wrote:
> > >>
> > >>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > >>>> On Mon, 4 Jan 2021 10:40:26 -0800
> > >>>> Ram Pai <[e-mail]> wrote:
> > >>
> > >>>>> The main difference between my proposal and the other proposal is...
> > >>>>>
> > >>>>>   In my proposal the guest makes the compatibility decision and acts
> > >>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> > >>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> > >>>>>   compatibility decision, because it wont know in advance, if the guest
> > >>>>>   will or will-not switch-to-secure.
> > >>>>>   
> > >>>>
> > >>>> You have a point there when you say that QEMU does not know in advance,
> > >>>> if the guest will or will-not switch-to-secure. I made that argument
> > >>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > >>>> was to flip that property on demand when the conversion occurs. David
> > >>>> explained to me that this is not possible for ppc, and that having the
> > >>>> "securable-guest-memory" property (or whatever the name will be)
> > >>>> specified is a strong indication, that the VM is intended to be used as
> > >>>> a secure VM (thus it is OK to hurt the case where the guest does not
> > >>>> try to transition). That argument applies here as well.  
> > >>>
> > >>> As suggested by Cornelia Huck, what if QEMU disabled the
> > >>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > >>> Offcourse; this has to be done with a big fat warning stating
> > >>> "secure-guest-memory" feature is disabled on the machine.
> > >>> Doing so, will continue to support guest that do not try to transition.
> > >>> Guest that try to transition will fail and terminate themselves.
> > >>
> > >> Just to recap the s390x situation:
> > >>
> > >> - We currently offer a cpu feature that indicates secure execution to
> > >>   be available to the guest if the host supports it.
> > >> - When we introduce the secure object, we still need to support
> > >>   previous configurations and continue to offer the cpu feature, even
> > >>   if the secure object is not specified.
> > >> - As migration is currently not supported for secured guests, we add a
> > >>   blocker once the guest actually transitions. That means that
> > >>   transition fails if --only-migratable was specified on the command
> > >>   line. (Guests not transitioning will obviously not notice anything.)
> > >> - With the secure object, we will already fail starting QEMU if
> > >>   --only-migratable was specified.
> > >>
> > >> My suggestion is now that we don't even offer the cpu feature if
> > >> --only-migratable has been specified. For a guest that does not want to
> > >> transition to secure mode, nothing changes; a guest that wants to
> > >> transition to secure mode will notice that the feature is not available
> > >> and fail appropriately (or ultimately, when the ultravisor call fails).
> > >> We'd still fail starting QEMU for the secure object + --only-migratable
> > >> combination.
> > >>
> > >> Does that make sense?
> > > 
> > > It's a little unusual; I don't think we have any other cases where
> > > --only-migratable changes the behaviour; I think it normally only stops
> > > you doing something that would have made it unmigratable or causes
> > > an operation that would make it unmigratable to fail.
> > 
> > I would like to NOT block this feature with --only-migrateable. A guest
> > can startup unprotected (and then is is migrateable). the migration blocker
> > is really a dynamic aspect during runtime. 
> 
> But the point of --only-migratable is to turn things that would have
> blocked migration into failures, so that a VM started with
> --only-migratable is *always* migratable.

I believe, the proposed behavior, does follow the above rule. The
VM started with --only-migratable will always be migratable. Any
behavior; in the guest, to the contrary will disallow the behavior or
terminate the guest, but will never let the VM transition to a
non-migratable state.


RP

Ram Pai Jan. 15, 2021, 6:55 p.m. UTC | #34

On Wed, Jan 13, 2021 at 09:06:29AM +0100, Cornelia Huck wrote:
> On Tue, 12 Jan 2021 10:55:11 -0800
> Ram Pai <[e-mail]> wrote:
> 
> > On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:
> > > On Mon, 11 Jan 2021 11:58:30 -0800
> > > Ram Pai <[e-mail]> wrote:
> > >   
> > > > On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:  
> > > > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > > > Ram Pai <[e-mail]> wrote:
> > > > >     
> > > > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:    
> > > > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > > > Ram Pai <[e-mail]> wrote:    
> > > > >     
> > > > > > > > The main difference between my proposal and the other proposal is...
> > > > > > > > 
> > > > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > > > >   will or will-not switch-to-secure.
> > > > > > > >       
> > > > > > > 
> > > > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > > > was to flip that property on demand when the conversion occurs. David
> > > > > > > explained to me that this is not possible for ppc, and that having the
> > > > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > > > try to transition). That argument applies here as well.      
> > > > > > 
> > > > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > > > Offcourse; this has to be done with a big fat warning stating
> > > > > > "secure-guest-memory" feature is disabled on the machine.
> > > > > > Doing so, will continue to support guest that do not try to transition.
> > > > > > Guest that try to transition will fail and terminate themselves.    
> > > > > 
> > > > > Just to recap the s390x situation:
> > > > > 
> > > > > - We currently offer a cpu feature that indicates secure execution to
> > > > >   be available to the guest if the host supports it.
> > > > > - When we introduce the secure object, we still need to support
> > > > >   previous configurations and continue to offer the cpu feature, even
> > > > >   if the secure object is not specified.
> > > > > - As migration is currently not supported for secured guests, we add a
> > > > >   blocker once the guest actually transitions. That means that
> > > > >   transition fails if --only-migratable was specified on the command
> > > > >   line. (Guests not transitioning will obviously not notice anything.)
> > > > > - With the secure object, we will already fail starting QEMU if
> > > > >   --only-migratable was specified.
> > > > > 
> > > > > My suggestion is now that we don't even offer the cpu feature if
> > > > > --only-migratable has been specified. For a guest that does not want to
> > > > > transition to secure mode, nothing changes; a guest that wants to
> > > > > transition to secure mode will notice that the feature is not available
> > > > > and fail appropriately (or ultimately, when the ultravisor call fails).    
> > > > 
> > > > 
> > > > On POWER, secure-execution is not **automatically** enabled even when
> > > > the host supports it.  The feature is enabled only if the secure-object
> > > > is configured, and the host supports it.  
> > > 
> > > Yes, the cpu feature on s390x is simply pre-existing.
> > >   
> > > > 
> > > > However the behavior proposed above will be consistent on POWER and
> > > > on s390x,  when '--only-migratable' is specified and 'secure-object'
> > > > is NOT specified.
> > > > 
> > > > So I am in agreement till now. 
> > > > 
> > > >   
> > > > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > > > combination.    
> > > > 
> > > > Why fail? 
> > > > 
> > > > Instead, print a warning and  disable the secure-object; which will
> > > > disable your cpu-feature. Guests that do not transition to secure, will
> > > > continue to operate, and guests that transition to secure, will fail.  
> > > 
> > > But that would be consistent with how other non-migratable objects are
> > > handled, no? It's simply a case of incompatible options on the command
> > > line.  
> > 
> > Actually the two options are inherently NOT incompatible.  Halil also
> > mentioned this in one of his replies.
> > 
> > Its just that the current implementation is lacking, which will be fixed
> > in the near future. 
> > 
> > We can design it upfront, with the assumption that they both are compatible.
> > In the short term  disable one; preferrably the secure-object, if both 
> > options are specified. In the long term, remove the restriction, when
> > the implemetation is complete.
> 
> Can't we simply mark the object as non-migratable now, and then remove
> that later? I don't see what is so special about it.

This is fine too. 

However I am told that libvirt has some assumptions, where it assumes
that the VM is guaranteed to be migratable if '--only-migratable' is
specified. Silently turning off that option can be bad.

Dr. David Alan Gilbert Jan. 18, 2021, 5:39 p.m. UTC | #35

* David Gibson ([e-mail]) wrote:
> On Thu, Jan 14, 2021 at 11:25:17AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Jan 13, 2021 at 12:42:26PM +0000, Dr. David Alan Gilbert wrote:
> > > * Cornelia Huck ([e-mail]) wrote:
> > > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > > Ram Pai <[e-mail]> wrote:
> > > > 
> > > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> > > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > > Ram Pai <[e-mail]> wrote:
> > > > 
> > > > > > > The main difference between my proposal and the other proposal is...
> > > > > > > 
> > > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > > >   will or will-not switch-to-secure.
> > > > > > >   
> > > > > > 
> > > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > > was to flip that property on demand when the conversion occurs. David
> > > > > > explained to me that this is not possible for ppc, and that having the
> > > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > > try to transition). That argument applies here as well.  
> > > > > 
> > > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > > Offcourse; this has to be done with a big fat warning stating
> > > > > "secure-guest-memory" feature is disabled on the machine.
> > > > > Doing so, will continue to support guest that do not try to transition.
> > > > > Guest that try to transition will fail and terminate themselves.
> > > > 
> > > > Just to recap the s390x situation:
> > > > 
> > > > - We currently offer a cpu feature that indicates secure execution to
> > > >   be available to the guest if the host supports it.
> > > > - When we introduce the secure object, we still need to support
> > > >   previous configurations and continue to offer the cpu feature, even
> > > >   if the secure object is not specified.
> > > > - As migration is currently not supported for secured guests, we add a
> > > >   blocker once the guest actually transitions. That means that
> > > >   transition fails if --only-migratable was specified on the command
> > > >   line. (Guests not transitioning will obviously not notice anything.)
> > > > - With the secure object, we will already fail starting QEMU if
> > > >   --only-migratable was specified.
> > > > 
> > > > My suggestion is now that we don't even offer the cpu feature if
> > > > --only-migratable has been specified. For a guest that does not want to
> > > > transition to secure mode, nothing changes; a guest that wants to
> > > > transition to secure mode will notice that the feature is not available
> > > > and fail appropriately (or ultimately, when the ultravisor call fails).
> > > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > > combination.
> > > > 
> > > > Does that make sense?
> > > 
> > > It's a little unusual; I don't think we have any other cases where
> > > --only-migratable changes the behaviour; I think it normally only stops
> > > you doing something that would have made it unmigratable or causes
> > > an operation that would make it unmigratable to fail.
> > 
> > I agree,  --only-migratable is supposed to be a *behavioural* toggle
> > for QEMU. It must /not/ have any impact on the guest ABI.
> > 
> > A management application needs to be able to add/remove --only-migratable
> > at will without changing the exposing guest ABI.
> 
> At the qemu level, it sounds like the right thing to do is to fail
> outright if all of the below are true:
>  1. --only-migratable is specified
>  2. -cpu host is specified
>  3. unpack isn't explicitly disabled
>  4. the host CPU actually does have the unpack facility
> 
> That can be changed if & when migration support is added for PV.

That sounds right to me.

Dave

> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson

Cornelia Huck Jan. 19, 2021, 8:19 a.m. UTC | #36

On Fri, 15 Jan 2021 10:55:14 -0800
Ram Pai <[e-mail]> wrote:

> On Wed, Jan 13, 2021 at 09:06:29AM +0100, Cornelia Huck wrote:
> > On Tue, 12 Jan 2021 10:55:11 -0800
> > Ram Pai <[e-mail]> wrote:
> >   
> > > On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:  
> > > > On Mon, 11 Jan 2021 11:58:30 -0800
> > > > Ram Pai <[e-mail]> wrote:
> > > >     
> > > > > On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:    
> > > > > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > > > > Ram Pai <[e-mail]> wrote:
> > > > > >       
> > > > > > > On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:      
> > > > > > > > On Mon, 4 Jan 2021 10:40:26 -0800
> > > > > > > > Ram Pai <[e-mail]> wrote:      
> > > > > >       
> > > > > > > > > The main difference between my proposal and the other proposal is...
> > > > > > > > > 
> > > > > > > > >   In my proposal the guest makes the compatibility decision and acts
> > > > > > > > >   accordingly.  In the other proposal QEMU makes the compatibility
> > > > > > > > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > > > > > > > >   compatibility decision, because it wont know in advance, if the guest
> > > > > > > > >   will or will-not switch-to-secure.
> > > > > > > > >         
> > > > > > > > 
> > > > > > > > You have a point there when you say that QEMU does not know in advance,
> > > > > > > > if the guest will or will-not switch-to-secure. I made that argument
> > > > > > > > regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> > > > > > > > was to flip that property on demand when the conversion occurs. David
> > > > > > > > explained to me that this is not possible for ppc, and that having the
> > > > > > > > "securable-guest-memory" property (or whatever the name will be)
> > > > > > > > specified is a strong indication, that the VM is intended to be used as
> > > > > > > > a secure VM (thus it is OK to hurt the case where the guest does not
> > > > > > > > try to transition). That argument applies here as well.        
> > > > > > > 
> > > > > > > As suggested by Cornelia Huck, what if QEMU disabled the
> > > > > > > "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > > > > > > Offcourse; this has to be done with a big fat warning stating
> > > > > > > "secure-guest-memory" feature is disabled on the machine.
> > > > > > > Doing so, will continue to support guest that do not try to transition.
> > > > > > > Guest that try to transition will fail and terminate themselves.      
> > > > > > 
> > > > > > Just to recap the s390x situation:
> > > > > > 
> > > > > > - We currently offer a cpu feature that indicates secure execution to
> > > > > >   be available to the guest if the host supports it.
> > > > > > - When we introduce the secure object, we still need to support
> > > > > >   previous configurations and continue to offer the cpu feature, even
> > > > > >   if the secure object is not specified.
> > > > > > - As migration is currently not supported for secured guests, we add a
> > > > > >   blocker once the guest actually transitions. That means that
> > > > > >   transition fails if --only-migratable was specified on the command
> > > > > >   line. (Guests not transitioning will obviously not notice anything.)
> > > > > > - With the secure object, we will already fail starting QEMU if
> > > > > >   --only-migratable was specified.
> > > > > > 
> > > > > > My suggestion is now that we don't even offer the cpu feature if
> > > > > > --only-migratable has been specified. For a guest that does not want to
> > > > > > transition to secure mode, nothing changes; a guest that wants to
> > > > > > transition to secure mode will notice that the feature is not available
> > > > > > and fail appropriately (or ultimately, when the ultravisor call fails).      
> > > > > 
> > > > > 
> > > > > On POWER, secure-execution is not **automatically** enabled even when
> > > > > the host supports it.  The feature is enabled only if the secure-object
> > > > > is configured, and the host supports it.    
> > > > 
> > > > Yes, the cpu feature on s390x is simply pre-existing.
> > > >     
> > > > > 
> > > > > However the behavior proposed above will be consistent on POWER and
> > > > > on s390x,  when '--only-migratable' is specified and 'secure-object'
> > > > > is NOT specified.
> > > > > 
> > > > > So I am in agreement till now. 
> > > > > 
> > > > >     
> > > > > > We'd still fail starting QEMU for the secure object + --only-migratable
> > > > > > combination.      
> > > > > 
> > > > > Why fail? 
> > > > > 
> > > > > Instead, print a warning and  disable the secure-object; which will
> > > > > disable your cpu-feature. Guests that do not transition to secure, will
> > > > > continue to operate, and guests that transition to secure, will fail.    
> > > > 
> > > > But that would be consistent with how other non-migratable objects are
> > > > handled, no? It's simply a case of incompatible options on the command
> > > > line.    
> > > 
> > > Actually the two options are inherently NOT incompatible.  Halil also
> > > mentioned this in one of his replies.
> > > 
> > > Its just that the current implementation is lacking, which will be fixed
> > > in the near future. 
> > > 
> > > We can design it upfront, with the assumption that they both are compatible.
> > > In the short term  disable one; preferrably the secure-object, if both 
> > > options are specified. In the long term, remove the restriction, when
> > > the implemetation is complete.  
> > 
> > Can't we simply mark the object as non-migratable now, and then remove
> > that later? I don't see what is so special about it.  
> 
> This is fine too. 
> 
> However I am told that libvirt has some assumptions, where it assumes
> that the VM is guaranteed to be migratable if '--only-migratable' is
> specified. Silently turning off that option can be bad.
> 

I meant "later" as in "when support for live migration has been added".
Mucking around with the options does not sound like a good idea.

Christian Borntraeger Jan. 19, 2021, 8:28 a.m. UTC | #37

On 18.01.21 18:39, Dr. David Alan Gilbert wrote:
> * David Gibson ([e-mail]) wrote:
>> On Thu, Jan 14, 2021 at 11:25:17AM +0000, Daniel P. Berrangé wrote:
>>> On Wed, Jan 13, 2021 at 12:42:26PM +0000, Dr. David Alan Gilbert wrote:
>>>> * Cornelia Huck ([e-mail]) wrote:
>>>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>>>> Ram Pai <[e-mail]> wrote:
>>>>>
>>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
>>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
>>>>>>> Ram Pai <[e-mail]> wrote:
>>>>>
>>>>>>>> The main difference between my proposal and the other proposal is...
>>>>>>>>
>>>>>>>>   In my proposal the guest makes the compatibility decision and acts
>>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
>>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
>>>>>>>>   compatibility decision, because it wont know in advance, if the guest
>>>>>>>>   will or will-not switch-to-secure.
>>>>>>>>   
>>>>>>>
>>>>>>> You have a point there when you say that QEMU does not know in advance,
>>>>>>> if the guest will or will-not switch-to-secure. I made that argument
>>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
>>>>>>> was to flip that property on demand when the conversion occurs. David
>>>>>>> explained to me that this is not possible for ppc, and that having the
>>>>>>> "securable-guest-memory" property (or whatever the name will be)
>>>>>>> specified is a strong indication, that the VM is intended to be used as
>>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
>>>>>>> try to transition). That argument applies here as well.  
>>>>>>
>>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
>>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
>>>>>> Offcourse; this has to be done with a big fat warning stating
>>>>>> "secure-guest-memory" feature is disabled on the machine.
>>>>>> Doing so, will continue to support guest that do not try to transition.
>>>>>> Guest that try to transition will fail and terminate themselves.
>>>>>
>>>>> Just to recap the s390x situation:
>>>>>
>>>>> - We currently offer a cpu feature that indicates secure execution to
>>>>>   be available to the guest if the host supports it.
>>>>> - When we introduce the secure object, we still need to support
>>>>>   previous configurations and continue to offer the cpu feature, even
>>>>>   if the secure object is not specified.
>>>>> - As migration is currently not supported for secured guests, we add a
>>>>>   blocker once the guest actually transitions. That means that
>>>>>   transition fails if --only-migratable was specified on the command
>>>>>   line. (Guests not transitioning will obviously not notice anything.)
>>>>> - With the secure object, we will already fail starting QEMU if
>>>>>   --only-migratable was specified.
>>>>>
>>>>> My suggestion is now that we don't even offer the cpu feature if
>>>>> --only-migratable has been specified. For a guest that does not want to
>>>>> transition to secure mode, nothing changes; a guest that wants to
>>>>> transition to secure mode will notice that the feature is not available
>>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
>>>>> We'd still fail starting QEMU for the secure object + --only-migratable
>>>>> combination.
>>>>>
>>>>> Does that make sense?
>>>>
>>>> It's a little unusual; I don't think we have any other cases where
>>>> --only-migratable changes the behaviour; I think it normally only stops
>>>> you doing something that would have made it unmigratable or causes
>>>> an operation that would make it unmigratable to fail.
>>>
>>> I agree,  --only-migratable is supposed to be a *behavioural* toggle
>>> for QEMU. It must /not/ have any impact on the guest ABI.
>>>
>>> A management application needs to be able to add/remove --only-migratable
>>> at will without changing the exposing guest ABI.
>>
>> At the qemu level, it sounds like the right thing to do is to fail
>> outright if all of the below are true:
>>  1. --only-migratable is specified
>>  2. -cpu host is specified
>>  3. unpack isn't explicitly disabled
>>  4. the host CPU actually does have the unpack facility
>>
>> That can be changed if & when migration support is added for PV.
> 
> That sounds right to me.

as startup will fail anyway if the guest cpu model enables unpack, but the host
cpu does not support it this can be simplified to forbid startup in qemu if
--only-migratable is combined with unpack being active in the guest cpu model.

This is actually independent from this patch set.  maybe just
something like

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 35179f9dc7ba..3b85ff4e31b2 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -26,6 +26,7 @@
 #include "qapi/qmp/qdict.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
 #endif
 #include "qapi/qapi-commands-machine-target.h"
@@ -878,6 +879,11 @@ static void check_compatibility(const S390CPUModel *max_model,
         return;
     }
 
+    if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) {
+        error_setg(errp, "The unpack facility is not compatible with "
+                  "the --only-migratable option");
+    }
+
     /* detect the missing features to properly report them */
     bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX);
     if (bitmap_empty(missing, S390_FEAT_MAX)) {

Cornelia Huck Jan. 19, 2021, 8:34 a.m. UTC | #38

On Tue, 19 Jan 2021 09:28:22 +0100
Christian Borntraeger <[e-mail]> wrote:

> On 18.01.21 18:39, Dr. David Alan Gilbert wrote:
> > * David Gibson ([e-mail]) wrote:  
> >> On Thu, Jan 14, 2021 at 11:25:17AM +0000, Daniel P. Berrangé wrote:  
> >>> On Wed, Jan 13, 2021 at 12:42:26PM +0000, Dr. David Alan Gilbert wrote:  
> >>>> * Cornelia Huck ([e-mail]) wrote:  
> >>>>> On Tue, 5 Jan 2021 12:41:25 -0800
> >>>>> Ram Pai <[e-mail]> wrote:
> >>>>>  
> >>>>>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:  
> >>>>>>> On Mon, 4 Jan 2021 10:40:26 -0800
> >>>>>>> Ram Pai <[e-mail]> wrote:  
> >>>>>  
> >>>>>>>> The main difference between my proposal and the other proposal is...
> >>>>>>>>
> >>>>>>>>   In my proposal the guest makes the compatibility decision and acts
> >>>>>>>>   accordingly.  In the other proposal QEMU makes the compatibility
> >>>>>>>>   decision and acts accordingly. I argue that QEMU cannot make a good
> >>>>>>>>   compatibility decision, because it wont know in advance, if the guest
> >>>>>>>>   will or will-not switch-to-secure.
> >>>>>>>>     
> >>>>>>>
> >>>>>>> You have a point there when you say that QEMU does not know in advance,
> >>>>>>> if the guest will or will-not switch-to-secure. I made that argument
> >>>>>>> regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >>>>>>> was to flip that property on demand when the conversion occurs. David
> >>>>>>> explained to me that this is not possible for ppc, and that having the
> >>>>>>> "securable-guest-memory" property (or whatever the name will be)
> >>>>>>> specified is a strong indication, that the VM is intended to be used as
> >>>>>>> a secure VM (thus it is OK to hurt the case where the guest does not
> >>>>>>> try to transition). That argument applies here as well.    
> >>>>>>
> >>>>>> As suggested by Cornelia Huck, what if QEMU disabled the
> >>>>>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> >>>>>> Offcourse; this has to be done with a big fat warning stating
> >>>>>> "secure-guest-memory" feature is disabled on the machine.
> >>>>>> Doing so, will continue to support guest that do not try to transition.
> >>>>>> Guest that try to transition will fail and terminate themselves.  
> >>>>>
> >>>>> Just to recap the s390x situation:
> >>>>>
> >>>>> - We currently offer a cpu feature that indicates secure execution to
> >>>>>   be available to the guest if the host supports it.
> >>>>> - When we introduce the secure object, we still need to support
> >>>>>   previous configurations and continue to offer the cpu feature, even
> >>>>>   if the secure object is not specified.
> >>>>> - As migration is currently not supported for secured guests, we add a
> >>>>>   blocker once the guest actually transitions. That means that
> >>>>>   transition fails if --only-migratable was specified on the command
> >>>>>   line. (Guests not transitioning will obviously not notice anything.)
> >>>>> - With the secure object, we will already fail starting QEMU if
> >>>>>   --only-migratable was specified.
> >>>>>
> >>>>> My suggestion is now that we don't even offer the cpu feature if
> >>>>> --only-migratable has been specified. For a guest that does not want to
> >>>>> transition to secure mode, nothing changes; a guest that wants to
> >>>>> transition to secure mode will notice that the feature is not available
> >>>>> and fail appropriately (or ultimately, when the ultravisor call fails).
> >>>>> We'd still fail starting QEMU for the secure object + --only-migratable
> >>>>> combination.
> >>>>>
> >>>>> Does that make sense?  
> >>>>
> >>>> It's a little unusual; I don't think we have any other cases where
> >>>> --only-migratable changes the behaviour; I think it normally only stops
> >>>> you doing something that would have made it unmigratable or causes
> >>>> an operation that would make it unmigratable to fail.  
> >>>
> >>> I agree,  --only-migratable is supposed to be a *behavioural* toggle
> >>> for QEMU. It must /not/ have any impact on the guest ABI.
> >>>
> >>> A management application needs to be able to add/remove --only-migratable
> >>> at will without changing the exposing guest ABI.  
> >>
> >> At the qemu level, it sounds like the right thing to do is to fail
> >> outright if all of the below are true:
> >>  1. --only-migratable is specified
> >>  2. -cpu host is specified
> >>  3. unpack isn't explicitly disabled
> >>  4. the host CPU actually does have the unpack facility
> >>
> >> That can be changed if & when migration support is added for PV.  
> > 
> > That sounds right to me.  
> 
> as startup will fail anyway if the guest cpu model enables unpack, but the host
> cpu does not support it this can be simplified to forbid startup in qemu if
> --only-migratable is combined with unpack being active in the guest cpu model.
> 
> This is actually independent from this patch set.

Yep, I think we should just go ahead and fix this.

>  maybe just
> something like
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 35179f9dc7ba..3b85ff4e31b2 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -26,6 +26,7 @@
>  #include "qapi/qmp/qdict.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/arch_init.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/pci/pci.h"
>  #endif
>  #include "qapi/qapi-commands-machine-target.h"
> @@ -878,6 +879,11 @@ static void check_compatibility(const S390CPUModel *max_model,
>          return;
>      }
>  
> +    if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) {
> +        error_setg(errp, "The unpack facility is not compatible with "
> +                  "the --only-migratable option");
> +    }
> +
>      /* detect the missing features to properly report them */
>      bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX);
>      if (bitmap_empty(missing, S390_FEAT_MAX)) {
> 
> 

Want to send this as a proper patch?

Daniel P. Berrangé Jan. 19, 2021, 9:59 a.m. UTC | #39

On Fri, Jan 15, 2021 at 10:55:14AM -0800, Ram Pai wrote:
> On Wed, Jan 13, 2021 at 09:06:29AM +0100, Cornelia Huck wrote:
> > On Tue, 12 Jan 2021 10:55:11 -0800
> > Ram Pai <[e-mail]> wrote:
> > 
> > > On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:
> > > Actually the two options are inherently NOT incompatible.  Halil also
> > > mentioned this in one of his replies.
> > > 
> > > Its just that the current implementation is lacking, which will be fixed
> > > in the near future. 
> > > 
> > > We can design it upfront, with the assumption that they both are compatible.
> > > In the short term  disable one; preferrably the secure-object, if both 
> > > options are specified. In the long term, remove the restriction, when
> > > the implemetation is complete.
> > 
> > Can't we simply mark the object as non-migratable now, and then remove
> > that later? I don't see what is so special about it.
> 
> This is fine too. 
> 
> However I am told that libvirt has some assumptions, where it assumes
> that the VM is guaranteed to be migratable if '--only-migratable' is
> specified. Silently turning off that option can be bad.

TO be clear libvirt does *not* currently use --only-migratable.

What you're describing here is QEMU's own definition of this flag

 $ qemu-system-x86_64 | grep migratable
 -only-migratable     allow only migratable devices


Regards,
Daniel

Patch

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)
+
+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);
+
     return 0;
 }