[v5] scripts/gdb: add lx_current support for riscv

  csr_sscratch CSR holds current task_struct address when hart is in
user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
"tp" with expected user mode value and place current task_struct address
again in csr_sscratch CSR.

This patch assumes "tp" is pointing to task_struct. If value in
csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
is correct address of current task_struct. This logic holds when
   - hart is in user space, "tp" will be less than csr_sscratch.
   - hart is in kernel space but not in trap handler, "tp" will be more
     than csr_sscratch (csr_sscratch being equal to 0).
   - hart is executing trap handler
       - "tp" is still pointing to user mode but csr_sscratch contains
          ptr to task_struct. Thus numerically higher.
       - "tp" is  pointing to task_struct but csr_sscratch now contains
          either 0 or numerically smaller value (transiently holds
          user mode tp)

Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

---
Since patch has changed a little bit from v1 and I didn't include
changelog earlier, here it is.

v1 --> v2:
 - added logic to locate task_struct irrespective of priv
 - made locating task_struct agnostic to bitness(32 vs 64).
 - added caching of ulong type in scripts/gdb/linux/utils.py
 - added more descriptive commit message

v2 --> v3:
 - amended commit message and source line to fit column width

v3 --> v4:
 - amended commit message and remove whitespace in source
 - added Reviewed-by for reviewers

v4 --> v5:
 - changing the order of changelog and sign off/review tags in commit
---
---
 scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
 scripts/gdb/linux/utils.py |  5 +++++
 2 files changed, 20 insertions(+)
  

On Tue, Nov 15, 2022 at 02:10:51PM -0800, Deepak Gupta wrote:
> csr_sscratch CSR holds current task_struct address when hart is in
> user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
> register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
> "tp" with expected user mode value and place current task_struct address
> again in csr_sscratch CSR.
> 
> This patch assumes "tp" is pointing to task_struct. If value in
> csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
> is correct address of current task_struct. This logic holds when
>    - hart is in user space, "tp" will be less than csr_sscratch.
>    - hart is in kernel space but not in trap handler, "tp" will be more
>      than csr_sscratch (csr_sscratch being equal to 0).
>    - hart is executing trap handler
>        - "tp" is still pointing to user mode but csr_sscratch contains
>           ptr to task_struct. Thus numerically higher.
>        - "tp" is  pointing to task_struct but csr_sscratch now contains
>           either 0 or numerically smaller value (transiently holds
>           user mode tp)
> 
> Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> 
> ---
> Since patch has changed a little bit from v1 and I didn't include
> changelog earlier, here it is.
> 
> v1 --> v2:
>  - added logic to locate task_struct irrespective of priv
>  - made locating task_struct agnostic to bitness(32 vs 64).
>  - added caching of ulong type in scripts/gdb/linux/utils.py
>  - added more descriptive commit message
> 
> v2 --> v3:
>  - amended commit message and source line to fit column width
> 
> v3 --> v4:
>  - amended commit message and remove whitespace in source
>  - added Reviewed-by for reviewers
> 
> v4 --> v5:
>  - changing the order of changelog and sign off/review tags in commit
> ---
> ---

Everything looks good, but you've got extra ---'s here. They don't hurt,
but if you're still ironing out your workflow you may want to keep in
mind that you don't need them. You only need one, which goes above the
changelog.

Thanks,
drew

