On Sun, Apr 10, 2011 at 10:11:15PM +0200, Alexander Graf wrote: > > On 10.04.2011, at 22:06, Aurelien Jarno wrote: > > > On Sun, Apr 10, 2011 at 09:26:26PM +0200, Alexander Graf wrote: > >> > >> On 10.04.2011, at 21:25, Aurelien Jarno wrote: > >> > >>> On Mon, Apr 04, 2011 at 04:32:10PM +0200, Alexander Graf wrote: > >>>> The feature bitmap in the s390 virtio machine is little endian. To > >>>> address for that, we need to bswap the values after reading them out. > >>>> > >>>> Signed-off-by: Alexander Graf <ag...@suse.de> > >>>> --- > >>>> hw/s390-virtio-bus.c | 4 ++-- > >>>> 1 files changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > >>>> index 58af164..60e0135 100644 > >>>> --- a/hw/s390-virtio-bus.c > >>>> +++ b/hw/s390-virtio-bus.c > >>>> @@ -223,7 +223,7 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) > >>>> cur_offs += num_vq * VIRTIO_VQCONFIG_LEN; > >>>> > >>>> /* Sync feature bitmap */ > >>>> - stl_phys(cur_offs, dev->host_features); > >>>> + stl_phys(cur_offs, bswap32(dev->host_features)); > >>> > >>> Is bswap32 correct here for both big and little endian guests? I don't > >>> really understand the reason why a bswap is needed here, especially > >>> given that AFAIK this code was already used when using KVM. > >> > >> This is target specific code. The s390-virtio-bus is s390 specific. And > >> yes, the code was also broken with KVM. That's how I first found it > >> actually. > >> > > > > So in short, s390-virtio-bus is specific to big endian machines, but is > > using little endian? That looks a bit weired. I guess it's probably to > > late to change the specification anyway... > > Yeah. In fact, it mixes big and little endian pieces. But it really is too > late to change the (unwritten) spec, unfortunately. > Ok, thanks for the explanations, I have applied it.
-- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net