Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

[Avi Kivity <avi@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 <mst@redhat.com>
> > > ---
> > >  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