Re: [Qemu-devel] [PATCH] memory: make ram device read/write endian sensitive

From: Peter Maydell <peter.maydell@linaro.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>,
	Yongji Xie <xyjxie@linux.vnet.ibm.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	zhong@linux.vnet.ibm.com,
	David Gibson <david@gibson.dropbear.id.au>,
	Paul Mackerras <paulus@samba.org>
Subject: Re: [Qemu-devel] [PATCH] memory: make ram device read/write endian sensitive
Date: Thu, 23 Feb 2017 15:29:53 +0000	[thread overview]
Message-ID: <CAFEAcA_BgiXrjW9aRNFkXEEjS+kjibJ_UisJtiLgrPh=0re5qA@mail.gmail.com> (raw)
In-Reply-To: <2565ef99-9c16-e836-08c6-0915f5d4b0f8@redhat.com>

On 23 February 2017 at 15:21, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 23/02/2017 15:35, Peter Maydell wrote:
>> On 23 February 2017 at 12:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>
>>> On 23/02/2017 13:26, Peter Maydell wrote:
>>>> On 23 February 2017 at 11:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> On 23/02/2017 12:34, Peter Maydell wrote:
>>>>>> We should probably update the doc comment to note that the
>>>>>> pointer is to host-endianness memory (and that this is not
>>>>>> like normal RAM which is target-endian)...
>>>>>
>>>>> I wouldn't call it host-endianness memory, and I disagree that normal
>>>>> RAM is target-endian---in both cases it's just a bunch of bytes.
>>>>>
>>>>> However, the access done by the MemoryRegionOps callbacks needs to match
>>>>> the endianness declared by the MemoryRegionOps themselves.
>>>>
>>>> Well, if the guest stores a bunch of integers to the memory, which
>>>> way round do you see them when you look at the bunch of bytes?
>>>
>>> You see them in whatever endianness the guest used.
>>
>> I'm confused. I said "normal RAM and this ramdevice memory are
>> different", and you seem to be saying they're the same. I don't
>> think they are (in particular I think with a BE guest on an
>> LE host they'll look different).
>
> No, they look entirely the same.  The only difference is that they go
> through MemoryRegionOps instead of memcpy.

Then we have a different problem, because the thing this patch
is claiming to fix is that the memory the device is backed by
(from vfio) is little-endian and we're not accessing it right.

RAM of the usual sort is target-endian (by which I mean "when the guest
does a write of 32-bits 0x12345678, and you look at the memory byte
by byte then the order of bytes is either 0x12 0x34 0x56 0x78 if
TARGET_LITTLE_ENDIAN or 0x78 0x56 0x34 0x12 if TARGET_BIG_ENDIAN").

AIUI what we want for this VFIO case is "when the guest does
a 32-bit write of 0x12345678 then the bytes are 0x12 0x34 0x56 0x78
regardless of whether TARGET_BIG_ENDIAN or not".

thanks
-- PMM