[PATCH 0/3 v3] [AArch64] Support tagged pointer

[PATCH 0/3 v3] [AArch64] Support tagged pointer

[Yao Qi]

ARMv8 supports tagged address, that is, the top one byte in address
is ignored.  It is always enabled on aarch64-linux.  See
https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt

Some parts of GDB related to address are updated in this patch series,

 - Memory access, like command 'x',
 - Setting hw breakpoint on some address,
 - Setting watchpoint on some address,

Address tag is treated as non-significant bits of address, so this patch
series add a new gdbarch significant_addr_bit, and use it in gdbarch to
get rid of non-significant bits.  This was the suggestion in the v2 review.
(https://sourceware.org/ml/gdb-patches/2017-10/msg00792.html)

When I test this patch series, and I find a regression in linespec.  I
posted the fix https://sourceware.org/ml/gdb-patches/2017-12/msg00158.html
Without this fix, this series causes a regression.

*** BLURB HERE ***

Yao Qi (3):
  Clear non-significant bits of address on memory access
  Adjust breakpoint address by clearing non-significant bits
  Clear non-significant bits of address in watchpoint

 gdb/aarch64-linux-tdep.c                          |   2 +
 gdb/breakpoint.c                                  |  22 ++---
 gdb/gdbarch.c                                     |  22 +++++
 gdb/gdbarch.h                                     |   8 ++
 gdb/gdbarch.sh                                    |   6 ++
 gdb/target.c                                      |   2 +
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   |  57 ++++++++++++
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 105 ++++++++++++++++++++++
 gdb/utils.c                                       |  17 ++++
 gdb/utils.h                                       |   3 +
 10 files changed, 233 insertions(+), 11 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp

-- 
1.9.1

[PATCH 3/3] Clear non-significant bits of address in watchpoint

[Yao Qi]

Nowadays, GDB can't set watchpoint on tagged address on AArch64,

(gdb) p p2
$1 = (int *) 0xf000fffffffff474
(gdb) watch *((int *) 0xf000fffffffff474)
Hardware watchpoint 2: *((int *) 0xf000fffffffff474)
(gdb) c
Continuing.
main () at
binutils-gdb/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c:45
45	  void (*func_ptr) (void) = foo;
Unexpected error setting hardware debug registers

This patch is about setting watchpoint on a tagged address.  Unlike
breakpoint, watchpoint record the expression rather than the address, and
when a watchpoint is fired, GDB checks the expression value changed
instead of matching address, so we can mask the watchpoint address by
getting rid of non-significant bits of address.

gdb:

2017-12-01  Yao Qi  <yao.qi@linaro.org>

	* breakpoint.c (update_watchpoint): Call
	significant_addr.

gdb/testsuite:

2017-12-01  Yao Qi  <yao.qi@linaro.org>

	* gdb.arch/aarch64-tagged-pointer.c (main): Update.
	* gdb.arch/aarch64-tagged-pointer.exp: Add tests for watchpoint.
---
 gdb/breakpoint.c                                  |  2 +-
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   |  1 +
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 22b3069..6570513 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1867,7 +1867,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 		  loc->gdbarch = get_type_arch (value_type (v));
 
 		  loc->pspace = frame_pspace;
-		  loc->address = addr;
+		  loc->address  = address_significant (loc->gdbarch, addr);
 
 		  if (bitsize != 0)
 		    {
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
index 9bfe41e..5754785 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
@@ -53,4 +53,5 @@ main (void)
     }
 
   sp1->i = 8765;
+  i = 1;
 }
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
index fcab1b7..c08993e 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
@@ -89,3 +89,17 @@ foreach_with_prefix bptype {"hbreak" "break"} {
     gdb_test "up" "\\(\*func_ptr\\) \\(\\).*" "caller is *func_ptr"
     delete_breakpoints
 }
+
+gdb_test "down"
+gdb_test "finish"
+# Watch on tagged pointer.
+gdb_test "watch *sp2"
+gdb_test "continue" \
+    "Continuing\\..*Hardware watchpoint \[0-9\]+.*" \
+    "run until watchpoint on s1"
+delete_breakpoints
+
+gdb_test "watch *p2"
+gdb_test "continue" \
+    "Continuing\\..*Hardware watchpoint \[0-9\]+.*" \
+    "run until watchpoint on i"
-- 
1.9.1

Re: [PATCH 3/3] Clear non-significant bits of address in watchpoint

[Pedro Alves]

Looks good to me with formatting nits pointed out below fixed.

On 12/08/2017 10:04 AM, Yao Qi wrote:

> 2017-12-01  Yao Qi  <yao.qi@linaro.org>
> 
> 	* breakpoint.c (update_watchpoint): Call
> 	significant_addr.

Wrong name: significant_addr -> address_significant.

Guess you went back and forth deciding the function name. :-)

> @@ -1867,7 +1867,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
>  		  loc->gdbarch = get_type_arch (value_type (v));
>  
>  		  loc->pspace = frame_pspace;
> -		  loc->address = addr;
> +		  loc->address  = address_significant (loc->gdbarch, addr);

Spurious space before '='.

Thanks,
Pedro Alves

[PATCH 1/3] Clear non-significant bits of address on memory access

[Yao Qi]

ARMv8 supports tagged address, that is, the top one byte in address
is ignored.  It is always enabled on aarch64-linux.  See
https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt

The tag in the tagged address is modeled as non-significant bits in
address, so this patch adds a new gdbarch method significant_addr_bit and
clear the non-significant bits (the top byte in ARMv8) of the virtual
address at the point before passing address to target cache layer.  IOW,
the address used in the target cache layer is already cleared.

Before this patch,
(gdb) x/x 0x0000000000411030
0x411030 <global>:	0x00000000
(gdb) x/x 0xf000000000411030
0xf000000000411030:	Cannot access memory at address 0xf000000000411030

After this patch,

(gdb) x/x 0x0000000000411030
0x411030 <global>:	0x00000000
(gdb) x/x 0xf000000000411030
0xf000000000411030:	0x00000000

Note that I used address_significant in paddress, but it causes a
regression gdb.base/long_long.exp, because gdb clears the non-significant
bits in address, but test still expects them.

p/a val.oct^M
$24 = 0x2ee53977053977^M
(gdb) FAIL: gdb.base/long_long.exp: p/a val.oct

so I defer the change there.

gdb:

2017-10-19  Yao Qi  <yao.qi@linaro.org>

	* aarch64-linux-tdep.c (aarch64_linux_init_abi): Install gdbarch
	significant_addr_bit.
	* gdbarch.sh (significant_addr_bit): New.
	* gdbarch.c, gdbarch.h: Re-generated.
	* target.c (memory_xfer_partial): Call address_significant.
	* utils.c (address_significant): New function.
	* utils.h (address_significant): Declare.

2017-10-19  Yao Qi  <yao.qi@linaro.org>

gdb/testsuite:

	* gdb.arch/aarch64-tagged-pointer.c: New file.
	* gdb.arch/aarch64-tagged-pointer.exp: New file.
---
 gdb/aarch64-linux-tdep.c                          |  2 +
 gdb/gdbarch.c                                     | 22 ++++++++
 gdb/gdbarch.h                                     |  8 +++
 gdb/gdbarch.sh                                    |  6 ++
 gdb/target.c                                      |  2 +
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   | 48 ++++++++++++++++
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 67 +++++++++++++++++++++++
 gdb/utils.c                                       | 17 ++++++
 gdb/utils.h                                       |  3 +
 9 files changed, 175 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index c2c2c30..b6c2a81 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1217,6 +1217,8 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_xml_syscall_file_name (gdbarch, "syscalls/aarch64-linux.xml");
   set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_number);
 
+  set_gdbarch_significant_addr_bit (gdbarch, 56);
+
   /* Displaced stepping.  */
   set_gdbarch_max_insn_length (gdbarch, 4 * DISPLACED_MODIFIED_INSNS);
   set_gdbarch_displaced_step_copy_insn (gdbarch,
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 007392c..8177f05 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -259,6 +259,7 @@ struct gdbarch
   int frame_red_zone_size;
   gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
   gdbarch_addr_bits_remove_ftype *addr_bits_remove;
+  int significant_addr_bit;
   gdbarch_software_single_step_ftype *software_single_step;
   gdbarch_single_step_through_delay_ftype *single_step_through_delay;
   gdbarch_print_insn_ftype *print_insn;
@@ -618,6 +619,8 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of stabs_argument_has_addr, invalid_p == 0 */
   /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0 */
   /* Skip verify of addr_bits_remove, invalid_p == 0 */
+  if (gdbarch->significant_addr_bit == 0)
+    gdbarch->significant_addr_bit = gdbarch_addr_bit (gdbarch);
   /* Skip verify of software_single_step, has predicate.  */
   /* Skip verify of single_step_through_delay, has predicate.  */
   /* Skip verify of print_insn, invalid_p == 0 */
@@ -1325,6 +1328,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: short_bit = %s\n",
                       plongest (gdbarch->short_bit));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: significant_addr_bit = %s\n",
+                      plongest (gdbarch->significant_addr_bit));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_single_step_through_delay_p() = %d\n",
                       gdbarch_single_step_through_delay_p (gdbarch));
   fprintf_unfiltered (file,
@@ -3216,6 +3222,22 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
 }
 
 int
