Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target

[Greg Kurz]

On Wed, 18 Jun 2014 18:38:14 +0800
Stefan Hajnoczi <address@hidden> wrote:

> On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote:
> > 
> > On 17.06.14 09:36, Stefan Hajnoczi wrote:
> > >On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote:
> > >>This version merges the changes requested during the v7 review, remarks 
> > >>from
> > >>ppc64 dump support review (yes, we talked about virtio there) and the 
> > >>work on
> > >>virtio subsections migration. Also two new patches have been added:
> > >>- patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf
> > >>- patch #9 prepares the work on the virtio_is_big_endian() helper
> > >>
> > >>The most significant changes are:
> > >>- introduction of a new CPU method for virtio
> > >>- endianness is taken from CPU that resets the device
> > >>- fastpath virtio memory accessors for fixed endian targets
> > >>- VMState based virtio subsections (compatibility friendly)
> > >I'm surprised it's not enough for the virtio device to have an
> > >endianness field (big/little).  It seems these patches make endianness
> > >depend on the CPUState through which the device is being accessed.
> > >
> > >Can you explain why it's necessary to check the CPUState?
> > 
> > They only check CPUState at the point in time of reset, as that's the only
> > case where we can derive the implicit endian configuration from :).
> 
> What bothers me is that real hardware can't do this.  Given that VIRTIO
> 1.0 is always little-endian I guess this is just a temporary hack for
> ppc little-endian.  Would be nice to add a comment so it's clear why
> this approach is being taken instead of a cleaner solution.
> 
> Stefan

Hi Stefan,

Previous versions of this patch set had such comments:

"virtio data structures are defined as "target endian", which assumes
that's a fixed value.  In fact, that actually means it's platform-specific.
The OASIS virtio 1.0 spec will fix this, by making all little endian.

We need to support both implementations and we want to share as much code
as possible."

but these lines got lost between v6 and v7... my bad... :-\

I agree all of this is a hack but:
- it has been on the table for nearly a year
- ppc LE distros are already available
- the memory accessors part makes sense for 1.0

and, speaking of 1.0, I couldn't find any clue about when QEMU would support
this (Cc'ing Rusty for his input), but we (IBM and distro partners) need
ppc LE support now.

Cheers.

-- 
Gregory Kurz                                     address@hidden
                                                 address@hidden
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.

signature.asc
Description: PGP signature