Re: [Qemu-devel] [PATCH v1 1/5] target-microblaze: Respect MSR.PVR as read-only

[Edgar E. Iglesias]

On Thu, Apr 19, 2018 at 09:56:40AM -1000, Richard Henderson wrote:
> On 04/19/2018 01:21 AM, Edgar E. Iglesias wrote:
> >  static inline void msr_write(DisasContext *dc, TCGv v)
> >  {
> > -    TCGv t;
> > -
> > -    t = tcg_temp_new();
> >      dc->cpustate_changed = 1;
> >      /* PVR bit is not writable.  */
> > -    tcg_gen_andi_tl(t, v, ~MSR_PVR);
> > -    tcg_gen_andi_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], MSR_PVR);
> > -    tcg_gen_or_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], v);
> > -    tcg_temp_free(t);
> > +    tcg_gen_deposit_tl(cpu_SR[SR_MSR], v, cpu_SR[SR_MSR], MSR_PVR_SHIFT, 
> > 1);
> >  }
> 
> Um... the old code was correct, but the new code isn't.
> 
> The new code sets msr to v, with bit 10 set to the old msr bit 0.
> 
> Why do you believe the old code to be wrong?

The old code was incorrectly ORing v instead of t...

What about the following?
    tcg_gen_shri_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], MSR_PVR_SHIFT);
    tcg_gen_deposit_tl(cpu_SR[SR_MSR], v, cpu_SR[SR_MSR], MSR_PVR_SHIFT, 1);

Cheers,
Edgar