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

2021-01-19 Thread Daniel P . Berrangé
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  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
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




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

2021-01-19 Thread Cornelia Huck
On Tue, 19 Jan 2021 09:28:22 +0100
Christian Borntraeger  wrote:

> On 18.01.21 18:39, Dr. David Alan Gilbert wrote:
> > * David Gibson (da...@gibson.dropbear.id.au) wrote:  
> >> On Thu, Jan 14, 2021 at 11:25:17AM +, Daniel P. Berrangé wrote:  
> >>> On Wed, Jan 13, 2021 at 12:42:26PM +, Dr. David Alan Gilbert wrote:  
>  * Cornelia Huck (coh...@redhat.com) wrote:  
> > On Tue, 5 Jan 2021 12:41:25 -0800
> > Ram Pai  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  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.

>  

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

2021-01-19 Thread Christian Borntraeger



On 18.01.21 18:39, Dr. David Alan Gilbert wrote:
> * David Gibson (da...@gibson.dropbear.id.au) wrote:
>> On Thu, Jan 14, 2021 at 11:25:17AM +, Daniel P. Berrangé wrote:
>>> On Wed, Jan 13, 2021 at 12:42:26PM +, Dr. David Alan Gilbert wrote:
 * Cornelia Huck (coh...@redhat.com) wrote:
> On Tue, 5 Jan 2021 12:41:25 -0800
> Ram Pai  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  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
 

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

2021-01-19 Thread Cornelia Huck
On Fri, 15 Jan 2021 10:55:14 -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  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  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  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  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 

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

2021-01-18 Thread Dr. David Alan Gilbert
* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Thu, Jan 14, 2021 at 11:25:17AM +, Daniel P. Berrangé wrote:
> > On Wed, Jan 13, 2021 at 12:42:26PM +, Dr. David Alan Gilbert wrote:
> > > * Cornelia Huck (coh...@redhat.com) wrote:
> > > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > > Ram Pai  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  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


-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




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

2021-01-15 Thread Ram Pai
On Wed, Jan 13, 2021 at 09:06:29AM +0100, Cornelia Huck wrote:
> On Tue, 12 Jan 2021 10:55:11 -0800
> Ram Pai  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  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  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  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.  
> > 
> > 

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

2021-01-15 Thread Ram Pai
On Thu, Jan 14, 2021 at 10:36:43AM +, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> > 
> > 
> > On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > > * Cornelia Huck (coh...@redhat.com) wrote:
> > >> On Tue, 5 Jan 2021 12:41:25 -0800
> > >> Ram Pai  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  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



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

2021-01-14 Thread David Gibson
On Thu, Jan 14, 2021 at 11:25:17AM +, Daniel P. Berrangé wrote:
> On Wed, Jan 13, 2021 at 12:42:26PM +, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck (coh...@redhat.com) wrote:
> > > On Tue, 5 Jan 2021 12:41:25 -0800
> > > Ram Pai  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  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.

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


signature.asc
Description: PGP signature


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

2021-01-14 Thread Daniel P . Berrangé
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
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




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

2021-01-14 Thread Christian Borntraeger
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 +
>>> Daniel P. Berrangé  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 (coh...@redhat.com) wrote:  
>>> On Thu, 14 Jan 2021 11:52:11 +0100
>>> Christian Borntraeger  wrote:
>>>  
 On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
>>
>>
>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
>>> * Cornelia Huck (coh...@redhat.com) wrote:
 On Tue, 5 Jan 2021 12:41:25 -0800
 Ram Pai  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  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 

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

2021-01-14 Thread Daniel P . Berrangé
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 +
> > Daniel P. Berrangé  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 (coh...@redhat.com) wrote:  
> > On Thu, 14 Jan 2021 11:52:11 +0100
> > Christian Borntraeger  wrote:
> >  
> >> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
> >>> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> 
> 
>  On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck (coh...@redhat.com) wrote:
> >> On Tue, 5 Jan 2021 12:41:25 -0800
> >> Ram Pai  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  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.
> 
> 

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

