Re: [PATCH v3 0/9] Generalize memory encryption models

From: David Gibson <david@gibson.dropbear.id.au>
To: Cornelia Huck <cohuck@redhat.com>
Cc: David Hildenbrand <david@redhat.com>,
	qemu-devel@nongnu.org, brijesh.singh@amd.com, pair@us.ibm.com,
	pbonzini@redhat.com, dgilbert@redhat.com, frankja@linux.ibm.com,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	kvm@vger.kernel.org, qemu-ppc@nongnu.org, mst@redhat.com,
	mdroth@linux.vnet.ibm.com, Richard Henderson <rth@twiddle.net>,
	pasic@linux.ibm.com, Eduardo Habkost <ehabkost@redhat.com>,
	qemu-s390x@nongnu.org
Subject: Re: [PATCH v3 0/9] Generalize memory encryption models
Date: Thu, 25 Jun 2020 15:25:18 +1000	[thread overview]
Message-ID: <20200625052518.GD172395@umbus.fritz.box> (raw)
In-Reply-To: <20200622140254.0dbe5d8c.cohuck@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4312 bytes --]

On Mon, Jun 22, 2020 at 02:02:54PM +0200, Cornelia Huck wrote:
> On Fri, 19 Jun 2020 12:10:13 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
> > On 19.06.20 12:05, Cornelia Huck wrote:
> > > On Fri, 19 Jun 2020 11:56:49 +0200
> > > David Hildenbrand <david@redhat.com> wrote:
> > >   
> > >>>>> For now this series covers just AMD SEV and POWER PEF.  I'm hoping it
> > >>>>> can be extended to cover the Intel and s390 mechanisms as well,
> > >>>>> though.      
> > >>>>
> > >>>> The only approach on s390x to not glue command line properties to the
> > >>>> cpu model would be to remove the CPU model feature and replace it by the
> > >>>> command line parameter. But that would, of course, be an incompatible break.    
> > >>>
> > >>> Yuck.
> > >>>
> > >>> We still need to provide the cpu feature to the *guest* in any case, no?    
> > >>
> > >> Yeah, but that could be wired up internally. Wouldn't consider it clean,
> > >> though (I second the "overengineered" above).  
> > > 
> > > Could an internally wired-up cpu feature be introspected? Also, what  
> > 
> > Nope. It would just be e.g., a "machine feature" indicated to the guest
> > via the STFL interface/instruction. I was tackling the introspect part
> > when asking David how to sense from upper layers. It would have to be
> > sense via a different interface as it would not longer be modeled as
> > part of CPU features in QEMU.
> > 
> > > happens if new cpu features are introduced that have a dependency on or
> > > a conflict with this one?  
> > 
> > Conflict: bail out in QEMU when incompatible options are specified.
> > Dependency: warn and continue/fixup (e.g., mask off?)
> 
> Masking off would likely be surprising to the user.
> 
> > Not clean I think.
> 
> I agree.
> 
> Still unsure how to bring this new machine property and the cpu feature
> together. Would be great to have the same interface everywhere, but
> having two distinct command line objects depend on each other sucks.

Kinda, but the reality is that hardware - virtual and otherwise -
frequently doesn't have entirely orthogonal configuration for each of
its components.  This is by no means new in that regard.

> Automatically setting the feature bit if pv is supported complicates
> things further.

AIUI, on s390 the "unpack" feature is available by default on recent
models.  In that case you could do this:

 * Don't modify either cpu or HTL options based on each other
 * Bail out if the user specifies a non "unpack" secure CPU along with
   the HTL option

Cases of note:
 - User specifies an old CPU model + htl
   or explicitly sets unpack=off + htl
	=> fails with an error, correctly
 - User specifies modern/default cpu + htl, with secure aware guest
 	=> works as a secure guest
 - User specifies modern/default cpu + htl, with non secure aware guest
	=> works, though not secure (and maybe slower than neccessary)
 - User specifies modern/default cpu, no htl, with non-secure guest
 	=> works, "unpack" feature is present but unused
 - User specifies modern/default cpu, no htl, secure guest
  	=> this is the worst one.  It kind of works by accident if
	   you've also  manually specified whatever virtio (and
	   anything else) options are necessary. Ugly, but no
	   different from the situation right now, IIUC

> (Is there any requirement that the machine object has been already set
> up before the cpu features are processed? Or the other way around?)

CPUs are usually created by the machine, so I believe we can count on
the machine object being there first.

> Does this have any implications when probing with the 'none' machine?

I'm not sure.  In your case, I guess the cpu bit would still show up
as before, so it would tell you base feature availability, but not
whether you can use the new configuration option.

Since the HTL option is generic, you could still set it on the "none"
machine, though it wouldn't really have any effect.  That is, if you
could create a suitable object to point it at, which would depend on
... details.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]