Re: [PATCH v4 03/57] accel/tcg: Introduce tlb_read_idx

[Richard Henderson]

On 5/7/23 11:09, Peter Maydell wrote:
> On Fri, 5 May 2023 at 19:57, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > On 5/4/23 16:02, Peter Maydell wrote:
> > > On Wed, 3 May 2023 at 08:15, Richard Henderson
> > > <richard.henderson@linaro.org> wrote:
> > > > Instead of playing with offsetof in various places, use
> > > > MMUAccessType to index an array.  This is easily defined
> > > > instead of the previous dummy padding array in the union.
> > > >
> > > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > > ---
> > >
> > > > @@ -1802,7 +1763,8 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
> > > > target_ulong addr,
> > > >        if (prot & PAGE_WRITE) {
> > > >            tlb_addr = tlb_addr_write(tlbe);
> > > >            if (!tlb_hit(tlb_addr, addr)) {
> > > > -            if (!VICTIM_TLB_HIT(addr_write, addr)) {
> > > > +            if (!victim_tlb_hit(env, mmu_idx, index, MMU_DATA_STORE,
> > > > +                                addr & TARGET_PAGE_MASK)) {
> > > >                    tlb_fill(env_cpu(env), addr, size,
> > > >                             MMU_DATA_STORE, mmu_idx, retaddr);
> > > >                    index = tlb_index(env, mmu_idx, addr);
> > > > @@ -1835,7 +1797,8 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
> > > > target_ulong addr,
> > > >        } else /* if (prot & PAGE_READ) */ {
> > > >            tlb_addr = tlbe->addr_read;
> >
> > read
> >
> > > >            if (!tlb_hit(tlb_addr, addr)) {
> > > > -            if (!VICTIM_TLB_HIT(addr_write, addr)) {
> >
> > write
> >
> > > > +            if (!victim_tlb_hit(env, mmu_idx, index, MMU_DATA_LOAD,
> > > > +                                addr & TARGET_PAGE_MASK)) {
> > >
> > > This was previously looking at addr_write, but now we pass
> > > MMU_DATA_LOAD ?
> >
> > This is the read portion of the read+write check.  The write portion is above 
> > in the
> > previous hunk.  So this is an existing error, fixed here, and I hadn't noticed.
>
> Yeah, I wondered if it was a pre-existing bug. We should split out
> the bug fix.

https://patchew.org/QEMU/20230505204049.352469-1-richard.henderson@linaro.org/

r~