2021-01-14 Thread Christian Borntraeger



On 14.01.21 15:04, Cornelia Huck wrote:
> On Thu, 14 Jan 2021 12:20:48 +
> Daniel P. Berrangé  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 (coh...@redhat.com) wrote:  
> On Thu, 14 Jan 2021 11:52:11 +0100
> Christian Borntraeger  wrote:
>  
>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
>>> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:


 On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> * Cornelia Huck (coh...@redhat.com) wrote:
>> On Tue, 5 Jan 2021 12:41:25 -0800
>> Ram Pai  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  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 

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

2021-01-14 Thread Cornelia Huck
On Thu, 14 Jan 2021 12:20:48 +
Daniel P. Berrangé  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 (coh...@redhat.com) wrote:  
> > >> On Thu, 14 Jan 2021 11:52:11 +0100
> > >> Christian Borntraeger  wrote:
> > >>  
> > >>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:  
> >  * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> > >
> > >
> > > On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > >> * Cornelia Huck (coh...@redhat.com) wrote:
> > >>> On Tue, 5 Jan 2021 12:41:25 -0800
> > >>> Ram Pai  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  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. 
> 

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

2021-01-14 Thread Daniel P . Berrangé
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 (coh...@redhat.com) wrote:
> >> On Thu, 14 Jan 2021 11:52:11 +0100
> >> Christian Borntraeger  wrote:
> >>
> >>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
>  * Christian Borntraeger (borntrae...@de.ibm.com) wrote:  
> >
> >
> > On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> >> * Cornelia Huck (coh...@redhat.com) wrote:  
> >>> On Tue, 5 Jan 2021 12:41:25 -0800
> >>> Ram Pai  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  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 

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

2021-01-14 Thread Christian Borntraeger



On 14.01.21 12:45, Dr. David Alan Gilbert wrote:
> * Cornelia Huck (coh...@redhat.com) wrote:
>> On Thu, 14 Jan 2021 11:52:11 +0100
>> Christian Borntraeger  wrote:
>>
>>> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
 * Christian Borntraeger (borntrae...@de.ibm.com) wrote:  
>
>
> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
>> * Cornelia Huck (coh...@redhat.com) wrote:  
>>> On Tue, 5 Jan 2021 12:41:25 -0800
>>> Ram Pai  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  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 

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

2021-01-14 Thread Dr. David Alan Gilbert
* Cornelia Huck (coh...@redhat.com) wrote:
> On Thu, 14 Jan 2021 11:52:11 +0100
> Christian Borntraeger  wrote:
> 
> > On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> > > * Christian Borntraeger (borntrae...@de.ibm.com) wrote:  
> > >>
> > >>
> > >> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> > >>> * Cornelia Huck (coh...@redhat.com) wrote:  
> >  On Tue, 5 Jan 2021 12:41:25 -0800
> >  Ram Pai  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  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

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

2021-01-14 Thread Daniel P . Berrangé
On Wed, Jan 13, 2021 at 12:42:26PM +, Dr. David Alan Gilbert wrote:
> * Cornelia Huck (coh...@redhat.com) wrote:
> > On Tue, 5 Jan 2021 12:41:25 -0800
> > Ram Pai  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  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
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




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

2021-01-14 Thread Daniel P . Berrangé
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  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  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
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




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

2021-01-14 Thread Cornelia Huck
On Thu, 14 Jan 2021 11:52:11 +0100
Christian Borntraeger  wrote:

> On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> > * Christian Borntraeger (borntrae...@de.ibm.com) wrote:  
> >>
> >>
> >> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:  
> >>> * Cornelia Huck (coh...@redhat.com) wrote:  
>  On Tue, 5 Jan 2021 12:41:25 -0800
>  Ram Pai  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  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.




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

2021-01-14 Thread Christian Borntraeger



