[Pdbg] [PATCH v2 22/39] gdbserver: check for attn using the SPATTN register

Nicholas Piggin npiggin at gmail.com
Tue May 10 19:50:29 AEST 2022


Excerpts from Joel Stanley's message of May 3, 2022 5:10 pm:
> On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin at 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 at 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 at 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 at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/pdbg
> 


More information about the Pdbg mailing list