All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yongji Xie <xyjxie@linux.vnet.ibm.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: pbonzini@redhat.com, crosthwaite.peter@gmail.com,
	rth@twiddle.net, qemu-devel@nongnu.org,
	alex.williamson@redhat.com, aik@ozlabs.ru,
	peter.maydell@linaro.org, paulus@samba.org,
	mdroth@linux.vnet.ibm.com, zhong@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH v2] memory: Introduce DEVICE_HOST_ENDIAN for ram device
Date: Wed, 1 Mar 2017 11:23:05 +0800	[thread overview]
Message-ID: <23cdce58-1d7e-8195-c02e-c21014ab5e2f@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170301003549.GD12571@umbus.fritz.box>

on 2017/3/1 8:35, David Gibson wrote:

> On Tue, Feb 28, 2017 at 06:12:56PM +0800, Yongji Xie wrote:
>> on 2017/2/28 8:41, David Gibson wrote:
>>
>>> On Mon, Feb 27, 2017 at 12:52:44PM +0800, Yongji Xie wrote:
>>>> At the moment ram device's memory regions are DEVICE_NATIVE_ENDIAN. It's
>>>> incorrect. This memory region is backed by a MMIO area in host, so the
>>>> uint64_t data that MemoryRegionOps read from/write to this area should be
>>>> host-endian rather than target-endian. Hence, current code does not work
>>>> when target and host endianness are different which is the most common case
>>>> on PPC64. To fix it, this introduces DEVICE_HOST_ENDIAN for the ram device.
>>>>
>>>> This has been tested on PPC64 BE/LE host/guest in all possible combinations
>>>> including TCG.
>>>>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>>
>>> The effect of the patch is certainly correct.  I remain a little
>>> concerned that the name "host endian" might cause more confusion than
>>> it resolves, but a better term isn't immediately obvious to me.
>> If the memory region's endianness indicates the endianness of multi-byte
>> value that
>> MemoryRegionOps read from/write to this memory region, should "host endian"
>> be reasonable?
>>
>> For a mmio store, QEMU just get a bunch of bytes in the memory at the
>> beginning.
>> Then we use ldX_p to load a target-endian multi-byte value from the memory.
>> Then
>> adjust_endianness() change the endianness of the multi-byte value from
>> target-endian
>> to memory region's endianness.
>>
>> For the mmap MMIO area, we should use host-endian multi-byte value to access
>> it.
>>
>> *(uint32_t *)(mr->ram_block->host + addr) = (uint32_t)data;
>>
>> Here it is the same as stl_he_p().
>>
>> The "host-endian" means we load a bunch of bytes as a host-endian value, and
>> write the
>> value to the mmap MMIO area. That's my understanding. Not sure if it's
>> correct.
> That's correct.  The difficulty is that generally the endian flag
> describes the device's endianness as it appears to the guest.  The
> guest doesn't (and shouldn't) know the host's endianness, so
> describing something as "host endian" is pretty weird from that point
> of view.  Basically the only way this can work is if the qemu device
> is treating all data from the guest as pieces of a bytestream and
> never interpreting things as multibyte values.
>

OK, I think I know what you mean. Indeed, it's hard to describe the ram 
device's
endianness from this point of view.  Just transfer the bytestream without
considering any endianness seems to be good.

Thanks,
Yongji

      reply	other threads:[~2017-03-01  3:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27  4:52 [Qemu-devel] [PATCH v2] memory: Introduce DEVICE_HOST_ENDIAN for ram device Yongji Xie
2017-02-28  0:41 ` David Gibson
2017-02-28  1:04   ` Alexey Kardashevskiy
2017-02-28 10:12   ` Yongji Xie
2017-03-01  0:35     ` David Gibson
2017-03-01  3:23       ` Yongji Xie [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23cdce58-1d7e-8195-c02e-c21014ab5e2f@linux.vnet.ibm.com \
    --to=xyjxie@linux.vnet.ibm.com \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=crosthwaite.peter@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=zhong@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.