On Wed, Mar 13, 2019 at 08:17:42AM +0100, Cédric Le Goater wrote: > On 3/13/19 4:20 AM, David Gibson wrote: > > 176dccee "target/ppc/spapr: Clear partition table entry when allocating > > hash table" reworked the H_REGISTER_PROCESS_TABLE hypercall, but > > unfortunately due to a small error no longer correctly sets the LPCR[GTSE] > > bit which allows the guest to directly execute (some types of) tlbie (TLB > > flush) instructions without involving the hypervisor. > > > > We got away with this, initially, because POWER9 did not have hypervisor > > mode enabled in its msr_mask, which meant we didn't actually run hypervisor > > privilege checks in TCG at all. However, da874d90 "target/ppc: add HV > > support for POWER9" turned on HV support on POWER9 for the benefit of the > > powernv machine type. > > > > This exposed the earlier bug in H_REGISTER_PROCESS_TABLE, and causes guests > > which rely on LPCR[GTSE] (i.e. basically all of them) to crash during early > > boot when their first tlbie instruction causes an unexpected trap. > > > > Fixes: 176dccee target/ppc/spapr: Clear partition table entry when > > allocating hash table > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > Nice catch. > > May be we should rename the flags from FLAG_XXXX to PROC_TBL_XXXX_FLAG ?
Meh, the current names are theoretically ambiguous, but they're
short. If they were truly global I'd be concerned, but given they're
#defined right above I don't think there's any urgency.
> Reviewed-by: Cédric Le Goater <c...@kaod.org>
>
> Thanks,
>
> C.
>
> > ---
> > hw/ppc/spapr_hcall.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 0761e10142..8a736797b9 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1400,7 +1400,8 @@ static target_ulong
> > h_register_process_table(PowerPCCPU *cpu,
> > else if (flags & FLAG_HASH_PROC_TBL) /* Hash with process tables */
> > update_lpcr |= LPCR_UPRT;
> > if (flags & FLAG_GTSE) /* Guest translation shootdown enable */
> > - update_lpcr |= FLAG_GTSE;
> > + update_lpcr |= LPCR_GTSE;
> > +
> > spapr_set_all_lpcrs(update_lpcr, LPCR_UPRT | LPCR_HR | LPCR_GTSE);
> >
> > if (kvm_enabled()) {
> >
>
--
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
