1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2] pseries: fix TCG migration

Laurent Vivier posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171128174310.15700-1-lvivier@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
hw/ppc/spapr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH v2] pseries: fix TCG migration
Posted by Laurent Vivier 6 years, 4 months ago 
Migration of pseries is broken with TCG because
QEMU tries to restore KVM MMU state unconditionally.

The result is a SIGSEGV in kvm_vm_ioctl():

  #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
      at qemu/accel/kvm/kvm-all.c:2032
  #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu (cpu=<optimized out>,
      radix=<optimized out>, gtse=<optimized out>, proc_tbl=<optimized out>)
      at qemu/target/ppc/kvm.c:396
  #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
      version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
  #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
      vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
      version_id=<optimized out>) at qemu/migration/vmstate.c:165
  #4  0x00000001005987e0 in vmstate_load (f=<optimized out>, se=<optimized out>)
      at qemu/migration/savevm.c:748

This patch fixes the problem by not calling the KVM function with the
TCG mode.

Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2: fix the comment to keep GDB backtrace lines starting with '#'

 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9efddeaee5..a471de6cab 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1570,7 +1570,7 @@ static int spapr_post_load(void *opaque, int version_id)
         err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
     }
 
-    if (spapr->patb_entry) {
+    if (kvm_enabled() && spapr->patb_entry) {
         PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
         bool radix = !!(spapr->patb_entry & PATBE1_GR);
         bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
-- 
2.13.6
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
Posted by Daniel Henrique Barboza 6 years, 4 months ago 

On 11/28/2017 03:43 PM, Laurent Vivier wrote:
> Migration of pseries is broken with TCG because
> QEMU tries to restore KVM MMU state unconditionally.
>
> The result is a SIGSEGV in kvm_vm_ioctl():
>
>    #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
>        at qemu/accel/kvm/kvm-all.c:2032
>    #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu (cpu=<optimized out>,
>        radix=<optimized out>, gtse=<optimized out>, proc_tbl=<optimized out>)
>        at qemu/target/ppc/kvm.c:396
>    #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
>        version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
>    #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
>        vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
>        version_id=<optimized out>) at qemu/migration/vmstate.c:165
>    #4  0x00000001005987e0 in vmstate_load (f=<optimized out>, se=<optimized out>)
>        at qemu/migration/savevm.c:748
>
> This patch fixes the problem by not calling the KVM function with the
> TCG mode.
>
> Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: fix the comment to keep GDB backtrace lines starting with '#'
>
>   hw/ppc/spapr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9efddeaee5..a471de6cab 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1570,7 +1570,7 @@ static int spapr_post_load(void *opaque, int version_id)
>           err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
>       }
>
> -    if (spapr->patb_entry) {
> +    if (kvm_enabled() && spapr->patb_entry) {

As an alternative, I believe you can also do:

if (!!spapr->patb_entry) {

because the !! will return false if patb_entry is either undefined (as in
TCG) or zero (as set in spapr_reallocate_hpt). This is the same logic used
in spapr_patb_entry_needed too.

But I am fine with what you did and it's also clearer, so


Reviewed-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>



>           PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>           bool radix = !!(spapr->patb_entry & PATBE1_GR);
>           bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
Posted by Suraj Jitindar Singh 6 years, 4 months ago 
On Tue, 2017-11-28 at 17:26 -0200, Daniel Henrique Barboza wrote:
> 
> On 11/28/2017 03:43 PM, Laurent Vivier wrote:
> > Migration of pseries is broken with TCG because
> > QEMU tries to restore KVM MMU state unconditionally.
> > 
> > The result is a SIGSEGV in kvm_vm_ioctl():
> > 
> >    #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
> >        at qemu/accel/kvm/kvm-all.c:2032
> >    #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu
> > (cpu=<optimized out>,
> >        radix=<optimized out>, gtse=<optimized out>,
> > proc_tbl=<optimized out>)
> >        at qemu/target/ppc/kvm.c:396
> >    #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
> >        version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
> >    #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
> >        vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
> >        version_id=<optimized out>) at qemu/migration/vmstate.c:165
> >    #4  0x00000001005987e0 in vmstate_load (f=<optimized out>,
> > se=<optimized out>)
> >        at qemu/migration/savevm.c:748
> > 
> > This patch fixes the problem by not calling the KVM function with
> > the
> > TCG mode.
> > 
> > Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> > v2: fix the comment to keep GDB backtrace lines starting with '#'
> > 
> >   hw/ppc/spapr.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 9efddeaee5..a471de6cab 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1570,7 +1570,7 @@ static int spapr_post_load(void *opaque, int
> > version_id)
> >           err = spapr_rtc_import_offset(&spapr->rtc, spapr-
> > >rtc_offset);
> >       }
> > 
> > -    if (spapr->patb_entry) {
> > +    if (kvm_enabled() && spapr->patb_entry) {
> 
> As an alternative, I believe you can also do:
> 
> if (!!spapr->patb_entry) {
> 
> because the !! will return false if patb_entry is either undefined
> (as in

Hmmm, no. patb_entry is definately used in tcg

> TCG) or zero (as set in spapr_reallocate_hpt). This is the same logic
> used
> in spapr_patb_entry_needed too.
> 
> But I am fine with what you did and it's also clearer, so
> 
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> 
> 
> 
> >           PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> >           bool radix = !!(spapr->patb_entry & PATBE1_GR);
> >           bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
> 
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
Posted by Suraj Jitindar Singh 6 years, 4 months ago 
On Tue, 2017-11-28 at 18:43 +0100, Laurent Vivier wrote:
> Migration of pseries is broken with TCG because
> QEMU tries to restore KVM MMU state unconditionally.
> 
> The result is a SIGSEGV in kvm_vm_ioctl():
> 
>   #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
>       at qemu/accel/kvm/kvm-all.c:2032
>   #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu (cpu=<optimized
> out>,
>       radix=<optimized out>, gtse=<optimized out>,
> proc_tbl=<optimized out>)
>       at qemu/target/ppc/kvm.c:396
>   #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
>       version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
>   #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
>       vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
>       version_id=<optimized out>) at qemu/migration/vmstate.c:165
>   #4  0x00000001005987e0 in vmstate_load (f=<optimized out>,
> se=<optimized out>)
>       at qemu/migration/savevm.c:748
> 
> This patch fixes the problem by not calling the KVM function with the
> TCG mode.
> 
> Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Ah, guess I never hit this because I never tested tcg migration with a
qemu compiled on a kvm enabled system. Nice catch :)

Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

