Re: [Qemu-devel] [PATCH] cputlb: Fix io_readx() to respect the access_type

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

Hi Alex,

Le sam. 20 avr. 2019 01:05, Alex Bennée <alex.bennee@linaro.org> a écrit :

>
> Shahab Vahedi <shahab.vahedi@gmail.com> writes:
>
> > This change adapts io_readx() to its input access_type. Currently
> > io_readx() treats any memory access as a read, although it has an
> > input argument "MMUAccessType access_type". This results in:
> >
> > 1) Calling the tlb_fill() only with MMU_DATA_LOAD
> > 2) Considering only entry->addr_read as the tlb_addr
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1825359
>
> This bug talks about the distinction between DATA_LOAD and INST_FETCH
> but...
>
> >
> > Signed-off-by: Shahab Vahedi <shahab.vahedi@gmail.com>
> > ---
> >  accel/tcg/cputlb.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> > index 88cc8389e9..0daac0e806 100644
> > --- a/accel/tcg/cputlb.c
> > +++ b/accel/tcg/cputlb.c
> > @@ -878,10 +878,13 @@ static uint64_t io_readx(CPUArchState *env,
> CPUIOTLBEntry *iotlbentry,
> >          CPUTLBEntry *entry;
> >          target_ulong tlb_addr;
> >
> > -        tlb_fill(cpu, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
> > +        tlb_fill(cpu, addr, size, access_type, mmu_idx, retaddr);
> >
> >          entry = tlb_entry(env, mmu_idx, addr);
> > -        tlb_addr = entry->addr_read;
> > +        tlb_addr =
> > +            (access_type == MMU_DATA_LOAD ) ? entry->addr_read  :
> > +            (access_type == MMU_DATA_STORE) ? entry->addr_write :
> > +            entry->addr_code;
>
> ...why do we care here about MMU_DATA_STORE?
>
> We could just assert (access_type == MMU_DATA_LOAD || access_type ==
> MMU_INST_FETCH) and then have:
>

Is asserting the best we can do here?


>   (access_type == MMU_DATA_LOAD ) ? entry->addr_read  : entry->addr_code
>
>
> >          if (!(tlb_addr & ~(TARGET_PAGE_MASK | TLB_RECHECK))) {
> >              /* RAM access */
> >              uintptr_t haddr = addr + entry->addend;
>
>
> --
> Alex Bennée
>
>