Re: [PATCH v3] target/ppc: fix Hash64 MMU update of PTE bit R

[Cédric Le Goater]

> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 163c90388a..8ebf85bad8 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -1414,7 +1414,7 @@ void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> > > >            kvmppc_write_hpte(ptex, pte0, pte1);
> > > >        } else {
> > > >            if (pte0 & HPTE64_V_VALID) {
> > > > -            stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
> > > > +            stq_p(spapr->htab + offset + HPTE64_R_BYTE_OFFSET, pte1);
> > >
> > > Urgh.. so, initially I thought this was wrong because I was confusing
> > > HPTE64_R_BYTE_OFFSET with HPTE64_R_R_BYTE_OFFSET.  I doubt I'd be the
> > > only one.
> > >
> > > Calling something a BYTE_OFFSET then doing an stq to it is pretty
> > > misleading I think.  WORD1_OFFSET or R_WORD_OFFSET might be better?
> >
> > How about (inspired from XIVE) :
> >
> >   #define HPTE64_W1    (HASH_PTE_SIZE_64 / 2)
> >   #define HPTE64_W1_R  14 // or HPTE64_W1 + 6
> >   #define HPTE64_W1_C  15 // or HPTE64_W1 + 7
>
> Good enough.

But a word is 32bits in Power. Let's use DW1 to be consistent with the
architecture. I think this is less confusing than using W2.

Thanks,

C.