This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 5/5] Add support for Intel PKRU register to GDB and GDBserver.
- From: Luis Machado <lgustavo at codesourcery dot com>
- To: Michael Sturm <michael dot sturm at intel dot com>, <mark dot kettenis at xs4all dot nl>, <palves at redhat dot com>, <eliz at gnu dot org>
- Cc: <gdb-patches at sourceware dot org>
- Date: Thu, 1 Dec 2016 19:59:43 -0600
- Subject: Re: [PATCH v2 5/5] Add support for Intel PKRU register to GDB and GDBserver.
- Authentication-results: sourceware.org; auth=none
- References: <1480599538-30543-1-git-send-email-michael.sturm@intel.com> <1480599538-30543-6-git-send-email-michael.sturm@intel.com>
- Reply-to: Luis Machado <lgustavo at codesourcery dot com>
On 12/01/2016 07:38 AM, Michael Sturm wrote:
diff --git a/gdb/NEWS b/gdb/NEWS
index a597405..d37c477 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,10 +3,16 @@
*** Changes since GDB 7.12
+* GDB now supports access to the PKU register on GNU/Linux. The register is
+ added by the Memory Protection Keys for Userspace feature which will be
+ available in future Intel CPUs.
+
* Building GDB and GDBserver now requires a C++11 compiler.
For example, GCC 4.8 or later.
+* GDB and GDBserver now require building with a C++ compiler.
+
Spurious change? Maybe a merge error.
# Linux object files. This is so we don't have to repeat
diff --git a/gdb/gdbserver/i387-fp.c b/gdb/gdbserver/i387-fp.c
index d0b0736..6f188d2 100644
--- a/gdb/gdbserver/i387-fp.c
+++ b/gdb/gdbserver/i387-fp.c
@@ -27,6 +27,7 @@ static const int num_avx512_zmmh_low_registers = 16;
static const int num_avx512_zmmh_high_registers = 16;
static const int num_avx512_ymmh_registers = 16;
static const int num_avx512_xmm_registers = 16;
+static const int num_pkeys_registers = 1;
/* Note: These functions preserve the reserved bits in control registers.
However, gdbserver promptly throws away that information. */
@@ -136,6 +137,10 @@ struct i387_xsave {
/* Space for 16 512-bit zmm16-31 values. */
unsigned char zmmh_high_space[1024];
+
+ /* Space for 1 32-bit PKRU register. The HW XSTATE size for this feature is
Two spaces after period.
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 672a6cf..c0963b6 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -189,6 +189,15 @@ struct gdbarch_tdep
/* YMM16-31 register names. Only used for tdesc_numbered_register. */
const char **ymm_avx512_register_names;
+ /* Number of PKEYS registers */
Period at the end of sentence here ...
+ int num_pkeys_regs;
+
+ /* Register number for PKRU register */
... here ...
+ int pkru_regnum;
+
+ /* PKEYS register names */
and here.
+ const char **pkeys_register_names;
+
/* Target description. */
const struct target_desc *tdesc;
@@ -284,7 +293,8 @@ enum i386_regnum
I386_K0_REGNUM, /* %k0 */
I386_K7_REGNUM = I386_K0_REGNUM + 7,
I386_ZMM0H_REGNUM, /* %zmm0h */
- I386_ZMM7H_REGNUM = I386_ZMM0H_REGNUM + 7
+ I386_ZMM7H_REGNUM = I386_ZMM0H_REGNUM + 7,
+ I386_PKRU_REGNUM
};
/* Register numbers of RECORD_REGMAP. */
@@ -324,6 +334,7 @@ enum record_i386_regnum
#define I386_AVX_NUM_REGS (I386_YMM7H_REGNUM + 1)
#define I386_MPX_NUM_REGS (I386_BNDSTATUS_REGNUM + 1)
#define I386_AVX512_NUM_REGS (I386_ZMM7H_REGNUM + 1)
+#define I386_PKEYS_NUM_REGS (I386_PKRU_REGNUM + 1)
/* Size of the largest register. */
#define I386_MAX_REGISTER_SIZE 64
@@ -345,6 +356,7 @@ extern int i386_bnd_regnum_p (struct gdbarch *gdbarch, int regnum);
extern int i386_k_regnum_p (struct gdbarch *gdbarch, int regnum);
extern int i386_zmm_regnum_p (struct gdbarch *gdbarch, int regnum);
extern int i386_zmmh_regnum_p (struct gdbarch *gdbarch, int regnum);
+extern int i386_pkru_regnum_p (struct gdbarch *gdbarch, int regnum);
extern const char *i386_pseudo_register_name (struct gdbarch *gdbarch,
int regnum);
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index ef3a631..dcc8f08 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -888,6 +888,19 @@ static int xsave_avx512_zmm_h_offset[] =
#define XSAVE_AVX512_ZMM_H_ADDR(tdep, xsave, regnum) \
(xsave + xsave_avx512_zmm_h_offset[regnum - I387_ZMM0H_REGNUM (tdep)])
+/* At xsave_pkeys_offset[REGNUM] you find the offset to the location
+ of the PKRU register data structure used by the "xsave"
+ instruction where GDB register REGNUM is stored. */
+
+static int xsave_pkeys_offset[] =
+{
+2688 + 0 * 8 /* %pkru (64 bits in XSTATE, 32-bit actually used by
Is the constant 2688 something we already #define-ed?
diff --git a/gdb/testsuite/gdb.arch/i386-pkru.c b/gdb/testsuite/gdb.arch/i386-pkru.c
new file mode 100644
index 0000000..dd8b8f7
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-pkru.c
@@ -0,0 +1,95 @@
+/* Test program for PKEYS registers.
+
+ Copyright 2015-2016 Free Software Foundation, Inc.
+
+ Contributed by Intel Corp. <michael.sturm@intel.com>
+
+ This file is part of GDB.
+
+ 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 <stdio.h>
+#include "x86-cpuid.h"
+
+#ifndef NOINLINE
+#define NOINLINE __attribute__ ((noinline))
+#endif
+
+unsigned int have_pkru (void) NOINLINE;
+
+static inline unsigned long
+rdpkru (void)
+{
+ unsigned int eax, edx;
+ unsigned int ecx = 0;
+ unsigned int pkru;
+
+ asm volatile (".byte 0x0f,0x01,0xee\n\t"
+ : "=a" (eax), "=d" (edx)
+ : "c" (ecx));
+ pkru = eax;
+ return pkru;
+}
+
+static inline void
+wrpkru (unsigned int pkru)
+{
+ unsigned int eax = pkru;
+ unsigned int ecx = 0;
+ unsigned int edx = 0;
+
+ asm volatile (".byte 0x0f,0x01,0xef\n\t"
+ : : "a" (eax), "c" (ecx), "d" (edx));
+}
+
+
Spurious newline.
+unsigned int NOINLINE
+have_pkru (void)
+{
+ unsigned int eax, ebx, ecx, edx;
+
+ if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+ return 0;
+
+ if ((ecx & bit_OSXSAVE) == bit_OSXSAVE)
+ {
+ if (__get_cpuid_max (0, NULL) < 7)
+ return 0;
+
+ __cpuid_count (7, 0, eax, ebx, ecx, edx);
+
+ if ((ecx & bit_PKU) == bit_PKU)
+ return 1;
+ else
+ return 0;
No need to return 0 here. We can just let it continue and return 0 below?
+ }
+ return 0;
+}
+
+int
+main (int argc, char **argv)
+{
+ unsigned int wr_value = 0x12345678;
+ unsigned int rd_value = 0x0;
+
+ if (have_pkru ())
+ {
+ wrpkru (wr_value);
+ asm ("nop\n\t"); /* break here 1. */
+
+ rd_value = rdpkru ();
+ asm ("nop\n\t"); /* break here 2. */
+ }
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/i386-pkru.exp b/gdb/testsuite/gdb.arch/i386-pkru.exp
new file mode 100644
index 0000000..f5e3da5
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-pkru.exp
@@ -0,0 +1,73 @@
+# Copyright 2015-2016 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <michael.sturm@intel.com>
+#
+# 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/>.
+
+standard_testfile
+
+if { ![istarget i?86-*-*] && ![istarget x86_64-*-* ] } {
+ verbose "Skipping PKEYS tests."
Drop the verbose call and ...
unsupported "skipping x86-specific tests"
or
unsupported "skipping x86 PKEYS tests"
or some other meaningful message.
+ return
+}
+
+set comp_flags "-I${srcdir}/../nat/"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+ [list debug additional_flags=${comp_flags}]] } {
Add 'untested "failed to compile"'
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+set supports_pkru 0
+set test "probe PKRU support"
+gdb_test_multiple "print have_pkru()" $test {
+ -re ".. = 1\r\n$gdb_prompt $" {
+ pass $test
+ set supports_pkru 1
+ }
+ -re ".. = 0\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
I don't think we need to check for $gdb_prompt explicitly here.
+
+if { !$supports_pkru } {
+ unsupported "processor does not support protection key feature."
+ return
+}
+
+# Test pkru register at startup
+# set test_string "0"
+
+gdb_test "print \$pkru" 0 "pkru formating"
+
"print pkru register" instead of "pkru formatting"?
+# Read values from pseudo registers.
+gdb_breakpoint [ gdb_get_line_number "break here 1" ]
+gdb_continue_to_breakpoint "break here 1" ".*break here 1.*"
+
+# set test_string ".*0x12345678.*"
+gdb_test "info register pkru" ".*pkru.*0x12345678.*" "read pkru register"
+
+# set test_string ".*0x44444444.*"
+gdb_test "print /x \$pkru = 0x44444444" "= 0x44444444" "set pkru value"
+gdb_test "info register pkru" ".*pkru.*0x44444444.*" "read value after setting value"
+
+gdb_breakpoint [ gdb_get_line_number "break here 2" ]
+gdb_continue_to_breakpoint "break here 2" ".*break here 2.*"
+
+gdb_test "print /x rd_value" ".*" "program variable after reading pkru"
print variable instead of program variable? I may be missing some
context here.