+gdbarch_significant_addr_bit (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_significant_addr_bit called\n");
+  return gdbarch->significant_addr_bit;
+}
+
+void
+set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch,
+                                  int significant_addr_bit)
+{
+  gdbarch->significant_addr_bit = significant_addr_bit;
+}
+
+int
 gdbarch_software_single_step_p (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index d2e6b6f..1a654b6 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -677,6 +677,14 @@ typedef CORE_ADDR (gdbarch_addr_bits_remove_ftype) (struct gdbarch *gdbarch, COR
 extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr);
 extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
 
+/* On some machines, not all bits of an address word are significant.
+   For example, on AArch64, the top bits of an address known as the "tag"
+   are ignored by the kernel, the hardware, etc. and can be regarded as
+   additional data associated with the address. */
+
+extern int gdbarch_significant_addr_bit (struct gdbarch *gdbarch);
+extern void set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch, int significant_addr_bit);
+
 /* FIXME/cagney/2001-01-18: This should be split in two.  A target method that
    indicates if the target needs software single step.  An ISA method to
    implement it.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 6459b12..1f165cf 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -621,6 +621,12 @@ m;CORE_ADDR;convert_from_func_ptr_addr;CORE_ADDR addr, struct target_ops *targ;a
 # possible it should be in TARGET_READ_PC instead).
 m;CORE_ADDR;addr_bits_remove;CORE_ADDR addr;addr;;core_addr_identity;;0
 
+# On some machines, not all bits of an address word are significant.
+# For example, on AArch64, the top bits of an address known as the "tag"
+# are ignored by the kernel, the hardware, etc. and can be regarded as
+# additional data associated with the address.
+v;int;significant_addr_bit;;;;;gdbarch_addr_bit (gdbarch);
+
 # FIXME/cagney/2001-01-18: This should be split in two.  A target method that
 # indicates if the target needs software single step.  An ISA method to
 # implement it.
diff --git a/gdb/target.c b/gdb/target.c
index 3b8b8ea..767a2ad 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1214,6 +1214,8 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
   if (len == 0)
     return TARGET_XFER_EOF;
 
+  memaddr = address_significant (target_gdbarch (), memaddr);
+
   /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
      breakpoint insns, thus hiding out from higher layers whether
      there are software breakpoints inserted in the code stream.  */
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
new file mode 100644
index 0000000..7c90132
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
@@ -0,0 +1,48 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+struct s
+{
+  int i;
+};
+
+static void
+foo (void)
+{
+}
+
+int
+main (void)
+{
+  struct s s1;
+  struct s *sp1, *sp2;
+  int i = 1234;
+  int *p1, *p2;
+
+  s1.i = 1234;
+  sp1 = &s1;
+  p1 = &i;
+  /* SP1 and SP2 have different tags, but point to the same address.  */
+  sp2 = (struct s *) ((uintptr_t) sp1 | 0xf000000000000000ULL);
+  p2 = (int *) ((uintptr_t) p1 | 0xf000000000000000ULL);
+
+  void (*func_ptr) (void) = foo;
+  func_ptr = (void (*) (void)) ((uintptr_t) func_ptr | 0xf000000000000000ULL);
+  sp2->i = 4321; /* breakpoint here.  */
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
new file mode 100644
index 0000000..4f2b44c
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
@@ -0,0 +1,67 @@
+# Copyright 2017 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# This file is part of the gdb testsuite.
+
+if {![is_aarch64_target]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint here"]
+gdb_continue_to_breakpoint "breakpoint here"
+
+# Test that GDB manages caches correctly for tagged address.
+# Read from P2,
+gdb_test "x p2" "$hex:\[\t \]+0x000004d2"
+gdb_test_no_output "set variable i = 5678"
+# Test that *P2 is updated.
+gdb_test "x p2" "$hex:\[\t \]+0x0000162e"
+
+# Read from SP1->i,
+gdb_test "print sp1->i" " = 1234"
+# Write to SP2->i,
+gdb_test_no_output "set variable sp2->i = 5678"
+# Test that SP1->i is updated.
+gdb_test "print sp1->i" " = 5678"
+
+gdb_test "x/d &sp2->i" "$hex:\[\t \]+5678"
+gdb_test "x/d &sp1->i" "$hex:\[\t \]+5678"
+
+# Test that the same disassembly is got when disassembling function vs
+# tagged function pointer.
+set insn1 ""
+set insn2 ""
+set test "disassemble foo,+8"
+gdb_test_multiple $test $test {
+    -re ":\[\t \]+(\[a-z\]*)\[ \r\n\]+.*:\[\t \]+(\[a-z\]*).*$gdb_prompt $" {
+	set insn1 $expect_out(1,string)
+	set insn2 $expect_out(2,string)
+	pass $test
+    }
+}
+
+gdb_test "disassemble func_ptr,+8" \
+    ":\[\t \]+$insn1\[ \r\n\]+.*:\[\t \]+$insn2.*"
diff --git a/gdb/utils.c b/gdb/utils.c
index b95dcfd..2f8f06f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2724,6 +2724,23 @@ When set, debugging messages will be marked with seconds and microseconds."),
 			   &setdebuglist, &showdebuglist);
 }
 
+/* See utils.h.  */
+
+CORE_ADDR
+address_significant (gdbarch *gdbarch, CORE_ADDR addr)
+{
+  /* Truncate address to the significant bits of a target address,
+     avoiding shifts larger or equal than the width of a CORE_ADDR.
+     The local variable ADDR_BIT stops the compiler reporting a shift
+     overflow when it won't occur.  */
+  int addr_bit = gdbarch_significant_addr_bit (gdbarch);
+
+  if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
+    addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
+
+  return addr;
+}
+
 const char *
 paddress (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 349ab93..b2e3d57 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -438,6 +438,9 @@ extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);
 #define gdb_print_host_address(ADDR, STREAM) \
   gdb_print_host_address_1 ((const void *) ADDR, STREAM)
 
+/* Return the address only having significant bits.  */
+extern CORE_ADDR address_significant (gdbarch *gdbarch, CORE_ADDR addr);
+
 /* Convert CORE_ADDR to string in platform-specific manner.
    This is usually formatted similar to 0x%lx.  */
 extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr);
-- 
1.9.1

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Pedro Alves]

On 12/08/2017 10:04 AM, Yao Qi wrote:
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1217,6 +1217,8 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_xml_syscall_file_name (gdbarch, "syscalls/aarch64-linux.xml");
>    set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_number);
>  
> +  set_gdbarch_significant_addr_bit (gdbarch, 56);
> +

I think adding the comment about "tag" here would be nice.

   The top bits of an address are known as the "tag" and are
   ignored by the kernel, the hardware, etc. and can be regarded
   as additional data associated with the address. */
   set_gdbarch_significant_addr_bit (gdbarch, 56);

BTW, since this is ignored by the hardware, should it be done
in aarch64-tdep.c instead of just for Linux?

Looks good to me otherwise.

Thanks,
Pedro Alves

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Ulrich Weigand]

Pedro Alves wrote:
> On 12/08/2017 10:04 AM, Yao Qi wrote:
> > --- a/gdb/aarch64-linux-tdep.c
> > +++ b/gdb/aarch64-linux-tdep.c
> > @@ -1217,6 +1217,8 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >    set_xml_syscall_file_name (gdbarch, "syscalls/aarch64-linux.xml");
> >    set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_number);
> >  
> > +  set_gdbarch_significant_addr_bit (gdbarch, 56);
> > +
> 
> I think adding the comment about "tag" here would be nice.
> 
>    The top bits of an address are known as the "tag" and are
>    ignored by the kernel, the hardware, etc. and can be regarded
>    as additional data associated with the address. */
>    set_gdbarch_significant_addr_bit (gdbarch, 56);
> 
> BTW, since this is ignored by the hardware, should it be done
> in aarch64-tdep.c instead of just for Linux?
> 
> Looks good to me otherwise.

This seems to duplicate the functionality of gdbarch_addr_bits_remove
to some extent ...  Could those be merged back again?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Yao Qi]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:

> This seems to duplicate the functionality of gdbarch_addr_bits_remove
> to some extent ...  Could those be merged back again?

They are a little bit different, addr_bits_remove only applies to PC (for
arm and mips), but significant_addr_bit applies to every address (both
data and instruction).  On the other hand, the LSB of address in arm and
mips is still significant, but it has some different meaning other than
address when it is the target address of branch.  We only clear the LSB
when it is the target address.  In normal memory access, watchpoing
setting, we don't clear it.

Simon asked the same question, see
https://sourceware.org/ml/gdb-patches/2017-11/msg00213.html

These two gdbarch methods are quite similar, but difficult to merge
these two, unless we bring more context in the gdbarch hook method.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Ulrich Weigand]

Yao Qi wrote:

> diff --git a/gdb/target.c b/gdb/target.c
> index 3b8b8ea..767a2ad 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1214,6 +1214,8 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
>    if (len == 0)
>      return TARGET_XFER_EOF;
>  
> +  memaddr = address_significant (target_gdbarch (), memaddr);
> +
>    /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
>       breakpoint insns, thus hiding out from higher layers whether
>       there are software breakpoints inserted in the code stream.  */

It turns out this breaks SPU multi-architecture debugging.  The problem is
that SPU memory addresses have 64 significant bits since we encode the
SPU ID in the upper 32 bits.  This means that spu-tdep.c needs to call
set_gdbarch_significant_addr_bits -- which is fine.

However, this doesn't fix the problem, since "target_gdbarch ()" at this
point may well return a (32-bit) ppc architecture, and then the address
is still truncated incorrectly.  In general, using target_gdbarch is
nearly never the right thing to do in generic code as long as we want
to support multi-architecture debugging.

Can this call not be pushed down further in the target stack, to below
the xfer_partial implementation in the spu-multiarch target?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Yao Qi]

On Tue, Dec 19, 2017 at 1:50 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Yao Qi wrote:
>
>> diff --git a/gdb/target.c b/gdb/target.c
>> index 3b8b8ea..767a2ad 100644
>> --- a/gdb/target.c
>> +++ b/gdb/target.c
>> @@ -1214,6 +1214,8 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
>>    if (len == 0)
>>      return TARGET_XFER_EOF;
>>
>> +  memaddr = address_significant (target_gdbarch (), memaddr);
>> +
>>    /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
>>       breakpoint insns, thus hiding out from higher layers whether
>>       there are software breakpoints inserted in the code stream.  */
>
> It turns out this breaks SPU multi-architecture debugging.  The problem is
> that SPU memory addresses have 64 significant bits since we encode the
> SPU ID in the upper 32 bits.  This means that spu-tdep.c needs to call
> set_gdbarch_significant_addr_bits -- which is fine.
>

Right, or SPU memory address has 63 significant bits?  The top bit is 1.

> However, this doesn't fix the problem, since "target_gdbarch ()" at this
> point may well return a (32-bit) ppc architecture, and then the address
> is still truncated incorrectly.  In general, using target_gdbarch is
> nearly never the right thing to do in generic code as long as we want
> to support multi-architecture debugging.
>

Can we use target_thread_architecture (inferior_ptid) instead?  If GDB
still access SPU memory even program stops in PPU, this doesn't work.

> Can this call not be pushed down further in the target stack, to below

I think you meant "Can this call be", without "not".

> the xfer_partial implementation in the spu-multiarch target?
>

I am stilling thinking how to do it...  alternatively, do you like the approach
that we pass 'address' to gdbarch significant_addr_bits, and teach
ppc32 gdbarch significant_addr_bits be aware of SPU address, that is,
return 64 if the top bit is one (this address is the SPU address),
otherwise, return 32 (this address is normal PPU address).

-- 
Yao (齐尧)

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Ulrich Weigand]

Yao Qi wrote:
> On Tue, Dec 19, 2017 at 1:50 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > It turns out this breaks SPU multi-architecture debugging.  The problem is
> > that SPU memory addresses have 64 significant bits since we encode the
> > SPU ID in the upper 32 bits.  This means that spu-tdep.c needs to call
> > set_gdbarch_significant_addr_bits -- which is fine.
> >
> 
> Right, or SPU memory address has 63 significant bits?  The top bit is 1.

It's a "merged" address space containing the PowerPC address space (with
top bit 0) as well as all the SPU address spaces (top bit 1, next 31 bits
the SPU ID, last 32 bit the address on that SPU).

I wouldn't need any of that if we actually supported "real" address spaces,
i.e. instead of always passing just a CORE_ADDR, use a pair of address-space
identifier and CORE_ADDR.  But that would be a huge change, and so I worked
around that by using the "merged" encoding.

> > However, this doesn't fix the problem, since "target_gdbarch ()" at this
> > point may well return a (32-bit) ppc architecture, and then the address
> > is still truncated incorrectly.  In general, using target_gdbarch is
> > nearly never the right thing to do in generic code as long as we want
> > to support multi-architecture debugging.
> >
> 
> Can we use target_thread_architecture (inferior_ptid) instead?  If GDB
> still access SPU memory even program stops in PPU, this doesn't work.

Yes, that should still work (e.g. when the program stops anywhere, GDB
will check software watchpoints that may be simultaneously set in SPU
and PPU memory).  Therefore target_thread_architecture doesn't really 
solve the problem either.

> > Can this call not be pushed down further in the target stack, to below
> 
> I think you meant "Can this call be", without "not".
> 
> > the xfer_partial implementation in the spu-multiarch target?
> >
> 
> I am stilling thinking how to do it...  alternatively, do you like the appr=
> oach
> that we pass 'address' to gdbarch significant_addr_bits, and teach
> ppc32 gdbarch significant_addr_bits be aware of SPU address, that is,
> return 64 if the top bit is one (this address is the SPU address),
> otherwise, return 32 (this address is normal PPU address).

Well, a simple workaround is to just always set significant_addr_bits
to 64 in ppc as well.  (This just doesn't matter for actual ppc addresses.)
But that doesn't really look like a clean solution for the generic
multi-architecture case ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/3] Clear non-significant bits of address on memory access

[Yao Qi]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:

