On 09/08/2014 02:06 AM, Greg Kurz wrote:
> On Sat, 6 Sep 2014 12:49:47 +1000
> Alexey Kardashevskiy <a...@ozlabs.ru> 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);
>> (gdb) s
>> stl_be_p (ptr=0x3fffb7f80028, v=0x4) at
>> /home/alexey/p/qemu/include/qemu/bswap.h:377
>> 377 {
>> (gdb) n
>>
>> ---
>> include/exec/cpu-all.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index f9d132f..f3a761a 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -152,7 +152,7 @@ static inline void tswap64s(uint64_t *s)
>> */
>>
>> /* target-endianness CPU memory access functions */
>> -#if defined(TARGET_WORDS_BIGENDIAN)
>> +#if defined(HOST_WORDS_BIGENDIAN)
>
> AFAIK these accessors implement target CPU accesses to memory... I
> don't see why they should be affected by the host endianness.
> If you have an endianness issue related to HOST_WORDS_BIGENDIAN,
> my guess is that you need to cancel the byteswap in hw/misc/vfio.c.
That is exactly the problem - swaps are cancelled already, VFIO is native
endian now.
>
>> #define lduw_p(p) lduw_be_p(p)
>> #define ldsw_p(p) ldsw_be_p(p)
>> #define ldl_p(p) ldl_be_p(p)
>
--
Alexey