>  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
>  scripts/gdb/linux/utils.py |  5 +++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 15fc4626d236..14c22f82449b 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -173,6 +173,21 @@ def get_current_task(cpu):
>           else:
>               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
>                                  "while running in userspace(EL0)")
> +    elif utils.is_target_arch("riscv"):
> +         current_tp = gdb.parse_and_eval("$tp")
> +         scratch_reg = gdb.parse_and_eval("$sscratch")
> +
> +         # by default tp points to current task
> +         current_task = current_tp.cast(task_ptr_type)
> +
> +         # scratch register is set 0 in trap handler after entering kernel.
> +         # When hart is in user mode, scratch register is pointing to task_struct.
> +         # and tp is used by user mode. So when scratch register holds larger value
> +         # (negative address as ulong is larger value) than tp, then use scratch register.
> +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> +             current_task = scratch_reg.cast(task_ptr_type)
> +
> +         return current_task.dereference()
>      else:
>          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
>                             "supported with this arch")
> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> index 1553f68716cc..ddaf3089170d 100644
> --- a/scripts/gdb/linux/utils.py
> +++ b/scripts/gdb/linux/utils.py
> @@ -35,12 +35,17 @@ class CachedType:
>  
>  
>  long_type = CachedType("long")
> +ulong_type = CachedType("ulong")
>  atomic_long_type = CachedType("atomic_long_t")
>  
>  def get_long_type():
>      global long_type
>      return long_type.get_type()
>  
> +def get_ulong_type():
> +    global ulong_type
> +    return ulong_type.get_type()
> +
>  def offset_of(typeobj, field):
>      element = gdb.Value(0).cast(typeobj)
>      return int(str(element[field].address).split()[0], 16)
> -- 
> 2.25.1
>
  

On Wed, Nov 16, 2022 at 09:16:49AM +0100, Andrew Jones wrote:
>On Tue, Nov 15, 2022 at 02:10:51PM -0800, Deepak Gupta wrote:
>> csr_sscratch CSR holds current task_struct address when hart is in
>> user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
>> register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
>> "tp" with expected user mode value and place current task_struct address
>> again in csr_sscratch CSR.
>>
>> This patch assumes "tp" is pointing to task_struct. If value in
>> csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
>> is correct address of current task_struct. This logic holds when
>>    - hart is in user space, "tp" will be less than csr_sscratch.
>>    - hart is in kernel space but not in trap handler, "tp" will be more
>>      than csr_sscratch (csr_sscratch being equal to 0).
>>    - hart is executing trap handler
>>        - "tp" is still pointing to user mode but csr_sscratch contains
>>           ptr to task_struct. Thus numerically higher.
>>        - "tp" is  pointing to task_struct but csr_sscratch now contains
>>           either 0 or numerically smaller value (transiently holds
>>           user mode tp)
>>
>> Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>>
>> ---
>> Since patch has changed a little bit from v1 and I didn't include
>> changelog earlier, here it is.
>>
>> v1 --> v2:
>>  - added logic to locate task_struct irrespective of priv
>>  - made locating task_struct agnostic to bitness(32 vs 64).
>>  - added caching of ulong type in scripts/gdb/linux/utils.py
>>  - added more descriptive commit message
>>
>> v2 --> v3:
>>  - amended commit message and source line to fit column width
>>
>> v3 --> v4:
>>  - amended commit message and remove whitespace in source
>>  - added Reviewed-by for reviewers
>>
>> v4 --> v5:
>>  - changing the order of changelog and sign off/review tags in commit
>> ---
>> ---
>
>Everything looks good, but you've got extra ---'s here. They don't hurt,
>but if you're still ironing out your workflow you may want to keep in
>mind that you don't need them. You only need one, which goes above the
>changelog.
>
>Thanks,
>drew
>

Noted. Thanks.