On 14.01.21 11:36, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
>>
>>
>> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
>>> * Cornelia Huck (coh...@redhat.com) wrote:
 On Tue, 5 Jan 2021 12:41:25 -0800
 Ram Pai  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  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. 



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

2021-01-14 Thread Dr. David Alan Gilbert
* Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> 
> 
> On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > * Cornelia Huck (coh...@redhat.com) wrote:
> >> On Tue, 5 Jan 2021 12:41:25 -0800
> >> Ram Pai  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  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

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




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

2021-01-14 Thread Christian Borntraeger



On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> * Cornelia Huck (coh...@redhat.com) wrote:
>> On Tue, 5 Jan 2021 12:41:25 -0800
>> Ram Pai  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  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. 



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

2021-01-13 Thread Dr. David Alan Gilbert
* Cornelia Huck (coh...@redhat.com) wrote:
> On Tue, 5 Jan 2021 12:41:25 -0800
> Ram Pai  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  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

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




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

2021-01-13 Thread Cornelia Huck
On Tue, 12 Jan 2021 10:55:11 -0800
Ram Pai  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  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  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  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 

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

2021-01-12 Thread Ram Pai
On Tue, Jan 12, 2021 at 09:19:43AM +0100, Cornelia Huck wrote:
> On Mon, 11 Jan 2021 11:58:30 -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  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  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.


-- 
Ram Pai



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

2021-01-12 Thread Cornelia Huck
On Mon, 11 Jan 2021 11:58:30 -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  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  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.




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

2021-01-11 Thread Ram Pai
On Mon, Jan 11, 2021 at 05:59:14PM +0100, Cornelia Huck wrote:
> On Tue, 5 Jan 2021 12:41:25 -0800
> Ram Pai  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  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



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

2021-01-11 Thread Cornelia Huck
On Tue, 5 Jan 2021 12:41:25 -0800
Ram Pai  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  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?




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