>> I am stilling thinking how to do it...  alternatively, do you like the appr=
>> oach
>> that we pass 'address' to gdbarch significant_addr_bits, and teach
>> ppc32 gdbarch significant_addr_bits be aware of SPU address, that is,
>> return 64 if the top bit is one (this address is the SPU address),
>> otherwise, return 32 (this address is normal PPU address).
>
> Well, a simple workaround is to just always set significant_addr_bits
> to 64 in ppc as well.  (This just doesn't matter for actual ppc addresses.)
> But that doesn't really look like a clean solution for the generic
> multi-architecture case ...

Nowadays, we strip non-significant bits in address, pass the stripped
address to target cache and to_xfer_partial.  It works for aarch64, but
breaks ppc32/spu.  However, in ppc32/spu case, ppu address and spu
address is mixed together, differentiated by the top bit, so the number
of significant bits of address is 64, because if we can't remove any of
them.  IMO, it is reasonable to set significant_addr_bits to 64 in ppc.

I considered your suggestion that pushing address_significant call down,
below spu-multiarch target, that means, many target's to_xfer_partial
need to call address_significant, so I don't do that.  Secondly, in the
way you suggested, we still pass the original address to target cache,
which works for ppu/spu, but it doesn't work for aarch64.

-- 
Yao (齐尧)

[pushed] Fix Cell/B.E. regression (Re: [PATCH 1/3] Clear non-significant bits of address on memory access)

[Ulrich Weigand]

Yao Qi wrote:

> Nowadays, we strip non-significant bits in address, pass the stripped
> address to target cache and to_xfer_partial.  It works for aarch64, but
> breaks ppc32/spu.  However, in ppc32/spu case, ppu address and spu
> address is mixed together, differentiated by the top bit, so the number
> of significant bits of address is 64, because if we can't remove any of
> them.  IMO, it is reasonable to set significant_addr_bits to 64 in ppc.
> 
> I considered your suggestion that pushing address_significant call down,
> below spu-multiarch target, that means, many target's to_xfer_partial
> need to call address_significant, so I don't do that.  Secondly, in the
> way you suggested, we still pass the original address to target cache,
> which works for ppu/spu, but it doesn't work for aarch64.

I've now pushed the patch below, which fixes the regression for now.

Longer term, I think the correct fix would probably be to make address
spaces explit, e.g. by passing an address space identifer to xfer_partial.
The gdbarch associated with that address space should then determine
whether truncation is required ...

Bye,
Ulrich


gdb/ChangeLog:

	* spu-tdep.c (spu_gdbarch_init): Set set_gdbarch_significant_addr_bit
	to 64 bits.
	(ppc_linux_init_abi): Likewise, if Cell/B.E. is supported.

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 0e43a64..5120490 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1809,6 +1809,10 @@ ppc_linux_init_abi (struct gdbarch_info info,
 
       /* Cell/B.E. cross-architecture unwinder support.  */
       frame_unwind_prepend_unwinder (gdbarch, &ppu2spu_unwind);
+
+      /* We need to support more than "addr_bit" significant address bits
+         in order to support SPUADDR_ADDR encoded values.  */
+      set_gdbarch_significant_addr_bit (gdbarch, 64);
     }
 
   set_gdbarch_displaced_step_location (gdbarch,
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index fb9a5d8..dda3011 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2720,6 +2720,9 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_address_class_name_to_type_flags
     (gdbarch, spu_address_class_name_to_type_flags);
 
+  /* We need to support more than "addr_bit" significant address bits
+     in order to support SPUADDR_ADDR encoded values.  */
+  set_gdbarch_significant_addr_bit (gdbarch, 64);
 
   /* Inferior function calls.  */
   set_gdbarch_call_dummy_location (gdbarch, ON_STACK);

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [pushed] Fix Cell/B.E. regression (Re: [PATCH 1/3] Clear non-significant bits of address on memory access)

[Yao Qi]

On Wed, Dec 20, 2017 at 1:03 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> I've now pushed the patch below, which fixes the regression for now.
>

Thank you.

> Longer term, I think the correct fix would probably be to make address
> spaces explit, e.g. by passing an address space identifer to xfer_partial.
> The gdbarch associated with that address space should then determine
> whether truncation is required ...
>

Agreed.

-- 
Yao (齐尧)

[PATCH 2/3] Adjust breakpoint address by clearing non-significant bits

[Yao Qi]

Tag in tagged address on AArch64 is treated as a non-significant bits of
address, which can be got by gdbarch method significant_addr_bit, and gdb
can clear these bits.

With this patch, when user sets a breakpoint on tagged address on AArch64,
GDB will drop the top byte of address, and put breakpoint at the new place,
as shown below,

(gdb) hbreak *func_ptr
warning: Breakpoint address adjusted from 0xf000000000400690 to 0x00400690.
Hardware assisted breakpoint 2 at 0x400690

(gdb) break *func_ptr
warning: Breakpoint address adjusted from 0xf000000000400690 to 0x00400690.
Breakpoint 3 at 0x400690

When program hits a breakpoint, the stopped pc reported by Linux kernel is
the address *without* tag, so it is better the address recorded in
breakpoint location is the one without tag too, so we can still match
breakpoint location address and stopped pc reported by Linux kernel, by
simple compare.

gdb:

2017-10-24  Yao Qi  <yao.qi@linaro.org>

	* breakpoint.c (adjust_breakpoint_address): Call
	address_significant.

gdb/testsuite:

2017-10-24  Yao Qi  <yao.qi@linaro.org>

	* gdb.arch/aarch64-tagged-pointer.c (main): Update.
	* gdb.arch/aarch64-tagged-pointer.exp: Add test for breakpoint.
---
 gdb/breakpoint.c                                  | 20 +++++++++----------
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   |  8 ++++++++
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 24 +++++++++++++++++++++++
 3 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 76bfd53..22b3069 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6987,12 +6987,7 @@ static CORE_ADDR
 adjust_breakpoint_address (struct gdbarch *gdbarch,
 			   CORE_ADDR bpaddr, enum bptype bptype)
 {
-  if (!gdbarch_adjust_breakpoint_address_p (gdbarch))
-    {
-      /* Very few targets need any kind of breakpoint adjustment.  */
-      return bpaddr;
-    }
-  else if (bptype == bp_watchpoint
+  if (bptype == bp_watchpoint
            || bptype == bp_hardware_watchpoint
            || bptype == bp_read_watchpoint
            || bptype == bp_access_watchpoint
@@ -7014,11 +7009,16 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
     }
   else
     {
-      CORE_ADDR adjusted_bpaddr;
+      CORE_ADDR adjusted_bpaddr = bpaddr;
+
+      if (gdbarch_adjust_breakpoint_address_p (gdbarch))
+	{
+	  /* Some targets have architectural constraints on the placement
+	     of breakpoint instructions.  Obtain the adjusted address.  */
+	  adjusted_bpaddr = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
+	}
 
-      /* Some targets have architectural constraints on the placement
-         of breakpoint instructions.  Obtain the adjusted address.  */
-      adjusted_bpaddr = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
+      adjusted_bpaddr = address_significant (gdbarch, adjusted_bpaddr);
 
       /* An adjusted breakpoint address can significantly alter
          a user's expectations.  Print a warning if an adjustment
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
index 7c90132..9bfe41e 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
@@ -45,4 +45,12 @@ main (void)
   void (*func_ptr) (void) = foo;
   func_ptr = (void (*) (void)) ((uintptr_t) func_ptr | 0xf000000000000000ULL);
   sp2->i = 4321; /* breakpoint here.  */
+
+  for (int i = 0; i < 2; i++)
+    {
+      foo ();
+      (*func_ptr) ();
+    }
+
+  sp1->i = 8765;
 }
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
index 4f2b44c..fcab1b7 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
@@ -65,3 +65,27 @@ gdb_test_multiple $test $test {
 
 gdb_test "disassemble func_ptr,+8" \
     ":\[\t \]+$insn1\[ \r\n\]+.*:\[\t \]+$insn2.*"
+
+foreach_with_prefix bptype {"hbreak" "break"} {
+
+    # Set a breakpoint on a tagged address, func_ptr,
+    gdb_test "$bptype *func_ptr" \
+	"warning: Breakpoint address adjusted from .*reakpoint $decimal at .*" \
+	"breakpoint at *func_ptr"
+    # Resume the program and expect it hits foo,
+    gdb_test "continue" \
+	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\) at .*" \
+	"run until breakpoint set *func_ptr"
+    gdb_test "up" "foo \\(\\).*" "caller is foo"
+    delete_breakpoints
+
+    # Set a breakpoint on normal function, call it through tagged
+    # function pointer.
+    gdb_test "$bptype foo" "reakpoint $decimal at .*" \
+	"hardware breakpoint at foo"
+    gdb_test "continue" \
+	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\) at .*" \
+	"run until breakpoint set foo"
+    gdb_test "up" "\\(\*func_ptr\\) \\(\\).*" "caller is *func_ptr"
+    delete_breakpoints
+}
-- 
1.9.1

Re: [PATCH 2/3] Adjust breakpoint address by clearing non-significant bits

[Pedro Alves]

On 12/08/2017 10:04 AM, Yao Qi wrote:

> When program hits a breakpoint, the stopped pc reported by Linux kernel is
> the address *without* tag, so it is better the address recorded in
> breakpoint location is the one without tag too, so we can still match
> breakpoint location address and stopped pc reported by Linux kernel, by
> simple compare.
> 

Agreed.  LGTM with formatting nit below.

> 2017-10-24  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdb.arch/aarch64-tagged-pointer.c (main): Update.
> 	* gdb.arch/aarch64-tagged-pointer.exp: Add test for breakpoint.
> ---
>  gdb/breakpoint.c                                  | 20 +++++++++----------
>  gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   |  8 ++++++++
>  gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 24 +++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 76bfd53..22b3069 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6987,12 +6987,7 @@ static CORE_ADDR
>  adjust_breakpoint_address (struct gdbarch *gdbarch,
>  			   CORE_ADDR bpaddr, enum bptype bptype)
>  {
> -  if (!gdbarch_adjust_breakpoint_address_p (gdbarch))
> -    {
> -      /* Very few targets need any kind of breakpoint adjustment.  */
> -      return bpaddr;
> -    }
> -  else if (bptype == bp_watchpoint
> +  if (bptype == bp_watchpoint
>             || bptype == bp_hardware_watchpoint
>             || bptype == bp_read_watchpoint
>             || bptype == bp_access_watchpoint

Please indent the rest of the condition before pushing.

OK with that fixed.

> @@ -7014,11 +7009,16 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
>      }
>    else

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 12/08/2017 10:04 AM, Yao Qi wrote:
> ARMv8 supports tagged address, that is, the top one byte in address
> is ignored.  It is always enabled on aarch64-linux.  See
> https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
> 
> Some parts of GDB related to address are updated in this patch series,
> 
>  - Memory access, like command 'x',
>  - Setting hw breakpoint on some address,
>  - Setting watchpoint on some address,
> 
> Address tag is treated as non-significant bits of address, so this patch
> series add a new gdbarch significant_addr_bit, and use it in gdbarch to
> get rid of non-significant bits.  This was the suggestion in the v2 review.
> (https://sourceware.org/ml/gdb-patches/2017-10/msg00792.html)

Thanks much for the update.  I like this version a lot better.

> 
> When I test this patch series, and I find a regression in linespec.  I
> posted the fix https://sourceware.org/ml/gdb-patches/2017-12/msg00158.html
> Without this fix, this series causes a regression.

Thanks again for that fix.

Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Yao Qi]

On Fri, Dec 8, 2017 at 10:04 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> ARMv8 supports tagged address, that is, the top one byte in address
> is ignored.  It is always enabled on aarch64-linux.  See
> https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
>
> Some parts of GDB related to address are updated in this patch series,
>
>  - Memory access, like command 'x',
>  - Setting hw breakpoint on some address,
>  - Setting watchpoint on some address,
>

I pushed them in as we are close to branching.  We can still keep the
addr_bits_remove vs significant_addr_bit conversation ongoing.

-- 
Yao (齐尧)

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 8 December 2017 at 22:31, Yao Qi <qiyaoltc@gmail.com> wrote:

> On Fri, Dec 8, 2017 at 10:04 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> > ARMv8 supports tagged address, that is, the top one byte in address
> > is ignored.  It is always enabled on aarch64-linux.  See
> > https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
> >
> > Some parts of GDB related to address are updated in this patch series,
> >
> >  - Memory access, like command 'x',
> >  - Setting hw breakpoint on some address,
> >  - Setting watchpoint on some address,
> >
>
> I pushed them in as we are close to branching.  We can still keep the
> addr_bits_remove vs significant_addr_bit conversation ongoing.
>

This patch has broken kernel debugging using kgdb and openOCD.

Tagged address should only be considered while debugging only user-space
programs on linux.

I still need to understand the full background behind this patch but can we
pull it out of 8.1 release to make sure kernel debugging works?

--
> Yao (齐尧)
>

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 11 April 2018 at 05:15, Omair Javaid <omair.javaid@linaro.org> wrote:

> On 8 December 2017 at 22:31, Yao Qi <qiyaoltc@gmail.com> wrote:
>
>> On Fri, Dec 8, 2017 at 10:04 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
>> > ARMv8 supports tagged address, that is, the top one byte in address
>> > is ignored.  It is always enabled on aarch64-linux.  See
>> > https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
>> >
>> > Some parts of GDB related to address are updated in this patch series,
>> >
>> >  - Memory access, like command 'x',
>> >  - Setting hw breakpoint on some address,
>> >  - Setting watchpoint on some address,
>> >
>>
>> I pushed them in as we are close to branching.  We can still keep the
>> addr_bits_remove vs significant_addr_bit conversation ongoing.
>>
>
> This patch has broken kernel debugging using kgdb and openOCD.
>
> Tagged address should only be considered while debugging only user-space
> programs on linux.
>
> I still need to understand the full background behind this patch but can
> we pull it out of 8.1 release to make sure kernel debugging works?
>
>
Just found out that patch series posted here
https://sourceware.org/ml/gdb-patches/2017-12/msg00160.html does change
aarch64-linux-tdep only.
But the committed version here
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=a738ea1d41daeec0cccb4ab6671f4f6d53bd9e18
is
applying it to aarch64-tdep


> --
>> Yao (齐尧)
>>
>
>

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Simon Marchi]

On 2018-04-10 20:36, Omair Javaid wrote:
>> This patch has broken kernel debugging using kgdb and openOCD.
>> 
>> Tagged address should only be considered while debugging only 
>> user-space
>> programs on linux.
>> 
>> I still need to understand the full background behind this patch but 
>> can
>> we pull it out of 8.1 release to make sure kernel debugging works?
>> 
>> 
> Just found out that patch series posted here
> https://sourceware.org/ml/gdb-patches/2017-12/msg00160.html does change
> aarch64-linux-tdep only.
> But the committed version here
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=a738ea1d41daeec0cccb4ab6671f4f6d53bd9e18
> is
> applying it to aarch64-tdep

I think it was because of Pedro's suggestion in this reply:
https://sourceware.org/ml/gdb-patches/2017-12/msg00179.html

although there was now acknowledgement from Yao, so I can only guess.

As Pedro said, it's the hardware that ignores these top bits.  But it 
first has to be configured to do so, so we can't assume that all aarch64 
code behaves like this.  So far we only know that the Linux userspace 
threads work like that, so I think it would make sense to move it to 
aarch64-linux-tdep.c.  Did you confirm that it indeed fixes your 
problem?

I could see a fix for this getting into the 8.1 branch (not reverting 
the patch though).

Simon

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 04/11/2018 01:15 AM, Omair Javaid wrote:

> This patch has broken kernel debugging using kgdb and openOCD.

OOC, can you qualify this a bit more, please?

Does the kernel use the high bits for something?

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 11 April 2018 at 15:13, Pedro Alves <palves@redhat.com> wrote:

> On 04/11/2018 01:15 AM, Omair Javaid wrote:
>
> > This patch has broken kernel debugging using kgdb and openOCD.
>
> OOC, can you qualify this a bit more, please?
>
> Does the kernel use the high bits for something?
>

We can safely assume that top byte is 0 in case of user address space
on linux because it enables tagging support but not for kernel address
space.

According to linux memory layout of AArch64 given here:
https://www.kernel.org/doc/Documentation/arm64/memory.txt

"User addresses have bits 63:48 set to 0 while the kernel addresses have
the same bits set to 1. TTBRx selection is given by bit 63 of the
virtual address."

According to kernel document on tagged pointer support in AArch64
given here: https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt

The kernel configures the translation tables so that translations made
via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of
the virtual address ignored by the translation hardware. This frees up
this byte for application use.

With set_gdbarch_significant_addr_bit applied to aarch64-tdep
following happens when gdb tries reading kernel address space memory:

query the 0xffffffc000092698 memory data, GDB sent
"m00ffffc000092698,4" instead of "mffffffc000092698,4"




> Thanks,
> Pedro Alves
>

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 04/11/2018 12:12 PM, Omair Javaid wrote:
> On 11 April 2018 at 15:13, Pedro Alves <palves@redhat.com <mailto:palves@redhat.com>> wrote:
> 
>     On 04/11/2018 01:15 AM, Omair Javaid wrote:
> 
>     > This patch has broken kernel debugging using kgdb and openOCD.
> 
>     OOC, can you qualify this a bit more, please?
> 
>     Does the kernel use the high bits for something?
> 
> 
> We can safely assume that top byte is 0 in case of user address space on linux because it enables tagging support but not for kernel address space.
> 
> According to linux memory layout of AArch64 given here: https://www.kernel.org/doc/Documentation/arm64/memory.txt
> 
> "User addresses have bits 63:48 set to 0 while the kernel addresses have
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> the same bits set to 1. TTBRx selection is given by bit 63 of the
^^^^^^^^^^^^^^^^^^^^^^^^^
> virtual address."

Ah, that's clear as day now.

> 
> According to kernel document on tagged pointer support in AArch64 given here: https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
> 
> The kernel configures the translation tables so that translations made
> via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of
> the virtual address ignored by the translation hardware. This frees up
> this byte for application use.
> 
> With set_gdbarch_significant_addr_bit applied to aarch64-tdep following happens when gdb tries reading kernel address space memory:
> 
> query the 0xffffffc000092698 memory data, GDB sent "m00ffffc000092698,4" instead of "mffffffc000092698,4"
> 
OK, that makes a lot more sense now.  The above is the perfect
info to be included in a git commit log.  Want to submit a patch?

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 11 April 2018 at 16:19, Pedro Alves <palves@redhat.com> wrote:

> On 04/11/2018 12:12 PM, Omair Javaid wrote:
> > On 11 April 2018 at 15:13, Pedro Alves <palves@redhat.com <mailto:
> palves@redhat.com>> wrote:
> >
> >     On 04/11/2018 01:15 AM, Omair Javaid wrote:
> >
> >     > This patch has broken kernel debugging using kgdb and openOCD.
> >
> >     OOC, can you qualify this a bit more, please?
> >
> >     Does the kernel use the high bits for something?
> >
> >
> > We can safely assume that top byte is 0 in case of user address space on
> linux because it enables tagging support but not for kernel address space.
> >
> > According to linux memory layout of AArch64 given here:
> https://www.kernel.org/doc/Documentation/arm64/memory.txt
> >
> > "User addresses have bits 63:48 set to 0 while the kernel addresses have
>                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > the same bits set to 1. TTBRx selection is given by bit 63 of the
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> > virtual address."
>
> Ah, that's clear as day now.
>
> >
> > According to kernel document on tagged pointer support in AArch64 given
> here: https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt
> >
> > The kernel configures the translation tables so that translations made
> > via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of
> > the virtual address ignored by the translation hardware. This frees up
> > this byte for application use.
> >
> > With set_gdbarch_significant_addr_bit applied to aarch64-tdep following
> happens when gdb tries reading kernel address space memory:
> >
> > query the 0xffffffc000092698 memory data, GDB sent "m00ffffc000092698,4"
> instead of "mffffffc000092698,4"
> >
> OK, that makes a lot more sense now.  The above is the perfect
> info to be included in a git commit log.  Want to submit a patch?
>

Yes I can submit a patch that enables set_gdbarch_significant_addr_bit for
aarch64-linux-tdep only.

But a point to discuss here is the use-case where some people use
*-linux-gdb for debugging seamlessly between kernel and user-space.

There can be ways we can distinguish between user/kernel address space and
clear or set top byte of the address even in case of linux targets.

Does this sound something we should do?


>
> Thanks,
> Pedro Alves
>

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 04/11/2018 12:59 PM, Omair Javaid wrote:

> Yes I can submit a patch that enables set_gdbarch_significant_addr_bit for aarch64-linux-tdep only.
> 
> But a point to discuss here is the use-case where some people use *-linux-gdb for debugging seamlessly between kernel and user-space.
> 
> There can be ways we can distinguish between user/kernel address space and clear or set top byte of the address even in case of linux targets.
> 
> Does this sound something we should do? 

Yeah, why not.  

What are the pending kernel debugging patches using to distinguish
userspace and kernel debugging modes?  Off hand, I'd think we'd want to
make those separate ABIs / osabis / gdbarchs.

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 11 April 2018 at 23:27, Pedro Alves <palves@redhat.com> wrote:

> On 04/11/2018 12:59 PM, Omair Javaid wrote:
>
> > Yes I can submit a patch that enables set_gdbarch_significant_addr_bit
> for aarch64-linux-tdep only.
> >
> > But a point to discuss here is the use-case where some people use
> *-linux-gdb for debugging seamlessly between kernel and user-space.
> >
> > There can be ways we can distinguish between user/kernel address space
> and clear or set top byte of the address even in case of linux targets.
> >
> > Does this sound something we should do?
>
> Yeah, why not.
>
> What are the pending kernel debugging patches using to distinguish
> userspace and kernel debugging modes?  Off hand, I'd think we'd want to
> make those separate ABIs / osabis / gdbarchs.
>

Sorry for late reply on this I am out of office this week.

I have given this a thought and I propose to do the following:

Turn on pointer tagging on OSABI (LINUX) by default.

Add commands set aarch64 pointer-tagging show/enable/disable.

Once LKD patches for aarch64/arm land in our need for this will
automatically be solved.

^ Does this make sense?

For Linux Kernel Debugging we have separate OSABI (LINUX_KERNEL). Patch
snippet from LKD patches by Phillip given below:

+static enum gdb_osabi
+s390_lk_osabi_sniffer (bfd *abfd)
+{
+  if (bfd_get_section_by_name (abfd, ".reg-s390-prefix")
+      || bfd_get_section_by_name (abfd, "__ksymtab"))
+    return GDB_OSABI_LINUX_KERNEL;
+
+  return GDB_OSABI_UNKNOWN;
+}
+
+extern initialize_file_ftype _initialize_s390_lk_tdep; /*
-Wmissing-prototypes */
+
+void
+_initialize_s390_lk_tdep (void)
+{
+  /* Hook us into the OSABI mechanism.  */
+  gdbarch_register_osabi (bfd_arch_s390, bfd_mach_s390_64,
GDB_OSABI_LINUX_KERNEL,
+			  s390_lk_init_abi_64);
+  /* Add OSABI sniffer.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_s390, bfd_target_elf_flavour,
+				  s390_lk_osabi_sniffer);
+
+  /* Initialize the Linux kernel target descriptions.  */
+  initialize_tdesc_s390x_cr_linux64 ();
+  initialize_tdesc_s390x_vxcr_linux64 ();
+}


Thanks,
> Pedro Alves
>

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 04/16/2018 02:36 AM, Omair Javaid wrote:
> On 11 April 2018 at 23:27, Pedro Alves <palves@redhat.com> wrote:
> 
>> On 04/11/2018 12:59 PM, Omair Javaid wrote:
>>
>>> Yes I can submit a patch that enables set_gdbarch_significant_addr_bit
>> for aarch64-linux-tdep only.
>>>
>>> But a point to discuss here is the use-case where some people use
>> *-linux-gdb for debugging seamlessly between kernel and user-space.
>>>
>>> There can be ways we can distinguish between user/kernel address space
>> and clear or set top byte of the address even in case of linux targets.
>>>
>>> Does this sound something we should do?
>>
>> Yeah, why not.
>>
>> What are the pending kernel debugging patches using to distinguish
>> userspace and kernel debugging modes?  Off hand, I'd think we'd want to
>> make those separate ABIs / osabis / gdbarchs.
>>
> 
> Sorry for late reply on this I am out of office this week.
> 
> I have given this a thought and I propose to do the following:
> 
> Turn on pointer tagging on OSABI (LINUX) by default.
> 
> Add commands set aarch64 pointer-tagging show/enable/disable.
> 
> Once LKD patches for aarch64/arm land in our need for this will
> automatically be solved.

Makes sense, but I'd like to clarify usefulness of the separate
"set aarch64 pointer-tagging" command.
If indeed we're doing to end up with a separate osabi for the Linux
kernel, then "set osabi linux-kernel" will result
in disabling pointer-tagging too.  So, will it still be useful to have
the specific "set aarch64 pointer-tagging" commands?  Do you see
use cases for "set aarch64 pointer-tagging" beyond disabling it
for Linux kernel debugging?  I'm thinking that it may be useful
for bare metal debugging.  But, ideally, GDB would figure it out
on its own without user intervention.  Is there's some bit in some
register gdb could read that indicates whether tagging is enabled?

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 17 April 2018 at 03:57, Pedro Alves <palves@redhat.com> wrote:

> On 04/16/2018 02:36 AM, Omair Javaid wrote:
> > On 11 April 2018 at 23:27, Pedro Alves <palves@redhat.com> wrote:
> >
> >> On 04/11/2018 12:59 PM, Omair Javaid wrote:
> >>
> >>> Yes I can submit a patch that enables set_gdbarch_significant_addr_bit
> >> for aarch64-linux-tdep only.
> >>>
> >>> But a point to discuss here is the use-case where some people use
> >> *-linux-gdb for debugging seamlessly between kernel and user-space.
> >>>
> >>> There can be ways we can distinguish between user/kernel address space
> >> and clear or set top byte of the address even in case of linux targets.
> >>>
> >>> Does this sound something we should do?
> >>
> >> Yeah, why not.
> >>
> >> What are the pending kernel debugging patches using to distinguish
> >> userspace and kernel debugging modes?  Off hand, I'd think we'd want to
> >> make those separate ABIs / osabis / gdbarchs.
> >>
> >
> > Sorry for late reply on this I am out of office this week.
> >
> > I have given this a thought and I propose to do the following:
> >
> > Turn on pointer tagging on OSABI (LINUX) by default.
> >
> > Add commands set aarch64 pointer-tagging show/enable/disable.
> >
> > Once LKD patches for aarch64/arm land in our need for this will
> > automatically be solved.
>
> Makes sense, but I'd like to clarify usefulness of the separate
> "set aarch64 pointer-tagging" command.
> If indeed we're doing to end up with a separate osabi for the Linux
> kernel, then "set osabi linux-kernel" will result
> in disabling pointer-tagging too.  So, will it still be useful to have
> the specific "set aarch64 pointer-tagging" commands?  Do you see
> use cases for "set aarch64 pointer-tagging" beyond disabling it
> for Linux kernel debugging?  I'm thinking that it may be useful
> for bare metal debugging.  But, ideally, GDB would figure it out
> on its own without user intervention.  Is there's some bit in some
> register gdb could read that indicates whether tagging is enabled?
>
>
Pointer tagging information is stored in MMU registers so in linux
user-space we cannot actually read if pointer tagging is enabled or not
based on register bits.
JTAG debuggers should be able to read MMU registers and know whether
pointer tagging is enabled or not.

Rationale behind adding a separate command is to allow other application to
control pointer tagging for example bare-metal (non-linux OSes) which want
to use pointer tagging can enable it. I must admit I dont know of any such
use-case as of now.

Also I am not sure about the timeline of Linux Kernel patches going into
gdb and for now I thought of this command as the most suitable option.
Moreover some users might also be interested in combination where pointer
tagging is enabled but Linux Kernel threads support is disabled so I
thought we should give the control to the user in cases where we cannot
predict use-cases.


> Thanks,
> Pedro Alves
>

Re: Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Daniel Thompson]

On 20/04/18 15:33, Omair Javaid wrote:
> On 17 April 2018 at 03:57, Pedro Alves <palves@redhat.com> wrote:
> 
>> On 04/16/2018 02:36 AM, Omair Javaid wrote:
>>> On 11 April 2018 at 23:27, Pedro Alves <palves@redhat.com> wrote:
>>>
>>>> On 04/11/2018 12:59 PM, Omair Javaid wrote:
>>>>
>>>>> Yes I can submit a patch that enables set_gdbarch_significant_addr_bit
>>>> for aarch64-linux-tdep only.
>>>>>
>>>>> But a point to discuss here is the use-case where some people use
>>>> *-linux-gdb for debugging seamlessly between kernel and user-space.
>>>>>
>>>>> There can be ways we can distinguish between user/kernel address space
>>>> and clear or set top byte of the address even in case of linux targets.
>>>>>
>>>>> Does this sound something we should do?
>>>>
>>>> Yeah, why not.
>>>>
>>>> What are the pending kernel debugging patches using to distinguish
>>>> userspace and kernel debugging modes?  Off hand, I'd think we'd want to
>>>> make those separate ABIs / osabis / gdbarchs.
>>>>
>>>
>>> Sorry for late reply on this I am out of office this week.
>>>
>>> I have given this a thought and I propose to do the following:
>>>
>>> Turn on pointer tagging on OSABI (LINUX) by default.
>>>
>>> Add commands set aarch64 pointer-tagging show/enable/disable.
>>>
>>> Once LKD patches for aarch64/arm land in our need for this will
>>> automatically be solved.
>>
>> Makes sense, but I'd like to clarify usefulness of the separate
>> "set aarch64 pointer-tagging" command.
>> If indeed we're doing to end up with a separate osabi for the Linux
>> kernel, then "set osabi linux-kernel" will result
>> in disabling pointer-tagging too.  So, will it still be useful to have
>> the specific "set aarch64 pointer-tagging" commands?  Do you see
>> use cases for "set aarch64 pointer-tagging" beyond disabling it
>> for Linux kernel debugging?  I'm thinking that it may be useful
>> for bare metal debugging.  But, ideally, GDB would figure it out
>> on its own without user intervention.  Is there's some bit in some
>> register gdb could read that indicates whether tagging is enabled?
>>
>>
> Pointer tagging information is stored in MMU registers so in linux
> user-space we cannot actually read if pointer tagging is enabled or not
> based on register bits.
> JTAG debuggers should be able to read MMU registers and know whether
> pointer tagging is enabled or not.

Perhaps a dumb question but could gdb be persuaded to mask the pointers 
at a lower level.

The current patches end up masking the pointer tags relatively early, 
which results in masked pointers being sent via the gdb remote protocol 
(which is what causes some of the problems at present: kgdb and OpenOCD 
get asked for the wrong pointer).

If the pointers were masked as the arguments to ptrace() were marshaled 
this would behave much more like the real hardware and would make 
debugging Linux kernel mode entirely transparent (since you cannot 
ptrace() kernel memory we would never try masking out the tag).

More generally masking would become the problem of the register read 
code for the target. I suspect many JTAG debuggers would (and certainly 
should) already work correctly as their register reads can honour the 
TTBR0 status.


Daniel.

Re: Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 20 April 2018 at 21:13, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>
> On 20/04/18 15:33, Omair Javaid wrote:
>>
>> On 17 April 2018 at 03:57, Pedro Alves <palves@redhat.com> wrote:
>>
>>> On 04/16/2018 02:36 AM, Omair Javaid wrote:
>>>>
>>>> On 11 April 2018 at 23:27, Pedro Alves <palves@redhat.com> wrote:
>>>>
>>>>> On 04/11/2018 12:59 PM, Omair Javaid wrote:
>>>>>
>>>>>> Yes I can submit a patch that enables set_gdbarch_significant_addr_bit
>>>>>
>>>>> for aarch64-linux-tdep only.
>>>>>>
>>>>>>
>>>>>> But a point to discuss here is the use-case where some people use
>>>>>
>>>>> *-linux-gdb for debugging seamlessly between kernel and user-space.
>>>>>>
>>>>>>
>>>>>> There can be ways we can distinguish between user/kernel address space
>>>>>
>>>>> and clear or set top byte of the address even in case of linux targets.
>>>>>>
>>>>>>
>>>>>> Does this sound something we should do?
>>>>>
>>>>>
>>>>> Yeah, why not.
>>>>>
>>>>> What are the pending kernel debugging patches using to distinguish
>>>>> userspace and kernel debugging modes?  Off hand, I'd think we'd want to
>>>>> make those separate ABIs / osabis / gdbarchs.
>>>>>
>>>>
>>>> Sorry for late reply on this I am out of office this week.
>>>>
>>>> I have given this a thought and I propose to do the following:
>>>>
>>>> Turn on pointer tagging on OSABI (LINUX) by default.
>>>>
>>>> Add commands set aarch64 pointer-tagging show/enable/disable.
>>>>
>>>> Once LKD patches for aarch64/arm land in our need for this will
>>>> automatically be solved.
>>>
>>>
>>> Makes sense, but I'd like to clarify usefulness of the separate
>>> "set aarch64 pointer-tagging" command.
>>> If indeed we're doing to end up with a separate osabi for the Linux
>>> kernel, then "set osabi linux-kernel" will result
>>> in disabling pointer-tagging too.  So, will it still be useful to have
>>> the specific "set aarch64 pointer-tagging" commands?  Do you see
>>> use cases for "set aarch64 pointer-tagging" beyond disabling it
>>> for Linux kernel debugging?  I'm thinking that it may be useful
>>> for bare metal debugging.  But, ideally, GDB would figure it out
>>> on its own without user intervention.  Is there's some bit in some
>>> register gdb could read that indicates whether tagging is enabled?
>>>
>>>
>> Pointer tagging information is stored in MMU registers so in linux
>> user-space we cannot actually read if pointer tagging is enabled or not
>> based on register bits.
>> JTAG debuggers should be able to read MMU registers and know whether
>> pointer tagging is enabled or not.
>
>
> Perhaps a dumb question but could gdb be persuaded to mask the pointers at a lower level.
>
> The current patches end up masking the pointer tags relatively early, which results in masked pointers being sent via the gdb remote protocol (which is what causes some of the problems at present: kgdb and OpenOCD get asked for the wrong pointer).
>
> If the pointers were masked as the arguments to ptrace() were marshaled this would behave much more like the real hardware and would make debugging Linux kernel mode entirely transparent (since you cannot ptrace() kernel memory we would never try masking out the tag).

Although this can be done with a hook but will require some
fundamental changes to the way ptrace inf_ptrace_xfer_partial memory
accesses routines are written. Currently we use a generic
implementation inf_ptrace_xfer_partial for all target architectures.
Same is the case with GDBServer it just handles the ptrace calls
except in a few cases where we need extra architecture specific code
before ptrace call like setting hardware breakpoints watchpoints etc.

As top byte in tagged address is essentially data, pushing masking
down to gdbserver will mean that we ll be sending out data mangled as
part of the address. Passing mangled address over RSP expecting other
side will correct it doesnt sound right.

Lets see what Pedro has to see on this.

>
> More generally masking would become the problem of the register read code for the target. I suspect many JTAG debuggers would (and certainly should) already work correctly as their register reads can honour the TTBR0 status.
>
>
> Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

On 04/23/2018 08:50 AM, Omair Javaid wrote:
> On 20 April 2018 at 21:13, Daniel Thompson <daniel.thompson@linaro.org> wrote:

>> Perhaps a dumb question but could gdb be persuaded to mask the pointers at a lower level.
>>
>> The current patches end up masking the pointer tags relatively early, which results in masked pointers being sent via the gdb remote protocol (which is what causes some of the problems at present: kgdb and OpenOCD get asked for the wrong pointer).
>>
>> If the pointers were masked as the arguments to ptrace() were marshaled this would behave much more like the real hardware and would make debugging Linux kernel mode entirely transparent (since you cannot ptrace() kernel memory we would never try masking out the tag).
> 
> Although this can be done with a hook but will require some
> fundamental changes to the way ptrace inf_ptrace_xfer_partial memory
> accesses routines are written. Currently we use a generic
> implementation inf_ptrace_xfer_partial for all target architectures.
> Same is the case with GDBServer it just handles the ptrace calls
> except in a few cases where we need extra architecture specific code
> before ptrace call like setting hardware breakpoints watchpoints etc.
> 
> As top byte in tagged address is essentially data, pushing masking
> down to gdbserver will mean that we ll be sending out data mangled as
> part of the address. Passing mangled address over RSP expecting other
> side will correct it doesnt sound right.
> 
> Lets see what Pedro has to see on this.

It seems like you are proposing going back to what was originally
proposed in v1.  I think these urls link through all of the
history:

 v1:
 https://sourceware.org/ml/gdb-patches/2017-10/msg00593.html
 v2:
 https://sourceware.org/ml/gdb-patches/2017-10/msg00792.html
 v3:
 https://sourceware.org/ml/gdb-patches/2017-12/msg00159.html

As can be seen in v2, as soon we started considering watchpoints,
here:
  https://sourceware.org/ml/gdb-patches/2017-10/msg00793.html
the gdb core needed to be made aware of tagged pointers, the bit
masking could no longer be deferred to the lower level ptrace calls
alone.  And then given that the core of gdb needs to know to
mask out tagged address, by v3, we had no masking at the ptrace
level anymore.  So I'm not sure how that would work; we'd need
a more detailed proposal.

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Daniel Thompson]

On Tue, Apr 24, 2018 at 12:39:15PM +0100, Pedro Alves wrote:
> On 04/23/2018 08:50 AM, Omair Javaid wrote:
> > On 20 April 2018 at 21:13, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> 
> >> Perhaps a dumb question but could gdb be persuaded to mask the pointers at a lower level.
> >>
> >> The current patches end up masking the pointer tags relatively early, which results in masked pointers being sent via the gdb remote protocol (which is what causes some of the problems at present: kgdb and OpenOCD get asked for the wrong pointer).
> >>
> >> If the pointers were masked as the arguments to ptrace() were marshaled this would behave much more like the real hardware and would make debugging Linux kernel mode entirely transparent (since you cannot ptrace() kernel memory we would never try masking out the tag).
> > 
> > Although this can be done with a hook but will require some
> > fundamental changes to the way ptrace inf_ptrace_xfer_partial memory
> > accesses routines are written. Currently we use a generic
> > implementation inf_ptrace_xfer_partial for all target architectures.
> > Same is the case with GDBServer it just handles the ptrace calls
> > except in a few cases where we need extra architecture specific code
> > before ptrace call like setting hardware breakpoints watchpoints etc.
> > 
> > As top byte in tagged address is essentially data, pushing masking
> > down to gdbserver will mean that we ll be sending out data mangled as
> > part of the address. Passing mangled address over RSP expecting other
> > side will correct it doesnt sound right.
> > 
> > Lets see what Pedro has to see on this.
> 
> It seems like you are proposing going back to what was originally
> proposed in v1.  I think these urls link through all of the
> history:
> 
>  v1:
>  https://sourceware.org/ml/gdb-patches/2017-10/msg00593.html
>  v2:
>  https://sourceware.org/ml/gdb-patches/2017-10/msg00792.html
>  v3:
>  https://sourceware.org/ml/gdb-patches/2017-12/msg00159.html
> 
> As can be seen in v2, as soon we started considering watchpoints,
> here:
>   https://sourceware.org/ml/gdb-patches/2017-10/msg00793.html
> the gdb core needed to be made aware of tagged pointers, the bit
> masking could no longer be deferred to the lower level ptrace calls
> alone.  And then given that the core of gdb needs to know to
> mask out tagged address, by v3, we had no masking at the ptrace
> level anymore.  So I'm not sure how that would work; we'd need
> a more detailed proposal.

Thanks for the links.

If you've been round this loop once before I don't think I want to
encourage anyone to go round it again.


Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Pedro Alves]

Hi,

On 04/20/2018 03:33 PM, Omair Javaid wrote:

> Pointer tagging information is stored in MMU registers so in linux
> user-space we cannot actually read if pointer tagging is enabled or not
> based on register bits.
> JTAG debuggers should be able to read MMU registers and know whether
> pointer tagging is enabled or not.
> 
> Rationale behind adding a separate command is to allow other application to
> control pointer tagging for example bare-metal (non-linux OSes) which want
> to use pointer tagging can enable it. I must admit I dont know of any such
> use-case as of now.

Alright, that's in line with what I was thinking.  Though, bare metal
should have access to MMU registers too.  Ideally, things would Just Work
without user intervention.  But I don't mind starting by adding a
user-controllable knob, it might be a convenient escape hatch.  We can always
extend it from "on/off" -> "on/off/auto" setting, with auto the default
in future.

> Also I am not sure about the timeline of Linux Kernel patches going into
> gdb and for now I thought of this command as the most suitable option.
> Moreover some users might also be interested in combination where pointer
> tagging is enabled but Linux Kernel threads support is disabled so I
> thought we should give the control to the user in cases where we cannot
> predict use-cases.

If everyone agrees that proper Linux kernel support benefits from
its own osabi setting/name, then I don't see why we couldn't start by
adding the osabi setting as soon as we have a use for it, even if 
the larger Linux Kernel patches aren't ready yet.

Thanks,
Pedro Alves

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Daniel Thompson]

