Re: [Qemu-devel] [PATCH 3/3] target/ppc: filter out non-zero PCR values when using TCG

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

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

On Wed, Jun 13, 2018 at 10:19:15AM +0200, Greg Kurz wrote:
> On Wed, 13 Jun 2018 10:45:06 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Jun 12, 2018 at 07:04:15PM +0200, Greg Kurz wrote:
> > > Bits set in the PCR disable features of the processor. TCG currently
> > > doesn't implement that, ie, we always act like if PCR is all zeros.
> > > 
> > > But it is still possible for the PCR to have a non-null value. This may
> > > confuse the guest.
> > > 
> > > There are three distinct cases:
> > > 
> > > 1) a powernv guest doing mtspr SPR_PCR
> > > 
> > > 2) reset of a pseries guest if the max-cpu-compat machine property is set
> > > 
> > > 3) CAS of a pseries guest
> > > 
> > > This patch adds a ppc_store_pcr() helper that ensures we cannot put
> > > a non-null value in the PCR when using TCG. This helper also has
> > > error propagation support, so that each case listed above can be
> > > handled appropriately:
> > > 
> > > 1) since the powernv machine is mostly used for OpenPOWER FW devel,
> > >    we just print an error and let QEMU continue execution
> > > 
> > > 2) an error is printed and QEMU exits, ie, same behaviour as when
> > >    KVM doesn't support the requested compat mode
> > > 
> > > 3) an error is printed and QEMU reports H_HARDWARE to the guest
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>  
> > 
> > I'm not really convinced this is a good idea.  Printing a (non fatal)
> > error if the guest attempts to write a non-zero value to the PCR
> > should be ok.  However, you're generating a fatal error if the machine
> > tries to set the PCR in TCG mode.  That could easily happen using,
> > e.g. the cap-htm flag on a TCG guest.  That would take TCG from mostly
> > working, to refusing to run at all.
> > 
> 
> I'm confused... I don't see anything related to HTM in TCG. Also we have
> the following in cap_htm_apply():
> 
>     if (tcg_enabled()) {
>         error_setg(errp,
>                    "No Transactional Memory support in TCG, try cap-htm=off");
> 
> I'm probably missing something... can you enlighten me ?

Ok, so right now when cap-htm=off we don't actually enforce that, we
just don't advertise it to the guest.  We probably _should_ enforce
that, and the way we'd do it is to set the appropriate bit in the
PCR.  That'll do the right thing for KVM (well, once we update KVM to
actually pass on the PCR value) but would break TCG in conjunction
with your patch above.

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