[Qemu-devel] BUG: Re: [PATCH v3 11/14] ioport: Switch dispatching to memory core layer

[Alexey Kardashevskiy]

Hi!

This patch also breaks virtio on powerpc. I thought it was fixed
(reverted?) in the master branch from qemu.org but it is still there. What
did I miss?

The problem comes from "-drive file=virtimg/fc18guest,if=virtio".

My full command line is:
./qemu-system-ppc64 -L "qemu-ppc64-bios/" \
 -trace "events=qemu_trace_events" \
 -nodefaults \
 -chardev "stdio,id=char1,signal=off,mux=on" \
 -device "spapr-vty,id=ser1,chardev=char1" \
 -mon "chardev=char1,mode=readline,id=mon1" \
 -drive file=virtimg/fc18guest,if=virtio \
 -m "1024" \
 -machine "pseries" \
 -nographic \
 -vga "none" \
 -enable-kvm


This is what happens:

SLOF **********************************************************************
QEMU Starting
 Build Date = Apr 30 2013 14:04:00
 FW Version = git-8cfdfc43f4c4c8c8
 Press "s" to enter Open Firmware.

Populating /vdevice methods
Populating /vdevice/address@hidden

NVRAM: size=65536, fetch=200E, store=200F
Populating /vdevice/address@hidden
Populating /address@hidden
 Adapters on 0800000020000000
                     00 0000 (D) : 1af4 1001    virtio [ block ]
No NVRAM common partition, re-initializing...
claim failed!
Using default console: /vdevice/address@hidden

  Welcome to Open Firmware

  Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
  This program and the accompanying materials are made available
  under the terms of the BSD License available at
  http://www.opensource.org/licenses/bsd-license.php


Trying to load:  from: disk ... virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
virtioblk_read: Access beyond end of device!
[many of those]



On 07/11/2013 10:29 PM, Alexander Graf wrote:
> 
> On 24.06.2013, at 08:07, Jan Kiszka wrote:
> 
>> On 2013-06-23 22:50, Hervé Poussineau wrote:
>>> Jan Kiszka a écrit :
>>>> From: Jan Kiszka <address@hidden>
>>>>
>>>> The current ioport dispatcher is a complex beast, mostly due to the
>>>> need to deal with old portio interface users. But we can overcome it
>>>> without converting all portio users by embedding the required base
>>>> address of a MemoryRegionPortio access into that data structure. That
>>>> removes the need to have the additional MemoryRegionIORange structure
>>>> in the loop on every access.
>>>>
>>>> To handle old portio memory ops, we simply install dispatching handlers
>>>> for portio memory regions when registering them with the memory core.
>>>> This removes the need for the old_portio field.
>>>>
>>>> We can drop the additional aliasing of ioport regions and also the
>>>> special address space listener. cpu_in and cpu_out now simply call
>>>> address_space_read/write. And we can concentrate portio handling in a
>>>> single source file.
>>>>
>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>> ---
>>>
>>> ...
>>>
>>>> +
>>>> +static void portio_write(void *opaque, hwaddr addr, uint64_t data,
>>>> +                         unsigned size)
>>>> +{
>>>> +    MemoryRegionPortioList *mrpio = opaque;
>>>> +    const MemoryRegionPortio *mrp = find_portio(mrpio, addr, size,
>>>> true);
>>>> +
>>>> +    if (mrp) {
>>>> +        mrp->write(mrpio->portio_opaque, mrp->base + addr, data);
>>>> +    } else if (size == 2) {
>>>> +        mrp = find_portio(mrpio, addr, 1, true);
>>>> +        assert(mrp);
>>>> +        mrp->write(mrpio->portio_opaque, mrp->base + addr, data & 0xff);
>>>> +        mrp->write(mrpio->portio_opaque, mrp->base + addr + 1, data
>>>>>> 8);
>>>> +    }
>>>> +}
>>>> +
>>>> +static const MemoryRegionOps portio_ops = {
>>>> +    .read = portio_read,
>>>> +    .write = portio_write,
>>>> +    .valid.unaligned = true,
>>>> +    .impl.unaligned = true,
>>>> +};
>>>> +
>>>
>>> You need to mark these operations as DEVICE_LITTLE_ENDIAN.
>>> In portio_write above, you clearly assume that data is in LE format.
>>
>> Anything behind PIO is little endian, of course. Will add this.
> 
> This patch breaks VGA on PPC as it is in master today.
> 
> 
> Alex
> 
>>
>>>
>>> This fixes PPC PReP emulation, which would otherwise be broken with this
>>> patchset.
>>
>> Thanks,
>> Jan
>>
>>
> 
> 


-- 
Alexey