Re: [PATCH v5 4/5] hw/nvme: fix mmio read

[Peter Maydell]

On Tue, 20 Jul 2021 at 13:58, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 7/20/21 12:46 AM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > The new PMR test unearthed a long-standing issue with MMIO reads on
> > big-endian hosts.
> >
> > Fix this by unconditionally storing all controller registers in little
> > endian.

> My brain overflowed at this point, too many changes to track :/
>
> Would it make sense to split it? (per big function body maybe?)

I did review of (a previous version of) the patch by:
 (1) check that after the patch is applied there aren't any
     direct accesses to n->bar.anything which don't go via a
     ld*/st* (ie it didn't miss anything)
 (2) check that the accesses which use ldq/stq are consistently
     doing so for those fields and that they are uint64_t
 (3) read through the patch and check all the changes are ok
     (either using inline calls to the accessors, or fishing
     the value out at the start of the function)

It is a big patch, but I'm not sure splitting it apart helps much.

thanks
-- PMM