2021-01-05 Thread Ram Pai
On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> On Mon, 4 Jan 2021 10:40:26 -0800
> Ram Pai  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  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(_mig_blocker, "PEF: Migration is not 
> > > > > > > > > > implemented");
> > > > > > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > > > > > +migrate_add_blocker(pef_mig_blocker, _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 

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

2021-01-05 Thread Halil Pasic
On Mon, 4 Jan 2021 10:40:26 -0800
Ram Pai  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  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(_mig_blocker, "PEF: Migration is not 
> > > > > > > > > implemented");
> > > > > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > > > > +migrate_add_blocker(pef_mig_blocker, _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 

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

2021-01-04 Thread Ram Pai
On Mon, Jan 04, 2021 at 01:46:29PM +0100, Halil Pasic wrote:
> On Sun, 3 Jan 2021 23:15:50 -0800
> Ram Pai  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(_mig_blocker, "PEF: Migration is not 
> > > > > > > > implemented");
> > > > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > > > +migrate_add_blocker(pef_mig_blocker, _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 

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

2021-01-04 Thread Halil Pasic
On Sun, 3 Jan 2021 23:15:50 -0800
Ram Pai  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(_mig_blocker, "PEF: Migration is not 
> > > > > > > implemented");
> > > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > > +migrate_add_blocker(pef_mig_blocker, _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

[..]



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

2021-01-03 Thread Ram Pai
On Fri, Dec 18, 2020 at 12:41:11PM +0100, Cornelia Huck wrote:
> On Thu, 17 Dec 2020 15:15:30 +0100
> Greg Kurz  wrote:
> 
> > On Thu, 17 Dec 2020 12:38:42 +0100
> > Cornelia Huck  wrote:
> > 
> > > On Thu, 17 Dec 2020 16:47:36 +1100
> > > David Gibson  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  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 
> > > > > > Reviewed-by: Dr. David Alan Gilbert 
> > > > > > ---
> > > > > >  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(_mig_blocker, "PEF: Migration is not 
> > > > > > implemented");
> > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > +migrate_add_blocker(pef_mig_blocker, _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



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

2020-12-18 Thread Dr. David Alan Gilbert
* Cornelia Huck (coh...@redhat.com) wrote:
> On Thu, 17 Dec 2020 15:15:30 +0100
> Greg Kurz  wrote:
> 
> > On Thu, 17 Dec 2020 12:38:42 +0100
> > Cornelia Huck  wrote:
> > 
> > > On Thu, 17 Dec 2020 16:47:36 +1100
> > > David Gibson  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  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 
> > > > > > Reviewed-by: Dr. David Alan Gilbert 
> > > > > > ---
> > > > > >  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(_mig_blocker, "PEF: Migration is not 
> > > > > > implemented");
> > > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > > +migrate_add_blocker(pef_mig_blocker, _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;
> > > > > >  }
> > > > > >  
> > > > > 
> > > >   
> > >   
> > 
> 


-- 
Dr. David Alan Gilbert / 

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

2020-12-18 Thread Cornelia Huck
On Thu, 17 Dec 2020 15:15:30 +0100
Greg Kurz  wrote:

> On Thu, 17 Dec 2020 12:38:42 +0100
> Cornelia Huck  wrote:
> 
> > On Thu, 17 Dec 2020 16:47:36 +1100
> > David Gibson  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  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 
> > > > > Reviewed-by: Dr. David Alan Gilbert 
> > > > > ---
> > > > >  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(_mig_blocker, "PEF: Migration is not 
> > > > > implemented");
> > > > > +/* NB: This can fail if --only-migratable is used */
> > > > > +migrate_add_blocker(pef_mig_blocker, _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;
> > > > >  }
> > > > >  
> > > > 
> > >   
> >   
> 



pgpU8GI4QXIkp.pgp
Description: OpenPGP digital signature


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

2020-12-17 Thread Greg Kurz
On Thu, 17 Dec 2020 12:38:42 +0100
Cornelia Huck  wrote:

> On Thu, 17 Dec 2020 16:47:36 +1100
> David Gibson  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  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 
> > > > Reviewed-by: Dr. David Alan Gilbert 
> > > > ---
> > > >  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(_mig_blocker, "PEF: Migration is not implemented");
> > > > +/* NB: This can fail if --only-migratable is used */
> > > > +migrate_add_blocker(pef_mig_blocker, _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;
> > > >  }
> > > >
> > >   
> > 
> 



pgpRAPfFg0ojp.pgp
Description: OpenPGP digital signature


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

2020-12-17 Thread Cornelia Huck
On Thu, 17 Dec 2020 16:47:36 +1100
David Gibson  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  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 
> > > Reviewed-by: Dr. David Alan Gilbert 
> > > ---
> > >  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(_mig_blocker, "PEF: Migration is not implemented");
> > > +/* NB: This can fail if --only-migratable is used */
> > > +migrate_add_blocker(pef_mig_blocker, _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;
> > >  }
> > >
> >   
> 



pgpnGL6xQ7sMu.pgp
Description: OpenPGP digital signature


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

2020-12-16 Thread David Gibson
On Mon, Dec 14, 2020 at 06:22:40PM +0100, Cornelia Huck wrote:
> On Fri,  4 Dec 2020 16:44:13 +1100
> David Gibson  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 
> > Reviewed-by: Dr. David Alan Gilbert 
> > ---
> >  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(_mig_blocker, "PEF: Migration is not implemented");
> > +/* NB: This can fail if --only-migratable is used */
> > +migrate_add_blocker(pef_mig_blocker, _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;
> >  }
> >  
> 

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


signature.asc
Description: PGP signature


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

2020-12-14 Thread Cornelia Huck
On Fri,  4 Dec 2020 16:44:13 +1100
David Gibson  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 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  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(_mig_blocker, "PEF: Migration is not implemented");
> +/* NB: This can fail if --only-migratable is used */
> +migrate_add_blocker(pef_mig_blocker, _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;
>  }
>