Re: [Qemu-devel] [PATCH for-1.4] libi2c-omap: Fix endianness dependency

["Andreas Färber" <afaerber@suse.de>]

Am 02.02.2013 17:49, schrieb Peter Maydell:
> On 2 February 2013 16:45, Andreas Färber <afaerber@suse.de> wrote:
>> From: Andreas Färber <andreas.faerber@web.de>
>>
>> The libqos driver for omap_i2c currently does not work on Big Endian.
>> Introduce helpers for reading from and writing to 16-bit armel registers.
>>
>> This fixes tmp105-test failures on ppc.
>>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>>  tests/libi2c-omap.c |   51 ++++++++++++++++++++++++++++++++-------------------
>>  1 Datei geändert, 32 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)
>>
>> diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c
>> index 9be57e9..7d50ef2 100644
>> --- a/tests/libi2c-omap.c
>> +++ b/tests/libi2c-omap.c
>> @@ -12,6 +12,7 @@
>>  #include <string.h>
>>
>>  #include "qemu/osdep.h"
>> +#include "qemu/bswap.h"
>>  #include "libqtest.h"
>>
>>  enum OMAPI2CRegisters {
>> @@ -48,12 +49,24 @@ typedef struct OMAPI2C {
>>  } OMAPI2C;
>>
>>
>> +static inline void omap_i2c_read16(uint64_t addr, uint16_t *data)
>> +{
>> +    memread(addr, data, 2);
>> +    *data = le16_to_cpu(*data);
>> +}
>> +
>> +static inline void omap_i2c_write16(uint64_t addr, uint16_t data)
>> +{
>> +    data = cpu_to_le16(data);
>> +    memwrite(addr, &data, 2);
>> +}
> 
> There's nothing special about the OMAP i2c device that I know of:
> shouldn't the test code just be using a generic "write 16 bit value
> to memory with appropriate endianness for target CPU" function ?

I asked about where to address this issue [1], no concrete answers, and
this is the only solution I came up with.

libqtest.h has no generic endian-aware memread functions unlike Alex,
you or me expected. It reads a sequence of bytes from guest memory and
transmits them one-by-one over the text-based qtest protocol. Since they
are raw bytes I need to convert them here. tmp105-test itself was
transmitting byte arrays, so it did not need changes.

Looking into where that annoying "mipid" debug output is coming from is
a bit further down on my to-do list for 1.4.

I tested this patch to resolve the test breakage on ppc and not to
introduce a regression for x86_64. I don't want arm disabled like sparc.
Any qtest API remodelling would be 1.5 material now, I guess, and could
be hidden inside my helper functions.

Andreas

[1] http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg05081.html

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg