| Message ID | 20220420065013.222816-23-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gdbserver multi-threaded debugging and POWER9/10 support | expand |
On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin@gmail.com> wrote: > > When POWER9/10 execute an attn instruction, they set a bit in the SPATTN > register and quiesce the thread. > > This bit can be checked to confirm the thread hit an attn instruction, > rather than assuming a thread was quiesced because of attn. This makes > the gdb breakpoint code more robust in the presence of other direct > controls (e.g., host-based direct controls from OPAL). > > This change also clears the SPATTN bit which clears the exception > condition that raises the IPOLL global interrupt. That interrupt seems > to be involved with the IPOLL interrupt storm and lock-up on POWER9 > (although this does not fix it). > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > src/pdbgproxy.c | 79 ++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 68 insertions(+), 11 deletions(-) > > diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c > index 23971c25..d7ceac8d 100644 > --- a/src/pdbgproxy.c > +++ b/src/pdbgproxy.c > @@ -457,6 +457,59 @@ static void v_contc(uint64_t *stack, void *priv) > poll_interval = 1; > } > > +#define P9_SPATTN_AND 0x20010A98 > +#define P9_SPATTN 0x20010A99 > + > +#define P10_SPATTN_AND 0x20028498 > +#define P10_SPATTN 0x20028499 > + > +static bool thread_check_attn(struct pdbg_target *target) > +{ > + struct thread *thread = target_to_thread(target); > + struct pdbg_target *core; > + uint64_t spattn; > + > + if (pdbg_target_compatible(target, "ibm,power8-thread")) { > + return true; /* XXX */ I assume this is ok? Reviewed-by: Joel Stanley <joel@jms.id.au> > + } else if (pdbg_target_compatible(target, "ibm,power9-thread")) { > + core = pdbg_target_require_parent("core", target); > + if (pib_read(core, P9_SPATTN, &spattn)) { > + PR_ERROR("SPATTN read failed\n"); > + return false; > + } > + > + if (spattn & PPC_BIT(1 + 4*thread->id)) { > + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); > + > + if (pib_write(core, P9_SPATTN_AND, mask)) { > + PR_ERROR("SPATTN clear failed\n"); > + return false; > + } > + > + return true; > + } > + } else if (pdbg_target_compatible(target, "ibm,power10-thread")) { > + core = pdbg_target_require_parent("core", target); > + if (pib_read(core, P10_SPATTN, &spattn)) { > + PR_ERROR("SPATTN read failed\n"); > + return false; > + } > + > + if (spattn & PPC_BIT(1 + 4*thread->id)) { > + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); > + > + if (pib_write(core, P10_SPATTN_AND, mask)) { > + PR_ERROR("SPATTN clear failed\n"); > + return false; > + } > + > + return true; > + } > + } > + > + return false; > +} > + > static void interrupt(uint64_t *stack, void *priv) > { > struct thread_state status; > @@ -477,7 +530,6 @@ static void interrupt(uint64_t *stack, void *priv) > > static void poll(void) > { > - uint64_t nia; > struct thread_state status; > > thread_target->probe(thread_target); > @@ -495,17 +547,22 @@ static void poll(void) > > state = IDLE; > poll_interval = VCONT_POLL_DELAY; > - if (!(status.active)) { > - PR_ERROR("Thread inactive after trap\n"); > - send_response(fd, ERROR(EPERM)); > - return; > - } > > - /* Restore NIA */ > - if (thread_getnia(thread_target, &nia)) > - PR_ERROR("Error during getnia\n"); > - if (thread_putnia(thread_target, nia - 4)) > - PR_ERROR("Error during putnia\n"); > + if (thread_check_attn(thread_target)) { > + uint64_t nia; > + > + if (!(status.active)) { > + PR_ERROR("Thread inactive after trap\n"); > + send_response(fd, ERROR(EPERM)); > + return; > + } > + > + /* Restore NIA */ > + if (thread_getnia(thread_target, &nia)) > + PR_ERROR("Error during getnia\n"); > + if (thread_putnia(thread_target, nia - 4)) > + PR_ERROR("Error during putnia\n"); > + } > send_response(fd, TRAP); > break; > } > -- > 2.35.1 > > -- > Pdbg mailing list > Pdbg@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/pdbg
Excerpts from Joel Stanley's message of May 3, 2022 5:10 pm: > On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin@gmail.com> wrote: >> >> When POWER9/10 execute an attn instruction, they set a bit in the SPATTN >> register and quiesce the thread. >> >> This bit can be checked to confirm the thread hit an attn instruction, >> rather than assuming a thread was quiesced because of attn. This makes >> the gdb breakpoint code more robust in the presence of other direct >> controls (e.g., host-based direct controls from OPAL). >> >> This change also clears the SPATTN bit which clears the exception >> condition that raises the IPOLL global interrupt. That interrupt seems >> to be involved with the IPOLL interrupt storm and lock-up on POWER9 >> (although this does not fix it). >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> src/pdbgproxy.c | 79 ++++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 68 insertions(+), 11 deletions(-) >> >> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c >> index 23971c25..d7ceac8d 100644 >> --- a/src/pdbgproxy.c >> +++ b/src/pdbgproxy.c >> @@ -457,6 +457,59 @@ static void v_contc(uint64_t *stack, void *priv) >> poll_interval = 1; >> } >> >> +#define P9_SPATTN_AND 0x20010A98 >> +#define P9_SPATTN 0x20010A99 >> + >> +#define P10_SPATTN_AND 0x20028498 >> +#define P10_SPATTN 0x20028499 >> + >> +static bool thread_check_attn(struct pdbg_target *target) >> +{ >> + struct thread *thread = target_to_thread(target); >> + struct pdbg_target *core; >> + uint64_t spattn; >> + >> + if (pdbg_target_compatible(target, "ibm,power8-thread")) { >> + return true; /* XXX */ > > I assume this is ok? Yeah I'm not sure how P8 checks for this, existing code just assumes it's always true. Thanks, Nick > > Reviewed-by: Joel Stanley <joel@jms.id.au> > >> + } else if (pdbg_target_compatible(target, "ibm,power9-thread")) { >> + core = pdbg_target_require_parent("core", target); >> + if (pib_read(core, P9_SPATTN, &spattn)) { >> + PR_ERROR("SPATTN read failed\n"); >> + return false; >> + } >> + >> + if (spattn & PPC_BIT(1 + 4*thread->id)) { >> + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); >> + >> + if (pib_write(core, P9_SPATTN_AND, mask)) { >> + PR_ERROR("SPATTN clear failed\n"); >> + return false; >> + } >> + >> + return true; >> + } >> + } else if (pdbg_target_compatible(target, "ibm,power10-thread")) { >> + core = pdbg_target_require_parent("core", target); >> + if (pib_read(core, P10_SPATTN, &spattn)) { >> + PR_ERROR("SPATTN read failed\n"); >> + return false; >> + } >> + >> + if (spattn & PPC_BIT(1 + 4*thread->id)) { >> + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); >> + >> + if (pib_write(core, P10_SPATTN_AND, mask)) { >> + PR_ERROR("SPATTN clear failed\n"); >> + return false; >> + } >> + >> + return true; >> + } >> + } >> + >> + return false; >> +} >> + >> static void interrupt(uint64_t *stack, void *priv) >> { >> struct thread_state status; >> @@ -477,7 +530,6 @@ static void interrupt(uint64_t *stack, void *priv) >> >> static void poll(void) >> { >> - uint64_t nia; >> struct thread_state status; >> >> thread_target->probe(thread_target); >> @@ -495,17 +547,22 @@ static void poll(void) >> >> state = IDLE; >> poll_interval = VCONT_POLL_DELAY; >> - if (!(status.active)) { >> - PR_ERROR("Thread inactive after trap\n"); >> - send_response(fd, ERROR(EPERM)); >> - return; >> - } >> >> - /* Restore NIA */ >> - if (thread_getnia(thread_target, &nia)) >> - PR_ERROR("Error during getnia\n"); >> - if (thread_putnia(thread_target, nia - 4)) >> - PR_ERROR("Error during putnia\n"); >> + if (thread_check_attn(thread_target)) { >> + uint64_t nia; >> + >> + if (!(status.active)) { >> + PR_ERROR("Thread inactive after trap\n"); >> + send_response(fd, ERROR(EPERM)); >> + return; >> + } >> + >> + /* Restore NIA */ >> + if (thread_getnia(thread_target, &nia)) >> + PR_ERROR("Error during getnia\n"); >> + if (thread_putnia(thread_target, nia - 4)) >> + PR_ERROR("Error during putnia\n"); >> + } >> send_response(fd, TRAP); >> break; >> } >> -- >> 2.35.1 >> >> -- >> Pdbg mailing list >> Pdbg@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/pdbg >
diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c index 23971c25..d7ceac8d 100644 --- a/src/pdbgproxy.c +++ b/src/pdbgproxy.c @@ -457,6 +457,59 @@ static void v_contc(uint64_t *stack, void *priv) poll_interval = 1; } +#define P9_SPATTN_AND 0x20010A98 +#define P9_SPATTN 0x20010A99 + +#define P10_SPATTN_AND 0x20028498 +#define P10_SPATTN 0x20028499 + +static bool thread_check_attn(struct pdbg_target *target) +{ + struct thread *thread = target_to_thread(target); + struct pdbg_target *core; + uint64_t spattn; + + if (pdbg_target_compatible(target, "ibm,power8-thread")) { + return true; /* XXX */ + } else if (pdbg_target_compatible(target, "ibm,power9-thread")) { + core = pdbg_target_require_parent("core", target); + if (pib_read(core, P9_SPATTN, &spattn)) { + PR_ERROR("SPATTN read failed\n"); + return false; + } + + if (spattn & PPC_BIT(1 + 4*thread->id)) { + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); + + if (pib_write(core, P9_SPATTN_AND, mask)) { + PR_ERROR("SPATTN clear failed\n"); + return false; + } + + return true; + } + } else if (pdbg_target_compatible(target, "ibm,power10-thread")) { + core = pdbg_target_require_parent("core", target); + if (pib_read(core, P10_SPATTN, &spattn)) { + PR_ERROR("SPATTN read failed\n"); + return false; + } + + if (spattn & PPC_BIT(1 + 4*thread->id)) { + uint64_t mask = ~PPC_BIT(1 + 4*thread->id); + + if (pib_write(core, P10_SPATTN_AND, mask)) { + PR_ERROR("SPATTN clear failed\n"); + return false; + } + + return true; + } + } + + return false; +} + static void interrupt(uint64_t *stack, void *priv) { struct thread_state status; @@ -477,7 +530,6 @@ static void interrupt(uint64_t *stack, void *priv) static void poll(void) { - uint64_t nia; struct thread_state status; thread_target->probe(thread_target); @@ -495,17 +547,22 @@ static void poll(void) state = IDLE; poll_interval = VCONT_POLL_DELAY; - if (!(status.active)) { - PR_ERROR("Thread inactive after trap\n"); - send_response(fd, ERROR(EPERM)); - return; - } - /* Restore NIA */ - if (thread_getnia(thread_target, &nia)) - PR_ERROR("Error during getnia\n"); - if (thread_putnia(thread_target, nia - 4)) - PR_ERROR("Error during putnia\n"); + if (thread_check_attn(thread_target)) { + uint64_t nia; + + if (!(status.active)) { + PR_ERROR("Thread inactive after trap\n"); + send_response(fd, ERROR(EPERM)); + return; + } + + /* Restore NIA */ + if (thread_getnia(thread_target, &nia)) + PR_ERROR("Error during getnia\n"); + if (thread_putnia(thread_target, nia - 4)) + PR_ERROR("Error during putnia\n"); + } send_response(fd, TRAP); break; }
When POWER9/10 execute an attn instruction, they set a bit in the SPATTN register and quiesce the thread. This bit can be checked to confirm the thread hit an attn instruction, rather than assuming a thread was quiesced because of attn. This makes the gdb breakpoint code more robust in the presence of other direct controls (e.g., host-based direct controls from OPAL). This change also clears the SPATTN bit which clears the exception condition that raises the IPOLL global interrupt. That interrupt seems to be involved with the IPOLL interrupt storm and lock-up on POWER9 (although this does not fix it). Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- src/pdbgproxy.c | 79 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 11 deletions(-)