On Wed, Jun 13, 2018 at 04:26:39PM +0200, Greg Kurz wrote: > On Wed, 13 Jun 2018 22:05:02 +1000 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > On Wed, Jun 13, 2018 at 10:19:15AM +0200, Greg Kurz wrote: > > > On Wed, 13 Jun 2018 10:45:06 +1000 > > > David Gibson <da...@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 <gr...@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. > > Hmm... The granularity of the PCR bits is PowerISA versions, not individual > facilities AFAIK... or am I missing something again ?
Huh.. so. In the 3.0 ISA, there are only ISA version bits. But in the 2.07 ISA, there are ISA version bits *and* a bit to control HTM. I'm not quite sure what to make of that. I kind of love the fact that they incompatibly change the compatibility register. > Anyway, with the current code, if we start the guest with: > > -machine accel=tcg,max-cpu-compat=power8 -cpu POWER9 > > We get this in the guest: > > # grep ^cpu /proc/cpuinfo > cpu : POWER8 (architected), altivec supported > > This is the expected result of the CAS negotiation, but > the guest can still execute PowerISA 3.0 instructions > that should cause an invalid instruction exception. Right, this is a bug, albeit not a very high priority one. > I agree this shouldn't cause QEMU to exit, but I guess we should > at least leave the PCR to all zeroes, so that the guest view > is consistent with what TGC does (ie, raw mode). And maybe an > error message to indicate that the PCR is ignored in TCG... but > since that could be guest triggered, and the user cannot do anything > about it, I'm wondering if this should rather be a trace actually. > > Does it make sense for you ? TBH, I don't care all that much. There are a bunch of cases where TCG doesn't behave quite right, most without warnings. One more or less doesn't make a huge difference. -- 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