From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47690) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rwb1A-00065y-AT for qemu-devel@nongnu.org; Sun, 12 Feb 2012 10:07:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rwb18-0007JV-Sr for qemu-devel@nongnu.org; Sun, 12 Feb 2012 10:07:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:25578) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rwb18-0007JP-Kl for qemu-devel@nongnu.org; Sun, 12 Feb 2012 10:07:06 -0500 Date: Sun, 12 Feb 2012 17:06:59 +0200 From: "Michael S. Tsirkin" Message-ID: <20120212150658.GC27718@redhat.com> References: <20120212125202.GA23416@redhat.com> <4F37B853.1010405@redhat.com> <20120212134707.GB27718@redhat.com> <4F37C4C8.5000808@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F37C4C8.5000808@redhat.com> Subject: Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, jan.kiszka@siemens.com, rth@twiddle.net, qemu-devel@nongnu.org, blauwirbel@gmail.com, michael@walle.cc, david@gibson.dropbear.id.au, afaerber@suse.de, areis@redhat.com On Sun, Feb 12, 2012 at 03:55:20PM +0200, Avi Kivity wrote: > On 02/12/2012 03:47 PM, Michael S. Tsirkin wrote: > > On Sun, Feb 12, 2012 at 03:02:11PM +0200, Avi Kivity wrote: > > > On 02/12/2012 02:52 PM, Michael S. Tsirkin wrote: > > > > This is an attempt to document the endian > > > > field in memory API. As this is a confusing topic, > > > > it's best to make the text as explicit as possible. > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > --- > > > > docs/memory.txt | 28 ++++++++++++++++++++++++++++ > > > > 1 files changed, 28 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/docs/memory.txt b/docs/memory.txt > > > > index 5bbee8e..ff92b52 100644 > > > > --- a/docs/memory.txt > > > > +++ b/docs/memory.txt > > > > @@ -170,3 +170,31 @@ various constraints can be supplied to control how these callbacks are called: > > > > - .old_portio and .old_mmio can be used to ease porting from code using > > > > cpu_register_io_memory() and register_ioport(). They should not be used > > > > in new code. > > > > +- .endianness; specifies the device endian-ness, which affects > > > > + the value parameter passed from guest to write and returned > > > > + to guest from read callbacks, as follows: > > > > + void write(void *opaque, target_phys_addr_t addr, > > > > + uint64_t value, unsigned size) > > > > + uint64_t read(void *opaque, target_phys_addr_t addr, > > > > + unsigned size) > > > > + Legal values are: > > > > + DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in > > > > + host endian format. This makes it possible to do > > > > + math on values without type conversions. > > > > + Low size bytes in value are set, the rest are zero padded > > > > + on input and ignored on output. > > > > + DEVICE_LITTLE_ENDIAN - Callbacks accept and return value > > > > + in little endian format. This is appropriate > > > > + if you need to directly copy the data into device memory, > > > > + and the device programming interface is little endian > > > > + (true for most pci devices). > > > > + First size bytes in value are set, the rest are zero padded > > > > + on input and ignored on output. > > > > + DEVICE_BIG_ENDIAN - Callbacks accept and return value > > > > + in big endian format. > > > > + in little endian format. This is appropriate > > > > + if you need to directly copy the data into device memory, > > > > + and the device programming interface is big endian > > > > + (true e.g. for some system devices on big endian architectures). > > > > + Last size bytes in value are set, the rest are zero padded > > > > + on input and ignored on output. > > > > > > This is wrong. Callback data is always in host endianness. Device > > > endianness is about the device. > > > > > > For example, DEVICE_BIG_ENDIAN means that the device expects data in big > > > endian format. Qemu assumes the guest OS writes big endian data to the > > > device, so it swaps from big endian to host endian before calling the > > > callback. Similarly it will swap from host endian to big endian on read. > > > > > > DEVICE_NATIVE_ENDIAN means: > > > > > > defined(TARGET_WORDS_BIGENDIAN) ? DEVICE_BIG_ENDIAN : DEVICE_NATIVE_ENDIAN > > > > > > i.e. the device has the same endianness as the guest cpu. > > > > I think this boils down to the same thing in the end, no? > > Maybe. > > > However, it's a bad way to describe the setup > > for device writers: it documents the > > internal workings of qemu with multiple > > swaps. We need to document the end result. > > > > And, it is IMO confusing to say that 'a device expects data' > > this adds a speculative element where you > > are asked to think about what you would want to > > do and promised that this will be somehow > > satisfied. > > > > Instead, please specify what the API does, users > > can make their own decisions on when to use it. > > But "callbacks accept data in little endian format" implies that you > have to add a swap in the handler, > since you usually want data in host endian. > It's really really simple: > > If the device spec says "big endian, specify DEVICE_BIG_ENDIAN, and > treat the data naturally in the callback. > If the device spec says "little endian, specify DEVICE_LITTLE_ENDIAN, > and treat the data naturally in the callback. > > That's it. OKay, but I'm sure your API does not go read the spec, so we should not base the description on that :) Right? So I think the following is right? commit 02aa79aac9bec1c8c17d1b7b5405b59b649dfdb9 Author: Michael S. Tsirkin Date: Wed Feb 8 17:16:35 2012 +0200 docs: memory.txt document the endian field This is an attempt to document the endian field in memory API. As this is a confusing topic, add some examples. Signed-off-by: Michael S. Tsirkin diff --git a/docs/memory.txt b/docs/memory.txt index 5bbee8e..9132c86 100644 --- a/docs/memory.txt +++ b/docs/memory.txt @@ -170,3 +170,48 @@ various constraints can be supplied to control how these callbacks are called: - .old_portio and .old_mmio can be used to ease porting from code using cpu_register_io_memory() and register_ioport(). They should not be used in new code. +- .endianness; specifies the device endian-ness, which affects + the handling of the value parameter passed from guest to write + and returned to guest from read callbacks, as follows: + void write(void *opaque, target_phys_addr_t addr, + uint64_t value, unsigned size) + uint64_t read(void *opaque, target_phys_addr_t addr, + unsigned size) + value is always passed in the natural host format, + low size bytes in value are set, the rest are zero padded + on input and ignored on output. + Legal values for endian-ness are: + DEVICE_NATIVE_ENDIAN - The value is left in the format used by guest. + Note that although this is typically a fixed format as + guest drivers take care of endian conversions, + if host endian-ness does not match the device this will + result in "mixed endian" since the data is always + stored in low bits of value. + + To handle this data, on write, you typically need to first + convert to the appropriate type, removing the + padding. On read, handle the data in the appropriate + type and then convert to uint64_t, padding with leading zeroes. + + DEVICE_LITTLE_ENDIAN - The value is assumed to be + endian, and is converted to host endian. + DEVICE_BIG_ENDIAN - The value is assumed to be + big endian, and is converted to host endian. + + As an example, consider a little endian guest writing a 32 bit + value 0x12345678 into an MMIO register, on a big endian host. + The value passed to the write callback is documented below: + + DEVICE_NATIVE_ENDIAN - value = 0x0000000087654321 + Explanation: write callback will get the high bits + in value set to 0, and low bits set to data left + as is, that is in little endian format. + DEVICE_LITTLE_ENDIAN - value = 0x0000000012345678 + Explanation: the write callback will get the high bits + in value set to 0, and low bits set to data in big endian + format. + DEVICE_BIG_ENDIAN - value = 0x0000000087654321 + Explanation: the write callback will get the high bits + in value set to 0, and low bits set to data in little endian + format. +