All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: Deepak Gupta <debug@rivosinc.com>
Cc: <aou@eecs.berkeley.edu>, <jan.kiszka@siemens.com>,
	<kbingham@kernel.org>, <linux-kernel@vger.kernel.org>,
	<palmer@dabbelt.com>, <paul.walmsley@sifive.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	<linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv
Date: Tue, 15 Nov 2022 14:38:27 +0000	[thread overview]
Message-ID: <Y3OkY6myfea00BaK@wendy> (raw)
In-Reply-To: <20221115084923.1822572-1-debug@rivosinc.com>

Hey Deepak,

On Tue, Nov 15, 2022 at 12:49:23AM -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_scratch CSR.
> 
> This patch assumes "tp" is pointing to task_struct. If value in
> csr_scratch is numerically greater than "tp" then it assumes csr_scratch

nit: s/scratch/sscratch/ ?

> is correct address of current task_struct. This logic holds when
>    - hart is in user space, "tp" will be less than csr_scratch.
>    - hart is in kernel space but not in trap handler, "tp" will be more
>      than csr_scratch (csr_scratch being equal to 0).
>    - hart is executing trap handler
>        - "tp" is still pointing to user mode but csr_scratch contains
>           ptr to task_struct. Thus numerically higher.
>        - "tp" is  pointing to task_struct but csr_scratch 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>

I noticed when looking into patchwork complaining about checkpatch
errors in v2, that b4 had actually downloaded v3 but I could not see
this patch on the RISC-V list. I don't see a changelog anywhere here
from v2 either, nor did you pick up Drew's Reviewed-by.

What's the story there?

One really minor thing below. Should be able to fix it up trivially up
& submit a v4, CCing the linux-riscv list.

> ---
>  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..ca5215a660c7 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())):
                                                          ^^
extra space here?


> +             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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor.dooley@microchip.com>
To: Deepak Gupta <debug@rivosinc.com>
Cc: <aou@eecs.berkeley.edu>, <jan.kiszka@siemens.com>,
	<kbingham@kernel.org>, <linux-kernel@vger.kernel.org>,
	<palmer@dabbelt.com>, <paul.walmsley@sifive.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	<linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv
Date: Tue, 15 Nov 2022 14:38:27 +0000	[thread overview]
Message-ID: <Y3OkY6myfea00BaK@wendy> (raw)
In-Reply-To: <20221115084923.1822572-1-debug@rivosinc.com>

Hey Deepak,

On Tue, Nov 15, 2022 at 12:49:23AM -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_scratch CSR.
> 
> This patch assumes "tp" is pointing to task_struct. If value in
> csr_scratch is numerically greater than "tp" then it assumes csr_scratch

nit: s/scratch/sscratch/ ?

> is correct address of current task_struct. This logic holds when
>    - hart is in user space, "tp" will be less than csr_scratch.
>    - hart is in kernel space but not in trap handler, "tp" will be more
>      than csr_scratch (csr_scratch being equal to 0).
>    - hart is executing trap handler
>        - "tp" is still pointing to user mode but csr_scratch contains
>           ptr to task_struct. Thus numerically higher.
>        - "tp" is  pointing to task_struct but csr_scratch 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>

I noticed when looking into patchwork complaining about checkpatch
errors in v2, that b4 had actually downloaded v3 but I could not see
this patch on the RISC-V list. I don't see a changelog anywhere here
from v2 either, nor did you pick up Drew's Reviewed-by.

What's the story there?

One really minor thing below. Should be able to fix it up trivially up
& submit a v4, CCing the linux-riscv list.

> ---
>  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..ca5215a660c7 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())):
                                                          ^^
extra space here?


> +             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

  reply	other threads:[~2022-11-15 14:39 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-11 19:59 Updating python gdb command lx_current for riscv debug
2022-11-11 19:59 ` debug
2022-11-11 19:59 ` [PATCH] gdb-script: updated " debug
2022-11-11 19:59   ` debug
2022-11-13  0:13   ` Conor Dooley
2022-11-13  0:13     ` Conor Dooley
2022-11-13 14:14     ` Conor Dooley
2022-11-13 14:14       ` Conor Dooley
2022-11-13 11:06   ` Andrew Jones
2022-11-13 11:06     ` Andrew Jones
2022-11-15  1:29   ` [PATCH v2] scripts/gdb: add lx_current support " Deepak Gupta
2022-11-15  1:29     ` Deepak Gupta
2022-11-15  6:46     ` Andrew Jones
2022-11-15  6:46       ` Andrew Jones
2022-11-15  8:49     ` [PATCH v3] " Deepak Gupta
2022-11-15 14:38       ` Conor Dooley [this message]
2022-11-15 14:38         ` Conor Dooley
     [not found]         ` <CAKC1njRi9C0m3JKpu0ebAFCC25161EST=tFFWiAj1yZBbnak6A@mail.gmail.com>
2022-11-15 18:06           ` Conor.Dooley
2022-11-15 18:06             ` Conor.Dooley
2022-11-15 18:43             ` Deepak Gupta
2022-11-15 18:43               ` Deepak Gupta
2022-11-15 18:06           ` Andrew Jones
2022-11-15 18:06             ` Andrew Jones
2022-11-15 20:40         ` [PATCH v4] " Deepak Gupta
2022-11-15 20:40           ` Deepak Gupta
2022-11-15 21:23           ` Conor.Dooley
2022-11-15 21:23             ` Conor.Dooley
2022-11-15 22:10             ` [PATCH v5] " Deepak Gupta
2022-11-15 22:10               ` Deepak Gupta
2022-11-16  8:16               ` Andrew Jones
2022-11-16  8:16                 ` Andrew Jones
2022-11-16 22:24                 ` Deepak Gupta
2022-11-16 22:24                   ` Deepak Gupta
2022-12-09  1:24               ` Palmer Dabbelt
2022-12-09  1:24                 ` Palmer Dabbelt
2023-01-02  9:09               ` Jan Kiszka
2023-01-02  9:09                 ` Jan Kiszka
2023-10-05  5:29                 ` Hsieh-Tseng Shen
2023-10-05  5:29                   ` Hsieh-Tseng Shen
2023-10-26 23:41                   ` Deepak Gupta
2023-10-26 23:41                     ` Deepak Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3OkY6myfea00BaK@wendy \
    --to=conor.dooley@microchip.com \
    --cc=ajones@ventanamicro.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=debug@rivosinc.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kbingham@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.