diff mbox

docs: memory.txt document the endian field

Message ID 20120212125202.GA23416@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Feb. 12, 2012, 12:52 p.m. UTC
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(-)

Comments

Avi Kivity Feb. 12, 2012, 1:02 p.m. UTC | #1
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.
Michael S. Tsirkin Feb. 12, 2012, 1:47 p.m. UTC | #2
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?

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.

> -- 
> error compiling committee.c: too many arguments to function
Avi Kivity Feb. 12, 2012, 1:55 p.m. UTC | #3
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.
diff mbox

Patch

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.