Re: [PATCH 08/25] block/nvme: Simplify device reset

[Keith Busch]

On Tue, Oct 27, 2020 at 04:53:31PM +0100, Philippe Mathieu-Daudé wrote:
> On 10/27/20 3:58 PM, Keith Busch wrote:
> > On Tue, Oct 27, 2020 at 02:55:30PM +0100, Philippe Mathieu-Daudé wrote:
> >> Avoid multiple endianess conversion by using device endianess.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >>  block/nvme.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/block/nvme.c b/block/nvme.c
> >> index e95d59d3126..be14350f959 100644
> >> --- a/block/nvme.c
> >> +++ b/block/nvme.c
> >> @@ -755,7 +755,7 @@ static int nvme_init(BlockDriverState *bs, const char 
> >> *device, int namespace,
> >>      timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000);
> >>  
> >>      /* Reset device to get a clean state. */
> >> -    regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE);
> >> +    regs->cc &= const_le32(0xFE);
> > 
> > This doesn't look right. The 'regs' is an MMIO address, correct? Memory
> > mappings use the CPU native access.
> 
> cc is little-endian uint32_t.

Well, yes and no. PCI is defined as a little endian transport, so all
CPUs have to automatically convert from their native format when
accessing memory mapped addresses over that transport, so you always use
the arch native format from the host software.

This isn't just for CC. This includes all memory mapped registers, so
this driver's CSTS, AQA, doorbells, etc... shouldn't have any endian
swapping.

See also: every other nvme driver. :)