This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/1] Add support for Intel PKRU register to GDB and GDBserver.
- From: Pedro Alves <palves at redhat dot com>
- To: Michael Sturm <michael dot sturm at intel dot com>, mark dot kettenis at xs4all dot nl, eliz at gnu dot org
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 20 Jan 2016 16:05:10 +0000
- Subject: Re: [PATCH 1/1] Add support for Intel PKRU register to GDB and GDBserver.
- Authentication-results: sourceware.org; auth=none
- References: <1450367024-8646-1-git-send-email-michael dot sturm at intel dot com>
On 12/17/2015 03:43 PM, Michael Sturm wrote:
> Intel(R) 64 and IA-32 Architecures Software Developer's
(Architectures)
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 484d98d..74103f0 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -150,6 +150,10 @@ show remote exec-event-feature-packet
>
> *** Changes in GDB 7.10
>
> +* 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.
> +
Please remember to move this to the 7.11 section.
> +
> /* Supported mask and size of the extended state. */
> #define X86_XSTATE_X87_MASK X86_XSTATE_X87
> #define X86_XSTATE_SSE_MASK (X86_XSTATE_X87 | X86_XSTATE_SSE)
> #define X86_XSTATE_AVX_MASK (X86_XSTATE_SSE_MASK | X86_XSTATE_AVX)
> #define X86_XSTATE_MPX_MASK (X86_XSTATE_AVX_MASK | X86_XSTATE_MPX)
> #define X86_XSTATE_AVX512_MASK (X86_XSTATE_AVX_MASK | X86_XSTATE_AVX512)
> -#define X86_XSTATE_MPX_AVX512_MASK (X86_XSTATE_MPX_MASK | X86_XSTATE_AVX512)
> +#define X86_XSTATE_MPX_AVX512_MASK (X86_XSTATE_MPX_MASK | X86_XSTATE_AVX512 | X86_XSTATE_PKRU)
Looks like X86_XSTATE_MPX_AVX512_MASK ends up misnamed?
> diff --git a/gdb/features/i386/32bit-pkeys.xml b/gdb/features/i386/32bit-pkeys.xml
> new file mode 100644
> index 0000000..d4702e2
> --- /dev/null
> +++ b/gdb/features/i386/32bit-pkeys.xml
> @@ -0,0 +1,13 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2015 Free Software Foundation, Inc.
You'll need to write "2015-2016" now. Here and all other new files
in the patch.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.i386.pkeys">
> +
> + <reg name="pkru" bitsize="32" type="uint32"/>
> +
> +</feature>
> diff --git a/gdb/features/i386/64bit-pkeys.xml b/gdb/features/i386/64bit-pkeys.xml
If I'm reading right, this is exactly the same as "32bit-pkeys.xml". Any reason
to have two files?
> @@ -951,7 +968,8 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
> if (regclass != none)
> {
> /* Get `xstat_bv'. */
> - const gdb_byte *xstate_bv_p = XSAVE_XSTATE_BV_ADDR (regs);
> + const unsigned long *xstate_bv_p =
> + (unsigned long*) XSAVE_XSTATE_BV_ADDR (regs);
This ...
> @@ -1333,15 +1383,21 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
> if ((regclass & check))
> {
> gdb_byte raw[I386_MAX_REGISTER_SIZE];
> - gdb_byte *xstate_bv_p = XSAVE_XSTATE_BV_ADDR (regs);
> - unsigned int xstate_bv = 0;
> + unsigned long *xstate_bv_p =
> + (unsigned long*) XSAVE_XSTATE_BV_ADDR (regs);
> + unsigned long xstate_bv = 0;
> /* The supported bits in `xstat_bv' are 1 byte. */
> - unsigned int clear_bv = (~(*xstate_bv_p)) & tdep->xcr0;
> + unsigned long clear_bv = (~(*xstate_bv_p)) & tdep->xcr0;
> gdb_byte *p;
... looks problematic for a host-independant file, which should work
with big endian hosts, and hosts that don't support unaligned accesses.
> +gdb_test "info register pkru" ".*pkru$test_string" "Read pkru register"
> +
> +set test_string ".*0x44444444.*"
> +gdb_test "print /x \$pkru = 0x44444444" "= 0x44444444" "Set pkru value"
> +gdb_test "info register pkru" ".*pkru$test_string" "Read value after setting value"
Please lowercase test messages.
> +
> +send_gdb "quit\n"
>
Please remove this.
Thanks,
Pedro Alves