On Tue, Apr 24, 2018 at 12:48:19PM +0100, Pedro Alves wrote:
> Hi,
> 
> On 04/20/2018 03:33 PM, Omair Javaid wrote:
> 
> > Pointer tagging information is stored in MMU registers so in linux
> > user-space we cannot actually read if pointer tagging is enabled or not
> > based on register bits.
> > JTAG debuggers should be able to read MMU registers and know whether
> > pointer tagging is enabled or not.
> > 
> > Rationale behind adding a separate command is to allow other application to
> > control pointer tagging for example bare-metal (non-linux OSes) which want
> > to use pointer tagging can enable it. I must admit I dont know of any such
> > use-case as of now.
> 
> Alright, that's in line with what I was thinking.  Though, bare metal
> should have access to MMU registers too.  Ideally, things would Just Work
> without user intervention.  But I don't mind starting by adding a
> user-controllable knob, it might be a convenient escape hatch.  We can always
> extend it from "on/off" -> "on/off/auto" setting, with auto the default
> in future.

For bare metal cases this is not a simple on/off control!

Top byte ignore (a.k.a. pointer tagging) has separate on/off switches 
for TTBR0 (0x0 upwards) and TTBR1 (0xffffffffffff downwards) *and* we 
have to know the respective sizes of TTBR0 and TTBR1 to be sure which
table we are using.


