* [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET
@ 2017-08-08 17:59 Nicholas Piggin
2017-08-09 4:05 ` David Gibson
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Piggin @ 2017-08-08 17:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Nicholas Piggin, qemu-ppc, David Gibson
Unicast H_SIGNAL_SYS_RESET does not find the target CPU if it
is not the current CPU.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Unfortunately this slipped through without my noticing because the
Linux driver for NMI IPIs has a fallback to using regular IPIs, and
because Linux did not make much use of unicasts. A new watchdog
has started using them. After this patch, this function works properly:
Watchdog CPU:0 detected Hard LOCKUP other CPUS:3
*** Unicast NMI IPI is sent here ***
Watchdog CPU:3 Hard LOCKUP
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc3-00305-ge84cf82ae73a-dirty #1191
task: c00000001e440000 task.stack: c00000001e480000
NIP: c000000000023800 LR: c00000000000da28 CTR: c000000000626db0
REGS: c00000001ff97d80 TRAP: 0100 Not tainted (4.13.0-rc3-00305-ge84cf82ae73a-dirty)
MSR: 8000000002001033 <SF,VEC,ME,IR,DR,RI,LE>
CR: 48000224 XER: 20000000
CFAR: c00000000002380c SOFTE: 0
GPR00: c00000000000d9cc c00000001e483dc0 c000000000ebd900 000000000007d000
GPR04: f000000000078680 c00000001e1a0048 c00000001e1a0048 0000000000000001
GPR08: 0000000000000000 0000000000075036 00000003d5f633c2 0000000000000020
GPR12: 0000000000000000 c00000000fd80f00 c00000000000d988 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000b60
NIP [c000000000023800] udelay+0x40/0x60
LR [c00000000000da28] kernel_init+0xa8/0x1b0
Call Trace:
[c00000001e483dc0] [c00000000000d9cc] kernel_init+0x4c/0x1b0 (unreliable)
[c00000001e483e30] [c00000000000bb1c] ret_from_kernel_thread+0x5c/0xc0
Instruction dump:
7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840 409d0028 48000014
60000000 60000000 60000000 60420000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
hw/ppc/spapr_hcall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 72ea5a8247..f50e979b43 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1432,7 +1432,9 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
} else {
/* Unicast */
CPU_FOREACH(cs) {
- if (cpu->cpu_dt_id == target) {
+ PowerPCCPU *c = POWERPC_CPU(cs);
+
+ if (c->cpu_dt_id == target) {
run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
return H_SUCCESS;
}
--
2.13.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET
2017-08-08 17:59 [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET Nicholas Piggin
@ 2017-08-09 4:05 ` David Gibson
2017-08-09 5:07 ` Nicholas Piggin
0 siblings, 1 reply; 4+ messages in thread
From: David Gibson @ 2017-08-09 4:05 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: qemu-devel, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]
On Wed, Aug 09, 2017 at 03:59:36AM +1000, Nicholas Piggin wrote:
> Unicast H_SIGNAL_SYS_RESET does not find the target CPU if it
> is not the current CPU.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> Unfortunately this slipped through without my noticing because the
> Linux driver for NMI IPIs has a fallback to using regular IPIs, and
> because Linux did not make much use of unicasts. A new watchdog
> has started using them. After this patch, this function works
> properly:
In fact this bug was already fixed in the for-2.11 branch. If you've
hit this for real, I guess it's more important than I realized, so
I'll pull it into 2.10 instead.
>
> Watchdog CPU:0 detected Hard LOCKUP other CPUS:3
> *** Unicast NMI IPI is sent here ***
> Watchdog CPU:3 Hard LOCKUP
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc3-00305-ge84cf82ae73a-dirty #1191
> task: c00000001e440000 task.stack: c00000001e480000
> NIP: c000000000023800 LR: c00000000000da28 CTR: c000000000626db0
> REGS: c00000001ff97d80 TRAP: 0100 Not tainted (4.13.0-rc3-00305-ge84cf82ae73a-dirty)
> MSR: 8000000002001033 <SF,VEC,ME,IR,DR,RI,LE>
> CR: 48000224 XER: 20000000
> CFAR: c00000000002380c SOFTE: 0
> GPR00: c00000000000d9cc c00000001e483dc0 c000000000ebd900 000000000007d000
> GPR04: f000000000078680 c00000001e1a0048 c00000001e1a0048 0000000000000001
> GPR08: 0000000000000000 0000000000075036 00000003d5f633c2 0000000000000020
> GPR12: 0000000000000000 c00000000fd80f00 c00000000000d988 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000b60
> NIP [c000000000023800] udelay+0x40/0x60
> LR [c00000000000da28] kernel_init+0xa8/0x1b0
> Call Trace:
> [c00000001e483dc0] [c00000000000d9cc] kernel_init+0x4c/0x1b0 (unreliable)
> [c00000001e483e30] [c00000000000bb1c] ret_from_kernel_thread+0x5c/0xc0
> Instruction dump:
> 7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840 409d0028 48000014
> 60000000 60000000 60000000 60420000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
>
> hw/ppc/spapr_hcall.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 72ea5a8247..f50e979b43 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1432,7 +1432,9 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> } else {
> /* Unicast */
> CPU_FOREACH(cs) {
> - if (cpu->cpu_dt_id == target) {
> + PowerPCCPU *c = POWERPC_CPU(cs);
> +
> + if (c->cpu_dt_id == target) {
> run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> return H_SUCCESS;
> }
--
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] 4+ messages in thread* Re: [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET
2017-08-09 4:05 ` David Gibson
@ 2017-08-09 5:07 ` Nicholas Piggin
2017-08-09 5:45 ` David Gibson
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Piggin @ 2017-08-09 5:07 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-devel, qemu-ppc
On Wed, 9 Aug 2017 14:05:46 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Aug 09, 2017 at 03:59:36AM +1000, Nicholas Piggin wrote:
> > Unicast H_SIGNAL_SYS_RESET does not find the target CPU if it
> > is not the current CPU.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >
> > Unfortunately this slipped through without my noticing because the
> > Linux driver for NMI IPIs has a fallback to using regular IPIs, and
> > because Linux did not make much use of unicasts. A new watchdog
> > has started using them. After this patch, this function works
> > properly:
>
> In fact this bug was already fixed in the for-2.11 branch. If you've
> hit this for real, I guess it's more important than I realized, so
> I'll pull it into 2.10 instead.
Oh sorry I didn't notice the 2.11 branch fix. Yes please pull it
into 2.10 if possible.
Thanks,
Nick
>
> >
> > Watchdog CPU:0 detected Hard LOCKUP other CPUS:3
> > *** Unicast NMI IPI is sent here ***
> > Watchdog CPU:3 Hard LOCKUP
> > Modules linked in:
> > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc3-00305-ge84cf82ae73a-dirty #1191
> > task: c00000001e440000 task.stack: c00000001e480000
> > NIP: c000000000023800 LR: c00000000000da28 CTR: c000000000626db0
> > REGS: c00000001ff97d80 TRAP: 0100 Not tainted (4.13.0-rc3-00305-ge84cf82ae73a-dirty)
> > MSR: 8000000002001033 <SF,VEC,ME,IR,DR,RI,LE>
> > CR: 48000224 XER: 20000000
> > CFAR: c00000000002380c SOFTE: 0
> > GPR00: c00000000000d9cc c00000001e483dc0 c000000000ebd900 000000000007d000
> > GPR04: f000000000078680 c00000001e1a0048 c00000001e1a0048 0000000000000001
> > GPR08: 0000000000000000 0000000000075036 00000003d5f633c2 0000000000000020
> > GPR12: 0000000000000000 c00000000fd80f00 c00000000000d988 0000000000000000
> > GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000b60
> > NIP [c000000000023800] udelay+0x40/0x60
> > LR [c00000000000da28] kernel_init+0xa8/0x1b0
> > Call Trace:
> > [c00000001e483dc0] [c00000000000d9cc] kernel_init+0x4c/0x1b0 (unreliable)
> > [c00000001e483e30] [c00000000000bb1c] ret_from_kernel_thread+0x5c/0xc0
> > Instruction dump:
> > 7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840 409d0028 48000014
> > 60000000 60000000 60000000 60420000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
> >
> > hw/ppc/spapr_hcall.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 72ea5a8247..f50e979b43 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1432,7 +1432,9 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> > } else {
> > /* Unicast */
> > CPU_FOREACH(cs) {
> > - if (cpu->cpu_dt_id == target) {
> > + PowerPCCPU *c = POWERPC_CPU(cs);
> > +
> > + if (c->cpu_dt_id == target) {
> > run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > return H_SUCCESS;
> > }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET
2017-08-09 5:07 ` Nicholas Piggin
@ 2017-08-09 5:45 ` David Gibson
0 siblings, 0 replies; 4+ messages in thread
From: David Gibson @ 2017-08-09 5:45 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: qemu-devel, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]
On Wed, Aug 09, 2017 at 03:07:19PM +1000, Nicholas Piggin wrote:
> On Wed, 9 Aug 2017 14:05:46 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > On Wed, Aug 09, 2017 at 03:59:36AM +1000, Nicholas Piggin wrote:
> > > Unicast H_SIGNAL_SYS_RESET does not find the target CPU if it
> > > is not the current CPU.
> > >
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >
> > > Unfortunately this slipped through without my noticing because the
> > > Linux driver for NMI IPIs has a fallback to using regular IPIs, and
> > > because Linux did not make much use of unicasts. A new watchdog
> > > has started using them. After this patch, this function works
> > > properly:
> >
> > In fact this bug was already fixed in the for-2.11 branch. If you've
> > hit this for real, I guess it's more important than I realized, so
> > I'll pull it into 2.10 instead.
>
> Oh sorry I didn't notice the 2.11 branch fix. Yes please pull it
> into 2.10 if possible.
Already pulled into my ppc-for-2.10 tree, I'm preparing a pull request
at this moment.
>
> Thanks,
> Nick
>
> >
> > >
> > > Watchdog CPU:0 detected Hard LOCKUP other CPUS:3
> > > *** Unicast NMI IPI is sent here ***
> > > Watchdog CPU:3 Hard LOCKUP
> > > Modules linked in:
> > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc3-00305-ge84cf82ae73a-dirty #1191
> > > task: c00000001e440000 task.stack: c00000001e480000
> > > NIP: c000000000023800 LR: c00000000000da28 CTR: c000000000626db0
> > > REGS: c00000001ff97d80 TRAP: 0100 Not tainted (4.13.0-rc3-00305-ge84cf82ae73a-dirty)
> > > MSR: 8000000002001033 <SF,VEC,ME,IR,DR,RI,LE>
> > > CR: 48000224 XER: 20000000
> > > CFAR: c00000000002380c SOFTE: 0
> > > GPR00: c00000000000d9cc c00000001e483dc0 c000000000ebd900 000000000007d000
> > > GPR04: f000000000078680 c00000001e1a0048 c00000001e1a0048 0000000000000001
> > > GPR08: 0000000000000000 0000000000075036 00000003d5f633c2 0000000000000020
> > > GPR12: 0000000000000000 c00000000fd80f00 c00000000000d988 0000000000000000
> > > GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > > GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > > GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > > GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000b60
> > > NIP [c000000000023800] udelay+0x40/0x60
> > > LR [c00000000000da28] kernel_init+0xa8/0x1b0
> > > Call Trace:
> > > [c00000001e483dc0] [c00000000000d9cc] kernel_init+0x4c/0x1b0 (unreliable)
> > > [c00000001e483e30] [c00000000000bb1c] ret_from_kernel_thread+0x5c/0xc0
> > > Instruction dump:
> > > 7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840 409d0028 48000014
> > > 60000000 60000000 60000000 60420000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
> > >
> > > hw/ppc/spapr_hcall.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index 72ea5a8247..f50e979b43 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -1432,7 +1432,9 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> > > } else {
> > > /* Unicast */
> > > CPU_FOREACH(cs) {
> > > - if (cpu->cpu_dt_id == target) {
> > > + PowerPCCPU *c = POWERPC_CPU(cs);
> > > +
> > > + if (c->cpu_dt_id == target) {
> > > run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > > return H_SUCCESS;
> > > }
> >
>
--
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] 4+ messages in thread
end of thread, other threads:[~2017-08-09 5:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-08 17:59 [Qemu-devel] [PATCH] ppc/spapr: fix unicast H_SIGNAL_SYS_RESET Nicholas Piggin
2017-08-09 4:05 ` David Gibson
2017-08-09 5:07 ` Nicholas Piggin
2017-08-09 5:45 ` David Gibson
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).