On 10.01.2012, at 23:02, Anthony Liguori wrote:
> On 01/10/2012 05:35 AM, Benjamin Herrenschmidt wrote:
>> The virtio config area in PIO space is a bit special. The initial
>> header is little endian but the rest (device specific) is guest
>> native endian.
>>
>> The PIO accessors for PCI on machines that don't have native IO ports
>> assume that all PIO is little endian, which works fine for everything
>> except the above.
>>
>> A complicated way to fix it would be to split the BAR into two memory
>> regions with different endianess settings, but this isn't practical
>> to do, besides, the PIO code doesn't honor region endianness anyway
>> (I have a patch for that too but it isn't necessary at this stage).
>>
>> So I decided to go for the quick fix instead which consists of
>> reverting the swap in virtio-pci in selected places, hoping that when
>> we eventually do a "v2" of the virtio protocols, we sort that out once
>> and for all using a fixed endian setting for everything.
>>
>> Unfortunately, that mean moving virtio-pci from Makefile.objs to
>> Makefile.target so we can use TARGET_WORDS_BIGENDIAN which would
>> otherwise be poisoned.
>>
>> Signed-off-by: Benjamin Herrenschmidt<benh@xxxxxxxxxxxxxxxxxxx>
>> ---
>> Makefile.objs | 1 -
>> Makefile.target | 1 +
>> hw/virtio-pci.c | 24 ++++++++++++++++++++++--
>> 3 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 4f6d26c..b721fca 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -186,7 +186,6 @@ hw-obj-y =
>> hw-obj-y += vl.o loader.o
>> hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
>> hw-obj-y += usb-libhw.o
>> -hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>> hw-obj-y += fw_cfg.o
>> hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
>> hw-obj-$(CONFIG_PCI) += msix.o msi.o
>> diff --git a/Makefile.target b/Makefile.target
>> index ef6834b..03d44c3 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -191,6 +191,7 @@ obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o
>> # need to fix this properly
>> obj-$(CONFIG_NO_PCI) += pci-stub.o
>> obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
>> +obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>> obj-y += vhost_net.o
>> obj-$(CONFIG_VHOST_NET) += vhost.o
>> obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o
>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>> index 77b75bc..ca70e42 100644
>> --- a/hw/virtio-pci.c
>> +++ b/hw/virtio-pci.c
>> @@ -412,20 +412,34 @@ static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
>> {
>> VirtIOPCIProxy *proxy = opaque;
>> uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
>> + uint16_t val;
>> if (addr< config)
>> return virtio_ioport_read(proxy, addr);
>> addr -= config;
>> - return virtio_config_readw(proxy->vdev, addr);
>> + val = virtio_config_readw(proxy->vdev, addr);
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> + /* virtio is odd, ioports are LE but config space is target native
>> + * endian. However, in qemu, all PIO is LE, so we need to re-swap
>> + * on BE targets
>> + */
>> + val = bswap16(val);
>> +#endif
>
> I think this is the only reasonable way to do it, but I'd suggest adding target_is_bigendian() to arch_init.c that way we wouldn't have to move the object from libhw.
No. Libhw shouldn't be able to know anything about target endianness. If a device is as brokenly spec'ed as virtio and is coupled to the "main CPU endianness", it clearly belongs with the CPU, not into libhw.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
