On 06/19/2014 10:42 AM, Alexey Kardashevskiy wrote: > 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.
I'll remove "tcg" and what else is missing here? I thought the original commit log already had all answers on the questions you askes (or clarified :) ) Thanks! > > >> >>> 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 <a...@ozlabs.ru> >>> --- >>> 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