diff mbox series

[3/3] LAM: Support kernel space debugging

Message ID 20240327074739.2969623-4-christina.schimpe@intel.com
State New
Headers
Series Add amd64 LAM watchpoint support |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Schimpe, Christina March 27, 2024, 7:47 a.m. UTC 
  From: Christina Schimpe <christina.schimpe@intel.com>

Sign-extend watchpoint address for kernel space support.
---
 gdb/amd64-linux-tdep.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Willgerodt, Felix April 24, 2024, 11:11 a.m. UTC | #1 
> -----Original Message-----
> From: Schimpe, Christina <christina.schimpe@intel.com>
> Sent: Mittwoch, 27. März 2024 08:48
> To: gdb-patches@sourceware.org
> Cc: Schimpe, Christina <christina.schimpe@intel.com>
> Subject: [PATCH 3/3] LAM: Support kernel space debugging
> 
> From: Christina Schimpe <christina.schimpe@intel.com>
> 
> Sign-extend watchpoint address for kernel space support.
> ---
>  gdb/amd64-linux-tdep.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 94c62277623..3498a6d4632 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -51,6 +51,8 @@
> 
>  #include <sstream>
> 
> +/* Bit 63 is used to select between a kernel-space and user-space address.  */
> +#define VA_RANGE_SELECT_BIT_MASK  0x8000000000000000UL
>  #define DEFAULT_TAG_MASK 0xffffffffffffffffULL
> 
>  /* Mapping between the general-purpose registers in `struct user'
> @@ -1852,10 +1854,21 @@ amd64_linux_lam_untag_mask ()
>  static CORE_ADDR
>  amd64_linux_remove_non_addr_bits_wpt (gdbarch *gdbarch, CORE_ADDR
> addr)
>  {
> +  /* The topmost bit tells us if we have a kernel-space address or a user-space
> +     address.  */
> +  const bool kernel_address = (addr & VA_RANGE_SELECT_BIT_MASK) != 0;
> +
> +  CORE_ADDR mask = amd64_linux_lam_untag_mask ();
>    /* Clear insignificant bits of a target address using the untag mask.
>       The untag mask preserves the topmost bit, which distinguishes user space
>       from kernel space address.  */
> -  return (addr & amd64_linux_lam_untag_mask ());
> +  addr &= mask;
> +
> +  /* Sign-extend if we have a kernel-space address.  */
> +  if (kernel_address)
> +    addr |= ~mask;
> +
> +  return addr;

Ah, here is probably where we should add the comment I complained about in
patch 2.

I don't really know much about how kernel space debugging works.
(We can't get a kernel address when doing user-space debugging, can we?)
While this probably doesn't hurt much, I wonder if it isn't just dead code.
Is this file (amd64-linux-tdep.c) even used when doing kernel debugging?
And we still read the mask from /proc/pid/status. Is that available?

Maybe someone with more experience on this can chime in.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Schimpe, Christina May 17, 2024, 2:07 p.m. UTC | #2 
Hi Felix,
 
> Ah, here is probably where we should add the comment I complained about in
> patch 2.

Yes, I removed it from patch 2. Thanks.

> I don't really know much about how kernel space debugging works.
> (We can't get a kernel address when doing user-space debugging, can we?)
> While this probably doesn't hurt much, I wonder if it isn't just dead code.
> Is this file (amd64-linux-tdep.c) even used when doing kernel debugging?
> And we still read the mask from /proc/pid/status. Is that available?

I can't really test this as we don't have LAM support for kernel space in the
linux kernel, yet. 
This also makes me wonder if it's safe to have this patch before we can test it
or if we should better wait.

I agree with Felix, it would be great if someone with more experience on this
could help us here.

Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
diff mbox series

Patch

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 94c62277623..3498a6d4632 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -51,6 +51,8 @@ 
 
 #include <sstream>
 
+/* Bit 63 is used to select between a kernel-space and user-space address.  */
+#define VA_RANGE_SELECT_BIT_MASK  0x8000000000000000UL
 #define DEFAULT_TAG_MASK 0xffffffffffffffffULL
 
 /* Mapping between the general-purpose registers in `struct user'
@@ -1852,10 +1854,21 @@  amd64_linux_lam_untag_mask ()
 static CORE_ADDR
 amd64_linux_remove_non_addr_bits_wpt (gdbarch *gdbarch, CORE_ADDR addr)
 {
+  /* The topmost bit tells us if we have a kernel-space address or a user-space
+     address.  */
+  const bool kernel_address = (addr & VA_RANGE_SELECT_BIT_MASK) != 0;
+
+  CORE_ADDR mask = amd64_linux_lam_untag_mask ();
   /* Clear insignificant bits of a target address using the untag mask.
      The untag mask preserves the topmost bit, which distinguishes user space
      from kernel space address.  */
-  return (addr & amd64_linux_lam_untag_mask ());
+  addr &= mask;
+
+  /* Sign-extend if we have a kernel-space address.  */
+  if (kernel_address)
+    addr |= ~mask;
+
+  return addr;
 }
 
 static void