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

[Peter Maydell]

On Sun, 21 Apr 2019 at 09:03, Shahab Vahedi <address@hidden> wrote:
>
> Hi Peter,
>
> On Sat, Apr 20, 2019 at 07:57:31PM +0100, Peter Maydell wrote:
> > On Fri, 19 Apr 2019 at 12:46, Shahab Vahedi <address@hidden> wrote:
> > >
> > > 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
> > >
> > > Signed-off-by: Shahab Vahedi <address@hidden>
> > > ---
> > > Changelog:
> > > - Extra space before closing parenthesis is removed
> > >
> > >  accel/tcg/cputlb.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > Hi; this patch mostly looks good; thanks for submitting it.
> >
> Please let me remind you that there is a newer [PATCH v3].

Oops, yes, I missed that (was going through a lot of
list emails after being away for a bit). Sorry.

> I have a question though: Richard (Henderson) has already
> _reviewed_ [PATCH v3]. Is it OK if I change the code further
> and submit yet a newer version?

If you think it's necessary, you can -- for instance if
you find a bug or other problem in it. If you change
the code much then you should drop the reviewed-by:
line as the code needs re-review. If the change is
entirely trivial (eg fixing a comment typo) then you
can let the reviewed-by tag stand (ie include it in
the version you send out).

But if you're thinking of doing it just to fiddle
with the ?: style issue I suggested above, don't
bother -- it doesn't matter that much (and not
trying to deal with all 3 cases means you don't
have the nested ?: anyway).

thanks
-- PMM