* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
2017-11-28 17:43 [Qemu-devel] [PATCH v2] pseries: fix TCG migration Laurent Vivier
@ 2017-11-28 19:26 ` Daniel Henrique Barboza
2017-11-28 23:55 ` Suraj Jitindar Singh
2017-11-28 23:58 ` Suraj Jitindar Singh
2017-11-29 1:04 ` [Qemu-devel] " David Gibson
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Henrique Barboza @ 2017-11-28 19:26 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Bharata B Rao, qemu-ppc, David Gibson
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);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
2017-11-28 19:26 ` [Qemu-devel] [Qemu-ppc] " Daniel Henrique Barboza
@ 2017-11-28 23:55 ` Suraj Jitindar Singh
0 siblings, 0 replies; 6+ messages in thread
From: Suraj Jitindar Singh @ 2017-11-28 23:55 UTC (permalink / raw)
To: Daniel Henrique Barboza, Laurent Vivier, qemu-devel
Cc: David Gibson, qemu-ppc, Bharata B Rao
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);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
2017-11-28 17:43 [Qemu-devel] [PATCH v2] pseries: fix TCG migration Laurent Vivier
2017-11-28 19:26 ` [Qemu-devel] [Qemu-ppc] " Daniel Henrique Barboza
@ 2017-11-28 23:58 ` Suraj Jitindar Singh
2017-11-29 4:39 ` Bharata B Rao
2017-11-29 1:04 ` [Qemu-devel] " David Gibson
2 siblings, 1 reply; 6+ messages in thread
From: Suraj Jitindar Singh @ 2017-11-28 23:58 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Daniel Henrique Barboza, Bharata B Rao, qemu-ppc, David Gibson
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);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pseries: fix TCG migration
2017-11-28 23:58 ` Suraj Jitindar Singh
@ 2017-11-29 4:39 ` Bharata B Rao
0 siblings, 0 replies; 6+ messages in thread
From: Bharata B Rao @ 2017-11-29 4:39 UTC (permalink / raw)
To: Suraj Jitindar Singh
Cc: Laurent Vivier, qemu-devel, Daniel Henrique Barboza, qemu-ppc,
David Gibson
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] pseries: fix TCG migration
2017-11-28 17:43 [Qemu-devel] [PATCH v2] pseries: fix TCG migration Laurent Vivier
2017-11-28 19:26 ` [Qemu-devel] [Qemu-ppc] " Daniel Henrique Barboza
2017-11-28 23:58 ` Suraj Jitindar Singh
@ 2017-11-29 1:04 ` David Gibson
2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2017-11-29 1:04 UTC (permalink / raw)
To: Laurent Vivier
Cc: qemu-devel, qemu-ppc, Bharata B Rao, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread