qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
@ 2013-05-30 15:46 Luiz Capitulino
  2013-05-30 16:03 ` Paolo Bonzini
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-30 15:46 UTC (permalink / raw)
  To: jordan.l.justen; +Cc: pbonzini, qemu-devel, xiaoguangrong

The culprit is commit:

commit 235e8982ad393e5611cb892df54881c872eea9e1
Author: Jordan Justen <jordan.l.justen@intel.com>
Date:   Wed May 29 01:27:26 2013 -0700

    kvm: support using KVM_MEM_READONLY flag for regions

I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
first call to kvm_vm_ioctl().

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 15:46 [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument Luiz Capitulino
@ 2013-05-30 16:03 ` Paolo Bonzini
  2013-05-30 16:08   ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2013-05-30 16:03 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: jordan.l.justen, qemu-devel, xiaoguangrong

Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
> The culprit is commit:
> 
> commit 235e8982ad393e5611cb892df54881c872eea9e1
> Author: Jordan Justen <jordan.l.justen@intel.com>
> Date:   Wed May 29 01:27:26 2013 -0700
> 
>     kvm: support using KVM_MEM_READONLY flag for regions
> 
> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
> first call to kvm_vm_ioctl().
> 

Reproducer?

Paolo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 16:03 ` Paolo Bonzini
@ 2013-05-30 16:08   ` Luiz Capitulino
  2013-05-30 16:50     ` Jordan Justen
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-30 16:08 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: jordan.l.justen, qemu-devel, xiaoguangrong

On Thu, 30 May 2013 18:03:04 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
> > The culprit is commit:
> > 
> > commit 235e8982ad393e5611cb892df54881c872eea9e1
> > Author: Jordan Justen <jordan.l.justen@intel.com>
> > Date:   Wed May 29 01:27:26 2013 -0700
> > 
> >     kvm: support using KVM_MEM_READONLY flag for regions
> > 
> > I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
> > first call to kvm_vm_ioctl().
> > 
> 
> Reproducer?

I just try to start a VM (HEAD 87d23f7):

~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
QEMU 1.5.50 monitor - type 'help' for more information
(qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
~/work/virt/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 16:08   ` Luiz Capitulino
@ 2013-05-30 16:50     ` Jordan Justen
  2013-05-30 17:03       ` Luiz Capitulino
  2013-05-31  6:51       ` Xiao Guangrong
  0 siblings, 2 replies; 21+ messages in thread
From: Jordan Justen @ 2013-05-30 16:50 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Paolo Bonzini, Xiao Guangrong, qemu-devel, Justen, Jordan L

On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Thu, 30 May 2013 18:03:04 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>> > The culprit is commit:
>> >
>> > commit 235e8982ad393e5611cb892df54881c872eea9e1
>> > Author: Jordan Justen <jordan.l.justen@intel.com>
>> > Date:   Wed May 29 01:27:26 2013 -0700
>> >
>> >     kvm: support using KVM_MEM_READONLY flag for regions
>> >
>> > I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>> > first call to kvm_vm_ioctl().

As noted in the code, the first call is for KVM commit 75d61fbc.

I'm not sure we want to fail if an error occurs when making that call.
(I'm pretty sure we don't want to in fact.)

Xiao, any thoughts?

>> Reproducer?
>
> I just try to start a VM (HEAD 87d23f7):
>
> ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
> QEMU 1.5.50 monitor - type 'help' for more information
> (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> ~/work/virt/

Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
to update my kernel.

Does the firmware behave as a ROM for you?

-Jordan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 16:50     ` Jordan Justen
@ 2013-05-30 17:03       ` Luiz Capitulino
  2013-05-30 17:32         ` Jordan Justen
  2013-05-31  6:51       ` Xiao Guangrong
  1 sibling, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-30 17:03 UTC (permalink / raw)
  To: Jordan Justen; +Cc: Paolo Bonzini, Xiao Guangrong, qemu-devel, Justen, Jordan L

On Thu, 30 May 2013 09:50:10 -0700
Jordan Justen <jljusten@gmail.com> wrote:

> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Thu, 30 May 2013 18:03:04 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> >> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
> >> > The culprit is commit:
> >> >
> >> > commit 235e8982ad393e5611cb892df54881c872eea9e1
> >> > Author: Jordan Justen <jordan.l.justen@intel.com>
> >> > Date:   Wed May 29 01:27:26 2013 -0700
> >> >
> >> >     kvm: support using KVM_MEM_READONLY flag for regions
> >> >
> >> > I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
> >> > first call to kvm_vm_ioctl().
> 
> As noted in the code, the first call is for KVM commit 75d61fbc.
> 
> I'm not sure we want to fail if an error occurs when making that call.
> (I'm pretty sure we don't want to in fact.)
> 
> Xiao, any thoughts?
> 
> >> Reproducer?
> >
> > I just try to start a VM (HEAD 87d23f7):
> >
> > ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
> > QEMU 1.5.50 monitor - type 'help' for more information
> > (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> > ~/work/virt/
> 
> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
> to update my kernel.
> 
> Does the firmware behave as a ROM for you?

I think so:

(qemu) info roms
fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
(qemu) 

Is this what you're asking?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 17:03       ` Luiz Capitulino
@ 2013-05-30 17:32         ` Jordan Justen
  2013-05-30 17:56           ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Jordan Justen @ 2013-05-30 17:32 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Paolo Bonzini, Xiao Guangrong, qemu-devel, Justen, Jordan L

On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
<lcapitulino@redhat.com> wrote:
> On Thu, 30 May 2013 09:50:10 -0700
> Jordan Justen <jljusten@gmail.com> wrote:
>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > On Thu, 30 May 2013 18:03:04 +0200
>> > Paolo Bonzini <pbonzini@redhat.com> wrote:
>> >
>> >> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>> >> > The culprit is commit:
>> >> >
>> >> > commit 235e8982ad393e5611cb892df54881c872eea9e1
>> >> > Author: Jordan Justen <jordan.l.justen@intel.com>
>> >> > Date:   Wed May 29 01:27:26 2013 -0700
>> >> >
>> >> >     kvm: support using KVM_MEM_READONLY flag for regions
>> >> >
>> >> > I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>> >> > first call to kvm_vm_ioctl().
>>
>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>
>> I'm not sure we want to fail if an error occurs when making that call.
>> (I'm pretty sure we don't want to in fact.)
>>
>> Xiao, any thoughts?
>>
>> >> Reproducer?
>> >
>> > I just try to start a VM (HEAD 87d23f7):
>> >
>> > ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
>> > QEMU 1.5.50 monitor - type 'help' for more information
>> > (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
>> > ~/work/virt/
>>
>> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
>> to update my kernel.
>>
>> Does the firmware behave as a ROM for you?
>
> I think so:
>
> (qemu) info roms
> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
> addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
> (qemu)
>
> Is this what you're asking?

I guess I was meaning ... if you write to an address such as
0xfffffff0, does it update as RAM, or does it retain the original
value?

This is easy to test in OVMF at the EFI shell, but I'm not sure how
you could easily test it otherwise.

Does the system actually boot for you after the error message?

-Jordan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 17:32         ` Jordan Justen
@ 2013-05-30 17:56           ` Luiz Capitulino
  2013-05-30 18:05             ` Paolo Bonzini
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-30 17:56 UTC (permalink / raw)
  To: Jordan Justen; +Cc: Paolo Bonzini, Xiao Guangrong, qemu-devel, Justen, Jordan L

