2022-11-11 20:07:03

by Deepak Gupta

[permalink] [raw]
Subject: Updating python gdb command lx_current for riscv

scripts/gdb has a good deal of useful commands to locate critical kernel
data structures from gdb. lx_current misses support for locating current
task on this cpu. This patch fixes that.
It's a small patch. Please merge.




2022-11-11 20:07:58

by Deepak Gupta

[permalink] [raw]
Subject: [PATCH] gdb-script: updated lx_current for riscv

From: Deepak Gupta <[email protected]>

lx_current python gdb command defined in scripts/gdb/cpus.py updated
to support riscv architecture.

Signed-off-by: Deepak Gupta <[email protected]>
---
scripts/gdb/linux/cpus.py | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
index 15fc4626d236..ce6703f1e35a 100644
--- a/scripts/gdb/linux/cpus.py
+++ b/scripts/gdb/linux/cpus.py
@@ -173,6 +173,14 @@ 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_task_addr = gdb.parse_and_eval("$tp")
+ if((current_task_addr.cast(utils.get_long_type()) >> 63) != 0):
+ current_task = current_task_addr.cast(task_ptr_type)
+ return current_task.dereference()
+ else:
+ raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
+ "while running in userspace")
else:
raise gdb.GdbError("Sorry, obtaining the current task is not yet "
"supported with this arch")
--
2.25.1


2022-11-13 01:10:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] gdb-script: updated lx_current for riscv

On Fri, Nov 11, 2022 at 11:59:38AM -0800, [email protected] wrote:
> From: Deepak Gupta <[email protected]>
>
> lx_current python gdb command defined in scripts/gdb/cpus.py updated
> to support riscv architecture.

The commit which added support for arm64 gave an explanation of why
SP_EL0 needed to be checked. Would be nice if you could do the same here
for RISC-V. See 526940e39626 ("scripts/gdb: add lx_current support for
arm64") for what I mean.

While you're at it, "scripts/gdb: add support for RISC-V" would appear
to be a more standard $subject for this file.

Thanks,
Conor.

>
> Signed-off-by: Deepak Gupta <[email protected]>
> ---
> scripts/gdb/linux/cpus.py | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 15fc4626d236..ce6703f1e35a 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -173,6 +173,14 @@ 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_task_addr = gdb.parse_and_eval("$tp")
> + if((current_task_addr.cast(utils.get_long_type()) >> 63) != 0):
> + current_task = current_task_addr.cast(task_ptr_type)
> + return current_task.dereference()
> + else:
> + raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
> + "while running in userspace")
> else:
> raise gdb.GdbError("Sorry, obtaining the current task is not yet "
> "supported with this arch")
> --
> 2.25.1
>

2022-11-13 11:17:18

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH] gdb-script: updated lx_current for riscv

On Fri, Nov 11, 2022 at 11:59:38AM -0800, [email protected] wrote:
> From: Deepak Gupta <[email protected]>
>
> lx_current python gdb command defined in scripts/gdb/cpus.py updated
> to support riscv architecture.
>
> Signed-off-by: Deepak Gupta <[email protected]>
> ---
> scripts/gdb/linux/cpus.py | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> index 15fc4626d236..ce6703f1e35a 100644
> --- a/scripts/gdb/linux/cpus.py
> +++ b/scripts/gdb/linux/cpus.py
> @@ -173,6 +173,14 @@ 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_task_addr = gdb.parse_and_eval("$tp")
> + if((current_task_addr.cast(utils.get_long_type()) >> 63) != 0):

Shouldn't there be a get_long_type().sizeof == 8 check somewhere before
shifting by 63? Or are 32-bit targets not supported at all for some
reason?

Thanks,
drew

> + current_task = current_task_addr.cast(task_ptr_type)
> + return current_task.dereference()
> + else:
> + raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
> + "while running in userspace")
> else:
> raise gdb.GdbError("Sorry, obtaining the current task is not yet "
> "supported with this arch")
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-11-13 15:32:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] gdb-script: updated lx_current for riscv

On Sun, Nov 13, 2022 at 12:13:40AM +0000, Conor Dooley wrote:
> On Fri, Nov 11, 2022 at 11:59:38AM -0800, [email protected] wrote:
> > From: Deepak Gupta <[email protected]>
> >
> > lx_current python gdb command defined in scripts/gdb/cpus.py updated
> > to support riscv architecture.
>
> The commit which added support for arm64 gave an explanation of why
> SP_EL0 needed to be checked. Would be nice if you could do the same here
> for RISC-V. See 526940e39626 ("scripts/gdb: add lx_current support for
> arm64") for what I mean.
>
> While you're at it, "scripts/gdb: add support for RISC-V" would appear

Realised I omitted a word there, sorry: s/add support/add lx_current support/

> to be a more standard $subject for this file.
>
> Thanks,
> Conor.
>
> >
> > Signed-off-by: Deepak Gupta <[email protected]>
> > ---
> > scripts/gdb/linux/cpus.py | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/scripts/gdb/linux/cpus.py b/scripts/gdb/linux/cpus.py
> > index 15fc4626d236..ce6703f1e35a 100644
> > --- a/scripts/gdb/linux/cpus.py
> > +++ b/scripts/gdb/linux/cpus.py
> > @@ -173,6 +173,14 @@ 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_task_addr = gdb.parse_and_eval("$tp")
> > + if((current_task_addr.cast(utils.get_long_type()) >> 63) != 0):
> > + current_task = current_task_addr.cast(task_ptr_type)
> > + return current_task.dereference()
> > + else:
> > + raise gdb.GdbError("Sorry, obtaining the current task is not allowed "
> > + "while running in userspace")
> > else:
> > raise gdb.GdbError("Sorry, obtaining the current task is not yet "
> > "supported with this arch")
> > --
> > 2.25.1
> >

2022-11-15 01:49:08

by Deepak Gupta

[permalink] [raw]
Subject: [PATCH v2] 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_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 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.
- hart is executing trap handler
- "tp" is still pointing to user mode but csr_scratch contains ptr to task_struct.
numerically higher.
- "tp" is now pointing to task_struct but csr_scratch now contains either 0 or numerically
smaller value.

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

Signed-off-by: Deepak Gupta <[email protected]>
---
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..fd818d7896ce 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.
+ # So when scratch register holds higher value (negative address as ulong is bigger 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


2022-11-15 07:09:40

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2] scripts/gdb: add lx_current support for riscv

On Mon, Nov 14, 2022 at 05:29:17PM -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 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.
> - hart is executing trap handler
> - "tp" is still pointing to user mode but csr_scratch contains ptr to task_struct.
> numerically higher.
> - "tp" is now pointing to task_struct but csr_scratch now contains either 0 or numerically
> smaller value.

When is (csr_scratch != 0 && csr_scratch < tp)? IIUC, it should always be
zero or >= tp.

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

Please wrap commit message lines at ~74.

>
> Signed-off-by: Deepak Gupta <[email protected]>
> ---
> 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..fd818d7896ce 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.
> + # So when scratch register holds higher value (negative address as ulong is bigger value) than tp,

Please wrap source lines at 100.

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

Besides the line wrapping and the commit message clarification this looks
good to me.

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2022-11-15 09:50:25

by Deepak Gupta

[permalink] [raw]
Subject: [PATCH v3] 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_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
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 <[email protected]>
---
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())):
+ 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


2022-11-15 14:53:03

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv

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 <[email protected]>

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

2022-11-15 18:19:17

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv

On Tue, Nov 15, 2022 at 09:49:10AM -0800, Deepak Gupta wrote:
...
> On Tue, Nov 15, 2022 at 6:38 AM Conor Dooley <[email protected]>
> wrote:
>
> > Hey Deepak,
> >
> > On Tue, Nov 15, 2022 at 12:49:23AM -0800, Deepak Gupta wrote:
...
> > > + if (scratch_reg.cast(utils.get_ulong_type()) >
> > current_tp.cast(utils.get_ulong_type())):
> > ^^
> > extra space here?
>
>
> I don't see the space in the patch. Can you clarify which space you're
> talking about here?

The same one I pointed out in v2. The one after the greater-than sign.
Is your editor not using a monospaced font?

Thanks,
drew

2022-11-15 18:29:47

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv

Hey Deepak,

On 15/11/2022 17:49, Deepak Gupta wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> Since I am new to all this. I've had some oversight and am still learning the flow.
> Rest inline.

No worries chief. Worth noting is that this mail came in html
form, which the mailing lists reject. Noone outside of the
direct CC list will see this mail. May be worth asking some
of the other Rivos lads how they do their plain text emailing.

ik Palmer's got his hand rolled stuff, so maybe he's not the
best to ask - but try Bjorn or Atish?

>
> On Tue, Nov 15, 2022 at 6:38 AM Conor Dooley <[email protected] <mailto:[email protected]>> wrote:
>
> 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/ ?
>
>
> Will fix it.
>  
>
>
> > 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 <[email protected] <mailto:[email protected]>>
>
> 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'll make sure to add the risc-v list on the next spin up.
>
>
> I don't see a changelog anywhere here
> from v2 either
>
>
> I had been taking inputs and squashing commits on my end.
> You want me to send a changelog of changes between versions of patches.

Yeah, it's nice to say something like:
v2 -> v3:
- reworded commit message
- fixed compile error in bar.c if !CONFIG_FOO

Makes it easier for reviewers to see what changed between
versions.

>  
>
> , nor did you pick up Drew's Reviewed-by.
>
>
> I should've done that. My mistake and apologize.
> I'll fix it in my next submission.
>  
>
>
> 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?
>
>
> I don't see the space in the patch. Can you clarify which space you're talking about here?

There's a double space between the > and current_tp.
I put a ^^ under it, but if you've not got a monospace font, which since
you're replying in html you probably don't, it may not align for you.

Hope that helps,
Conor.

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

