On 21/02/2017 17:21, Alex Williamson wrote:
> On Tue, 21 Feb 2017 14:46:55 +0800
> Yongji Xie <xyj...@linux.vnet.ibm.com> wrote:
>
>> At the moment ram device's memory regions are NATIVE_ENDIAN. This does
>> not work on PPC64 because VFIO PCI device is little endian but PPC64
>> always defines static macro TARGET_WORDS_BIGENDIAN.
>>
>> This fixes endianness for ram device the same way as it is done
>> for VFIO region in commit 6758008e2c4e79fb6bd04fe8e7a41665fa583965.
>
> The referenced commit was to vfio code where the endianness is fixed,
> here you're modifying shared generic code to assume the same
> endianness as vfio. That seems wrong.
Is the goal to have the same endianness as VFIO? Or is it just a trick
to ensure the number of swaps is always 0 or 2, so that they cancel away?
In other words, would Yongji's patch just work if it used
DEVICE_BIG_ENDIAN and beNN_to_cpu/cpu_to_beNN? If so, then I think the
patch is okay.
Paolo
>
> Alex
>
>> Signed-off-by: Yongji Xie <xyj...@linux.vnet.ibm.com>
>> ---
>> memory.c | 14 +++++++-------
>> 1 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index 6c58373..1ccb99f 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1139,13 +1139,13 @@ static uint64_t memory_region_ram_device_read(void
>> *opaque,
>> data = *(uint8_t *)(mr->ram_block->host + addr);
>> break;
>> case 2:
>> - data = *(uint16_t *)(mr->ram_block->host + addr);
>> + data = le16_to_cpu(*(uint16_t *)(mr->ram_block->host + addr));
>> break;
>> case 4:
>> - data = *(uint32_t *)(mr->ram_block->host + addr);
>> + data = le32_to_cpu(*(uint32_t *)(mr->ram_block->host + addr));
>> break;
>> case 8:
>> - data = *(uint64_t *)(mr->ram_block->host + addr);
>> + data = le64_to_cpu(*(uint64_t *)(mr->ram_block->host + addr));
>> break;
>> }
>>
>> @@ -1166,13 +1166,13 @@ static void memory_region_ram_device_write(void
>> *opaque, hwaddr addr,
>> *(uint8_t *)(mr->ram_block->host + addr) = (uint8_t)data;
>> break;
>> case 2:
>> - *(uint16_t *)(mr->ram_block->host + addr) = (uint16_t)data;
>> + *(uint16_t *)(mr->ram_block->host + addr) =
>> cpu_to_le16((uint16_t)data);
>> break;
>> case 4:
>> - *(uint32_t *)(mr->ram_block->host + addr) = (uint32_t)data;
>> + *(uint32_t *)(mr->ram_block->host + addr) =
>> cpu_to_le32((uint32_t)data);
>> break;
>> case 8:
>> - *(uint64_t *)(mr->ram_block->host + addr) = data;
>> + *(uint64_t *)(mr->ram_block->host + addr) = cpu_to_le64(data);
>> break;
>> }
>> }
>> @@ -1180,7 +1180,7 @@ static void memory_region_ram_device_write(void
>> *opaque, hwaddr addr,
>> static const MemoryRegionOps ram_device_mem_ops = {
>> .read = memory_region_ram_device_read,
>> .write = memory_region_ram_device_write,
>> - .endianness = DEVICE_NATIVE_ENDIAN,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> .valid = {
>> .min_access_size = 1,
>> .max_access_size = 8,
>
