Re: [PATCH v2 06/12] target/ppc: Add PPR32 SPR

[Nicholas Piggin]

On Wed May 22, 2024 at 3:40 AM AEST, Richard Henderson wrote:
> On 5/20/24 18:30, Nicholas Piggin wrote:
> > +void spr_write_ppr32(DisasContext *ctx, int sprn, int gprn)
> > +{
> > +    TCGv t0 = tcg_temp_new();
> > +
> > +    tcg_gen_shli_tl(t0, cpu_gpr[gprn], 32);
> > +    gen_store_spr(SPR_PPR, t0);
> > +    spr_store_dump_spr(SPR_PPR);
> > +}
>
> The documentation isn't clear on whether this zaps the low 32 bits. If the 
> low bits of PPR 
> are {reserved, must-be-zero, undefined} or suchlike, this is fine.
>
> If not, then you need a deposit here, to preserve those bits, e.g.:
>
>      gen_load_spr(t0, SPR_PPR);
>      tcg_gen_deposit_tl(t0, t0, cpu_gpr[gprn], 32, 32);
>      gen_store_spr(SPR_PPR, t0);
>
> Anyway, it might be best to add a comment here re the above.

Oh good catch. The other bits are reserved which means they can return 0
but it's not necessary. We implement all the bits though, so we should
not have mtPPR32 zeroing out the other half. In theory we probably can
since they're "undefined", but it doesn't seem nice. Actually now I look
the ISA says reserved bits in SPRs should return 0 for reads in
user-mode which we get wrong in a few places.

Anyway yes, for now I'll go with your deposit. Thank you.

Thanks,
Nick