>>  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
>>  scripts/gdb/linux/utils.py |  5 +++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
>> index 15fc4626d236..14c22f82449b 100644
>> --- a/scripts/gdb/linux/cpus.py
>> +++ b/scripts/gdb/linux/cpus.py
>> @@ -173,6 +173,21 @@ def get_current_task(cpu):
>>           else:
>>               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
>>                                  "while running in userspace(EL0)")
>> +    elif utils.is_target_arch("riscv"):
>> +         current_tp = gdb.parse_and_eval("$tp")
>> +         scratch_reg = gdb.parse_and_eval("$sscratch")
>> +
>> +         # by default tp points to current task
>> +         current_task = current_tp.cast(task_ptr_type)
>> +
>> +         # scratch register is set 0 in trap handler after entering kernel.
>> +         # When hart is in user mode, scratch register is pointing to task_struct.
>> +         # and tp is used by user mode. So when scratch register holds larger value
>> +         # (negative address as ulong is larger value) than tp, then use scratch register.
>> +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
>> +             current_task = scratch_reg.cast(task_ptr_type)
>> +
>> +         return current_task.dereference()
>>      else:
>>          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
>>                             "supported with this arch")
>> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
>> index 1553f68716cc..ddaf3089170d 100644
>> --- a/scripts/gdb/linux/utils.py
>> +++ b/scripts/gdb/linux/utils.py
>> @@ -35,12 +35,17 @@ class CachedType:
>>
>>
>>  long_type = CachedType("long")
>> +ulong_type = CachedType("ulong")
>>  atomic_long_type = CachedType("atomic_long_t")
>>
>>  def get_long_type():
>>      global long_type
>>      return long_type.get_type()
>>
>> +def get_ulong_type():
>> +    global ulong_type
>> +    return ulong_type.get_type()
>> +
>>  def offset_of(typeobj, field):
>>      element = gdb.Value(0).cast(typeobj)
>>      return int(str(element[field].address).split()[0], 16)
>> --
>> 2.25.1
>>
  

On Tue, 15 Nov 2022 14:10:51 PST (-0800), debug@rivosinc.com wrote:
> csr_sscratch CSR holds current task_struct address when hart is in
> user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
> register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
> "tp" with expected user mode value and place current task_struct address
> again in csr_sscratch CSR.
>
> This patch assumes "tp" is pointing to task_struct. If value in
> csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
> is correct address of current task_struct. This logic holds when
>    - hart is in user space, "tp" will be less than csr_sscratch.
>    - hart is in kernel space but not in trap handler, "tp" will be more
>      than csr_sscratch (csr_sscratch being equal to 0).
>    - hart is executing trap handler
>        - "tp" is still pointing to user mode but csr_sscratch contains
>           ptr to task_struct. Thus numerically higher.
>        - "tp" is  pointing to task_struct but csr_sscratch now contains
>           either 0 or numerically smaller value (transiently holds
>           user mode tp)
>
> Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> ---
> Since patch has changed a little bit from v1 and I didn't include
> changelog earlier, here it is.
>
> v1 --> v2:
>  - added logic to locate task_struct irrespective of priv
>  - made locating task_struct agnostic to bitness(32 vs 64).
>  - added caching of ulong type in scripts/gdb/linux/utils.py
>  - added more descriptive commit message
>
> v2 --> v3:
>  - amended commit message and source line to fit column width
>
> v3 --> v4:
>  - amended commit message and remove whitespace in source
>  - added Reviewed-by for reviewers
>
> v4 --> v5:
>  - changing the order of changelog and sign off/review tags in commit
> ---
> ---
>  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
>  scripts/gdb/linux/utils.py |  5 +++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 15fc4626d236..14c22f82449b 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -173,6 +173,21 @@ def get_current_task(cpu):
>           else:
>               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
>                                  "while running in userspace(EL0)")
> +    elif utils.is_target_arch("riscv"):
> +         current_tp = gdb.parse_and_eval("$tp")
> +         scratch_reg = gdb.parse_and_eval("$sscratch")
> +
> +         # by default tp points to current task
> +         current_task = current_tp.cast(task_ptr_type)
> +
> +         # scratch register is set 0 in trap handler after entering kernel.
> +         # When hart is in user mode, scratch register is pointing to task_struct.
> +         # and tp is used by user mode. So when scratch register holds larger value
> +         # (negative address as ulong is larger value) than tp, then use scratch register.
> +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> +             current_task = scratch_reg.cast(task_ptr_type)
> +
> +         return current_task.dereference()
>      else:
>          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
>                             "supported with this arch")
> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> index 1553f68716cc..ddaf3089170d 100644
> --- a/scripts/gdb/linux/utils.py
> +++ b/scripts/gdb/linux/utils.py
> @@ -35,12 +35,17 @@ class CachedType:
>
>
>  long_type = CachedType("long")
> +ulong_type = CachedType("ulong")
>  atomic_long_type = CachedType("atomic_long_t")
>
>  def get_long_type():
>      global long_type
>      return long_type.get_type()
>
> +def get_ulong_type():
> +    global ulong_type
> +    return ulong_type.get_type()
> +
>  def offset_of(typeobj, field):
>      element = gdb.Value(0).cast(typeobj)
>      return int(str(element[field].address).split()[0], 16)

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