> > Also I am not sure about the timeline of Linux Kernel patches going into
> > gdb and for now I thought of this command as the most suitable option.
> > Moreover some users might also be interested in combination where pointer
> > tagging is enabled but Linux Kernel threads support is disabled so I
> > thought we should give the control to the user in cases where we cannot
> > predict use-cases.
> 
> If everyone agrees that proper Linux kernel support benefits from
> its own osabi setting/name, then I don't see why we couldn't start by
> adding the osabi setting as soon as we have a use for it, even if 
> the larger Linux Kernel patches aren't ready yet.

Following on from the above, for aarch64-linux-tdep we can apply domain
knowledge regarding how things are configured. Here we know that TTBR0 
is guaranteed to have top byte ignore set, TTBR1 does not *and* we
also know (from memory-layout.txt) that TTBR0 is sufficiently small 
that bit 55 can be used to discriminate between the two cases.

In others words regardless of whether we are running at EL0 or EL1 then 
I think we should mask the top byte from pointers if and only if bit 55 
is unset, otherwise leave them as they are.


Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 24 April 2018 at 21:05, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Tue, Apr 24, 2018 at 12:48:19PM +0100, Pedro Alves wrote:
>> Hi,
>>
>> On 04/20/2018 03:33 PM, Omair Javaid wrote:
>>
>> > Pointer tagging information is stored in MMU registers so in linux
>> > user-space we cannot actually read if pointer tagging is enabled or not
>> > based on register bits.
>> > JTAG debuggers should be able to read MMU registers and know whether
>> > pointer tagging is enabled or not.
>> >
>> > Rationale behind adding a separate command is to allow other application to
>> > control pointer tagging for example bare-metal (non-linux OSes) which want
>> > to use pointer tagging can enable it. I must admit I dont know of any such
>> > use-case as of now.
>>
>> Alright, that's in line with what I was thinking.  Though, bare metal
>> should have access to MMU registers too.  Ideally, things would Just Work
>> without user intervention.  But I don't mind starting by adding a
>> user-controllable knob, it might be a convenient escape hatch.  We can always
>> extend it from "on/off" -> "on/off/auto" setting, with auto the default
>> in future.
>
> For bare metal cases this is not a simple on/off control!
>
> Top byte ignore (a.k.a. pointer tagging) has separate on/off switches
> for TTBR0 (0x0 upwards) and TTBR1 (0xffffffffffff downwards) *and* we
> have to know the respective sizes of TTBR0 and TTBR1 to be sure which
> table we are using.
>
>
>> > Also I am not sure about the timeline of Linux Kernel patches going into
>> > gdb and for now I thought of this command as the most suitable option.
>> > Moreover some users might also be interested in combination where pointer
>> > tagging is enabled but Linux Kernel threads support is disabled so I
>> > thought we should give the control to the user in cases where we cannot
>> > predict use-cases.
>>
>> If everyone agrees that proper Linux kernel support benefits from
>> its own osabi setting/name, then I don't see why we couldn't start by
>> adding the osabi setting as soon as we have a use for it, even if
>> the larger Linux Kernel patches aren't ready yet.
>
> Following on from the above, for aarch64-linux-tdep we can apply domain
> knowledge regarding how things are configured. Here we know that TTBR0
> is guaranteed to have top byte ignore set, TTBR1 does not *and* we
> also know (from memory-layout.txt) that TTBR0 is sufficiently small
> that bit 55 can be used to discriminate between the two cases.
>
> In others words regardless of whether we are running at EL0 or EL1 then
> I think we should mask the top byte from pointers if and only if bit 55
> is unset, otherwise leave them as they are.

