[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian |
Date: |
Thu, 19 Jun 2014 21:21:56 -0600 |
On Thu, 2014-06-19 at 11:53 +1000, Alexey Kardashevskiy wrote:
> 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
> :) )
Yeah, it seems like it's correct otherwise and the patch itself looks
ok. Thanks,
Alex
> >>> 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 <address@hidden>
> >>> ---
> >>> 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)
> >>
> >>
> >>
> >
> >
>
>