> ---
> v2: fix the comment to keep GDB backtrace lines starting with '#'
> 
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9efddeaee5..a471de6cab 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1570,7 +1570,7 @@ static int spapr_post_load(void *opaque, int
> version_id)
>          err = spapr_rtc_import_offset(&spapr->rtc, spapr-
> >rtc_offset);
>      }
>  
> -    if (spapr->patb_entry) {
> +    if (kvm_enabled() && spapr->patb_entry) {
>          PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>          bool radix = !!(spapr->patb_entry & PATBE1_GR);
>          bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
Posted by Bharata B Rao 6 years, 4 months ago 
On Wed, Nov 29, 2017 at 10:58:29AM +1100, Suraj Jitindar Singh wrote:
> On Tue, 2017-11-28 at 18:43 +0100, Laurent Vivier wrote:
> > Migration of pseries is broken with TCG because
> > QEMU tries to restore KVM MMU state unconditionally.
> > 
> > The result is a SIGSEGV in kvm_vm_ioctl():
> > 
> >   #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
> >       at qemu/accel/kvm/kvm-all.c:2032
> >   #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu (cpu=<optimized
> > out>,
> >       radix=<optimized out>, gtse=<optimized out>,
> > proc_tbl=<optimized out>)
> >       at qemu/target/ppc/kvm.c:396
> >   #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
> >       version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
> >   #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
> >       vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
> >       version_id=<optimized out>) at qemu/migration/vmstate.c:165
> >   #4  0x00000001005987e0 in vmstate_load (f=<optimized out>,
> > se=<optimized out>)
> >       at qemu/migration/savevm.c:748
> > 
> > This patch fixes the problem by not calling the KVM function with the
> > TCG mode.
> > 
> > Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Ah, guess I never hit this because I never tested tcg migration with a
> qemu compiled on a kvm enabled system. Nice catch :)

Same here. In fact I had the kvm_enabled() check in the initial verions
but removed later as kvmppc_configure_v3_mmu() was defined separately
as nop for !CONFIG_KVM. But obviously the above combination wasn't covered.

Regards,
Bharata.
Re: [Qemu-devel] [PATCH v2] pseries: fix TCG migration
Posted by David Gibson 6 years, 4 months ago 
On Tue, Nov 28, 2017 at 06:43:10PM +0100, Laurent Vivier wrote:
> Migration of pseries is broken with TCG because
> QEMU tries to restore KVM MMU state unconditionally.
> 
> The result is a SIGSEGV in kvm_vm_ioctl():
> 
>   #0  kvm_vm_ioctl (s=0x0, type=-2146390353)
>       at qemu/accel/kvm/kvm-all.c:2032
>   #1  0x00000001003e3e2c in kvmppc_configure_v3_mmu (cpu=<optimized out>,
>       radix=<optimized out>, gtse=<optimized out>, proc_tbl=<optimized out>)
>       at qemu/target/ppc/kvm.c:396
>   #2  0x00000001002f8b88 in spapr_post_load (opaque=0x1019103c0,
>       version_id=<optimized out>) at qemu/hw/ppc/spapr.c:1578
>   #3  0x000000010059e4cc in vmstate_load_state (f=0x106230000,
>       vmsd=0x1009479e0 <vmstate_spapr>, opaque=0x1019103c0,
>       version_id=<optimized out>) at qemu/migration/vmstate.c:165
>   #4  0x00000001005987e0 in vmstate_load (f=<optimized out>, se=<optimized out>)
>       at qemu/migration/savevm.c:748
> 
> This patch fixes the problem by not calling the KVM function with the
> TCG mode.
> 
> Fixes: d39c90f5f3 ("spapr: Fix migration of Radix guests")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: fix the comment to keep GDB backtrace lines starting with '#'

Applied to ppc-for-2.11.

> 
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9efddeaee5..a471de6cab 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1570,7 +1570,7 @@ static int spapr_post_load(void *opaque, int version_id)
>          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
>      }
>  
> -    if (spapr->patb_entry) {
> +    if (kvm_enabled() && spapr->patb_entry) {
>          PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>          bool radix = !!(spapr->patb_entry & PATBE1_GR);
>          bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.