On Thu, 30 May 2013 10:32:36 -0700
Jordan Justen <jljusten@gmail.com> wrote:

> On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
> <lcapitulino@redhat.com> wrote:
> > On Thu, 30 May 2013 09:50:10 -0700
> > Jordan Justen <jljusten@gmail.com> wrote:
> >> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >> > On Thu, 30 May 2013 18:03:04 +0200
> >> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> >
> >> >> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
> >> >> > The culprit is commit:
> >> >> >
> >> >> > commit 235e8982ad393e5611cb892df54881c872eea9e1
> >> >> > Author: Jordan Justen <jordan.l.justen@intel.com>
> >> >> > Date:   Wed May 29 01:27:26 2013 -0700
> >> >> >
> >> >> >     kvm: support using KVM_MEM_READONLY flag for regions
> >> >> >
> >> >> > I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
> >> >> > first call to kvm_vm_ioctl().
> >>
> >> As noted in the code, the first call is for KVM commit 75d61fbc.
> >>
> >> I'm not sure we want to fail if an error occurs when making that call.
> >> (I'm pretty sure we don't want to in fact.)
> >>
> >> Xiao, any thoughts?
> >>
> >> >> Reproducer?
> >> >
> >> > I just try to start a VM (HEAD 87d23f7):
> >> >
> >> > ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
> >> > QEMU 1.5.50 monitor - type 'help' for more information
> >> > (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> >> > ~/work/virt/
> >>
> >> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
> >> to update my kernel.
> >>
> >> Does the firmware behave as a ROM for you?
> >
> > I think so:
> >
> > (qemu) info roms
> > fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
> > addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
> > (qemu)
> >
> > Is this what you're asking?
> 
> I guess I was meaning ... if you write to an address such as
> 0xfffffff0, does it update as RAM, or does it retain the original
> value?
> 
> This is easy to test in OVMF at the EFI shell, but I'm not sure how
> you could easily test it otherwise.

I could try to hack something, but maybe not today.

> Does the system actually boot for you after the error message?

No, I get an abort. That's what kvm_set_phys_mem() does when
kvm_set_user_memory_region() fails:

(gdb) bt
#0  0x00007f01f8592ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
#1  0x00007f01f8594358 in __GI_abort () at abort.c:90
#2  0x00007f01fedb57f1 in kvm_set_phys_mem (section=0x7f01ee6975d0, add=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:692
#3  0x00007f01fedb5bfa in kvm_region_del (listener=0x7f01ff2bc360 <kvm_memory_listener>, section=0x7f01ee6975d0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:795
#4  0x00007f01fedbc92d in address_space_update_topology_pass (as=0x7f01ffadfa60 <address_space_memory>, old_view=..., new_view=..., adding=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:689
#5  0x00007f01fedbd125 in address_space_update_topology (as=0x7f01ffadfa60 <address_space_memory>) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:725
#6  0x00007f01fedbd29c in memory_region_transaction_commit () at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:750
#7  0x00007f01fec1b977 in i440fx_update_memory_mappings (d=0x7f0200511940) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:131
#8  0x00007f01fec1bab3 in i440fx_write_config (dev=0x7f0200511940, address=94, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:154
#9  0x00007f01fec29a9a in pci_host_config_write_common (pci_dev=0x7f0200511940, addr=94, limit=256, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:54
#10 0x00007f01fec29ba2 in pci_data_write (s=0x7f0200502a50, addr=2147483742, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:75
#11 0x00007f01fec29d5e in pci_host_data_write (opaque=0x7f02004fa1c0, addr=2, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:128
#12 0x00007f01fedbac6e in memory_region_write_accessor (opaque=0x7f02004fc598, addr=2, value=0x7f01ee697a70, size=1, shift=0, mask=255) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:334
#13 0x00007f01fedbad50 in access_with_adjusted_size (addr=2, value=0x7f01ee697a70, size=1, access_size_min=1, access_size_max=4, access=0x7f01fedbabe9 <memory_region_write_accessor>, opaque=0x7f02004fc598)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:364
#14 0x00007f01fedbb1b8 in memory_region_iorange_write (iorange=0x7f0200503550, offset=2, width=1, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:439
#15 0x00007f01fedb33aa in ioport_writeb_thunk (opaque=0x7f0200503550, addr=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:212
#16 0x00007f01fedb2d84 in ioport_write (index=0, address=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:83
#17 0x00007f01fedb3924 in cpu_outb (addr=3326, val=51 '3') at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:289
#18 0x00007f01fedb76c5 in kvm_handle_io (port=3326, data=0x7f01fea6e000, direction=1, size=1, count=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1507
#19 0x00007f01fedb7d59 in kvm_cpu_exec (env=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1659
#20 0x00007f01fed3dc3f in qemu_kvm_cpu_thread_fn (arg=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/cpus.c:759
#21 0x00007f01fcf02d15 in start_thread (arg=0x7f01ee698700) at pthread_create.c:308
#22 0x00007f01f864f48d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114
(gdb) 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 17:56           ` Luiz Capitulino
@ 2013-05-30 18:05             ` Paolo Bonzini
  2013-05-30 20:32               ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2013-05-30 18:05 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Justen, Jordan L, Jordan Justen, qemu-devel, Xiao Guangrong

Il 30/05/2013 19:56, Luiz Capitulino ha scritto:
> On Thu, 30 May 2013 10:32:36 -0700
> Jordan Justen <jljusten@gmail.com> wrote:
> 
>> On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
>> <lcapitulino@redhat.com> wrote:
>>> On Thu, 30 May 2013 09:50:10 -0700
>>> Jordan Justen <jljusten@gmail.com> wrote:
>>>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>>>> On Thu, 30 May 2013 18:03:04 +0200
>>>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>
>>>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>>>> The culprit is commit:
>>>>>>>
>>>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>>>
>>>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>>>
>>>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>>>> first call to kvm_vm_ioctl().
>>>>
>>>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>>>
>>>> I'm not sure we want to fail if an error occurs when making that call.
>>>> (I'm pretty sure we don't want to in fact.)
>>>>
>>>> Xiao, any thoughts?
>>>>
>>>>>> Reproducer?
>>>>>
>>>>> I just try to start a VM (HEAD 87d23f7):
>>>>>
>>>>> ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
>>>>> QEMU 1.5.50 monitor - type 'help' for more information
>>>>> (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
>>>>> ~/work/virt/
>>>>
>>>> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
>>>> to update my kernel.
>>>>
>>>> Does the firmware behave as a ROM for you?
>>>
>>> I think so:
>>>
>>> (qemu) info roms
>>> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
>>> addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
>>> (qemu)
>>>
>>> Is this what you're asking?
>>
>> I guess I was meaning ... if you write to an address such as
>> 0xfffffff0, does it update as RAM, or does it retain the original
>> value?
>>
>> This is easy to test in OVMF at the EFI shell, but I'm not sure how
>> you could easily test it otherwise.
> 
> I could try to hack something, but maybe not today.

Just put a breakpoint on pflash_cfi01_register and see if it is reached.

I cannot reproduce it, but I'm also on 3.8.x.  Will look at it tomorrow.

Paolo

>> Does the system actually boot for you after the error message?
> 
> No, I get an abort. That's what kvm_set_phys_mem() does when
> kvm_set_user_memory_region() fails:
> 
> (gdb) bt
> #0  0x00007f01f8592ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
> #1  0x00007f01f8594358 in __GI_abort () at abort.c:90
> #2  0x00007f01fedb57f1 in kvm_set_phys_mem (section=0x7f01ee6975d0, add=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:692
> #3  0x00007f01fedb5bfa in kvm_region_del (listener=0x7f01ff2bc360 <kvm_memory_listener>, section=0x7f01ee6975d0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:795
> #4  0x00007f01fedbc92d in address_space_update_topology_pass (as=0x7f01ffadfa60 <address_space_memory>, old_view=..., new_view=..., adding=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:689
> #5  0x00007f01fedbd125 in address_space_update_topology (as=0x7f01ffadfa60 <address_space_memory>) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:725
> #6  0x00007f01fedbd29c in memory_region_transaction_commit () at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:750
> #7  0x00007f01fec1b977 in i440fx_update_memory_mappings (d=0x7f0200511940) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:131
> #8  0x00007f01fec1bab3 in i440fx_write_config (dev=0x7f0200511940, address=94, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:154
> #9  0x00007f01fec29a9a in pci_host_config_write_common (pci_dev=0x7f0200511940, addr=94, limit=256, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:54
> #10 0x00007f01fec29ba2 in pci_data_write (s=0x7f0200502a50, addr=2147483742, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:75
> #11 0x00007f01fec29d5e in pci_host_data_write (opaque=0x7f02004fa1c0, addr=2, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:128
> #12 0x00007f01fedbac6e in memory_region_write_accessor (opaque=0x7f02004fc598, addr=2, value=0x7f01ee697a70, size=1, shift=0, mask=255) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:334
> #13 0x00007f01fedbad50 in access_with_adjusted_size (addr=2, value=0x7f01ee697a70, size=1, access_size_min=1, access_size_max=4, access=0x7f01fedbabe9 <memory_region_write_accessor>, opaque=0x7f02004fc598)
>     at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:364
> #14 0x00007f01fedbb1b8 in memory_region_iorange_write (iorange=0x7f0200503550, offset=2, width=1, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:439
> #15 0x00007f01fedb33aa in ioport_writeb_thunk (opaque=0x7f0200503550, addr=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:212
> #16 0x00007f01fedb2d84 in ioport_write (index=0, address=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:83
> #17 0x00007f01fedb3924 in cpu_outb (addr=3326, val=51 '3') at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:289
> #18 0x00007f01fedb76c5 in kvm_handle_io (port=3326, data=0x7f01fea6e000, direction=1, size=1, count=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1507
> #19 0x00007f01fedb7d59 in kvm_cpu_exec (env=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1659
> #20 0x00007f01fed3dc3f in qemu_kvm_cpu_thread_fn (arg=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/cpus.c:759
> #21 0x00007f01fcf02d15 in start_thread (arg=0x7f01ee698700) at pthread_create.c:308
> #22 0x00007f01f864f48d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114
> (gdb) 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 18:05             ` Paolo Bonzini
@ 2013-05-30 20:32               ` Luiz Capitulino
  2013-05-30 21:23                 ` Paolo Bonzini
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-30 20:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Justen, Jordan L, Jordan Justen, qemu-devel, Xiao Guangrong

On Thu, 30 May 2013 20:05:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 30/05/2013 19:56, Luiz Capitulino ha scritto:
> > On Thu, 30 May 2013 10:32:36 -0700
> > Jordan Justen <jljusten@gmail.com> wrote:
> > 
> >> On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
> >> <lcapitulino@redhat.com> wrote:
> >>> On Thu, 30 May 2013 09:50:10 -0700
> >>> Jordan Justen <jljusten@gmail.com> wrote:
> >>>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >>>>> On Thu, 30 May 2013 18:03:04 +0200
> >>>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>>>>
> >>>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
> >>>>>>> The culprit is commit:
> >>>>>>>
> >>>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
> >>>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
> >>>>>>> Date:   Wed May 29 01:27:26 2013 -0700
> >>>>>>>
> >>>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
> >>>>>>>
> >>>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
> >>>>>>> first call to kvm_vm_ioctl().
> >>>>
> >>>> As noted in the code, the first call is for KVM commit 75d61fbc.
> >>>>
> >>>> I'm not sure we want to fail if an error occurs when making that call.
> >>>> (I'm pretty sure we don't want to in fact.)
> >>>>
> >>>> Xiao, any thoughts?
> >>>>
> >>>>>> Reproducer?
> >>>>>
> >>>>> I just try to start a VM (HEAD 87d23f7):
> >>>>>
> >>>>> ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
> >>>>> QEMU 1.5.50 monitor - type 'help' for more information
> >>>>> (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> >>>>> ~/work/virt/
> >>>>
> >>>> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
> >>>> to update my kernel.
> >>>>
> >>>> Does the firmware behave as a ROM for you?
> >>>
> >>> I think so:
> >>>
> >>> (qemu) info roms
> >>> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
> >>> addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
> >>> (qemu)
> >>>
> >>> Is this what you're asking?
> >>
> >> I guess I was meaning ... if you write to an address such as
> >> 0xfffffff0, does it update as RAM, or does it retain the original
> >> value?
> >>
> >> This is easy to test in OVMF at the EFI shell, but I'm not sure how
> >> you could easily test it otherwise.
> > 
> > I could try to hack something, but maybe not today.
> 
> Just put a breakpoint on pflash_cfi01_register and see if it is reached.

Reached on a regular boot right?

It's not reached, with or without the offending commit.

> 
> I cannot reproduce it, but I'm also on 3.8.x.  Will look at it tomorrow.
> 
> Paolo
> 
> >> Does the system actually boot for you after the error message?
> > 
> > No, I get an abort. That's what kvm_set_phys_mem() does when
> > kvm_set_user_memory_region() fails:
> > 
> > (gdb) bt
> > #0  0x00007f01f8592ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
> > #1  0x00007f01f8594358 in __GI_abort () at abort.c:90
> > #2  0x00007f01fedb57f1 in kvm_set_phys_mem (section=0x7f01ee6975d0, add=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:692
> > #3  0x00007f01fedb5bfa in kvm_region_del (listener=0x7f01ff2bc360 <kvm_memory_listener>, section=0x7f01ee6975d0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:795
> > #4  0x00007f01fedbc92d in address_space_update_topology_pass (as=0x7f01ffadfa60 <address_space_memory>, old_view=..., new_view=..., adding=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:689
> > #5  0x00007f01fedbd125 in address_space_update_topology (as=0x7f01ffadfa60 <address_space_memory>) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:725
> > #6  0x00007f01fedbd29c in memory_region_transaction_commit () at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:750
> > #7  0x00007f01fec1b977 in i440fx_update_memory_mappings (d=0x7f0200511940) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:131
> > #8  0x00007f01fec1bab3 in i440fx_write_config (dev=0x7f0200511940, address=94, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:154
> > #9  0x00007f01fec29a9a in pci_host_config_write_common (pci_dev=0x7f0200511940, addr=94, limit=256, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:54
> > #10 0x00007f01fec29ba2 in pci_data_write (s=0x7f0200502a50, addr=2147483742, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:75
> > #11 0x00007f01fec29d5e in pci_host_data_write (opaque=0x7f02004fa1c0, addr=2, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:128
> > #12 0x00007f01fedbac6e in memory_region_write_accessor (opaque=0x7f02004fc598, addr=2, value=0x7f01ee697a70, size=1, shift=0, mask=255) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:334
> > #13 0x00007f01fedbad50 in access_with_adjusted_size (addr=2, value=0x7f01ee697a70, size=1, access_size_min=1, access_size_max=4, access=0x7f01fedbabe9 <memory_region_write_accessor>, opaque=0x7f02004fc598)
> >     at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:364
> > #14 0x00007f01fedbb1b8 in memory_region_iorange_write (iorange=0x7f0200503550, offset=2, width=1, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:439
> > #15 0x00007f01fedb33aa in ioport_writeb_thunk (opaque=0x7f0200503550, addr=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:212
> > #16 0x00007f01fedb2d84 in ioport_write (index=0, address=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:83
> > #17 0x00007f01fedb3924 in cpu_outb (addr=3326, val=51 '3') at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:289
> > #18 0x00007f01fedb76c5 in kvm_handle_io (port=3326, data=0x7f01fea6e000, direction=1, size=1, count=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1507
> > #19 0x00007f01fedb7d59 in kvm_cpu_exec (env=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1659
> > #20 0x00007f01fed3dc3f in qemu_kvm_cpu_thread_fn (arg=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/cpus.c:759
> > #21 0x00007f01fcf02d15 in start_thread (arg=0x7f01ee698700) at pthread_create.c:308
> > #22 0x00007f01f864f48d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114
> > (gdb) 
> > 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 20:32               ` Luiz Capitulino
@ 2013-05-30 21:23                 ` Paolo Bonzini
  2013-05-30 23:43                   ` Jordan Justen
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2013-05-30 21:23 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Justen, Jordan L, Jordan Justen, qemu-devel, Xiao Guangrong

Il 30/05/2013 22:32, Luiz Capitulino ha scritto:
> On Thu, 30 May 2013 20:05:29 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 30/05/2013 19:56, Luiz Capitulino ha scritto:
>>> On Thu, 30 May 2013 10:32:36 -0700
>>> Jordan Justen <jljusten@gmail.com> wrote:
>>>
>>>> On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
>>>> <lcapitulino@redhat.com> wrote:
>>>>> On Thu, 30 May 2013 09:50:10 -0700
>>>>> Jordan Justen <jljusten@gmail.com> wrote:
>>>>>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>>>>>> On Thu, 30 May 2013 18:03:04 +0200
>>>>>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>>>
>>>>>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>>>>>> The culprit is commit:
>>>>>>>>>
>>>>>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>>>>>
>>>>>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>>>>>
>>>>>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>>>>>> first call to kvm_vm_ioctl().
>>>>>>
>>>>>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>>>>>
>>>>>> I'm not sure we want to fail if an error occurs when making that call.
>>>>>> (I'm pretty sure we don't want to in fact.)
>>>>>>
>>>>>> Xiao, any thoughts?
>>>>>>
>>>>>>>> Reproducer?
>>>>>>>
>>>>>>> I just try to start a VM (HEAD 87d23f7):
>>>>>>>
>>>>>>> ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
>>>>>>> QEMU 1.5.50 monitor - type 'help' for more information
>>>>>>> (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
>>>>>>> ~/work/virt/
>>>>>>
>>>>>> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
>>>>>> to update my kernel.
>>>>>>
>>>>>> Does the firmware behave as a ROM for you?
>>>>>
>>>>> I think so:
>>>>>
>>>>> (qemu) info roms
>>>>> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
>>>>> addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
>>>>> (qemu)
>>>>>
>>>>> Is this what you're asking?
>>>>
>>>> I guess I was meaning ... if you write to an address such as
>>>> 0xfffffff0, does it update as RAM, or does it retain the original
>>>> value?
>>>>
>>>> This is easy to test in OVMF at the EFI shell, but I'm not sure how
>>>> you could easily test it otherwise.
>>>
>>> I could try to hack something, but maybe not today.
>>
>> Just put a breakpoint on pflash_cfi01_register and see if it is reached.
> 
> Reached on a regular boot right?
> 
> It's not reached, with or without the offending commit.

Thanks.  Reproduced with 3.9.4.

Paolo

>> I cannot reproduce it, but I'm also on 3.8.x.  Will look at it tomorrow.
>>
>> Paolo
>>
>>>> Does the system actually boot for you after the error message?
>>>
>>> No, I get an abort. That's what kvm_set_phys_mem() does when
>>> kvm_set_user_memory_region() fails:
>>>
>>> (gdb) bt
>>> #0  0x00007f01f8592ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
>>> #1  0x00007f01f8594358 in __GI_abort () at abort.c:90
>>> #2  0x00007f01fedb57f1 in kvm_set_phys_mem (section=0x7f01ee6975d0, add=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:692
>>> #3  0x00007f01fedb5bfa in kvm_region_del (listener=0x7f01ff2bc360 <kvm_memory_listener>, section=0x7f01ee6975d0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:795
>>> #4  0x00007f01fedbc92d in address_space_update_topology_pass (as=0x7f01ffadfa60 <address_space_memory>, old_view=..., new_view=..., adding=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:689
>>> #5  0x00007f01fedbd125 in address_space_update_topology (as=0x7f01ffadfa60 <address_space_memory>) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:725
>>> #6  0x00007f01fedbd29c in memory_region_transaction_commit () at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:750
>>> #7  0x00007f01fec1b977 in i440fx_update_memory_mappings (d=0x7f0200511940) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:131
>>> #8  0x00007f01fec1bab3 in i440fx_write_config (dev=0x7f0200511940, address=94, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:154
>>> #9  0x00007f01fec29a9a in pci_host_config_write_common (pci_dev=0x7f0200511940, addr=94, limit=256, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:54
>>> #10 0x00007f01fec29ba2 in pci_data_write (s=0x7f0200502a50, addr=2147483742, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:75
>>> #11 0x00007f01fec29d5e in pci_host_data_write (opaque=0x7f02004fa1c0, addr=2, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:128
>>> #12 0x00007f01fedbac6e in memory_region_write_accessor (opaque=0x7f02004fc598, addr=2, value=0x7f01ee697a70, size=1, shift=0, mask=255) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:334
>>> #13 0x00007f01fedbad50 in access_with_adjusted_size (addr=2, value=0x7f01ee697a70, size=1, access_size_min=1, access_size_max=4, access=0x7f01fedbabe9 <memory_region_write_accessor>, opaque=0x7f02004fc598)
>>>     at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:364
>>> #14 0x00007f01fedbb1b8 in memory_region_iorange_write (iorange=0x7f0200503550, offset=2, width=1, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:439
>>> #15 0x00007f01fedb33aa in ioport_writeb_thunk (opaque=0x7f0200503550, addr=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:212
>>> #16 0x00007f01fedb2d84 in ioport_write (index=0, address=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:83
>>> #17 0x00007f01fedb3924 in cpu_outb (addr=3326, val=51 '3') at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:289
>>> #18 0x00007f01fedb76c5 in kvm_handle_io (port=3326, data=0x7f01fea6e000, direction=1, size=1, count=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1507
>>> #19 0x00007f01fedb7d59 in kvm_cpu_exec (env=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1659
>>> #20 0x00007f01fed3dc3f in qemu_kvm_cpu_thread_fn (arg=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/cpus.c:759
>>> #21 0x00007f01fcf02d15 in start_thread (arg=0x7f01ee698700) at pthread_create.c:308
>>> #22 0x00007f01f864f48d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114
>>> (gdb) 
>>>
>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 21:23                 ` Paolo Bonzini
@ 2013-05-30 23:43                   ` Jordan Justen
  0 siblings, 0 replies; 21+ messages in thread
From: Jordan Justen @ 2013-05-30 23:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Justen, Jordan L, Xiao Guangrong, qemu-devel, Luiz Capitulino

On Thu, May 30, 2013 at 2:23 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 30/05/2013 22:32, Luiz Capitulino ha scritto:
>> On Thu, 30 May 2013 20:05:29 +0200
>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> Il 30/05/2013 19:56, Luiz Capitulino ha scritto:
>>>> On Thu, 30 May 2013 10:32:36 -0700
>>>> Jordan Justen <jljusten@gmail.com> wrote:
>>>>
>>>>> On Thu, May 30, 2013 at 10:03 AM, Luiz Capitulino
>>>>> <lcapitulino@redhat.com> wrote:
>>>>>> On Thu, 30 May 2013 09:50:10 -0700
>>>>>> Jordan Justen <jljusten@gmail.com> wrote:
>>>>>>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>>>>>>> On Thu, 30 May 2013 18:03:04 +0200
>>>>>>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>>>>
>>>>>>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>>>>>>> The culprit is commit:
>>>>>>>>>>
>>>>>>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>>>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>>>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>>>>>>
>>>>>>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>>>>>>
>>>>>>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>>>>>>> first call to kvm_vm_ioctl().
>>>>>>>
>>>>>>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>>>>>>
>>>>>>> I'm not sure we want to fail if an error occurs when making that call.
>>>>>>> (I'm pretty sure we don't want to in fact.)
>>>>>>>
>>>>>>> Xiao, any thoughts?
>>>>>>>
>>>>>>>>> Reproducer?
>>>>>>>>
>>>>>>>> I just try to start a VM (HEAD 87d23f7):
>>>>>>>>
>>>>>>>> ~/work/virt/ sudo ./qemu-qmp -drive file=disks/test.img,if=virtio,cache=none,aio=native -enable-kvm -m 1G -monitor stdio -cpu host -snapshot
>>>>>>>> QEMU 1.5.50 monitor - type 'help' for more information
>>>>>>>> (qemu) kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
>>>>>>>> ~/work/virt/
>>>>>>>
>>>>>>> Sorry. I am working with Linux 3.8.0, and I don't see this. I'll try
>>>>>>> to update my kernel.
>>>>>>>
>>>>>>> Does the firmware behave as a ROM for you?
>>>>>>
>>>>>> I think so:
>>>>>>
>>>>>> (qemu) info roms
>>>>>> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
>>>>>> addr=00000000fffe0000 size=0x020000 mem=rom name="bios.bin"
>>>>>> (qemu)
>>>>>>
>>>>>> Is this what you're asking?
>>>>>
>>>>> I guess I was meaning ... if you write to an address such as
>>>>> 0xfffffff0, does it update as RAM, or does it retain the original
>>>>> value?
>>>>>
>>>>> This is easy to test in OVMF at the EFI shell, but I'm not sure how
>>>>> you could easily test it otherwise.
>>>>
>>>> I could try to hack something, but maybe not today.
>>>
>>> Just put a breakpoint on pflash_cfi01_register and see if it is reached.
>>
>> Reached on a regular boot right?
>>
>> It's not reached, with or without the offending commit.
>
> Thanks.  Reproduced with 3.9.4.

I have also reproduced it on 3.9.0. I'll look into it.

Is there a bug filed on this?

-Jordan

>>> I cannot reproduce it, but I'm also on 3.8.x.  Will look at it tomorrow.
>>>
>>> Paolo
>>>
>>>>> Does the system actually boot for you after the error message?
>>>>
>>>> No, I get an abort. That's what kvm_set_phys_mem() does when
>>>> kvm_set_user_memory_region() fails:
>>>>
>>>> (gdb) bt
>>>> #0  0x00007f01f8592ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
>>>> #1  0x00007f01f8594358 in __GI_abort () at abort.c:90
>>>> #2  0x00007f01fedb57f1 in kvm_set_phys_mem (section=0x7f01ee6975d0, add=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:692
>>>> #3  0x00007f01fedb5bfa in kvm_region_del (listener=0x7f01ff2bc360 <kvm_memory_listener>, section=0x7f01ee6975d0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:795
>>>> #4  0x00007f01fedbc92d in address_space_update_topology_pass (as=0x7f01ffadfa60 <address_space_memory>, old_view=..., new_view=..., adding=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:689
>>>> #5  0x00007f01fedbd125 in address_space_update_topology (as=0x7f01ffadfa60 <address_space_memory>) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:725
>>>> #6  0x00007f01fedbd29c in memory_region_transaction_commit () at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:750
>>>> #7  0x00007f01fec1b977 in i440fx_update_memory_mappings (d=0x7f0200511940) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:131
>>>> #8  0x00007f01fec1bab3 in i440fx_write_config (dev=0x7f0200511940, address=94, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci-host/piix.c:154
>>>> #9  0x00007f01fec29a9a in pci_host_config_write_common (pci_dev=0x7f0200511940, addr=94, limit=256, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:54
>>>> #10 0x00007f01fec29ba2 in pci_data_write (s=0x7f0200502a50, addr=2147483742, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:75
>>>> #11 0x00007f01fec29d5e in pci_host_data_write (opaque=0x7f02004fa1c0, addr=2, val=51, len=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/hw/pci/pci_host.c:128
>>>> #12 0x00007f01fedbac6e in memory_region_write_accessor (opaque=0x7f02004fc598, addr=2, value=0x7f01ee697a70, size=1, shift=0, mask=255) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:334
>>>> #13 0x00007f01fedbad50 in access_with_adjusted_size (addr=2, value=0x7f01ee697a70, size=1, access_size_min=1, access_size_max=4, access=0x7f01fedbabe9 <memory_region_write_accessor>, opaque=0x7f02004fc598)
>>>>     at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:364
>>>> #14 0x00007f01fedbb1b8 in memory_region_iorange_write (iorange=0x7f0200503550, offset=2, width=1, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/memory.c:439
>>>> #15 0x00007f01fedb33aa in ioport_writeb_thunk (opaque=0x7f0200503550, addr=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:212
>>>> #16 0x00007f01fedb2d84 in ioport_write (index=0, address=3326, data=51) at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:83
>>>> #17 0x00007f01fedb3924 in cpu_outb (addr=3326, val=51 '3') at /home/lcapitulino/work/src/upstream/qmp-unstable/ioport.c:289
>>>> #18 0x00007f01fedb76c5 in kvm_handle_io (port=3326, data=0x7f01fea6e000, direction=1, size=1, count=1) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1507
>>>> #19 0x00007f01fedb7d59 in kvm_cpu_exec (env=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/kvm-all.c:1659
>>>> #20 0x00007f01fed3dc3f in qemu_kvm_cpu_thread_fn (arg=0x7f02004e24e0) at /home/lcapitulino/work/src/upstream/qmp-unstable/cpus.c:759
>>>> #21 0x00007f01fcf02d15 in start_thread (arg=0x7f01ee698700) at pthread_create.c:308
>>>> #22 0x00007f01f864f48d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114
>>>> (gdb)
>>>>
>>>
>>
>>
>>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-30 16:50     ` Jordan Justen
  2013-05-30 17:03       ` Luiz Capitulino
@ 2013-05-31  6:51       ` Xiao Guangrong
  2013-05-31  7:14         ` Jordan Justen
  2013-05-31  8:23         ` Paolo Bonzini
  1 sibling, 2 replies; 21+ messages in thread
From: Xiao Guangrong @ 2013-05-31  6:51 UTC (permalink / raw)
  To: Jordan Justen
  Cc: Paolo Bonzini, qemu-devel, Justen, Jordan L, Luiz Capitulino

On 05/31/2013 12:50 AM, Jordan Justen wrote:
> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> On Thu, 30 May 2013 18:03:04 +0200
>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>> The culprit is commit:
>>>>
>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>
>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>
>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>> first call to kvm_vm_ioctl().
> 
> As noted in the code, the first call is for KVM commit 75d61fbc.
> 
> I'm not sure we want to fail if an error occurs when making that call.
> (I'm pretty sure we don't want to in fact.)
> 
> Xiao, any thoughts?

I have reproduced this bug, it seems that the bug is caused by double free
the memslot. After these change, it can boot the guest now.

diff --git a/kvm-all.c b/kvm-all.c
index 8e7bbf8..405480e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
     if (s->migration_log) {
         mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
     }
-    if (mem.flags & KVM_MEM_READONLY) {
+
+    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
         /* Set the slot size to 0 before setting the slot to the desired
          * value. This is needed based on KVM commit 75d61fbc. */
         mem.memory_size = 0;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-31  6:51       ` Xiao Guangrong
@ 2013-05-31  7:14         ` Jordan Justen
  2013-05-31  8:01           ` Jordan Justen
  2013-05-31  8:23         ` Paolo Bonzini
  1 sibling, 1 reply; 21+ messages in thread
From: Jordan Justen @ 2013-05-31  7:14 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Paolo Bonzini, qemu-devel, Justen, Jordan L, Luiz Capitulino

On Thu, May 30, 2013 at 11:51 PM, Xiao Guangrong
<xiaoguangrong@linux.vnet.ibm.com> wrote:
> On 05/31/2013 12:50 AM, Jordan Justen wrote:
>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> On Thu, 30 May 2013 18:03:04 +0200
>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>> The culprit is commit:
>>>>>
>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>
>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>
>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>> first call to kvm_vm_ioctl().
>>
>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>
>> I'm not sure we want to fail if an error occurs when making that call.
>> (I'm pretty sure we don't want to in fact.)
>>
>> Xiao, any thoughts?
>
> I have reproduced this bug, it seems that the bug is caused by double free
> the memslot. After these change, it can boot the guest now.

I think that I might have found that the flash memory had an issue
with transitioning from flash to rom mode with code like this on the
3.8 kernel. I'll test it on 3.8 & 3.9.

-Jordan

> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-31  7:14         ` Jordan Justen
@ 2013-05-31  8:01           ` Jordan Justen
  0 siblings, 0 replies; 21+ messages in thread
From: Jordan Justen @ 2013-05-31  8:01 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Paolo Bonzini, qemu-devel, Justen, Jordan L, Luiz Capitulino

On Fri, May 31, 2013 at 12:14 AM, Jordan Justen <jljusten@gmail.com> wrote:
> On Thu, May 30, 2013 at 11:51 PM, Xiao Guangrong
> <xiaoguangrong@linux.vnet.ibm.com> wrote:
>> On 05/31/2013 12:50 AM, Jordan Justen wrote:
>>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>>> On Thu, 30 May 2013 18:03:04 +0200
>>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>
>>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>>> The culprit is commit:
>>>>>>
>>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>>
>>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>>
>>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>>> first call to kvm_vm_ioctl().
>>>
>>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>>
>>> I'm not sure we want to fail if an error occurs when making that call.
>>> (I'm pretty sure we don't want to in fact.)
>>>
>>> Xiao, any thoughts?
>>
>> I have reproduced this bug, it seems that the bug is caused by double free
>> the memslot. After these change, it can boot the guest now.
>
> I think that I might have found that the flash memory had an issue
> with transitioning from flash to rom mode with code like this on the
> 3.8 kernel. I'll test it on 3.8 & 3.9.

Yes, the issue still occurs on 3.9, and even the "double free" doesn't
work on 3.9 like it did on 3.8.

This issue is a bit of a corner case for flash, but still important. I
hope maybe Xiao might have time to help me understand it. (I'll email
Xiao off-list about this.)

But, for now, I think Xiao's patch will fix things for the non-flash
case, and will not totally break flash support either.

-Jordan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
  2013-05-31  6:51       ` Xiao Guangrong
  2013-05-31  7:14         ` Jordan Justen
@ 2013-05-31  8:23         ` Paolo Bonzini
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
  1 sibling, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2013-05-31  8:23 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Justen, Jordan L, Jordan Justen, qemu-devel, Luiz Capitulino

Il 31/05/2013 08:51, Xiao Guangrong ha scritto:
> On 05/31/2013 12:50 AM, Jordan Justen wrote:
>> On Thu, May 30, 2013 at 9:08 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> On Thu, 30 May 2013 18:03:04 +0200
>>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>> Il 30/05/2013 17:46, Luiz Capitulino ha scritto:
>>>>> The culprit is commit:
>>>>>
>>>>> commit 235e8982ad393e5611cb892df54881c872eea9e1
>>>>> Author: Jordan Justen <jordan.l.justen@intel.com>
>>>>> Date:   Wed May 29 01:27:26 2013 -0700
>>>>>
>>>>>     kvm: support using KVM_MEM_READONLY flag for regions
>>>>>
>>>>> I'm running 3.9.2-200.fc18, btw. And, error checking is missing on the
>>>>> first call to kvm_vm_ioctl().
>>
>> As noted in the code, the first call is for KVM commit 75d61fbc.
>>
>> I'm not sure we want to fail if an error occurs when making that call.
>> (I'm pretty sure we don't want to in fact.)
>>
>> Xiao, any thoughts?
> 
> I have reproduced this bug, it seems that the bug is caused by double free
> the memslot. After these change, it can boot the guest now.
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;
> 
> 
> 
> 

Thanks, can you submit it for uq/master?  Please Cc kvm@vger.kernel.org
too, and use [PATCH uq/master] as the prefix.

Paolo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:23         ` Paolo Bonzini
@ 2013-05-31  8:52           ` Xiao Guangrong
  2013-05-31 12:27             ` Paolo Bonzini
                               ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Xiao Guangrong @ 2013-05-31  8:52 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gleb Natapov, KVM, Justen, Jordan L, qemu-devel, Luiz Capitulino,
	Jordan Justen

Luiz Capitulino reported that guest refused to boot and qemu
complained with:
kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument

It is caused by commit 235e8982ad that did double free for the memslot
so that the second one raises the -EINVAL error

Fix it by reset memory size only if it is needed

Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 kvm-all.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 8e7bbf8..405480e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
     if (s->migration_log) {
         mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
     }
-    if (mem.flags & KVM_MEM_READONLY) {
+
+    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
         /* Set the slot size to 0 before setting the slot to the desired
          * value. This is needed based on KVM commit 75d61fbc. */
         mem.memory_size = 0;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
@ 2013-05-31 12:27             ` Paolo Bonzini
  2013-05-31 12:39             ` Luiz Capitulino
                               ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2013-05-31 12:27 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Gleb Natapov, KVM, Justen, Jordan L, qemu-devel, Luiz Capitulino,
	Jordan Justen

Il 31/05/2013 10:52, Xiao Guangrong ha scritto:
> Luiz Capitulino reported that guest refused to boot and qemu
> complained with:
> kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> 
> It is caused by commit 235e8982ad that did double free for the memslot
> so that the second one raises the -EINVAL error
> 
> Fix it by reset memory size only if it is needed
> 
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  kvm-all.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
  2013-05-31 12:27             ` Paolo Bonzini
@ 2013-05-31 12:39             ` Luiz Capitulino
  2013-06-02 17:35             ` Richard W.M. Jones
                               ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2013-05-31 12:39 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Gleb Natapov, KVM, Justen, Jordan L, qemu-devel, Paolo Bonzini,
	Jordan Justen

On Fri, 31 May 2013 16:52:18 +0800
Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote:

> Luiz Capitulino reported that guest refused to boot and qemu
> complained with:
> kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> 
> It is caused by commit 235e8982ad that did double free for the memslot
> so that the second one raises the -EINVAL error
> 
> Fix it by reset memory size only if it is needed
> 
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>

Thanks Xiao for the fix, and thanks everyone for debugging this issue!

> ---
>  kvm-all.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
  2013-05-31 12:27             ` Paolo Bonzini
  2013-05-31 12:39             ` Luiz Capitulino
@ 2013-06-02 17:35             ` Richard W.M. Jones
  2013-06-02 22:08             ` Jordan Justen
  2013-06-03  6:57             ` Gleb Natapov
  4 siblings, 0 replies; 21+ messages in thread
From: Richard W.M. Jones @ 2013-06-02 17:35 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Gleb Natapov, KVM, Justen, Jordan L, qemu-devel, Luiz Capitulino,
	Paolo Bonzini, Jordan Justen

On Fri, May 31, 2013 at 04:52:18PM +0800, Xiao Guangrong wrote:
> Luiz Capitulino reported that guest refused to boot and qemu
> complained with:
> kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> 
> It is caused by commit 235e8982ad that did double free for the memslot
> so that the second one raises the -EINVAL error
> 
> Fix it by reset memory size only if it is needed
> 
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  kvm-all.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;

Tested and fixes it for me too.

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
                               ` (2 preceding siblings ...)
  2013-06-02 17:35             ` Richard W.M. Jones
@ 2013-06-02 22:08             ` Jordan Justen
  2013-06-03  6:57             ` Gleb Natapov
  4 siblings, 0 replies; 21+ messages in thread
From: Jordan Justen @ 2013-06-02 22:08 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Gleb Natapov, KVM, Justen, Jordan L, qemu-devel, Luiz Capitulino,
	Paolo Bonzini

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On Fri, May 31, 2013 at 1:52 AM, Xiao Guangrong
<xiaoguangrong@linux.vnet.ibm.com> wrote:
> Luiz Capitulino reported that guest refused to boot and qemu
> complained with:
> kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
>
> It is caused by commit 235e8982ad that did double free for the memslot
> so that the second one raises the -EINVAL error
>
> Fix it by reset memory size only if it is needed
>
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  kvm-all.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;
> --
> 1.7.7.6
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem
  2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
                               ` (3 preceding siblings ...)
  2013-06-02 22:08             ` Jordan Justen
@ 2013-06-03  6:57             ` Gleb Natapov
  4 siblings, 0 replies; 21+ messages in thread
From: Gleb Natapov @ 2013-06-03  6:57 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: KVM, Justen, Jordan L, qemu-devel, Luiz Capitulino, Paolo Bonzini,
	Jordan Justen

On Fri, May 31, 2013 at 04:52:18PM +0800, Xiao Guangrong wrote:
> Luiz Capitulino reported that guest refused to boot and qemu
> complained with:
> kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument
> 
> It is caused by commit 235e8982ad that did double free for the memslot
> so that the second one raises the -EINVAL error
> 
> Fix it by reset memory size only if it is needed
> 
> Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Thanks, applied.

> ---
>  kvm-all.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e7bbf8..405480e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -206,7 +206,8 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      if (s->migration_log) {
>          mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
>      }
> -    if (mem.flags & KVM_MEM_READONLY) {
> +
> +    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
>          /* Set the slot size to 0 before setting the slot to the desired
>           * value. This is needed based on KVM commit 75d61fbc. */
>          mem.memory_size = 0;
> -- 
> 1.7.7.6

--
			Gleb.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-06-03  6:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-30 15:46 [Qemu-devel] [BUG]: kvm_set_phys_mem: error unregistering overlapping slot: Invalid argument Luiz Capitulino
2013-05-30 16:03 ` Paolo Bonzini
2013-05-30 16:08   ` Luiz Capitulino
2013-05-30 16:50     ` Jordan Justen
2013-05-30 17:03       ` Luiz Capitulino
2013-05-30 17:32         ` Jordan Justen
2013-05-30 17:56           ` Luiz Capitulino
2013-05-30 18:05             ` Paolo Bonzini
2013-05-30 20:32               ` Luiz Capitulino
2013-05-30 21:23                 ` Paolo Bonzini
2013-05-30 23:43                   ` Jordan Justen
2013-05-31  6:51       ` Xiao Guangrong
2013-05-31  7:14         ` Jordan Justen
2013-05-31  8:01           ` Jordan Justen
2013-05-31  8:23         ` Paolo Bonzini
2013-05-31  8:52           ` [Qemu-devel] [PATCH uq/master] fix double free the memslot in kvm_set_phys_mem Xiao Guangrong
2013-05-31 12:27             ` Paolo Bonzini
2013-05-31 12:39             ` Luiz Capitulino
2013-06-02 17:35             ` Richard W.M. Jones
2013-06-02 22:08             ` Jordan Justen
2013-06-03  6:57             ` Gleb Natapov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).