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

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, Jun 25, 2020 at 09:06:05AM +0200, David Hildenbrand wrote:
> >> 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.
> 
> CPU model initialization is one of the first things machine
> initialization code does on s390x.

As it is for most platforms, but still, the values of machine
properties are available to you at this point.

> static void ccw_init(MachineState *machine)
> {
>     [... memory init ...]
>     s390_sclp_init();
>     s390_memory_init(machine->ram);
>     /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>     s390_init_cpus(machine);
>     [...]
> }
> 
> > 
> >> 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.
> > 
> 
> The important point is that we never want the (expanded) host cpu model
> look different when either specifying or not specifying the HTL
> property.

Ah, yes, I see your point.  So my current suggestion will satisfy
that, basically it is:

cpu has unpack (inc. by default) && htl specified
	=> works (allowing secure), as expected

!cpu has unpack && htl specified
	=> bails out with an error

!cpu has unpack && !htl specified
	=> works for a non-secure guest, as expected
	=> guest will fail if it attempts to go secure

cpu has unpack && !htl specified
	=> works as expected for a non-secure guest (unpack feature is
	   present, but unused)
	=> secure guest may work "by accident", but only if all virtio
	   properties have the right values, which is the user's
	   problem

That last case is kinda ugly, but I think it's tolerable.

> We don't want to run into issues where libvirt probes and gets
> host model X, but when using that probed model (automatically) for a
> guest domain, we suddenly cannot run X anymore.
> 

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