Thanks!
  

On 15.11.22 23:10, Deepak Gupta wrote:
> csr_sscratch CSR holds current task_struct address when hart is in
> user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
> register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
> "tp" with expected user mode value and place current task_struct address
> again in csr_sscratch CSR.
> 
> This patch assumes "tp" is pointing to task_struct. If value in
> csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
> is correct address of current task_struct. This logic holds when
>    - hart is in user space, "tp" will be less than csr_sscratch.
>    - hart is in kernel space but not in trap handler, "tp" will be more
>      than csr_sscratch (csr_sscratch being equal to 0).
>    - hart is executing trap handler
>        - "tp" is still pointing to user mode but csr_sscratch contains
>           ptr to task_struct. Thus numerically higher.
>        - "tp" is  pointing to task_struct but csr_sscratch now contains
>           either 0 or numerically smaller value (transiently holds
>           user mode tp)
> 
> Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> 
> ---
> Since patch has changed a little bit from v1 and I didn't include
> changelog earlier, here it is.
> 
> v1 --> v2:
>  - added logic to locate task_struct irrespective of priv
>  - made locating task_struct agnostic to bitness(32 vs 64).
>  - added caching of ulong type in scripts/gdb/linux/utils.py
>  - added more descriptive commit message
> 
> v2 --> v3:
>  - amended commit message and source line to fit column width
> 
> v3 --> v4:
>  - amended commit message and remove whitespace in source
>  - added Reviewed-by for reviewers
> 
> v4 --> v5:
>  - changing the order of changelog and sign off/review tags in commit
> ---
> ---
>  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
>  scripts/gdb/linux/utils.py |  5 +++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 15fc4626d236..14c22f82449b 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -173,6 +173,21 @@ def get_current_task(cpu):
>           else:
>               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
>                                  "while running in userspace(EL0)")
> +    elif utils.is_target_arch("riscv"):
> +         current_tp = gdb.parse_and_eval("$tp")
> +         scratch_reg = gdb.parse_and_eval("$sscratch")
> +
> +         # by default tp points to current task
> +         current_task = current_tp.cast(task_ptr_type)
> +
> +         # scratch register is set 0 in trap handler after entering kernel.
> +         # When hart is in user mode, scratch register is pointing to task_struct.
> +         # and tp is used by user mode. So when scratch register holds larger value
> +         # (negative address as ulong is larger value) than tp, then use scratch register.
> +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> +             current_task = scratch_reg.cast(task_ptr_type)

Why not if-else for the assignment here?

> +
> +         return current_task.dereference()
>      else:
>          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
>                             "supported with this arch")
> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> index 1553f68716cc..ddaf3089170d 100644
> --- a/scripts/gdb/linux/utils.py
> +++ b/scripts/gdb/linux/utils.py
> @@ -35,12 +35,17 @@ class CachedType:
>  
>  
>  long_type = CachedType("long")
> +ulong_type = CachedType("ulong")
>  atomic_long_type = CachedType("atomic_long_t")
>  
>  def get_long_type():
>      global long_type
>      return long_type.get_type()
>  
> +def get_ulong_type():
> +    global ulong_type
> +    return ulong_type.get_type()
> +
>  def offset_of(typeobj, field):
>      element = gdb.Value(0).cast(typeobj)
>      return int(str(element[field].address).split()[0], 16)

Looks good to me otherwise.

Jan
  

