On 09/08/2014 10:04 PM, Alexander Graf wrote:
>
>
> On 08.09.14 02:06, Alexey Kardashevskiy wrote:
>> On 09/08/2014 06:35 AM, Alexander Graf wrote:
>>>
>>>
>>> On 06.09.14 04:49, Alexey Kardashevskiy wrote:
>>>> At the moment VFIO's BARs are NATIVE_ENDIAN. The idea is that since
>>>> it does not parse BARs content and just provides transport, it should
>>>> not do byte swaps, the guest does it anyway. That worked fine while
>>>> the host was big-endian and it does not work when the host is
>>>> little-endian.
>>>> This happens because ./configure defines static macro
>>>> TARGET_WORDS_BIGENDIAN
>>>> for ppc64 and since there is no ppc64le - endianness of host and guest does
>>>> not match and stl_p&co swaps bytes as shown below.
>>>>
>>>> The proposed patch at the end of this mail fixes the issue for VFIO in
>>>> any combination of host-guest, BE-LE (need to double check with BE host).
>>>> However since I fail to grasp the idea of having statically defined
>>>> TARGET_WORDS_BIGENDIAN, I assume I am missing something big here.
>>>>
>>>> What would the correct fix be here? Thanks!
>>>>
>>>>
>>>>
>>>> Breakpoint 9, vfio_bar_read (opaque=0x10de2e98, addr=0x6f4, size=0x4) at
>>>> /home/alexey/p/qemu/hw/misc/vfio.c:1140
>>>> 1140 {
>>>> Missing separate debuginfos, use: zypper install
>>>> glibc-debuginfo-2.19-15.1.ppc64le
>>>> libgcc_s1-debuginfo-4.8.3+r212056-4.17.ppc64le
>>>> libglib-2_0-0-debuginfo-2.38.2-5.8.ppc64le libgthread-2_0-0-debu
>>>> ginfo-2.38.2-5.8.ppc64le libpcre1-debuginfo-8.33-3.253.ppc64le
>>>> libpixman-1-0-debuginfo-0.32.6-1.1.ppc64le
>>>> libstdc++6-debuginfo-4.8.3+r212056-4.17.ppc64le
>>>> libz1-debuginfo-1.2.8-4.65.ppc64le
>>>> (gdb) bt
>>>> #0 vfio_bar_read (opaque=0x10de2e98, addr=0x6f4, size=0x4) at
>>>> /home/alexey/p/qemu/hw/misc/vfio.c:1140
>>>> #1 0x00000000100a4bd4 in memory_region_read_accessor (mr=0x10de2ea8,
>>>> addr=0x6f4, value=0x3fffb77de320, size=0x4, shift=0x0, mask=0xffffffff) at
>>>> /home/alexey/p/qemu/memory.c:410
>>>> #2 0x00000000100a5000 in access_with_adjusted_size (addr=0x6f4,
>>>> value=0x3fffb77de320, size=0x4, access_size_min=0x1, access_size_max=0x4,
>>>> access=0x100a4b38 <memory_region_read_accessor>, mr=0x1
>>>> 0de2ea8) at /home/alexey/p/qemu/memory.c:475
>>>> #3 0x00000000100a84ac in memory_region_dispatch_read1 (mr=0x10de2ea8,
>>>> addr=0x6f4, size=0x4) at /home/alexey/p/qemu/memory.c:1097
>>>> #4 0x00000000100a85d8 in memory_region_dispatch_read (mr=0x10de2ea8,
>>>> addr=0x6f4, pval=0x3fffb77de458, size=0x4) at
>>>> /home/alexey/p/qemu/memory.c:1119
>>>> #5 0x00000000100ace24 in io_mem_read (mr=0x10de2ea8, addr=0x6f4,
>>>> pval=0x3fffb77de458, size=0x4) at /home/alexey/p/qemu/memory.c:1967
>>>> #6 0x0000000010013bf8 in address_space_rw (as=0x1081d410
>>>> <address_space_memory>, addr=0x1a0b00006f4, buf=0x3fffb7f80028 "",
>>>> len=0x4, is_write=0x0) at /home/alexey/p/qemu/exec.c:2076
>>>> #7 0x0000000010013fa4 in cpu_physical_memory_rw (addr=0x1a0b00006f4,
>>>> buf=0x3fffb7f80028 "", len=0x4, is_write=0x0) at
>>>> /home/alexey/p/qemu/exec.c:2121
>>>> #8 0x00000000100a015c in kvm_cpu_exec (cpu=0x3fffb77e0010) at
>>>> /home/alexey/p/qemu/kvm-all.c:1770
>>>> #9 0x000000001007ab18 in qemu_kvm_cpu_thread_fn (arg=0x3fffb77e0010) at
>>>> /home/alexey/p/qemu/cpus.c:940
>>>> #10 0x00003fffb7828a64 in start_thread () from /lib64/libpthread.so.0
>>>> #11 0x00003fffb7993b00 in clone () from /lib64/libc.so.6
>>>> (gdb) n
>>>> 1141 VFIOBAR *bar = opaque;
>>>> (gdb)
>>>> 1148 uint64_t data = 0;
>>>> (gdb)
>>>> 1150 if (pread(bar->fd, &buf, size, bar->fd_offset + addr) != size)
>>>> {
>>>> (gdb)
>>>> 1156 switch (size) {
>>>> (gdb)
>>>> 1164 data = buf.dword;
>>>> (gdb)
>>>> 1165 break;
>>>> (gdb)
>>>> 1171 if (addr == 0x6f4) {
>>>> (gdb)
>>>> 1172 printf("%s %u size=%d val=%lx\n", __func__, __LINE__,
>>>> size, data);
>>>> (gdb)
>>>> vfio_bar_read 1172 size=4 val=4
>>>> 1187 vfio_eoi(container_of(bar, VFIODevice, bars[bar->nr]));
>>>> (gdb)
>>>> 1189 return data;
>>>> (gdb)
>>>> 1190 }
>>>> (gdb)
>>>> memory_region_read_accessor (mr=0x10de2ea8, addr=0x6f4,
>>>> value=0x3fffb77de320, size=0x4, shift=0x0, mask=0xffffffff) at
>>>> /home/alexey/p/qemu/memory.c:411
>>>> 411 trace_memory_region_ops_read(mr, addr, tmp, size);
>>>> (gdb)
>>>> 412 *value |= (tmp & mask) << shift;
>>>> (gdb)
>>>> 413 }
>>>> (gdb)
>>>> access_with_adjusted_size (addr=0x6f4, value=0x3fffb77de320, size=0x4,
>>>> access_size_min=0x1, access_size_max=0x4, access=0x100a4b38
>>>> <memory_region_read_accessor>, mr=0x10de2ea8) at /home/alexey/p
>>>> /qemu/memory.c:474
>>>> 474 for (i = 0; i < size; i += access_size) {
>>>> (gdb)
>>>> 483 }
>>>> (gdb)
>>>> memory_region_dispatch_read1 (mr=0x10de2ea8, addr=0x6f4, size=0x4) at
>>>> /home/alexey/p/qemu/memory.c:1106
>>>> 1106 return data;
>>>> (gdb)
>>>> 1107 }
>>>> (gdb)
>>>> memory_region_dispatch_read (mr=0x10de2ea8, addr=0x6f4,
>>>> pval=0x3fffb77de458, size=0x4) at /home/alexey/p/qemu/memory.c:1120
>>>> 1120 adjust_endianness(mr, pval, size);
>>>> (gdb) s
>>>> adjust_endianness (mr=0x10de2ea8, data=0x3fffb77de458, size=0x4) at
>>>> /home/alexey/p/qemu/memory.c:364
>>>> 364 {
>>>> (gdb) n
>>>> 365 if (memory_region_wrong_endianness(mr)) {
>>>> (gdb)
>>>> 382 }
>>>> (gdb)
>>>> memory_region_dispatch_read (mr=0x10de2ea8, addr=0x6f4,
>>>> pval=0x3fffb77de458, size=0x4) at /home/alexey/p/qemu/memory.c:1121
>>>> 1121 return false;
>>>> (gdb)
>>>> 1122 }
>>>> (gdb)
>>>> io_mem_read (mr=0x10de2ea8, addr=0x6f4, pval=0x3fffb77de458, size=0x4) at
>>>> /home/alexey/p/qemu/memory.c:1968
>>>> 1968 }
>>>> (gdb)
>>>> address_space_rw (as=0x1081d410 <address_space_memory>,
>>>> addr=0x1a0b00006f4, buf=0x3fffb7f80028 "", len=0x4, is_write=0x0) at
>>>> /home/alexey/p/qemu/exec.c:2077
>>>> 2077 stl_p(buf, val);
>>>
>>> How do you end up in stl_p on an mmio read? That sounds odd.
>>
>> Sorry, I am not following you here. What would not be odd here? It is
>> emulated mmio, stl_p() stores to run->mmio.data.
>
> Ok, I'm slowly starting to grasp the problem. Can you please try the
> following patch?
This or I simply revert "Make BARs native endian" and fix ROM BAR.
commit c40708176a6b52b73bec14796b7c71b882ceb102
Author: Alexey Kardashevskiy <a...@ozlabs.ru>
AuthorDate: Mon Jun 30 09:52:58 2014 -0600
Commit: Alex Williamson <alex.william...@redhat.com>
CommitDate: Mon Jun 30 09:52:58 2014 -0600
vfio: Make BARs native endian
>
> Alex
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 40dcaa6..6283636 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -1095,13 +1095,13 @@ static void vfio_bar_write(void *opaque, hwaddr
> addr,
>
> switch (size) {
> case 1:
> - buf.byte = data;
> + stb_p(&buf.byte, data);
> break;
> case 2:
> - buf.word = data;
> + stw_le_p(&buf.word, data);
> break;
> case 4:
> - buf.dword = data;
> + stl_le_p(&buf.dword, data);
> break;
> default:
> hw_error("vfio: unsupported write size, %d bytes", size);
> @@ -1155,13 +1155,13 @@ static uint64_t vfio_bar_read(void *opaque,
>
> switch (size) {
> case 1:
> - data = buf.byte;
> + data = ldub_p(&buf.byte);
> break;
> case 2:
> - data = buf.word;
> + data = lduw_le_p(&buf.word);
> break;
> case 4:
> - data = buf.dword;
> + data = ldl_le_p(&buf.dword);
> break;
> default:
> hw_error("vfio: unsupported read size, %d bytes", size);
> @@ -1188,7 +1188,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_NATIVE_ENDIAN,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> static void vfio_pci_load_rom(VFIODevice *vdev)
>
--
Alexey