What I am understanding here is that you are basing your decision on
the fact that:

"User addresses have bits 63:48 set to 0 while the kernel addresses have
the same bits set to 1. TTBRx selection is given by bit 63 of the
virtual address."

Sounds legitimate for now but are we ever going to use more than
48-bit virtual addresses in arm64 linux?

This actually means we wont need any set pointer-tagging command and
can modify existing implementation. Sounds good?

>
>
> Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Andrew Pinski]

On Tue, Apr 24, 2018 at 4:41 PM, Omair Javaid <omair.javaid@linaro.org> wrote:
> On 24 April 2018 at 21:05, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> On Tue, Apr 24, 2018 at 12:48:19PM +0100, Pedro Alves wrote:
>>> Hi,
>>>
>>> On 04/20/2018 03:33 PM, Omair Javaid wrote:
>>>
>>> > Pointer tagging information is stored in MMU registers so in linux
>>> > user-space we cannot actually read if pointer tagging is enabled or not
>>> > based on register bits.
>>> > JTAG debuggers should be able to read MMU registers and know whether
>>> > pointer tagging is enabled or not.
>>> >
>>> > Rationale behind adding a separate command is to allow other application to
>>> > control pointer tagging for example bare-metal (non-linux OSes) which want
>>> > to use pointer tagging can enable it. I must admit I dont know of any such
>>> > use-case as of now.
>>>
>>> Alright, that's in line with what I was thinking.  Though, bare metal
>>> should have access to MMU registers too.  Ideally, things would Just Work
>>> without user intervention.  But I don't mind starting by adding a
>>> user-controllable knob, it might be a convenient escape hatch.  We can always
>>> extend it from "on/off" -> "on/off/auto" setting, with auto the default
>>> in future.
>>
>> For bare metal cases this is not a simple on/off control!
>>
>> Top byte ignore (a.k.a. pointer tagging) has separate on/off switches
>> for TTBR0 (0x0 upwards) and TTBR1 (0xffffffffffff downwards) *and* we
>> have to know the respective sizes of TTBR0 and TTBR1 to be sure which
>> table we are using.
>>
>>
>>> > Also I am not sure about the timeline of Linux Kernel patches going into
>>> > gdb and for now I thought of this command as the most suitable option.
>>> > Moreover some users might also be interested in combination where pointer
>>> > tagging is enabled but Linux Kernel threads support is disabled so I
>>> > thought we should give the control to the user in cases where we cannot
>>> > predict use-cases.
>>>
>>> If everyone agrees that proper Linux kernel support benefits from
>>> its own osabi setting/name, then I don't see why we couldn't start by
>>> adding the osabi setting as soon as we have a use for it, even if
>>> the larger Linux Kernel patches aren't ready yet.
>>
>> Following on from the above, for aarch64-linux-tdep we can apply domain
>> knowledge regarding how things are configured. Here we know that TTBR0
>> is guaranteed to have top byte ignore set, TTBR1 does not *and* we
>> also know (from memory-layout.txt) that TTBR0 is sufficiently small
>> that bit 55 can be used to discriminate between the two cases.
>>
>> In others words regardless of whether we are running at EL0 or EL1 then
>> I think we should mask the top byte from pointers if and only if bit 55
>> is unset, otherwise leave them as they are.
>
> What I am understanding here is that you are basing your decision on
> the fact that:
>
> "User addresses have bits 63:48 set to 0 while the kernel addresses have
> the same bits set to 1. TTBRx selection is given by bit 63 of the
> virtual address."
>
> Sounds legitimate for now but are we ever going to use more than
> 48-bit virtual addresses in arm64 linux?

YES.  52bit VA userspace addresses are coming soon.

Thanks,
Andrew

>
> This actually means we wont need any set pointer-tagging command and
> can modify existing implementation. Sounds good?
>
>>
>>
>> Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Daniel Thompson]