On Mon, Jan 02, 2023 at 10:09:01AM +0100, Jan Kiszka wrote:
> On 15.11.22 23:10, Deepak Gupta wrote:
> > csr_sscratch CSR holds current task_struct address when hart is in
> > user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
> > register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
> > "tp" with expected user mode value and place current task_struct address
> > again in csr_sscratch CSR.
> > 
> > This patch assumes "tp" is pointing to task_struct. If value in
> > csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
> > is correct address of current task_struct. This logic holds when
> >    - hart is in user space, "tp" will be less than csr_sscratch.
> >    - hart is in kernel space but not in trap handler, "tp" will be more
> >      than csr_sscratch (csr_sscratch being equal to 0).
> >    - hart is executing trap handler
> >        - "tp" is still pointing to user mode but csr_sscratch contains
> >           ptr to task_struct. Thus numerically higher.
> >        - "tp" is  pointing to task_struct but csr_sscratch now contains
> >           either 0 or numerically smaller value (transiently holds
> >           user mode tp)
> > 
> > Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
> > 
> > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > 
> > ---
> > Since patch has changed a little bit from v1 and I didn't include
> > changelog earlier, here it is.
> > 
> > v1 --> v2:
> >  - added logic to locate task_struct irrespective of priv
> >  - made locating task_struct agnostic to bitness(32 vs 64).
> >  - added caching of ulong type in scripts/gdb/linux/utils.py
> >  - added more descriptive commit message
> > 
> > v2 --> v3:
> >  - amended commit message and source line to fit column width
> > 
> > v3 --> v4:
> >  - amended commit message and remove whitespace in source
> >  - added Reviewed-by for reviewers
> > 
> > v4 --> v5:
> >  - changing the order of changelog and sign off/review tags in commit
> > ---
> > ---
> >  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
> >  scripts/gdb/linux/utils.py |  5 +++++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> > index 15fc4626d236..14c22f82449b 100644
> > --- a/scripts/gdb/linux/cpus.py
> > +++ b/scripts/gdb/linux/cpus.py
> > @@ -173,6 +173,21 @@ def get_current_task(cpu):
> >           else:
> >               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
> >                                  "while running in userspace(EL0)")
> > +    elif utils.is_target_arch("riscv"):
> > +         current_tp = gdb.parse_and_eval("$tp")
> > +         scratch_reg = gdb.parse_and_eval("$sscratch")
> > +
> > +         # by default tp points to current task
> > +         current_task = current_tp.cast(task_ptr_type)
> > +
> > +         # scratch register is set 0 in trap handler after entering kernel.
> > +         # When hart is in user mode, scratch register is pointing to task_struct.
> > +         # and tp is used by user mode. So when scratch register holds larger value
> > +         # (negative address as ulong is larger value) than tp, then use scratch register.
> > +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> > +             current_task = scratch_reg.cast(task_ptr_type)
> 
> Why not if-else for the assignment here?
> 
> > +
> > +         return current_task.dereference()
> >      else:
> >          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
> >                             "supported with this arch")
> > diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> > index 1553f68716cc..ddaf3089170d 100644
> > --- a/scripts/gdb/linux/utils.py
> > +++ b/scripts/gdb/linux/utils.py
> > @@ -35,12 +35,17 @@ class CachedType:
> >  
> >  
> >  long_type = CachedType("long")
> > +ulong_type = CachedType("ulong")
> >  atomic_long_type = CachedType("atomic_long_t")
> >  
> >  def get_long_type():
> >      global long_type
> >      return long_type.get_type()
> >  
> > +def get_ulong_type():
> > +    global ulong_type
> > +    return ulong_type.get_type()
> > +
> >  def offset_of(typeobj, field):
> >      element = gdb.Value(0).cast(typeobj)
> >      return int(str(element[field].address).split()[0], 16)
> 
> Looks good to me otherwise.
> 
> Jan
> 
> -- 
> Siemens AG, Technology
> Competence Center Embedded Linux

This patch had been pending for quite a while, and not sure if it's
still acceptable to be merged, but from my testing it's working fine
for me. Nevertheless, the v5 patch now has conflict with the current
master, so I've slightly modified for reference. It would be helpful
if Deepak can send v6 later on.

