Re: [Qemu-devel] BUG report Re: [PATCH] spapr-pci: change endianness for io ports space

[Alexander Graf <agraf@suse.de>]

Am 12.07.2013 um 10:59 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:

> Hi!
> 
> Got a small lesson from Ben how to post stuff to the list and here are more
> details :)
> 
> Every PCI device which uses IO ports (at least vga, e1000, virtio-pci,
> rtl8139) is broken in the master branch of QEMU for powerpc-kvm. The
> problem is exactly with endianness. For example, setup_vq() from
> drivers/virtio/virtio_pci.c (guest kernel) fails on:
> 
> iowrite16(msix_vec, vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
> 
> as QEMU receives msix_vec==0x100 instead of 0x1 in virtio_ioport_write().
> 
> The patch which broke it is b40acf99bef69fa8ab0f9092ff162fde945eec12
> "ioport: Switch dispatching to memory core layer".
> 
> The patch below fixes this but I am not really sure it is correct. Is it?

It is not. Please read my mail on the topic.

Alex

> Thanks.
> 
> 
> On 07/12/2013 05:37 PM, Alexey Kardashevskiy wrote:
>> sPAPR PHB emulates IO ports on PCI via a special memory region which
>> routes all reads/writes further via cpu_in*/cpu_out* which are eventually
>> processed by MemoryRegionOps implemented by devices.
>> 
>> As devices normally take care of endianness themselves by setting
>> correct MemoryRegionOps::endianness for their memory regions, it is
>> better to set endianness in spapr_io_ops to NATIVE.
>> 
>> Tested on sPAPR KVM with e1000, rtl8139, virtio-net.
>> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> 
>> I would really appreciate if someone told me what exactly changed
>> in QEMU and broke spapr-pci. It used to work... Thanks!
>> 
>> ---
>> hw/ppc/spapr_pci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index ca588aa..dfe4d04 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -472,7 +472,7 @@ static void spapr_io_write(void *opaque, hwaddr addr,
>> }
>> 
>> static const MemoryRegionOps spapr_io_ops = {
>> -    .endianness = DEVICE_LITTLE_ENDIAN,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>     .read = spapr_io_read,
>>     .write = spapr_io_write
>> };
> 
> 
> -- 
> Alexey