Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 5/6] qtest: define target cpu endianness conversion function

[Laurent Vivier]

On 28/09/2016 04:40, David Gibson wrote:
> On Tue, Sep 27, 2016 at 08:55:58PM +0200, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>  tests/libqtest.h | 57 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>>
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index 4be1f77..4c47f2d 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -17,6 +17,7 @@
>>  #ifndef LIBQTEST_H
>>  #define LIBQTEST_H
>>  
>> +#include "qemu/bswap.h"
>>  #include "qapi/qmp/qdict.h"
>>  
>>  typedef struct QTestState QTestState;
>> @@ -891,6 +892,62 @@ static inline bool target_big_endian(void)
>>      return qtest_big_endian(global_qtest);
>>  }
>>  
>> +/* Endianness conversion function between target cpu and specified endianess
> 
> Since the qtest accelerator essentially replaces the target cpu, it's
> not really clear to me how the (default) endianness of the target cpu
> comes into play.
> 
> IIUC for the purposes of the test code you'll eventually want things
> converted to the *host* endianness - since that's the test code is
> actually running on.

Values are read/written from/to memory by the qtest accelerator using
cpu_physical_memory_read()/cpu_physical_memory_write() (see qtest.c) and
then sent to the test program using ASCII protocol. So the values are
read/written using endianness of the guest.

So when we read a value from little-endian device on big endian guest
the value must be converted (like leXX_to_cpu() function on a host) but
in our case the involved endianness is the one from the guest (which is
reading the value from the memory) not the one from the host (which is
manipulating the value).

> 
>> + * uint16_t target_le16_to_cpu(uint16_t v);
>> + * uint32_t target_le32_to_cpu(uint32_t v);
>> + * uint64_t target_le64_to_cpu(uint64_t v);
>> + * uint16_t target_be16_to_cpu(uint16_t v);
>> + * uint32_t target_be32_to_cpu(uint32_t v);
>> + * uint64_t target_be64_to_cpu(uint64_t v);
>> + *
>> + * Convert the value @v from the specified format to the native
>> + * endianness of the host CPU by byteswapping if necessary, and
>                         ^^^^^^^^
> Should this be target CPU?

yes

>> + * return the converted value.
>> + *
>> + * uint16_t target_cpu_to_le16(uint16_t v);
>> + * uint32_t target_cpu_to_le32(uint32_t v);
>> + * uint64_t target_cpu_to_le64(uint64_t v);
>> + * uint16_t target_cpu_to_be16(uint16_t v);
>> + * uint32_t target_cpu_to_be32(uint32_t v);
>> + * uint64_t target_cpu_to_be64(uint64_t v);
>> + *
>> + * Convert the value @v from the native endianness of the host CPU to
>                                                     and here ^^^^^^^^
>> + * the specified format by byteswapping if necessary, and return
>> + * the converted value.
>> + *
>> + * Both target_X_to_cpu() and target_cpu_to_X() perform the same operation; 
>> you
>> + * should use whichever one is better documenting of the function your
>> + * code is performing.
>> + *
>> + */
>> +
>> +#define le_bswap(s, v, size) (qtest_big_endian(s) ? bswap ## size(v) : (v))
>> +#define be_bswap(s, v, size) (qtest_big_endian(s) ? (v) : bswap ## size(v))
>> +
>> +#define TARGET_CPU_CONVERT(endian, size, type)\
>> +static inline type target_ ## endian ## size ## _to_cpu(type v)\
>> +{\
>> +    return glue(endian, _bswap)(global_qtest, v, size);\
>> +} \
>> +\
>> +static inline type target_cpu_to_ ## endian ## size(type v)\
>> +{\
>> +    return glue(endian, _bswap)(global_qtest, v, size);\
>> +}
>> +
>> +TARGET_CPU_CONVERT(be, 16, uint16_t)
>> +TARGET_CPU_CONVERT(be, 32, uint32_t)
>> +TARGET_CPU_CONVERT(be, 64, uint64_t)
>> +
>> +TARGET_CPU_CONVERT(le, 16, uint16_t)
>> +TARGET_CPU_CONVERT(le, 32, uint32_t)
>> +TARGET_CPU_CONVERT(le, 64, uint64_t)
>> +
>> +#undef TARGET_CPU_CONVERT
>> +#undef be_bswap
>> +#undef le_bswap
>> +
>>  QDict *qmp_fd_receive(int fd);
>>  void qmp_fd_sendv(int fd, const char *fmt, va_list ap);
>>  void qmp_fd_send(int fd, const char *fmt, ...);
> 

Thanks,
Laurent