From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:44976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RwZtx-000599-0v for qemu-devel@nongnu.org; Sun, 12 Feb 2012 08:55:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RwZtv-0005TQ-3g for qemu-devel@nongnu.org; Sun, 12 Feb 2012 08:55:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1334) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RwZtu-0005TK-Qc for qemu-devel@nongnu.org; Sun, 12 Feb 2012 08:55:35 -0500 Message-ID: <4F37C4C8.5000808@redhat.com> Date: Sun, 12 Feb 2012 15:55:20 +0200 From: Avi Kivity MIME-Version: 1.0 References: <20120212125202.GA23416@redhat.com> <4F37B853.1010405@redhat.com> <20120212134707.GB27718@redhat.com> In-Reply-To: <20120212134707.GB27718@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" 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 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. -- error compiling committee.c: too many arguments to function