Tested-by: Hsieh-Tseng Shen <woodrow.shen@sifive.com>

v5 --> v6:
 - dropped cache type "ulong" in scripts/gdb/linux/utils.py as it
 already exists in the current upstream
---
 scripts/gdb/linux/cpus.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
index 255dc18cb9da..f8325cab5f1b 100644
--- a/scripts/gdb/linux/cpus.py
+++ b/scripts/gdb/linux/cpus.py
@@ -179,6 +179,21 @@ def get_current_task(cpu):
         else:
             raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
                                "while running in userspace(EL0)")
+    elif utils.is_target_arch("riscv"):
+        current_tp = gdb.parse_and_eval("$tp")
+        scratch_reg = gdb.parse_and_eval("$sscratch")
+
+        # by default tp points to current task
+        current_task = current_tp.cast(task_ptr_type)
+
+        # scratch register is set 0 in trap handler after entering kernel.
+        # When hart is in user mode, scratch register is pointing to task_struct.
+        # and tp is used by user mode. So when scratch register holds larger value
+        # (negative address as ulong is larger value) than tp, then use scratch register.
+        if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
+            current_task = scratch_reg.cast(task_ptr_type)
+
+            return current_task.dereference()
     else:
         raise gdb.GdbError("Sorry, obtaining the current task is not yet "
                            "supported with this arch")
--
2.31.1

Thank you
  

On Wed, Oct 4, 2023 at 10:29 PM Hsieh-Tseng Shen
<woodrow.shen@sifive.com> wrote:
>
> On Mon, Jan 02, 2023 at 10:09:01AM +0100, Jan Kiszka wrote:
> > On 15.11.22 23:10, Deepak Gupta wrote:
> > > csr_sscratch CSR holds current task_struct address when hart is in
> > > user space. Trap handler on entry spills csr_sscratch into "tp" (x2)
> > > register and zeroes out csr_sscratch CSR. Trap handler on exit reloads
> > > "tp" with expected user mode value and place current task_struct address
> > > again in csr_sscratch CSR.
> > >
> > > This patch assumes "tp" is pointing to task_struct. If value in
> > > csr_sscratch is numerically greater than "tp" then it assumes csr_sscratch
> > > is correct address of current task_struct. This logic holds when
> > >    - hart is in user space, "tp" will be less than csr_sscratch.
> > >    - hart is in kernel space but not in trap handler, "tp" will be more
> > >      than csr_sscratch (csr_sscratch being equal to 0).
> > >    - hart is executing trap handler
> > >        - "tp" is still pointing to user mode but csr_sscratch contains
> > >           ptr to task_struct. Thus numerically higher.
> > >        - "tp" is  pointing to task_struct but csr_sscratch now contains
> > >           either 0 or numerically smaller value (transiently holds
> > >           user mode tp)
> > >
> > > Patch also adds new cached type "ulong" in scripts/gdb/linux/utils.py
> > >
> > > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > >
> > > ---
> > > Since patch has changed a little bit from v1 and I didn't include
> > > changelog earlier, here it is.
> > >
> > > v1 --> v2:
> > >  - added logic to locate task_struct irrespective of priv
> > >  - made locating task_struct agnostic to bitness(32 vs 64).
> > >  - added caching of ulong type in scripts/gdb/linux/utils.py
> > >  - added more descriptive commit message
> > >
> > > v2 --> v3:
> > >  - amended commit message and source line to fit column width
> > >
> > > v3 --> v4:
> > >  - amended commit message and remove whitespace in source
> > >  - added Reviewed-by for reviewers
> > >
> > > v4 --> v5:
> > >  - changing the order of changelog and sign off/review tags in commit
> > > ---
> > > ---
> > >  scripts/gdb/linux/cpus.py  | 15 +++++++++++++++
> > >  scripts/gdb/linux/utils.py |  5 +++++
> > >  2 files changed, 20 insertions(+)
> > >
> > > diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> > > index 15fc4626d236..14c22f82449b 100644
> > > --- a/scripts/gdb/linux/cpus.py
> > > +++ b/scripts/gdb/linux/cpus.py
> > > @@ -173,6 +173,21 @@ def get_current_task(cpu):
> > >           else:
> > >               raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
> > >                                  "while running in userspace(EL0)")
> > > +    elif utils.is_target_arch("riscv"):
> > > +         current_tp = gdb.parse_and_eval("$tp")
> > > +         scratch_reg = gdb.parse_and_eval("$sscratch")
> > > +
> > > +         # by default tp points to current task
> > > +         current_task = current_tp.cast(task_ptr_type)
> > > +
> > > +         # scratch register is set 0 in trap handler after entering kernel.
> > > +         # When hart is in user mode, scratch register is pointing to task_struct.
> > > +         # and tp is used by user mode. So when scratch register holds larger value
> > > +         # (negative address as ulong is larger value) than tp, then use scratch register.
> > > +         if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> > > +             current_task = scratch_reg.cast(task_ptr_type)
> >
> > Why not if-else for the assignment here?
> >
> > > +
> > > +         return current_task.dereference()
> > >      else:
> > >          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
> > >                             "supported with this arch")
> > > diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> > > index 1553f68716cc..ddaf3089170d 100644
> > > --- a/scripts/gdb/linux/utils.py
> > > +++ b/scripts/gdb/linux/utils.py
> > > @@ -35,12 +35,17 @@ class CachedType:
> > >
> > >
> > >  long_type = CachedType("long")
> > > +ulong_type = CachedType("ulong")
> > >  atomic_long_type = CachedType("atomic_long_t")
> > >
> > >  def get_long_type():
> > >      global long_type
> > >      return long_type.get_type()
> > >
> > > +def get_ulong_type():
> > > +    global ulong_type
> > > +    return ulong_type.get_type()
> > > +
> > >  def offset_of(typeobj, field):
> > >      element = gdb.Value(0).cast(typeobj)
> > >      return int(str(element[field].address).split()[0], 16)
> >
> > Looks good to me otherwise.
> >
> > Jan
> >
> > --
> > Siemens AG, Technology
> > Competence Center Embedded Linux
>
> This patch had been pending for quite a while, and not sure if it's
> still acceptable to be merged, but from my testing it's working fine
> for me. Nevertheless, the v5 patch now has conflict with the current
> master, so I've slightly modified for reference. It would be helpful
> if Deepak can send v6 later on.

