This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 0/4] Add support for PKRU register to GDB and GDBServer.


This patch series adds support for the registers added by the
Memory Protection Keys for Userspace (PKU aka PKEYs).
Native and remote debugging are covered by this patch.

The feedback I got during the first review of this patch raised
questions regarding the naming of XSTATE masks and target descriptors.
in addition, Walfred started working on patches that included renaming
of XSTATE masks and target descriptors. These patches have been submitted
by now.
(https://sourceware.org/ml/gdb-patches/2016-04/msg00329.html)
(https://sourceware.org/ml/gdb-patches/2016-04/msg00328.html)

To address this, I split and extended the original patch to a series of four
patches. This is also the main reason for the long time it took to reply
to the initial feedback by Pedro - sorry for that!

The first three patches implement prerequisites, while fourth patch
actually adds the new feature.

I'll include the original feedback I received below and comment on it.
I hope that it'll make it easier for everybody to follow what is going on!

Thanks and Regards,
Michael

On 20/01/2016 17:05, Pedro Alves wrote:
> 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.
>
Moved to changes after 7.11 section. This is part of patch 4 of this series.
>> +
>>  /* 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?
>
I agree. With the changes submitted by Walfred on target descriptors for
AVX+MPX, we decided to rename all post-AVX masks and target descriptors
to have a more consistent naming.
I've included a patch that does all the renaming. I've also included a patch
that adds another configuration required (AVX + AVX512). This configuration
also needs to be used for the respective X32 variant, which included MPX
up to know, although MPX is not available in X32 mode.
>> 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.
>
Ok.
>> +
>> +     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?
>
Actually, the only reason is symmetry and the fact that there is no file yet
that is not prefixed by 32bit/64bit. Maybe keeping both files will make
maintenance a little bit less effort in case differences between 32bit and
64bit are introduced in future implementations.

>> @@ -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.
>
I've addressed this in the first patch of this series.
>> +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.
>
Ok.
>> +
>> +send_gdb "quit\n"
>>
>
> Please remove this.
>
Ok.
> Thanks,
> Pedro Alves
>
>
>

Michael Sturm (4):
  Change xstate_bv handling to use 8 bytes of data.
  Rename target descriptors to reflect actual content of descriptor.
  Add target descriptor for avx-avx512.
  Add support for Intel PKRU register to GDB and GDBserver.

 gdb/NEWS                                           |   4 +
 gdb/amd64-linux-nat.c                              |   1 +
 gdb/amd64-linux-tdep.c                             |  24 +-
 gdb/amd64-linux-tdep.h                             |   7 +-
 gdb/amd64-tdep.c                                   |  28 +-
 gdb/amd64-tdep.h                                   |   5 +-
 gdb/common/x86-xstate.h                            |  21 +-
 gdb/doc/gdb.texinfo                                |   4 +
 gdb/features/Makefile                              |  77 +++--
 gdb/features/i386/32bit-pkeys.xml                  |  13 +
 gdb/features/i386/64bit-pkeys.xml                  |  13 +
 gdb/features/i386/amd64-avx-avx512-linux.c         | 284 ++++++++++++++++++
 gdb/features/i386/amd64-avx-avx512-linux.xml       |  19 ++
 gdb/features/i386/amd64-avx-avx512.c               | 279 ++++++++++++++++++
 gdb/features/i386/amd64-avx-avx512.xml             |  17 ++
 gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c | 325 +++++++++++++++++++++
 .../i386/amd64-avx-mpx-avx512-pku-linux.xml        |  22 ++
 gdb/features/i386/amd64-avx-mpx-avx512-pku.c       | 320 ++++++++++++++++++++
 gdb/features/i386/amd64-avx-mpx-avx512-pku.xml     |  19 ++
 gdb/features/i386/amd64-avx512-linux.c             | 322 --------------------
 gdb/features/i386/amd64-avx512-linux.xml           |  20 --
 gdb/features/i386/amd64-avx512.c                   | 317 --------------------
 gdb/features/i386/amd64-avx512.xml                 |  18 --
 gdb/features/i386/i386-avx-avx512-linux.c          | 170 +++++++++++
 gdb/features/i386/i386-avx-avx512-linux.xml        |  19 ++
 gdb/features/i386/i386-avx-avx512.c                | 165 +++++++++++
 gdb/features/i386/i386-avx-avx512.xml              |  17 ++
 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c  | 211 +++++++++++++
 .../i386/i386-avx-mpx-avx512-pku-linux.xml         |  22 ++
 gdb/features/i386/i386-avx-mpx-avx512-pku.c        | 206 +++++++++++++
 gdb/features/i386/i386-avx-mpx-avx512-pku.xml      |  19 ++
 gdb/features/i386/i386-avx512-linux.c              | 208 -------------
 gdb/features/i386/i386-avx512-linux.xml            |  20 --
 gdb/features/i386/i386-avx512.c                    | 203 -------------
 gdb/features/i386/i386-avx512.xml                  |  18 --
 gdb/features/i386/x32-avx-avx512-linux.c           | 284 ++++++++++++++++++
 gdb/features/i386/x32-avx-avx512-linux.xml         |  19 ++
 gdb/features/i386/x32-avx-avx512.c                 | 279 ++++++++++++++++++
 gdb/features/i386/x32-avx-avx512.xml               |  17 ++
 gdb/features/i386/x32-avx512-linux.c               | 322 --------------------
 gdb/features/i386/x32-avx512-linux.xml             |  20 --
 gdb/features/i386/x32-avx512.c                     | 317 --------------------
 gdb/features/i386/x32-avx512.xml                   |  18 --
 gdb/gdbserver/Makefile.in                          |  50 ++--
 gdb/gdbserver/configure.srv                        |  24 +-
 gdb/gdbserver/i387-fp.c                            |  47 ++-
 gdb/gdbserver/linux-amd64-ipa.c                    |   9 +-
 gdb/gdbserver/linux-i386-ipa.c                     |  10 +-
 gdb/gdbserver/linux-x86-low.c                      |  45 ++-
 gdb/gdbserver/linux-x86-tdesc.h                    |  29 +-
 gdb/i386-linux-nat.c                               |   2 +-
 gdb/i386-linux-tdep.c                              |  14 +-
 gdb/i386-linux-tdep.h                              |   6 +-
 gdb/i386-tdep.c                                    |  76 ++++-
 gdb/i386-tdep.h                                    |  14 +-
 gdb/i387-tdep.c                                    | 108 ++++++-
 gdb/i387-tdep.h                                    |   5 +
 gdb/nat/x86-gcc-cpuid.h                            |   4 +
 gdb/regformats/i386/amd64-avx-avx512-linux.dat     | 151 ++++++++++
 gdb/regformats/i386/amd64-avx-avx512.dat           | 150 ++++++++++
 .../i386/amd64-avx-mpx-avx512-pku-linux.dat        | 157 ++++++++++
 gdb/regformats/i386/amd64-avx-mpx-avx512-pku.dat   | 157 ++++++++++
 gdb/regformats/i386/amd64-avx512-linux.dat         | 157 ----------
 gdb/regformats/i386/amd64-avx512.dat               | 156 ----------
 gdb/regformats/i386/i386-avx-avx512-linux.dat      |  71 +++++
 gdb/regformats/i386/i386-avx-avx512.dat            |  70 +++++
 .../i386/i386-avx-mpx-avx512-pku-linux.dat         |  78 +++++
 gdb/regformats/i386/i386-avx-mpx-avx512-pku.dat    |  77 +++++
 gdb/regformats/i386/i386-avx512-linux.dat          |  77 -----
 gdb/regformats/i386/i386-avx512.dat                |  76 -----
 gdb/regformats/i386/x32-avx-avx512-linux.dat       | 151 ++++++++++
 gdb/regformats/i386/x32-avx-avx512.dat             | 150 ++++++++++
 gdb/regformats/i386/x32-avx512-linux.dat           | 157 ----------
 gdb/regformats/i386/x32-avx512.dat                 | 156 ----------
 gdb/testsuite/gdb.arch/i386-pkru.c                 |  95 ++++++
 gdb/testsuite/gdb.arch/i386-pkru.exp               |  73 +++++
 gdb/x86-linux-nat.c                                |  20 +-
 77 files changed, 4594 insertions(+), 2741 deletions(-)
 create mode 100644 gdb/features/i386/32bit-pkeys.xml
 create mode 100644 gdb/features/i386/64bit-pkeys.xml
 create mode 100644 gdb/features/i386/amd64-avx-avx512-linux.c
 create mode 100644 gdb/features/i386/amd64-avx-avx512-linux.xml
 create mode 100644 gdb/features/i386/amd64-avx-avx512.c
 create mode 100644 gdb/features/i386/amd64-avx-avx512.xml
 create mode 100644 gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c
 create mode 100644 gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.xml
 create mode 100644 gdb/features/i386/amd64-avx-mpx-avx512-pku.c
 create mode 100644 gdb/features/i386/amd64-avx-mpx-avx512-pku.xml
 delete mode 100644 gdb/features/i386/amd64-avx512-linux.c
 delete mode 100644 gdb/features/i386/amd64-avx512-linux.xml
 delete mode 100644 gdb/features/i386/amd64-avx512.c
 delete mode 100644 gdb/features/i386/amd64-avx512.xml
 create mode 100644 gdb/features/i386/i386-avx-avx512-linux.c
 create mode 100644 gdb/features/i386/i386-avx-avx512-linux.xml
 create mode 100644 gdb/features/i386/i386-avx-avx512.c
 create mode 100644 gdb/features/i386/i386-avx-avx512.xml
 create mode 100644 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
 create mode 100644 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.xml
 create mode 100644 gdb/features/i386/i386-avx-mpx-avx512-pku.c
 create mode 100644 gdb/features/i386/i386-avx-mpx-avx512-pku.xml
 delete mode 100644 gdb/features/i386/i386-avx512-linux.c
 delete mode 100644 gdb/features/i386/i386-avx512-linux.xml
 delete mode 100644 gdb/features/i386/i386-avx512.c
 delete mode 100644 gdb/features/i386/i386-avx512.xml
 create mode 100644 gdb/features/i386/x32-avx-avx512-linux.c
 create mode 100644 gdb/features/i386/x32-avx-avx512-linux.xml
 create mode 100644 gdb/features/i386/x32-avx-avx512.c
 create mode 100644 gdb/features/i386/x32-avx-avx512.xml
 delete mode 100644 gdb/features/i386/x32-avx512-linux.c
 delete mode 100644 gdb/features/i386/x32-avx512-linux.xml
 delete mode 100644 gdb/features/i386/x32-avx512.c
 delete mode 100644 gdb/features/i386/x32-avx512.xml
 create mode 100644 gdb/regformats/i386/amd64-avx-avx512-linux.dat
 create mode 100644 gdb/regformats/i386/amd64-avx-avx512.dat
 create mode 100644 gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
 create mode 100644 gdb/regformats/i386/amd64-avx-mpx-avx512-pku.dat
 delete mode 100644 gdb/regformats/i386/amd64-avx512-linux.dat
 delete mode 100644 gdb/regformats/i386/amd64-avx512.dat
 create mode 100644 gdb/regformats/i386/i386-avx-avx512-linux.dat
 create mode 100644 gdb/regformats/i386/i386-avx-avx512.dat
 create mode 100644 gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
 create mode 100644 gdb/regformats/i386/i386-avx-mpx-avx512-pku.dat
 delete mode 100644 gdb/regformats/i386/i386-avx512-linux.dat
 delete mode 100644 gdb/regformats/i386/i386-avx512.dat
 create mode 100644 gdb/regformats/i386/x32-avx-avx512-linux.dat
 create mode 100644 gdb/regformats/i386/x32-avx-avx512.dat
 delete mode 100644 gdb/regformats/i386/x32-avx512-linux.dat
 delete mode 100644 gdb/regformats/i386/x32-avx512.dat
 create mode 100644 gdb/testsuite/gdb.arch/i386-pkru.c
 create mode 100644 gdb/testsuite/gdb.arch/i386-pkru.exp

-- 
1.8.4.2


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]