On Mon, 4 Jun 2018 17:59:45 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Thu, May 31, 2018 at 01:27:33PM +0200, Greg Kurz wrote: > > On Thu, 31 May 2018 09:38:10 +0200 > > Cédric Le Goater <c...@kaod.org> wrote: > > > > > On 05/30/2018 04:42 PM, Joel Stanley wrote: > > > > The powerpc Linux kernel[1] and skiboot firmware[2] recently gained > > > > changes > > > > that cause the Processor Compatibility Register (PCR) SPR to be cleared. > > > > > > > > These changes cause Linux to fail to boot on the Qemu powernv machine > > > > with an error: > > > > > > > > Trying to write privileged spr 338 (0x152) at 0000000030017f0c > > > > > > > > With this patch Qemu makes this register available as a hypervisor > > > > privileged register. > > > > > > > > Note that bits set in this register disable features of the processor. > > > > Currently the only register state that is supported is when the register > > > > is zeroed (enable all features). This is sufficient for guests to > > > > once again boot. > > > > > > > > [1] https://lkml.kernel.org/r/20180518013742.24095-1-mi...@neuling.org > > > > [2] https://patchwork.ozlabs.org/patch/915932/ > > > > > > > > Signed-off-by: Joel Stanley <j...@jms.id.au> > > > > --- > > > > target/ppc/helper.h | 1 + > > > > target/ppc/misc_helper.c | 10 ++++++++++ > > > > target/ppc/translate_init.inc.c | 9 +++++++-- > > > > 3 files changed, 18 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > > > > index 19453c68138a..d751f0e21909 100644 > > > > --- a/target/ppc/helper.h > > > > +++ b/target/ppc/helper.h > > > > @@ -17,6 +17,7 @@ DEF_HELPER_2(pminsn, void, env, i32) > > > > DEF_HELPER_1(rfid, void, env) > > > > DEF_HELPER_1(hrfid, void, env) > > > > DEF_HELPER_2(store_lpcr, void, env, tl) > > > > +DEF_HELPER_2(store_pcr, void, env, tl) > > > > #endif > > > > DEF_HELPER_1(check_tlb_flush_local, void, env) > > > > DEF_HELPER_1(check_tlb_flush_global, void, env) > > > > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > > > > index 8c8cba5cc6f1..40c39d08ad14 100644 > > > > --- a/target/ppc/misc_helper.c > > > > +++ b/target/ppc/misc_helper.c > > > > @@ -20,6 +20,7 @@ > > > > #include "cpu.h" > > > > #include "exec/exec-all.h" > > > > #include "exec/helper-proto.h" > > > > +#include "qemu/error-report.h" > > > > > > > > #include "helper_regs.h" > > > > > > > > @@ -186,6 +187,15 @@ void ppc_store_msr(CPUPPCState *env, target_ulong > > > > value) > > > > hreg_store_msr(env, value, 0); > > > > } > > > > > > > > +void helper_store_pcr(CPUPPCState *env, target_ulong value) > > > > +{ > > > > + if (value != 0) { > > > > + error_report("Unimplemented PCR value 0x"TARGET_FMT_lx, value); > > > > + return; > > > > + } > > > > + env->spr[SPR_PCR] = value; > > > > > > shouldn't we use pcc->pcr_mask ? and check pcc->pcr_supported also ? > > > > > > > pcc->pcr_mask and ppc->pcr_supported only make sense for pseries machine > > types (ie, when the spapr machine code call ppc_*_compat() functions). > > That's really not true. pcr_mask is relevant at the hardware level, > not just for pseries. It's the mask of bits that are implemented in > the PCR for this CPU. > Oops... dumb me stroke again... So indeed, the pcr_mask should be applied here, but the (value != 0) check makes sense anyway since TCG currently ignores the PCR. > pcr_supported isn't really relevant here regardless of whether we're > talking about pseries or pnv. It's basically just a kind of weird > encoding of which compat modes are supported by the cpu. > > > The case here is different: we're running a fully emulated pnv machine, > > ie, PCR can only be set by mtspr() called within the pnv guest. But TCG > > doesn't implement the compatibility mode logic, ie, the CPU always run > > in "raw" mode, ie, we only support PCR == 0, actually. > > > > So, this patch looks good for me. I'm just not sure about what is > > causing the build break with patchew though... > > > > > C. > > > > > > > +} > > > > + > > > > /* This code is lifted from MacOnLinux. It is called whenever > > > > * THRM1,2 or 3 is read an fixes up the values in such a way > > > > * that will make MacOS not hang. These registers exist on some > > > > diff --git a/target/ppc/translate_init.inc.c > > > > b/target/ppc/translate_init.inc.c > > > > index ab782cb32aaa..bebe6ec5333c 100644 > > > > --- a/target/ppc/translate_init.inc.c > > > > +++ b/target/ppc/translate_init.inc.c > > > > @@ -456,6 +456,10 @@ static void spr_write_hid0_601(DisasContext *ctx, > > > > int sprn, int gprn) > > > > /* Must stop the translation as endianness may have changed */ > > > > gen_stop_exception(ctx); > > > > } > > > > +static void spr_write_pcr(DisasContext *ctx, int sprn, int gprn) > > > > +{ > > > > + gen_helper_store_pcr(cpu_env, cpu_gpr[gprn]); > > > > +} > > > > #endif > > > > > > > > /* Unified bats */ > > > > @@ -7957,11 +7961,12 @@ static void gen_spr_power6_common(CPUPPCState > > > > *env) > > > > #endif > > > > /* > > > > * Register PCR to report POWERPC_EXCP_PRIV_REG instead of > > > > - * POWERPC_EXCP_INVAL_SPR. > > > > + * POWERPC_EXCP_INVAL_SPR in userspace. Permit hypervisor access. > > > > */ > > > > - spr_register(env, SPR_PCR, "PCR", > > > > + spr_register_hv(env, SPR_PCR, "PCR", > > > > SPR_NOACCESS, SPR_NOACCESS, > > > > SPR_NOACCESS, SPR_NOACCESS, > > > > + &spr_read_generic, &spr_write_pcr, > > > > 0x00000000); > > > > } > > > > > > > > > > > > > > > > >
pgpsoy62184Iw.pgp
Description: OpenPGP digital signature
