diff mbox

spapr-pci: remove io ports workaround

Message ID 20131209163357.14448.60087.stgit@bahia.local
State New
Headers show

Commit Message

Greg Kurz Dec. 9, 2013, 4:33 p.m. UTC
In the past, IO space could not be mapped into the memory address space
so we introduced a workaround for that. Nowadays it does not look
necessary so we can remove the workaround and make sPAPR PCI
configuration simplier.

This workaround has also an evil side effect with virtio devices: because
all PHBs have their .io region at the same address, the devices
get mapped in the .io-alias region of every PHB (AKA. mapped multiple times).
This breaks the ioeventfd feature and causes qemu to abort() when running
with KVM and asking for more than one PHB:

$ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
  -hda /local/greg/images/fedora-be.qcow2 \
  -device virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
  -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \
  -device spapr-pci-host-bridge,index=15
kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
Aborted

This will prevent to use virtio and VFIO passthrough at the same time, since
VFIO needs a dedicated PHB to work on ppc.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

Comments

Alexey Kardashevskiy Dec. 10, 2013, 2:43 a.m. UTC | #1
On 12/10/2013 03:33 AM, Greg Kurz wrote:
> In the past, IO space could not be mapped into the memory address space
> so we introduced a workaround for that. Nowadays it does not look
> necessary so we can remove the workaround and make sPAPR PCI
> configuration simplier.
> 
> This workaround has also an evil side effect with virtio devices: because
> all PHBs have their .io region at the same address, the devices
> get mapped in the .io-alias region of every PHB (AKA. mapped multiple times).
> This breaks the ioeventfd feature and causes qemu to abort() when running
> with KVM and asking for more than one PHB:
> 
> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>   -hda /local/greg/images/fedora-be.qcow2 \
>   -device virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>   -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \
>   -device spapr-pci-host-bridge,index=15
> kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
> Aborted
> 
> This will prevent to use virtio and VFIO passthrough at the same time, since
> VFIO needs a dedicated PHB to work on ppc.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>


I have not seen this version yet so please remove me from "SOB". The patch
you replied to was eventually reworked and went to upstream as
66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf

This one might be correct too but I want to try this first :)


> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_pci.c |   13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 7763149..7d29431 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -564,23 +564,14 @@ static int spapr_phb_init(SysBusDevice *s)
>      memory_region_add_subregion(get_system_memory(), sphb->mem_win_addr,
>                                  &sphb->memwindow);
>  
> -    /* On ppc, we only have MMIO no specific IO space from the CPU
> -     * perspective.  In theory we ought to be able to embed the PCI IO
> -     * memory region direction in the system memory space.  However,
> -     * if any of the IO BAR subregions use the old_portio mechanism,
> -     * that won't be processed properly unless accessed from the
> -     * system io address space.  This hack to bounce things via
> -     * system_io works around the problem until all the users of
> -     * old_portion are updated */
> +    /* Initialize IO regions */
>      sprintf(namebuf, "%s.io", sphb->dtbusname);
>      memory_region_init(&sphb->iospace, OBJECT(sphb),
>                         namebuf, SPAPR_PCI_IO_WIN_SIZE);
> -    /* FIXME: fix to support multiple PHBs */
> -    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
>  
>      sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
>      memory_region_init_alias(&sphb->iowindow, OBJECT(sphb), namebuf,
> -                             get_system_io(), 0, SPAPR_PCI_IO_WIN_SIZE);
> +                             &sphb->iospace, 0, SPAPR_PCI_IO_WIN_SIZE);
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>      /*
>
Greg Kurz Dec. 10, 2013, 7:47 a.m. UTC | #2
On Tue, 10 Dec 2013 13:43:05 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 12/10/2013 03:33 AM, Greg Kurz wrote:
> > In the past, IO space could not be mapped into the memory address space
> > so we introduced a workaround for that. Nowadays it does not look
> > necessary so we can remove the workaround and make sPAPR PCI
> > configuration simplier.
> > 
> > This workaround has also an evil side effect with virtio devices:
> > because all PHBs have their .io region at the same address, the devices
> > get mapped in the .io-alias region of every PHB (AKA. mapped multiple
> > times). This breaks the ioeventfd feature and causes qemu to abort()
> > when running with KVM and asking for more than one PHB:
> > 
> > $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
> >   -hda /local/greg/images/fedora-be.qcow2 \
> >   -device
> > virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
> > -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
> > spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
> > ioeventfd: File exists Aborted
> > 
> > This will prevent to use virtio and VFIO passthrough at the same time,
> > since VFIO needs a dedicated PHB to work on ppc.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> 
> I have not seen this version yet so please remove me from "SOB". The patch
> you replied to was eventually reworked and went to upstream as
> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
> 

Hi Alex,

I agree you have not seen this version yet... The patch I replied to was my
primary source of inspiration and contains these bits, hence the SOB. 
Anyway, the SOB is now removed until you decide to add one yourself. :)

> This one might be correct too but I want to try this first :)
> 

Well, I hope it is. Please try it.

Thanks for the feedback.

Cheers.

--
Greg

> 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr_pci.c |   13 ++-----------
> >  1 file changed, 2 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 7763149..7d29431 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -564,23 +564,14 @@ static int spapr_phb_init(SysBusDevice *s)
> >      memory_region_add_subregion(get_system_memory(),
> > sphb->mem_win_addr, &sphb->memwindow);
> >  
> > -    /* On ppc, we only have MMIO no specific IO space from the CPU
> > -     * perspective.  In theory we ought to be able to embed the PCI IO
> > -     * memory region direction in the system memory space.  However,
> > -     * if any of the IO BAR subregions use the old_portio mechanism,
> > -     * that won't be processed properly unless accessed from the
> > -     * system io address space.  This hack to bounce things via
> > -     * system_io works around the problem until all the users of
> > -     * old_portion are updated */
> > +    /* Initialize IO regions */
> >      sprintf(namebuf, "%s.io", sphb->dtbusname);
> >      memory_region_init(&sphb->iospace, OBJECT(sphb),
> >                         namebuf, SPAPR_PCI_IO_WIN_SIZE);
> > -    /* FIXME: fix to support multiple PHBs */
> > -    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
> >  
> >      sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
> >      memory_region_init_alias(&sphb->iowindow, OBJECT(sphb), namebuf,
> > -                             get_system_io(), 0, 
SPAPR_PCI_IO_WIN_SIZE);
> > +                             &sphb->iospace, 0, SPAPR_PCI_IO_WIN_SIZE);
> >      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
> >                                  &sphb->iowindow);
> >      /*
> > 
> 
>
Alexey Kardashevskiy Dec. 11, 2013, 6:47 a.m. UTC | #3
On 12/10/2013 06:47 PM, Greg Kurz wrote:
> On Tue, 10 Dec 2013 13:43:05 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>> In the past, IO space could not be mapped into the memory address space
>>> so we introduced a workaround for that. Nowadays it does not look
>>> necessary so we can remove the workaround and make sPAPR PCI
>>> configuration simplier.
>>>
>>> This workaround has also an evil side effect with virtio devices:
>>> because all PHBs have their .io region at the same address, the devices
>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>> when running with KVM and asking for more than one PHB:
>>>
>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>   -hda /local/greg/images/fedora-be.qcow2 \
>>>   -device
>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>> ioeventfd: File exists Aborted
>>>
>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>> since VFIO needs a dedicated PHB to work on ppc.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>>
>> I have not seen this version yet so please remove me from "SOB". The patch
>> you replied to was eventually reworked and went to upstream as
>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>
> 
> Hi Alex,
> 
> I agree you have not seen this version yet... The patch I replied to was my
> primary source of inspiration and contains these bits, hence the SOB. 
> Anyway, the SOB is now removed until you decide to add one yourself. :)
> 
>> This one might be correct too but I want to try this first :)
>>
> 
> Well, I hope it is. Please try it.


Yep. Tried. Looks good, did not break a thing as far as I can tell, even
VGA works :)


Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Alexey Kardashevskiy Dec. 11, 2013, 7:07 a.m. UTC | #4
On 12/11/2013 05:47 PM, Alexey Kardashevskiy wrote:
> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>> On Tue, 10 Dec 2013 13:43:05 +1100
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>> In the past, IO space could not be mapped into the memory address space
>>>> so we introduced a workaround for that. Nowadays it does not look
>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>> configuration simplier.
>>>>
>>>> This workaround has also an evil side effect with virtio devices:
>>>> because all PHBs have their .io region at the same address, the devices
>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>> when running with KVM and asking for more than one PHB:
>>>>
>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>   -hda /local/greg/images/fedora-be.qcow2 \
>>>>   -device
>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>> ioeventfd: File exists Aborted
>>>>
>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>>
>>> I have not seen this version yet so please remove me from "SOB". The patch
>>> you replied to was eventually reworked and went to upstream as
>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>>
>>
>> Hi Alex,
>>
>> I agree you have not seen this version yet... The patch I replied to was my
>> primary source of inspiration and contains these bits, hence the SOB. 
>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>>
>>> This one might be correct too but I want to try this first :)
>>>
>>
>> Well, I hope it is. Please try it.
> 
> 
> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
> VGA works :)
> 
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Hm. Nack. This fails:

./qemu-system-ppc64 \
 -trace "events=qemu_trace_events" \
 -L "qemu-ppc64-bios/" \
 -nographic \
 -vga "none" \
 -device \
 virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
 -drive \
file=virtimg/fc19_16GB.qcow2,if=none,id=drive0,readonly=off,format=qcow2,media=disk,werror=stop,rerror=stop,discard=on
\
 -S \
 -m "2048" \
 -machine "pseries" \
 -enable-kvm




command line: BOOT_IMAGE=/vmlinux-3.10.0-rc7-aik-guest+
root=UUID=27cde746-128e-4528-b4de-44a00d807ea0 ro rd.md=0 rd.lvm=0 rd.dm=0
vconsole.font=latarcyrheb-sun16 vconsole.keymap=us rd.luks=0 console=hvc0 debug
memory layout at init:
  memory_limit : 0000000000000000 (16 MB aligned)
  alloc_bottom : 0000000003400000
  alloc_top    : 0000000030000000
  alloc_top_hi : 0000000080000000
  rmo_top      : 0000000030000000
  ram_top      : 0000000080000000
instantiating rtas at 0x000000002fff0000... done
boot cpu hw idx 0
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x0000000003410000 -> 0x0000000003410817
Device tree struct  0x0000000003420000 -> 0x0000000003430000
[Switching to Thread 0x3fff8ca4eee0 (LWP 10370)]

Breakpoint 8, unassigned_mem_accepts (opaque=0x0, addr=0x10080000052,
size=0x1, is_write=0x1)
    at /home/alexey/p/qemu/memory.c:892
892	    return false;
Missing separate debuginfos, use: debuginfo-install
glusterfs-api-3.4.0-8.fc19.ppc64 glusterfs-libs-3.4.0-8.fc19.ppc64
gnutls-3.1.16-1.fc19.ppc64 keyutils-libs-1.5.6-1.fc19.ppc64
libgcc-4.8.2-1.fc19.ppc64 libgcrypt-1.5.3-2.fc19.ppc64
libibverbs-1.1.7-3.fc19.ppc64 libiscsi-1.7.0-5.fc19.ppc64
libpng-1.5.13-2.fc19.ppc64 librdmacm-1.0.17-1.fc19.ppc64
libusbx-1.0.16-3.fc19.ppc64 systemd-libs-204-17.fc19.ppc64
usbredir-0.6-2.fc19.ppc64
(gdb) bt
#0  unassigned_mem_accepts (opaque=0x0, addr=0x10080000052, size=0x1,
is_write=0x1)
    at /home/alexey/p/qemu/memory.c:892
#1  0x00000000103cb238 in memory_region_access_valid (mr=0x10b76ec8
<io_mem_unassigned>,
    addr=0x10080000052, size=0x1, is_write=0x1) at
/home/alexey/p/qemu/memory.c:928
#2  0x00000000103cb4bc in memory_region_dispatch_write (mr=0x10b76ec8
<io_mem_unassigned>,
    addr=0x10080000052, data=0x80, size=0x1) at
/home/alexey/p/qemu/memory.c:976
#3  0x00000000103cebec in io_mem_write (mr=0x10b76ec8 <io_mem_unassigned>,
addr=0x10080000052,
    val=0x80, size=0x1) at /home/alexey/p/qemu/memory.c:1748
#4  0x0000000010329b54 in address_space_rw (as=0x10b80cf0
<address_space_memory>,
    addr=0x10080000052, buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1)
    at /home/alexey/p/qemu/exec.c:1941
#5  0x000000001032a000 in cpu_physical_memory_rw (addr=0x10080000052,
    buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1) at
/home/alexey/p/qemu/exec.c:2010
#6  0x00000000103231c4 in cpu_physical_memory_write (addr=0x10080000052,
buf=0x3fff8ad9e190,
    len=0x1) at /home/alexey/p/qemu/include/exec/cpu-common.h:68
#7  0x000000001032b5b0 in stb_phys (addr=0x10080000052, val=0x80)
    at /home/alexey/p/qemu/exec.c:2506
#8  0x00000000103a0c10 in h_logical_store (cpu=0x3fff8ada0010,
spapr=0x100118ca410,
    opcode=0x40, args=0x3fff8bfc0030) at
/home/alexey/p/qemu/hw/ppc/spapr_hcall.c:564
#9  0x00000000103a140c in spapr_hypercall (cpu=0x3fff8ada0010, opcode=0x40,
args=0x3fff8bfc0030)
    at /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:737
#10 0x000000001041b080 in kvm_arch_handle_exit (cs=0x3fff8ada0010,
run=0x3fff8bfc0000)
    at /home/alexey/p/qemu/target-ppc/kvm.c:1223
#11 0x00000000103c5cbc in kvm_cpu_exec (cpu=0x3fff8ada0010)
    at /home/alexey/p/qemu/kvm-all.c:1726
#12 0x000000001031902c in qemu_kvm_cpu_thread_fn (arg=0x3fff8ada0010)
    at /home/alexey/p/qemu/cpus.c:874
#13 0x00000080bcd0c29c in start_thread (arg=0x3fff8ad9eee0) at
pthread_create.c:310
#14 0x00000080bcb1ddb0 in .__clone ()
    at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111



Without your patch, unassigned_mem_accepts() is not called so it tells us
that IO stopped working after your patch.

Unfortunately I do not have good 3D imagination, do not understand this
memory region well enough to give advice and cannot tell quickly what
exactly is wrong here :)
Greg Kurz Dec. 17, 2013, 7:52 a.m. UTC | #5
On Wed, 11 Dec 2013 18:07:58 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> Hm. Nack. This fails:
> 
> ./qemu-system-ppc64 \
>  -trace "events=qemu_trace_events" \
>  -L "qemu-ppc64-bios/" \
>  -nographic \
>  -vga "none" \
>  -device \
>  virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
>  -drive \
> file=virtimg/fc19_16GB.qcow2,if=none,id=drive0,readonly=off,format=qcow2,media=disk,werror=stop,rerror=stop,discard=on
> \
>  -S \
>  -m "2048" \
>  -machine "pseries" \
>  -enable-kvm
> 
> 
> 
> 
> command line: BOOT_IMAGE=/vmlinux-3.10.0-rc7-aik-guest+
> root=UUID=27cde746-128e-4528-b4de-44a00d807ea0 ro rd.md=0 rd.lvm=0 rd.dm=0
> vconsole.font=latarcyrheb-sun16 vconsole.keymap=us rd.luks=0 console=hvc0
> debug memory layout at init:
>   memory_limit : 0000000000000000 (16 MB aligned)
>   alloc_bottom : 0000000003400000
>   alloc_top    : 0000000030000000
>   alloc_top_hi : 0000000080000000
>   rmo_top      : 0000000030000000
>   ram_top      : 0000000080000000
> instantiating rtas at 0x000000002fff0000... done
> boot cpu hw idx 0
> copying OF device tree...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x0000000003410000 -> 0x0000000003410817
> Device tree struct  0x0000000003420000 -> 0x0000000003430000
> [Switching to Thread 0x3fff8ca4eee0 (LWP 10370)]
> 
> Breakpoint 8, unassigned_mem_accepts (opaque=0x0, addr=0x10080000052,
> size=0x1, is_write=0x1)
>     at /home/alexey/p/qemu/memory.c:892
> 892	    return false;
> Missing separate debuginfos, use: debuginfo-install
> glusterfs-api-3.4.0-8.fc19.ppc64 glusterfs-libs-3.4.0-8.fc19.ppc64
> gnutls-3.1.16-1.fc19.ppc64 keyutils-libs-1.5.6-1.fc19.ppc64
> libgcc-4.8.2-1.fc19.ppc64 libgcrypt-1.5.3-2.fc19.ppc64
> libibverbs-1.1.7-3.fc19.ppc64 libiscsi-1.7.0-5.fc19.ppc64
> libpng-1.5.13-2.fc19.ppc64 librdmacm-1.0.17-1.fc19.ppc64
> libusbx-1.0.16-3.fc19.ppc64 systemd-libs-204-17.fc19.ppc64
> usbredir-0.6-2.fc19.ppc64
> (gdb) bt
> #0  unassigned_mem_accepts (opaque=0x0, addr=0x10080000052, size=0x1,
> is_write=0x1)
>     at /home/alexey/p/qemu/memory.c:892
> #1  0x00000000103cb238 in memory_region_access_valid (mr=0x10b76ec8
> <io_mem_unassigned>,
>     addr=0x10080000052, size=0x1, is_write=0x1) at
> /home/alexey/p/qemu/memory.c:928
> #2  0x00000000103cb4bc in memory_region_dispatch_write (mr=0x10b76ec8
> <io_mem_unassigned>,
>     addr=0x10080000052, data=0x80, size=0x1) at
> /home/alexey/p/qemu/memory.c:976
> #3  0x00000000103cebec in io_mem_write (mr=0x10b76ec8 <io_mem_unassigned>,
> addr=0x10080000052,
>     val=0x80, size=0x1) at /home/alexey/p/qemu/memory.c:1748
> #4  0x0000000010329b54 in address_space_rw (as=0x10b80cf0
> <address_space_memory>,
>     addr=0x10080000052, buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1)
>     at /home/alexey/p/qemu/exec.c:1941
> #5  0x000000001032a000 in cpu_physical_memory_rw (addr=0x10080000052,
>     buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1) at
> /home/alexey/p/qemu/exec.c:2010
> #6  0x00000000103231c4 in cpu_physical_memory_write (addr=0x10080000052,
> buf=0x3fff8ad9e190,
>     len=0x1) at /home/alexey/p/qemu/include/exec/cpu-common.h:68
> #7  0x000000001032b5b0 in stb_phys (addr=0x10080000052, val=0x80)
>     at /home/alexey/p/qemu/exec.c:2506
> #8  0x00000000103a0c10 in h_logical_store (cpu=0x3fff8ada0010,
> spapr=0x100118ca410,
>     opcode=0x40, args=0x3fff8bfc0030) at
> /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:564
> #9  0x00000000103a140c in spapr_hypercall (cpu=0x3fff8ada0010,
> opcode=0x40, args=0x3fff8bfc0030)
>     at /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:737
> #10 0x000000001041b080 in kvm_arch_handle_exit (cs=0x3fff8ada0010,
> run=0x3fff8bfc0000)
>     at /home/alexey/p/qemu/target-ppc/kvm.c:1223
> #11 0x00000000103c5cbc in kvm_cpu_exec (cpu=0x3fff8ada0010)
>     at /home/alexey/p/qemu/kvm-all.c:1726
> #12 0x000000001031902c in qemu_kvm_cpu_thread_fn (arg=0x3fff8ada0010)
>     at /home/alexey/p/qemu/cpus.c:874
> #13 0x00000080bcd0c29c in start_thread (arg=0x3fff8ad9eee0) at
> pthread_create.c:310
> #14 0x00000080bcb1ddb0 in .__clone ()
>     at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111
> 
> 
> 
> Without your patch, unassigned_mem_accepts() is not called so it tells us
> that IO stopped working after your patch.
> 

Alex,

Can you elaborate ? Does the kernel fail to boot ?

> Unfortunately I do not have good 3D imagination, do not understand this
> memory region well enough to give advice and cannot tell quickly what
> exactly is wrong here :)
> 

Heh no problem. Let me share my findings then ! :)

First, if I pass kernel/initrd/cmdline directly to the qemu command
line, I don't get this weird access to 0x10080000052 at all (with or
without my patch).

Second, if I run the very same test WITHOUT my patch and set a breakpoint
in unassigned_io_write(), it pops with the same 0x10080000052 address.

Third but not least, I have not hit a single issue so far... I mean, when
the kernel is booted, virtio is functional with my patch, as far as I could
test (having / mounted on virtio, multiple 9p shares).

It proves the patch is not responsible for the "unassigned" thing, IMHO.

Thanks for your time anyway.

Cheers.

--
Greg
Alexey Kardashevskiy Dec. 17, 2013, 8:33 a.m. UTC | #6
On 12/17/2013 06:52 PM, Greg Kurz wrote:
> On Wed, 11 Dec 2013 18:07:58 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> Hm. Nack. This fails:
>>
>> ./qemu-system-ppc64 \
>>  -trace "events=qemu_trace_events" \
>>  -L "qemu-ppc64-bios/" \
>>  -nographic \
>>  -vga "none" \
>>  -device \
>>  virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
>>  -drive \
>> file=virtimg/fc19_16GB.qcow2,if=none,id=drive0,readonly=off,format=qcow2,media=disk,werror=stop,rerror=stop,discard=on
>> \
>>  -S \
>>  -m "2048" \
>>  -machine "pseries" \
>>  -enable-kvm
>>
>>
>>
>>
>> command line: BOOT_IMAGE=/vmlinux-3.10.0-rc7-aik-guest+
>> root=UUID=27cde746-128e-4528-b4de-44a00d807ea0 ro rd.md=0 rd.lvm=0 rd.dm=0
>> vconsole.font=latarcyrheb-sun16 vconsole.keymap=us rd.luks=0 console=hvc0
>> debug memory layout at init:
>>   memory_limit : 0000000000000000 (16 MB aligned)
>>   alloc_bottom : 0000000003400000
>>   alloc_top    : 0000000030000000
>>   alloc_top_hi : 0000000080000000
>>   rmo_top      : 0000000030000000
>>   ram_top      : 0000000080000000
>> instantiating rtas at 0x000000002fff0000... done
>> boot cpu hw idx 0
>> copying OF device tree...
>> Building dt strings...
>> Building dt structure...
>> Device tree strings 0x0000000003410000 -> 0x0000000003410817
>> Device tree struct  0x0000000003420000 -> 0x0000000003430000
>> [Switching to Thread 0x3fff8ca4eee0 (LWP 10370)]
>>
>> Breakpoint 8, unassigned_mem_accepts (opaque=0x0, addr=0x10080000052,
>> size=0x1, is_write=0x1)
>>     at /home/alexey/p/qemu/memory.c:892
>> 892	    return false;
>> Missing separate debuginfos, use: debuginfo-install
>> glusterfs-api-3.4.0-8.fc19.ppc64 glusterfs-libs-3.4.0-8.fc19.ppc64
>> gnutls-3.1.16-1.fc19.ppc64 keyutils-libs-1.5.6-1.fc19.ppc64
>> libgcc-4.8.2-1.fc19.ppc64 libgcrypt-1.5.3-2.fc19.ppc64
>> libibverbs-1.1.7-3.fc19.ppc64 libiscsi-1.7.0-5.fc19.ppc64
>> libpng-1.5.13-2.fc19.ppc64 librdmacm-1.0.17-1.fc19.ppc64
>> libusbx-1.0.16-3.fc19.ppc64 systemd-libs-204-17.fc19.ppc64
>> usbredir-0.6-2.fc19.ppc64
>> (gdb) bt
>> #0  unassigned_mem_accepts (opaque=0x0, addr=0x10080000052, size=0x1,
>> is_write=0x1)
>>     at /home/alexey/p/qemu/memory.c:892
>> #1  0x00000000103cb238 in memory_region_access_valid (mr=0x10b76ec8
>> <io_mem_unassigned>,
>>     addr=0x10080000052, size=0x1, is_write=0x1) at
>> /home/alexey/p/qemu/memory.c:928
>> #2  0x00000000103cb4bc in memory_region_dispatch_write (mr=0x10b76ec8
>> <io_mem_unassigned>,
>>     addr=0x10080000052, data=0x80, size=0x1) at
>> /home/alexey/p/qemu/memory.c:976
>> #3  0x00000000103cebec in io_mem_write (mr=0x10b76ec8 <io_mem_unassigned>,
>> addr=0x10080000052,
>>     val=0x80, size=0x1) at /home/alexey/p/qemu/memory.c:1748
>> #4  0x0000000010329b54 in address_space_rw (as=0x10b80cf0
>> <address_space_memory>,
>>     addr=0x10080000052, buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1)
>>     at /home/alexey/p/qemu/exec.c:1941
>> #5  0x000000001032a000 in cpu_physical_memory_rw (addr=0x10080000052,
>>     buf=0x3fff8ad9e190 "\200", len=0x1, is_write=0x1) at
>> /home/alexey/p/qemu/exec.c:2010
>> #6  0x00000000103231c4 in cpu_physical_memory_write (addr=0x10080000052,
>> buf=0x3fff8ad9e190,
>>     len=0x1) at /home/alexey/p/qemu/include/exec/cpu-common.h:68
>> #7  0x000000001032b5b0 in stb_phys (addr=0x10080000052, val=0x80)
>>     at /home/alexey/p/qemu/exec.c:2506
>> #8  0x00000000103a0c10 in h_logical_store (cpu=0x3fff8ada0010,
>> spapr=0x100118ca410,
>>     opcode=0x40, args=0x3fff8bfc0030) at
>> /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:564
>> #9  0x00000000103a140c in spapr_hypercall (cpu=0x3fff8ada0010,
>> opcode=0x40, args=0x3fff8bfc0030)
>>     at /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:737
>> #10 0x000000001041b080 in kvm_arch_handle_exit (cs=0x3fff8ada0010,
>> run=0x3fff8bfc0000)
>>     at /home/alexey/p/qemu/target-ppc/kvm.c:1223
>> #11 0x00000000103c5cbc in kvm_cpu_exec (cpu=0x3fff8ada0010)
>>     at /home/alexey/p/qemu/kvm-all.c:1726
>> #12 0x000000001031902c in qemu_kvm_cpu_thread_fn (arg=0x3fff8ada0010)
>>     at /home/alexey/p/qemu/cpus.c:874
>> #13 0x00000080bcd0c29c in start_thread (arg=0x3fff8ad9eee0) at
>> pthread_create.c:310
>> #14 0x00000080bcb1ddb0 in .__clone ()
>>     at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111
>>
>>
>>
>> Without your patch, unassigned_mem_accepts() is not called so it tells us
>> that IO stopped working after your patch.
>>
> 
> Alex,
> 
> Can you elaborate ? Does the kernel fail to boot ?
> 
>> Unfortunately I do not have good 3D imagination, do not understand this
>> memory region well enough to give advice and cannot tell quickly what
>> exactly is wrong here :)
>>
> 
> Heh no problem. Let me share my findings then ! :)
> 
> First, if I pass kernel/initrd/cmdline directly to the qemu command
> line, I don't get this weird access to 0x10080000052 at all (with or
> without my patch).
> 
> Second, if I run the very same test WITHOUT my patch and set a breakpoint
> in unassigned_io_write(), it pops with the same 0x10080000052 address.


It does not stop there when I try it "WITHOUT my patch", that's my entire
point.

I retried right now, with the upstream QEMU + KVM breakpoint stubs patch.

With your patch - it still 100% reproducible - gdb stops at
unassigned_io_write().

Without your patch there is no stopping in unassigned_io_write().

The exact command line is:
./qemu-system-ppc64 \
	-enable-kvm \
	-m 2048 \
	-L qemu-ppc64-bios/ \
	-machine pseries \
	-trace events=qemu_trace_events \
	-nographic \
	-vga none \
	-drive
id=id0,if=none,readonly=off,werror=stop,rerror=stop,discard=on,file=virtimg/fc19_16GB.qcow2,format=qcow2
\
	-device virtio-blk-pci,id=id1,drive=id0


There is always a chance that you have fixed one bug and make another show
up, this happens sometime, but we do not know for sure that this is the case :)


> Third but not least, I have not hit a single issue so far... I mean, when
> the kernel is booted, virtio is functional with my patch, as far as I could
> test (having / mounted on virtio, multiple 9p shares).


Yes, it is functional in this particular example. However something else
might get broken, something what makes use of IO ports, I do not even know
what it could be (need to test every emulated PCI device with all supported
distros to make sure OR understand what the difference is and fix it).



> It proves the patch is not responsible for the "unassigned" thing, IMHO.
> 
> Thanks for your time anyway.

Welcome :)
Alexander Graf Jan. 2, 2014, 9:04 p.m. UTC | #7
On 11.12.2013, at 07:47, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>> On Tue, 10 Dec 2013 13:43:05 +1100
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>> In the past, IO space could not be mapped into the memory address space
>>>> so we introduced a workaround for that. Nowadays it does not look
>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>> configuration simplier.
>>>> 
>>>> This workaround has also an evil side effect with virtio devices:
>>>> because all PHBs have their .io region at the same address, the devices
>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>> when running with KVM and asking for more than one PHB:
>>>> 
>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>  -hda /local/greg/images/fedora-be.qcow2 \
>>>>  -device
>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>> ioeventfd: File exists Aborted
>>>> 
>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>> 
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> 
>>> 
>>> I have not seen this version yet so please remove me from "SOB". The patch
>>> you replied to was eventually reworked and went to upstream as
>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>> 
>> 
>> Hi Alex,
>> 
>> I agree you have not seen this version yet... The patch I replied to was my
>> primary source of inspiration and contains these bits, hence the SOB. 
>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>> 
>>> This one might be correct too but I want to try this first :)
>>> 
>> 
>> Well, I hope it is. Please try it.
> 
> 
> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
> VGA works :)
> 
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Thanks, applied to ppc-next.


Alex
Alexey Kardashevskiy Jan. 2, 2014, 10:08 p.m. UTC | #8
On 01/03/2014 08:04 AM, Alexander Graf wrote:
> 
> On 11.12.2013, at 07:47, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>>> On Tue, 10 Dec 2013 13:43:05 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>>> In the past, IO space could not be mapped into the memory address space
>>>>> so we introduced a workaround for that. Nowadays it does not look
>>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>>> configuration simplier.
>>>>>
>>>>> This workaround has also an evil side effect with virtio devices:
>>>>> because all PHBs have their .io region at the same address, the devices
>>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>>> when running with KVM and asking for more than one PHB:
>>>>>
>>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>>  -hda /local/greg/images/fedora-be.qcow2 \
>>>>>  -device
>>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>>> ioeventfd: File exists Aborted
>>>>>
>>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>
>>>>
>>>> I have not seen this version yet so please remove me from "SOB". The patch
>>>> you replied to was eventually reworked and went to upstream as
>>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>>>
>>>
>>> Hi Alex,
>>>
>>> I agree you have not seen this version yet... The patch I replied to was my
>>> primary source of inspiration and contains these bits, hence the SOB. 
>>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>>>
>>>> This one might be correct too but I want to try this first :)
>>>>
>>>
>>> Well, I hope it is. Please try it.
>>
>>
>> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
>> VGA works :)
>>
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Thanks, applied to ppc-next.


Please read the rest of this thread. It does not visibly break things but
with this patch QEMU starts calling unassigned_mem_accepts() (normally
silent) which is not a good sign.
Alexander Graf Jan. 2, 2014, 10:09 p.m. UTC | #9
On 02.01.2014, at 23:08, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 01/03/2014 08:04 AM, Alexander Graf wrote:
>> 
>> On 11.12.2013, at 07:47, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> 
>>> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>>>> On Tue, 10 Dec 2013 13:43:05 +1100
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>>>> In the past, IO space could not be mapped into the memory address space
>>>>>> so we introduced a workaround for that. Nowadays it does not look
>>>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>>>> configuration simplier.
>>>>>> 
>>>>>> This workaround has also an evil side effect with virtio devices:
>>>>>> because all PHBs have their .io region at the same address, the devices
>>>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>>>> when running with KVM and asking for more than one PHB:
>>>>>> 
>>>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>>> -hda /local/greg/images/fedora-be.qcow2 \
>>>>>> -device
>>>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>>>> ioeventfd: File exists Aborted
>>>>>> 
>>>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>>>> 
>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> 
>>>>> 
>>>>> I have not seen this version yet so please remove me from "SOB". The patch
>>>>> you replied to was eventually reworked and went to upstream as
>>>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>>>> 
>>>> 
>>>> Hi Alex,
>>>> 
>>>> I agree you have not seen this version yet... The patch I replied to was my
>>>> primary source of inspiration and contains these bits, hence the SOB. 
>>>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>>>> 
>>>>> This one might be correct too but I want to try this first :)
>>>>> 
>>>> 
>>>> Well, I hope it is. Please try it.
>>> 
>>> 
>>> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
>>> VGA works :)
>>> 
>>> 
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> 
>> Thanks, applied to ppc-next.
> 
> 
> Please read the rest of this thread. It does not visibly break things but
> with this patch QEMU starts calling unassigned_mem_accepts() (normally
> silent) which is not a good sign.

Oops, I thought your comment meant this was fixed. I took it off the queue again :).


Alex
Alexey Kardashevskiy Jan. 3, 2014, 7:29 a.m. UTC | #10
On 03.01.2014 9:09, Alexander Graf wrote:
> 
> On 02.01.2014, at 23:08, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 01/03/2014 08:04 AM, Alexander Graf wrote:
>>>
>>> On 11.12.2013, at 07:47, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>>>>> On Tue, 10 Dec 2013 13:43:05 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>>>>> In the past, IO space could not be mapped into the memory address space
>>>>>>> so we introduced a workaround for that. Nowadays it does not look
>>>>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>>>>> configuration simplier.
>>>>>>>
>>>>>>> This workaround has also an evil side effect with virtio devices:
>>>>>>> because all PHBs have their .io region at the same address, the devices
>>>>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>>>>> when running with KVM and asking for more than one PHB:
>>>>>>>
>>>>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>>>> -hda /local/greg/images/fedora-be.qcow2 \
>>>>>>> -device
>>>>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>>>>> ioeventfd: File exists Aborted
>>>>>>>
>>>>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>>>>>
>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>>
>>>>>>
>>>>>> I have not seen this version yet so please remove me from "SOB". The patch
>>>>>> you replied to was eventually reworked and went to upstream as
>>>>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>>>>>
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>> I agree you have not seen this version yet... The patch I replied to was my
>>>>> primary source of inspiration and contains these bits, hence the SOB. 
>>>>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>>>>>
>>>>>> This one might be correct too but I want to try this first :)
>>>>>>
>>>>>
>>>>> Well, I hope it is. Please try it.
>>>>
>>>>
>>>> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
>>>> VGA works :)
>>>>
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>> Thanks, applied to ppc-next.
>>
>>
>> Please read the rest of this thread. It does not visibly break things but
>> with this patch QEMU starts calling unassigned_mem_accepts() (normally
>> silent) which is not a good sign.
> 
> Oops, I thought your comment meant this was fixed. I took it off the queue again :).

Thanks :)

And I have another bug like that - SPR registers are never
saved/restored via KVM's set-one-reg mechanism (with the loop through
all 1024 SPRs) because one_reg_id in SPR's descriptor is never
initialized (the macro misses assignment) and I made a patch for that
but cannot post it as some registers (AMR?) breaks HV KVM :)
Greg Kurz Jan. 6, 2014, 11:12 a.m. UTC | #11
On Fri, 03 Jan 2014 09:08:21 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> Please read the rest of this thread. It does not visibly break things but
> with this patch QEMU starts calling unassigned_mem_accepts() (normally
> silent) which is not a good sign.
> 
> 
> 

Hmm... this is only because this patch moves the PHB io region from the
system IO to the system memory space, but the bogus(?) write to unassigned
memory already exists.

I have tested against the current ppc-next (62d529a), with no
additional patch:

qemu-system-ppc64 \
-snapshot -S -monitor stdio -serial pty \
-nographic -nodefaults \
-machine type=pseries,accel=kvm -smp 1 -m 4G \
-device virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
-drive file=/local/greg/qemu/fedora-be.qcow2,if=none,id=drive0,readonly=off,\
       format=qcow2,media=disk,werror=stop,rerror=stop,discard=on

where fedora-be.qcow2 contains a stock fedora 19 for ppc64.

I have attached gdb to qemu and set a breakpoint in unassigned_io_write(), and
here is what I get again:

(gdb) b unassigned_io_write 
Breakpoint 1 at 0x1045d308: file /home/greg/Work/ibm/linux/qemu-agraf/ioport.c, line 54.
(gdb) c
Continuing.
[Thread 0x1ffffc5deef0 (LWP 11946) exited]
[New Thread 0x1ffffc5deef0 (LWP 11955)]
[Switching to Thread 0x1ffffbdaeef0 (LWP 11947)]

Breakpoint 1, unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
54      {
(gdb) where
#0  unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
#1  0x0000000010468f38 in memory_region_write_accessor (mr=0x10027615940, addr=82, value=0x1ffffbdadd68, size=1, shift=0, mask=255) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:440
#2  0x00000000104690c4 in access_with_adjusted_size (addr=82, value=0x1ffffbdadd68, size=1, access_size_min=1, access_size_max=4, access=@0x107ca670: 0x10468e5c <memory_region_write_accessor>, mr=0x10027615940)
    at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:472
#3  0x000000001046bc64 in memory_region_dispatch_write (mr=0x10027615940, addr=82, data=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:984
#4  0x000000001046fdc4 in io_mem_write (mr=0x10027615940, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:1749
#5  0x00000000103aca0c in address_space_rw (as=0x10c19638 <address_space_memory>, addr=1101659111506, buf=0x1ffffbdae117 "\200", len=1, is_write=true) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2002
#6  0x00000000103acf3c in cpu_physical_memory_rw (addr=1101659111506, buf=0x1ffffbdae117 "\200", len=1, is_write=1) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2071
#7  0x00000000103a44c4 in cpu_physical_memory_write (addr=1101659111506, buf=0x1ffffbdae117, len=1) at /home/greg/Work/ibm/linux/qemu-agraf/include/exec/cpu-common.h:68
#8  0x00000000103aeb2c in stb_phys (addr=1101659111506, val=128) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2600
#9  0x0000000010438550 in h_logical_store (cpu=0x10027d0f0d0, spapr=0x100276bb210, opcode=64, args=0x1ffffff80030) at /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:564
#10 0x0000000010438e74 in spapr_hypercall (cpu=0x10027d0f0d0, opcode=64, args=0x1ffffff80030) at /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:737
#11 0x00000000104cf424 in kvm_arch_handle_exit (cs=0x10027d0f0d0, run=0x1ffffff80000) at /home/greg/Work/ibm/linux/qemu-agraf/target-ppc/kvm.c:1223
#12 0x00000000104648a4 in kvm_cpu_exec (cpu=0x10027d0f0d0) at /home/greg/Work/ibm/linux/qemu-agraf/kvm-all.c:1736
#13 0x0000000010397f00 in qemu_kvm_cpu_thread_fn (arg=0x10027d0f0d0) at /home/greg/Work/ibm/linux/qemu-agraf/cpus.c:874
#14 0x00001fffff92c29c in start_thread (arg=0x1ffffbdaeef0) at pthread_create.c:310
#15 0x00001ffffde5de10 in .__clone ()
at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111

All I can say for the moment, is that I don't get that if I run qemu with
-kernel/-append/-initrd instead of following the grub2 path.

Any clue ?

Thanks.
Alexey Kardashevskiy Jan. 6, 2014, 11:12 p.m. UTC | #12
On 01/06/2014 10:12 PM, Greg Kurz wrote:
> On Fri, 03 Jan 2014 09:08:21 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> Please read the rest of this thread. It does not visibly break things but
>> with this patch QEMU starts calling unassigned_mem_accepts() (normally
>> silent) which is not a good sign.
>>
>>
>>
> 
> Hmm... this is only because this patch moves the PHB io region from the
> system IO to the system memory space, but the bogus(?) write to unassigned
> memory already exists.
> 
> I have tested against the current ppc-next (62d529a), with no
> additional patch:
> 
> qemu-system-ppc64 \
> -snapshot -S -monitor stdio -serial pty \
> -nographic -nodefaults \
> -machine type=pseries,accel=kvm -smp 1 -m 4G \
> -device virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
> -drive file=/local/greg/qemu/fedora-be.qcow2,if=none,id=drive0,readonly=off,\
>        format=qcow2,media=disk,werror=stop,rerror=stop,discard=on
> 
> where fedora-be.qcow2 contains a stock fedora 19 for ppc64.
> 
> I have attached gdb to qemu and set a breakpoint in unassigned_io_write(), and
> here is what I get again:
> 
> (gdb) b unassigned_io_write 
> Breakpoint 1 at 0x1045d308: file /home/greg/Work/ibm/linux/qemu-agraf/ioport.c, line 54.
> (gdb) c
> Continuing.
> [Thread 0x1ffffc5deef0 (LWP 11946) exited]
> [New Thread 0x1ffffc5deef0 (LWP 11955)]
> [Switching to Thread 0x1ffffbdaeef0 (LWP 11947)]
> 
> Breakpoint 1, unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
> 54      {
> (gdb) where
> #0  unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
> #1  0x0000000010468f38 in memory_region_write_accessor (mr=0x10027615940, addr=82, value=0x1ffffbdadd68, size=1, shift=0, mask=255) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:440
> #2  0x00000000104690c4 in access_with_adjusted_size (addr=82, value=0x1ffffbdadd68, size=1, access_size_min=1, access_size_max=4, access=@0x107ca670: 0x10468e5c <memory_region_write_accessor>, mr=0x10027615940)
>     at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:472
> #3  0x000000001046bc64 in memory_region_dispatch_write (mr=0x10027615940, addr=82, data=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:984
> #4  0x000000001046fdc4 in io_mem_write (mr=0x10027615940, addr=82, val=128, size=1) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:1749
> #5  0x00000000103aca0c in address_space_rw (as=0x10c19638 <address_space_memory>, addr=1101659111506, buf=0x1ffffbdae117 "\200", len=1, is_write=true) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2002
> #6  0x00000000103acf3c in cpu_physical_memory_rw (addr=1101659111506, buf=0x1ffffbdae117 "\200", len=1, is_write=1) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2071
> #7  0x00000000103a44c4 in cpu_physical_memory_write (addr=1101659111506, buf=0x1ffffbdae117, len=1) at /home/greg/Work/ibm/linux/qemu-agraf/include/exec/cpu-common.h:68
> #8  0x00000000103aeb2c in stb_phys (addr=1101659111506, val=128) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2600
> #9  0x0000000010438550 in h_logical_store (cpu=0x10027d0f0d0, spapr=0x100276bb210, opcode=64, args=0x1ffffff80030) at /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:564
> #10 0x0000000010438e74 in spapr_hypercall (cpu=0x10027d0f0d0, opcode=64, args=0x1ffffff80030) at /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:737
> #11 0x00000000104cf424 in kvm_arch_handle_exit (cs=0x10027d0f0d0, run=0x1ffffff80000) at /home/greg/Work/ibm/linux/qemu-agraf/target-ppc/kvm.c:1223
> #12 0x00000000104648a4 in kvm_cpu_exec (cpu=0x10027d0f0d0) at /home/greg/Work/ibm/linux/qemu-agraf/kvm-all.c:1736
> #13 0x0000000010397f00 in qemu_kvm_cpu_thread_fn (arg=0x10027d0f0d0) at /home/greg/Work/ibm/linux/qemu-agraf/cpus.c:874
> #14 0x00001fffff92c29c in start_thread (arg=0x1ffffbdaeef0) at pthread_create.c:310
> #15 0x00001ffffde5de10 in .__clone ()
> at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111
> 
> All I can say for the moment, is that I don't get that if I run qemu with
> -kernel/-append/-initrd instead of following the grub2 path.
> 
> Any clue ?


I've got nothing... Can you try without "ioeventfd=on"?
If you post gdb output next time, do "set radix 0x10" first :)
diff mbox

Patch

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 7763149..7d29431 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -564,23 +564,14 @@  static int spapr_phb_init(SysBusDevice *s)
     memory_region_add_subregion(get_system_memory(), sphb->mem_win_addr,
                                 &sphb->memwindow);
 
-    /* On ppc, we only have MMIO no specific IO space from the CPU
-     * perspective.  In theory we ought to be able to embed the PCI IO
-     * memory region direction in the system memory space.  However,
-     * if any of the IO BAR subregions use the old_portio mechanism,
-     * that won't be processed properly unless accessed from the
-     * system io address space.  This hack to bounce things via
-     * system_io works around the problem until all the users of
-     * old_portion are updated */
+    /* Initialize IO regions */
     sprintf(namebuf, "%s.io", sphb->dtbusname);
     memory_region_init(&sphb->iospace, OBJECT(sphb),
                        namebuf, SPAPR_PCI_IO_WIN_SIZE);
-    /* FIXME: fix to support multiple PHBs */
-    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
 
     sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
     memory_region_init_alias(&sphb->iowindow, OBJECT(sphb), namebuf,
-                             get_system_io(), 0, SPAPR_PCI_IO_WIN_SIZE);
+                             &sphb->iospace, 0, SPAPR_PCI_IO_WIN_SIZE);
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
     /*