2022-11-15 19:03:55

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH v3] scripts/gdb: add lx_current support for riscv

On Tue, Nov 15, 2022 at 06:06:34PM +0000, [email protected] wrote:
>Hey Deepak,
>
>On 15/11/2022 17:49, Deepak Gupta wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>> Since I am new to all this. I've had some oversight?and am still learning?the flow.
>> Rest inline.
>
>No worries chief. Worth noting is that this mail came in html
>form, which the mailing lists reject. Noone outside of the
>direct CC list will see this mail. May be worth asking some
>of the other Rivos lads how they do their plain text emailing.
>
>ik Palmer's got his hand rolled stuff, so maybe he's not the
>best to ask - but try Bjorn or Atish?

Sending this time from mutt. Hopefully no bounces.

>
>>
>> On Tue, Nov 15, 2022 at 6:38 AM Conor Dooley <[email protected] <mailto:[email protected]>> wrote:
>>
>> 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/ ?
>>
>>
>> Will fix it.
>> ?
>>
>>
>> > 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 <[email protected] <mailto:[email protected]>>
>>
>> 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'll make sure to add the risc-v list on the next spin up.
>>
>>
>> I don't see a changelog anywhere here
>> from v2 either
>>
>>
>> I had been taking inputs and squashing commits on my end.
>> You want me to send a changelog of changes between versions of patches.
>
>Yeah, it's nice to say something like:
>v2 -> v3:
>- reworded commit message
>- fixed compile error in bar.c if !CONFIG_FOO
>
>Makes it easier for reviewers to see what changed between
>versions.
>
>> ?
>>
>> , nor did you pick up Drew's Reviewed-by.
>>
>>
>> I should've done that. My mistake and apologize.
>> I'll fix it in my next submission.
>> ?
>>
>>
>> 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?
>>
>>
>> I don't see the space in the patch. Can you clarify which space you're talking about here?
>
>There's a double space between the > and current_tp.
>I put a ^^ under it, but if you've not got a monospace font, which since
>you're replying in html you probably don't, it may not align for you.
>
>Hope that helps,
>Conor.

Yes can see it now.

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

2022-11-15 21:03:24

by Deepak Gupta

[permalink] [raw]
Subject: [PATCH v4] 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

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

Signed-off-by: Deepak Gupta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
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


2022-11-15 21:44:53

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4] scripts/gdb: add lx_current support for riscv

Hey Deepak,

On 15/11/2022 20:40, Deepak Gupta wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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
>

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

This bit here needs to go below the --- line as it should not end up
in the commit logs (as everything below the --- is ignored.

>
> ---
> 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
> ---
>
> Signed-off-by: Deepak Gupta <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>

And this bit needs to go above the first --- line as it does need
to go into the commit logs.

The order is:
$subject

body

tags
---
changelog & comments

diffs

For example, this is what I see when I apply the patch:

$ b4 shazam [email protected]
Grabbing thread from lore.kernel.org/all/20221115204003.1866421-1-debug%40rivosinc.com/t.mbox.gz
Checking for newer revisions on https://lore.kernel.org/all/
Analyzing 13 messages in the thread
Will use the latest revision: v4
You can pick other revisions using the -vN flag
Checking attestation on all messages, may take a moment...
---
✓ [PATCH v4] scripts/gdb: add lx_current support for riscv
---
✓ Signed: DKIM/rivosinc-com.20210112.gappssmtp.com (From: [email protected])
---
Total patches: 1
---
Applying: scripts/gdb: add lx_current support for riscv

$ git show
commit 5c93617c68980f767e312fc51849d78093f56e72 (HEAD)
Author: Deepak Gupta <[email protected]>
Date: Tue Nov 15 12:40:03 2022 -0800

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

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

Taken a wee bit of the back and forth, Looks like you're nearly
there though!

Conor.

2022-11-15 22:19:37

by Deepak Gupta

[permalink] [raw]
Subject: [PATCH 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 <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>

---
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)
--
2.25.1


2022-11-16 08:51:05

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

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 <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
>
> ---
> 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
>

2022-11-16 22:40:15

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

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 <[email protected]>
>> Reviewed-by: Andrew Jones <[email protected]>
>>
>> ---
>> 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
>>

2022-12-09 02:10:47

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

On Tue, 15 Nov 2022 14:10:51 PST (-0800), [email protected] 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 <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
>
> ---
> 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 <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>

Thanks!

2023-01-02 09:22:34

by Jan Kiszka

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

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 <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
>
> ---
> 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

2023-10-05 14:21:49

by Hsieh-Tseng Shen

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

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 <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> >
> > ---
> > 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 <[email protected]>

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

2023-10-26 23:42:59

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH v5] scripts/gdb: add lx_current support for riscv

On Wed, Oct 4, 2023 at 10:29 PM Hsieh-Tseng Shen
<[email protected]> 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 <[email protected]>
> > > Reviewed-by: Andrew Jones <[email protected]>
> > >
> > > ---
> > > 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 <[email protected]>
>
> 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