On Tue, Oct 27, 2020 at 09:55:34AM -0700, Keith Busch wrote: > 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é <phi...@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. :)
I don't see the opposite in Linux. The Linux NVMe drivers use byteswap instructions because readl()/writel() and friends perform little-endian memory accesses, not native endian memory accesses: static int nvme_pci_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val) { writel(val, to_nvme_dev(ctrl)->bar + off); return 0; } arch/arm/include/asm/io.h: #define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); }) where the byteswap happens here: #define writel_relaxed(v,c) __raw_writel((__force u32) cpu_to_le32(v),c) The CPU is using explicit byteswaps, which matches what the QEMU driver is doing. Am I missing something? Stefan
signature.asc
Description: PGP signature