On Wed, Apr 25, 2018 at 04:41:40AM +0500, Omair Javaid wrote:
> >> If everyone agrees that proper Linux kernel support benefits from
> >> its own osabi setting/name, then I don't see why we couldn't start by
> >> adding the osabi setting as soon as we have a use for it, even if
> >> the larger Linux Kernel patches aren't ready yet.
> >
> > Following on from the above, for aarch64-linux-tdep we can apply domain
> > knowledge regarding how things are configured. Here we know that TTBR0
> > is guaranteed to have top byte ignore set, TTBR1 does not *and* we
> > also know (from memory-layout.txt) that TTBR0 is sufficiently small
> > that bit 55 can be used to discriminate between the two cases.
> >
> > In others words regardless of whether we are running at EL0 or EL1 then
> > I think we should mask the top byte from pointers if and only if bit 55
> > is unset, otherwise leave them as they are.
> 
> What I am understanding here is that you are basing your decision on
> the fact that:
> 
> "User addresses have bits 63:48 set to 0 while the kernel addresses have
> the same bits set to 1. TTBRx selection is given by bit 63 of the
> virtual address."
> 
> Sounds legitimate for now but are we ever going to use more than
> 48-bit virtual addresses in arm64 linux?

Almost guaranteed I would have thought!

However since the suggestion is *not* based on the assumption that bits 
63:48 are zero then I don't think this matters.
 
It is based on the assumption that bits 63:56 are unknown and cannot be
used for decision making (because tag 0xff is not reserved) and also
that bit 55 is not part of the VA. Bits 54:48 are not involved at all.

For 52-bit VAs (and any other number of bits <56) the hueristic remains
correct.

For 56-bit VAs the pointer tagging feature cannot survive without being
changed because with bit 55 allocated there would be no way for the
hardware to discriminate between TTBR0 and TTBR1 pointers either. Thus
whilst I don't deny the possibility that 56-bit addresses may eventually
happen, *any* implementation of pointer tagging support in gdb would
need to be updated at that point anyway.


Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 25 April 2018 at 13:04, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Wed, Apr 25, 2018 at 04:41:40AM +0500, Omair Javaid wrote:
>> >> If everyone agrees that proper Linux kernel support benefits from
>> >> its own osabi setting/name, then I don't see why we couldn't start by
>> >> adding the osabi setting as soon as we have a use for it, even if
>> >> the larger Linux Kernel patches aren't ready yet.
>> >
>> > Following on from the above, for aarch64-linux-tdep we can apply domain
>> > knowledge regarding how things are configured. Here we know that TTBR0
>> > is guaranteed to have top byte ignore set, TTBR1 does not *and* we
>> > also know (from memory-layout.txt) that TTBR0 is sufficiently small
>> > that bit 55 can be used to discriminate between the two cases.
>> >
>> > In others words regardless of whether we are running at EL0 or EL1 then
>> > I think we should mask the top byte from pointers if and only if bit 55
>> > is unset, otherwise leave them as they are.
>>
>> What I am understanding here is that you are basing your decision on
>> the fact that:
>>
>> "User addresses have bits 63:48 set to 0 while the kernel addresses have
>> the same bits set to 1. TTBRx selection is given by bit 63 of the
>> virtual address."
>>
>> Sounds legitimate for now but are we ever going to use more than
>> 48-bit virtual addresses in arm64 linux?
>
> Almost guaranteed I would have thought!
>
> However since the suggestion is *not* based on the assumption that bits
> 63:48 are zero then I don't think this matters.
>
> It is based on the assumption that bits 63:56 are unknown and cannot be
> used for decision making (because tag 0xff is not reserved) and also
> that bit 55 is not part of the VA. Bits 54:48 are not involved at all.
>
> For 52-bit VAs (and any other number of bits <56) the hueristic remains
> correct.
>
> For 56-bit VAs the pointer tagging feature cannot survive without being
> changed because with bit 55 allocated there would be no way for the
> hardware to discriminate between TTBR0 and TTBR1 pointers either. Thus
> whilst I don't deny the possibility that 56-bit addresses may eventually
> happen, *any* implementation of pointer tagging support in gdb would
> need to be updated at that point anyway.

Above discussion seems to have sufficient points in favor of using bit
55 as the indicator bit for pointer tagging on osabi Linux.
While we should also remove tagging support from non linux osabi.

Lets wait for a final agreement from Pedro on this.

>
>
> Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Daniel Thompson]

On Thu, Apr 26, 2018 at 01:11:04PM +0500, Omair Javaid wrote:
> On 25 April 2018 at 13:04, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > On Wed, Apr 25, 2018 at 04:41:40AM +0500, Omair Javaid wrote:
> >> >> If everyone agrees that proper Linux kernel support benefits from
> >> >> its own osabi setting/name, then I don't see why we couldn't start by
> >> >> adding the osabi setting as soon as we have a use for it, even if
> >> >> the larger Linux Kernel patches aren't ready yet.
> >> >
> >> > Following on from the above, for aarch64-linux-tdep we can apply domain
> >> > knowledge regarding how things are configured. Here we know that TTBR0
> >> > is guaranteed to have top byte ignore set, TTBR1 does not *and* we
> >> > also know (from memory-layout.txt) that TTBR0 is sufficiently small
> >> > that bit 55 can be used to discriminate between the two cases.
> >> >
> >> > In others words regardless of whether we are running at EL0 or EL1 then
> >> > I think we should mask the top byte from pointers if and only if bit 55
> >> > is unset, otherwise leave them as they are.
> >>
> >> What I am understanding here is that you are basing your decision on
> >> the fact that:
> >>
> >> "User addresses have bits 63:48 set to 0 while the kernel addresses have
> >> the same bits set to 1. TTBRx selection is given by bit 63 of the
> >> virtual address."
> >>
> >> Sounds legitimate for now but are we ever going to use more than
> >> 48-bit virtual addresses in arm64 linux?
> >
> > Almost guaranteed I would have thought!
> >
> > However since the suggestion is *not* based on the assumption that bits
> > 63:48 are zero then I don't think this matters.
> >
> > It is based on the assumption that bits 63:56 are unknown and cannot be
> > used for decision making (because tag 0xff is not reserved) and also
> > that bit 55 is not part of the VA. Bits 54:48 are not involved at all.
> >
> > For 52-bit VAs (and any other number of bits <56) the hueristic remains
> > correct.
> >
> > For 56-bit VAs the pointer tagging feature cannot survive without being
> > changed because with bit 55 allocated there would be no way for the
> > hardware to discriminate between TTBR0 and TTBR1 pointers either. Thus
> > whilst I don't deny the possibility that 56-bit addresses may eventually
> > happen, *any* implementation of pointer tagging support in gdb would
> > need to be updated at that point anyway.
> 
> Above discussion seems to have sufficient points in favor of using bit
> 55 as the indicator bit for pointer tagging on osabi Linux.

When you say "use as indicator bit" is it not clear if you are still
considering modal behaviour (pointer tagging is "on" or "off" based on
bit 55 of one of the pointer registers) or whether you hope to move the 
masking function into the tdep code (so instead of saying all pointers
at 56-bit, you can say all pointers need filtering in this manner).

Conditional masking based on bit 55 is idempotent so there's no risk
even if the filtering is applied multiple times at different places in
gdb!


> While we should also remove tagging support from non linux osabi.

Agree. 

This is perhaps more urgent since then at least a non-Linux gdb
can be used to debug the kernel. As things stand today no gdb-8.1
version can be used for AArch64 kernel debugging.


Daniel.

Re: [PATCH 0/3 v3] [AArch64] Support tagged pointer

[Omair Javaid]

On 27 April 2018 at 21:28, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Thu, Apr 26, 2018 at 01:11:04PM +0500, Omair Javaid wrote:
>> On 25 April 2018 at 13:04, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> > On Wed, Apr 25, 2018 at 04:41:40AM +0500, Omair Javaid wrote:
>> >> >> If everyone agrees that proper Linux kernel support benefits from
>> >> >> its own osabi setting/name, then I don't see why we couldn't start by
>> >> >> adding the osabi setting as soon as we have a use for it, even if
>> >> >> the larger Linux Kernel patches aren't ready yet.
>> >> >
>> >> > Following on from the above, for aarch64-linux-tdep we can apply domain
>> >> > knowledge regarding how things are configured. Here we know that TTBR0
>> >> > is guaranteed to have top byte ignore set, TTBR1 does not *and* we
>> >> > also know (from memory-layout.txt) that TTBR0 is sufficiently small
>> >> > that bit 55 can be used to discriminate between the two cases.
>> >> >
>> >> > In others words regardless of whether we are running at EL0 or EL1 then
>> >> > I think we should mask the top byte from pointers if and only if bit 55
>> >> > is unset, otherwise leave them as they are.
>> >>
>> >> What I am understanding here is that you are basing your decision on
>> >> the fact that:
>> >>
>> >> "User addresses have bits 63:48 set to 0 while the kernel addresses have
>> >> the same bits set to 1. TTBRx selection is given by bit 63 of the
>> >> virtual address."
>> >>
>> >> Sounds legitimate for now but are we ever going to use more than
>> >> 48-bit virtual addresses in arm64 linux?
>> >
>> > Almost guaranteed I would have thought!
>> >
>> > However since the suggestion is *not* based on the assumption that bits
>> > 63:48 are zero then I don't think this matters.
>> >
>> > It is based on the assumption that bits 63:56 are unknown and cannot be
>> > used for decision making (because tag 0xff is not reserved) and also
>> > that bit 55 is not part of the VA. Bits 54:48 are not involved at all.
>> >
>> > For 52-bit VAs (and any other number of bits <56) the hueristic remains
>> > correct.
>> >
>> > For 56-bit VAs the pointer tagging feature cannot survive without being
>> > changed because with bit 55 allocated there would be no way for the
>> > hardware to discriminate between TTBR0 and TTBR1 pointers either. Thus
>> > whilst I don't deny the possibility that 56-bit addresses may eventually
>> > happen, *any* implementation of pointer tagging support in gdb would
>> > need to be updated at that point anyway.
>>
>> Above discussion seems to have sufficient points in favor of using bit
>> 55 as the indicator bit for pointer tagging on osabi Linux.
>
> When you say "use as indicator bit" is it not clear if you are still
> considering modal behaviour (pointer tagging is "on" or "off" based on
> bit 55 of one of the pointer registers) or whether you hope to move the
> masking function into the tdep code (so instead of saying all pointers
> at 56-bit, you can say all pointers need filtering in this manner).
>
> Conditional masking based on bit 55 is idempotent so there's no risk
> even if the filtering is applied multiple times at different places in
> gdb!
>
>
>> While we should also remove tagging support from non linux osabi.
>
> Agree.
>
> This is perhaps more urgent since then at least a non-Linux gdb
> can be used to debug the kernel. As things stand today no gdb-8.1
> version can be used for AArch64 kernel debugging.

I have written a patch for this issue will be posting it on for review
after testing.
Approach i am using is to sign extend address based on 55th bit after
clearing top byte.
Clearing pointer tagging from aarch64-elf and only enabling it for
aarch64-linux.

>
>
> Daniel.