Thanks for testing it out.
I just sent out a v6 based on v6.6-rc5.

-Deepak

>
> Tested-by: Hsieh-Tseng Shen <woodrow.shen@sifive.com>
>
> v5 --> v6:
>  - dropped cache type "ulong" in scripts/gdb/linux/utils.py as it
>  already exists in the current upstream
> ---
>  scripts/gdb/linux/cpus.py | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 255dc18cb9da..f8325cab5f1b 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -179,6 +179,21 @@ def get_current_task(cpu):
>          else:
>              raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
>                                 "while running in userspace(EL0)")
> +    elif utils.is_target_arch("riscv"):
> +        current_tp = gdb.parse_and_eval("$tp")
> +        scratch_reg = gdb.parse_and_eval("$sscratch")
> +
> +        # by default tp points to current task
> +        current_task = current_tp.cast(task_ptr_type)
> +
> +        # scratch register is set 0 in trap handler after entering kernel.
> +        # When hart is in user mode, scratch register is pointing to task_struct.
> +        # and tp is used by user mode. So when scratch register holds larger value
> +        # (negative address as ulong is larger value) than tp, then use scratch register.
> +        if (scratch_reg.cast(utils.get_ulong_type()) > current_tp.cast(utils.get_ulong_type())):
> +            current_task = scratch_reg.cast(task_ptr_type)
> +
> +            return current_task.dereference()
>      else:
>          raise gdb.GdbError("Sorry, obtaining the current task is not yet "
>                             "supported with this arch")
> --
> 2.31.1
>
> Thank you