From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxQQj-0000PG-Er for qemu-devel@nongnu.org; Wed, 18 Jun 2014 20:42:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxQQd-00083D-3U for qemu-devel@nongnu.org; Wed, 18 Jun 2014 20:42:17 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:44745) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxQQc-000834-RD for qemu-devel@nongnu.org; Wed, 18 Jun 2014 20:42:11 -0400 Received: by mail-pa0-f48.google.com with SMTP id et14so1285561pad.7 for ; Wed, 18 Jun 2014 17:42:09 -0700 (PDT) Message-ID: <53A231DC.2040004@ozlabs.ru> Date: Thu, 19 Jun 2014 10:42:04 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1403091321-31721-1-git-send-email-aik@ozlabs.ru> <1403117875.3707.184.camel@ul30vt.home> In-Reply-To: <1403117875.3707.184.camel@ul30vt.home> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org, Nikunj A Dadhania , Alexander Graf On 06/19/2014 04:57 AM, Alex Williamson wrote: > On Wed, 2014-06-18 at 21:35 +1000, Alexey Kardashevskiy wrote: >> Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used. > > TCG can use the mmap too Oh, right. > >> Since this is just a transport between the guest and a device, there is >> no need to do endianness swapping. >> >> This changes BARs to use native endianness. Since non-ROM BARs were >> doing byte swapping, we need to remove it so does the patch. No change >> in behavior is expected though. >> >> ROM BARs were declared little endian but byte swapping was not >> implemented for them so they never actually worked on big endian systems. >> This fixes endiannes for ROM BARs: this declares them native endian and >> fixes access sizes. > > So the only actual behavior change we expect from this is to fix ROM > access on big endian where we had an unbalanced set of byte swaps. BAR > access worked because the byte swapping in the read/write handler > canceled the byte swapping in QEMU because we declared the BARs as > little endian. Otherwise we're just removing canceling byte swaps for > big endian. Is that correct? Yes. > > Also, as the kernel patch appears to not produce any behavioral change, > only eliminates canceling swaps, there's no dependency between patches, > right? Thanks, That is right. > > Alex > >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++---------- >> 1 file changed, 31 insertions(+), 10 deletions(-) >> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >> index 7437c2e..3eb3c71 100644 >> --- a/hw/misc/vfio.c >> +++ b/hw/misc/vfio.c >> @@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr, >> buf.byte = data; >> break; >> case 2: >> - buf.word = cpu_to_le16(data); >> + buf.word = data; >> break; >> case 4: >> - buf.dword = cpu_to_le32(data); >> + buf.dword = data; >> break; >> default: >> hw_error("vfio: unsupported write size, %d bytes", size); >> @@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque, >> data = buf.byte; >> break; >> case 2: >> - data = le16_to_cpu(buf.word); >> + data = buf.word; >> break; >> case 4: >> - data = le32_to_cpu(buf.dword); >> + data = buf.dword; >> break; >> default: >> hw_error("vfio: unsupported read size, %d bytes", size); >> @@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque, >> static const MemoryRegionOps vfio_bar_ops = { >> .read = vfio_bar_read, >> .write = vfio_bar_write, >> - .endianness = DEVICE_LITTLE_ENDIAN, >> + .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> static void vfio_pci_load_rom(VFIODevice *vdev) >> @@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev) >> static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size) >> { >> VFIODevice *vdev = opaque; >> - uint64_t val = ((uint64_t)1 << (size * 8)) - 1; >> + union { >> + uint8_t byte; >> + uint16_t word; >> + uint32_t dword; >> + uint64_t qword; >> + } buf; >> + uint64_t data = 0; >> >> /* Load the ROM lazily when the guest tries to read it */ >> if (unlikely(!vdev->rom && !vdev->rom_read_failed)) { >> vfio_pci_load_rom(vdev); >> } >> >> - memcpy(&val, vdev->rom + addr, >> + memcpy(&buf, vdev->rom + addr, >> (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0); >> >> + switch (size) { >> + case 1: >> + data = buf.byte; >> + break; >> + case 2: >> + data = buf.word; >> + break; >> + case 4: >> + data = buf.dword; >> + break; >> + default: >> + hw_error("vfio: unsupported read size, %d bytes", size); >> + break; >> + } >> + >> DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n", >> __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot, >> - vdev->host.function, addr, size, val); >> + vdev->host.function, addr, size, data); >> >> - return val; >> + return data; >> } >> >> static void vfio_rom_write(void *opaque, hwaddr addr, >> @@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr, >> static const MemoryRegionOps vfio_rom_ops = { >> .read = vfio_rom_read, >> .write = vfio_rom_write, >> - .endianness = DEVICE_LITTLE_ENDIAN, >> + .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> static bool vfio_blacklist_opt_rom(VFIODevice *vdev) > > > -- Alexey