mbox series

[v6,0/8] powerpc/watchpoint: Bug fixes plus new feature flag

Message ID 20200902042945.129369-1-ravi.bangoria@linux.ibm.com (mailing list archive)
Headers show
Series powerpc/watchpoint: Bug fixes plus new feature flag | expand

Message

Ravi Bangoria Sept. 2, 2020, 4:29 a.m. UTC
Patch #1 fixes issue for quardword instruction on p10 predecessors.
Patch #2 fixes issue for vector instructions.
Patch #3 fixes a bug about watchpoint not firing when created with
         ptrace PPC_PTRACE_SETHWDEBUG and CONFIG_HAVE_HW_BREAKPOINT=N.
         The fix uses HW_BRK_TYPE_PRIV_ALL for ptrace user which, I
         guess, should be fine because we don't leak any kernel
         addresses and PRIV_ALL will also help to cover scenarios when
         kernel accesses user memory.
Patch #4,#5 fixes infinite exception bug, again the bug happens only
         with CONFIG_HAVE_HW_BREAKPOINT=N.
Patch #6 fixes two places where we are missing to set hw_len.
Patch #7 introduce new feature bit PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
         which will be set when running on ISA 3.1 compliant machine.
Patch #8 finally adds selftest to test scenarios fixed by patch#2,#3
         and also moves MODE_EXACT tests outside of BP_RANGE condition.

Christophe, let me know if this series breaks something for 8xx.

v5: https://lore.kernel.org/r/20200825043617.1073634-1-ravi.bangoria@linux.ibm.com

v5->v6:
 - Fix build faulure reported by kernel test robot
 - patch #5. Use more compact if condition, suggested by Christophe


Ravi Bangoria (8):
  powerpc/watchpoint: Fix quarword instruction handling on p10
    predecessors
  powerpc/watchpoint: Fix handling of vector instructions
  powerpc/watchpoint/ptrace: Fix SETHWDEBUG when
    CONFIG_HAVE_HW_BREAKPOINT=N
  powerpc/watchpoint: Move DAWR detection logic outside of
    hw_breakpoint.c
  powerpc/watchpoint: Fix exception handling for
    CONFIG_HAVE_HW_BREAKPOINT=N
  powerpc/watchpoint: Add hw_len wherever missing
  powerpc/watchpoint/ptrace: Introduce PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
  powerpc/watchpoint/selftests: Tests for kernel accessing user memory

 Documentation/powerpc/ptrace.rst              |   1 +
 arch/powerpc/include/asm/hw_breakpoint.h      |  12 ++
 arch/powerpc/include/uapi/asm/ptrace.h        |   1 +
 arch/powerpc/kernel/Makefile                  |   3 +-
 arch/powerpc/kernel/hw_breakpoint.c           | 149 +---------------
 .../kernel/hw_breakpoint_constraints.c        | 162 ++++++++++++++++++
 arch/powerpc/kernel/process.c                 |  48 ++++++
 arch/powerpc/kernel/ptrace/ptrace-noadv.c     |   9 +-
 arch/powerpc/xmon/xmon.c                      |   1 +
 .../selftests/powerpc/ptrace/ptrace-hwbreak.c |  48 +++++-
 10 files changed, 282 insertions(+), 152 deletions(-)
 create mode 100644 arch/powerpc/kernel/hw_breakpoint_constraints.c

Comments

Michael Ellerman Sept. 17, 2020, 11:27 a.m. UTC | #1
On Wed, 2 Sep 2020 09:59:37 +0530, Ravi Bangoria wrote:
> Patch #1 fixes issue for quardword instruction on p10 predecessors.
> Patch #2 fixes issue for vector instructions.
> Patch #3 fixes a bug about watchpoint not firing when created with
>          ptrace PPC_PTRACE_SETHWDEBUG and CONFIG_HAVE_HW_BREAKPOINT=N.
>          The fix uses HW_BRK_TYPE_PRIV_ALL for ptrace user which, I
>          guess, should be fine because we don't leak any kernel
>          addresses and PRIV_ALL will also help to cover scenarios when
>          kernel accesses user memory.
> Patch #4,#5 fixes infinite exception bug, again the bug happens only
>          with CONFIG_HAVE_HW_BREAKPOINT=N.
> Patch #6 fixes two places where we are missing to set hw_len.
> Patch #7 introduce new feature bit PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>          which will be set when running on ISA 3.1 compliant machine.
> Patch #8 finally adds selftest to test scenarios fixed by patch#2,#3
>          and also moves MODE_EXACT tests outside of BP_RANGE condition.
> 
> [...]

Applied to powerpc/next.

[1/8] powerpc/watchpoint: Fix quadword instruction handling on p10 predecessors
      https://git.kernel.org/powerpc/c/4759c11ed20454b7b36db4ec15f7d5aa1519af4a
[2/8] powerpc/watchpoint: Fix handling of vector instructions
      https://git.kernel.org/powerpc/c/4441eb02333a9b46a0d919aa7a6d3b137b5f2562
[3/8] powerpc/watchpoint/ptrace: Fix SETHWDEBUG when CONFIG_HAVE_HW_BREAKPOINT=N
      https://git.kernel.org/powerpc/c/9b6b7c680cc20971444d9f836e49fc98848bcd0a
[4/8] powerpc/watchpoint: Move DAWR detection logic outside of hw_breakpoint.c
      https://git.kernel.org/powerpc/c/edc8dd99b29e4d705c45e2a3a6c01b096ee056db
[5/8] powerpc/watchpoint: Fix exception handling for CONFIG_HAVE_HW_BREAKPOINT=N
      https://git.kernel.org/powerpc/c/5b905d77987de065bdd3a2906816b5f143df087b
[6/8] powerpc/watchpoint: Add hw_len wherever missing
      https://git.kernel.org/powerpc/c/58da5984d2ea6d95f3f9d9e8dd9f7e1b0dddfb3c
[7/8] powerpc/watchpoint/ptrace: Introduce PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
      https://git.kernel.org/powerpc/c/fa725cc53d353110f39a9e5b9f60d6acb2c7ff49
[8/8] selftests/powerpc: Tests for kernel accessing user memory
      https://git.kernel.org/powerpc/c/ac234524056da4e0c081f682da3ea25cdaab737a

cheers
Rogerio Alves Sept. 17, 2020, 1:24 p.m. UTC | #2
On 9/2/20 1:29 AM, Ravi Bangoria wrote:
> Patch #1 fixes issue for quardword instruction on p10 predecessors.
> Patch #2 fixes issue for vector instructions.
> Patch #3 fixes a bug about watchpoint not firing when created with
>           ptrace PPC_PTRACE_SETHWDEBUG and CONFIG_HAVE_HW_BREAKPOINT=N.
>           The fix uses HW_BRK_TYPE_PRIV_ALL for ptrace user which, I
>           guess, should be fine because we don't leak any kernel
>           addresses and PRIV_ALL will also help to cover scenarios when
>           kernel accesses user memory.
> Patch #4,#5 fixes infinite exception bug, again the bug happens only
>           with CONFIG_HAVE_HW_BREAKPOINT=N.
> Patch #6 fixes two places where we are missing to set hw_len.
> Patch #7 introduce new feature bit PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>           which will be set when running on ISA 3.1 compliant machine.
> Patch #8 finally adds selftest to test scenarios fixed by patch#2,#3
>           and also moves MODE_EXACT tests outside of BP_RANGE condition.
> 
> Christophe, let me know if this series breaks something for 8xx.
> 
> v5: https://lore.kernel.org/r/20200825043617.1073634-1-ravi.bangoria@linux.ibm.com
> 
> v5->v6:
>   - Fix build faulure reported by kernel test robot
>   - patch #5. Use more compact if condition, suggested by Christophe
> 
> 
> Ravi Bangoria (8):
>    powerpc/watchpoint: Fix quarword instruction handling on p10
>      predecessors
>    powerpc/watchpoint: Fix handling of vector instructions
>    powerpc/watchpoint/ptrace: Fix SETHWDEBUG when
>      CONFIG_HAVE_HW_BREAKPOINT=N
>    powerpc/watchpoint: Move DAWR detection logic outside of
>      hw_breakpoint.c
>    powerpc/watchpoint: Fix exception handling for
>      CONFIG_HAVE_HW_BREAKPOINT=N
>    powerpc/watchpoint: Add hw_len wherever missing
>    powerpc/watchpoint/ptrace: Introduce PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>    powerpc/watchpoint/selftests: Tests for kernel accessing user memory
> 
>   Documentation/powerpc/ptrace.rst              |   1 +
>   arch/powerpc/include/asm/hw_breakpoint.h      |  12 ++
>   arch/powerpc/include/uapi/asm/ptrace.h        |   1 +
>   arch/powerpc/kernel/Makefile                  |   3 +-
>   arch/powerpc/kernel/hw_breakpoint.c           | 149 +---------------
>   .../kernel/hw_breakpoint_constraints.c        | 162 ++++++++++++++++++
>   arch/powerpc/kernel/process.c                 |  48 ++++++
>   arch/powerpc/kernel/ptrace/ptrace-noadv.c     |   9 +-
>   arch/powerpc/xmon/xmon.c                      |   1 +
>   .../selftests/powerpc/ptrace/ptrace-hwbreak.c |  48 +++++-
>   10 files changed, 282 insertions(+), 152 deletions(-)
>   create mode 100644 arch/powerpc/kernel/hw_breakpoint_constraints.c
> 

Tested this patch set for:
- SETHWDEBUG when CONFIG_HAVE_HW_BREAKPOINT=N = OK
- Fix exception handling for CONFIG_HAVE_HW_BREAKPOINT=N = OK
- Check for PPC_DEBUG_FEATURE_DATA_BP_ARCH_31 = OK
- Fix quarword instruction handling on p10 predecessors = OK
- Fix handling of vector instructions = OK

Also tested for:
- Set second watchpoint (P10 Mambo) = OK
- Infinity loop on sc instruction = OK
Ravi Bangoria Sept. 18, 2020, 8:31 a.m. UTC | #3
On 9/17/20 6:54 PM, Rogerio Alves wrote:
> On 9/2/20 1:29 AM, Ravi Bangoria wrote:
>> Patch #1 fixes issue for quardword instruction on p10 predecessors.
>> Patch #2 fixes issue for vector instructions.
>> Patch #3 fixes a bug about watchpoint not firing when created with
>>           ptrace PPC_PTRACE_SETHWDEBUG and CONFIG_HAVE_HW_BREAKPOINT=N.
>>           The fix uses HW_BRK_TYPE_PRIV_ALL for ptrace user which, I
>>           guess, should be fine because we don't leak any kernel
>>           addresses and PRIV_ALL will also help to cover scenarios when
>>           kernel accesses user memory.
>> Patch #4,#5 fixes infinite exception bug, again the bug happens only
>>           with CONFIG_HAVE_HW_BREAKPOINT=N.
>> Patch #6 fixes two places where we are missing to set hw_len.
>> Patch #7 introduce new feature bit PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>>           which will be set when running on ISA 3.1 compliant machine.
>> Patch #8 finally adds selftest to test scenarios fixed by patch#2,#3
>>           and also moves MODE_EXACT tests outside of BP_RANGE condition.
>>
[...]

> 
> Tested this patch set for:
> - SETHWDEBUG when CONFIG_HAVE_HW_BREAKPOINT=N = OK
> - Fix exception handling for CONFIG_HAVE_HW_BREAKPOINT=N = OK
> - Check for PPC_DEBUG_FEATURE_DATA_BP_ARCH_31 = OK
> - Fix quarword instruction handling on p10 predecessors = OK
> - Fix handling of vector instructions = OK
> 
> Also tested for:
> - Set second watchpoint (P10 Mambo) = OK
> - Infinity loop on sc instruction = OK

Thanks Rogerio!

Ravi
Michael Ellerman Sept. 18, 2020, 10:50 a.m. UTC | #4
Rogerio Alves <rcardoso@linux.ibm.com> writes:
> On 9/2/20 1:29 AM, Ravi Bangoria wrote:
>> Patch #1 fixes issue for quardword instruction on p10 predecessors.
>> Patch #2 fixes issue for vector instructions.
>> Patch #3 fixes a bug about watchpoint not firing when created with
>>           ptrace PPC_PTRACE_SETHWDEBUG and CONFIG_HAVE_HW_BREAKPOINT=N.
>>           The fix uses HW_BRK_TYPE_PRIV_ALL for ptrace user which, I
>>           guess, should be fine because we don't leak any kernel
>>           addresses and PRIV_ALL will also help to cover scenarios when
>>           kernel accesses user memory.
>> Patch #4,#5 fixes infinite exception bug, again the bug happens only
>>           with CONFIG_HAVE_HW_BREAKPOINT=N.
>> Patch #6 fixes two places where we are missing to set hw_len.
>> Patch #7 introduce new feature bit PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>>           which will be set when running on ISA 3.1 compliant machine.
>> Patch #8 finally adds selftest to test scenarios fixed by patch#2,#3
>>           and also moves MODE_EXACT tests outside of BP_RANGE condition.
>> 
>> Christophe, let me know if this series breaks something for 8xx.
>> 
>> v5: https://lore.kernel.org/r/20200825043617.1073634-1-ravi.bangoria@linux.ibm.com
>> 
>> v5->v6:
>>   - Fix build faulure reported by kernel test robot
>>   - patch #5. Use more compact if condition, suggested by Christophe
>> 
>> 
>> Ravi Bangoria (8):
>>    powerpc/watchpoint: Fix quarword instruction handling on p10
>>      predecessors
>>    powerpc/watchpoint: Fix handling of vector instructions
>>    powerpc/watchpoint/ptrace: Fix SETHWDEBUG when
>>      CONFIG_HAVE_HW_BREAKPOINT=N
>>    powerpc/watchpoint: Move DAWR detection logic outside of
>>      hw_breakpoint.c
>>    powerpc/watchpoint: Fix exception handling for
>>      CONFIG_HAVE_HW_BREAKPOINT=N
>>    powerpc/watchpoint: Add hw_len wherever missing
>>    powerpc/watchpoint/ptrace: Introduce PPC_DEBUG_FEATURE_DATA_BP_ARCH_31
>>    powerpc/watchpoint/selftests: Tests for kernel accessing user memory
>> 
>>   Documentation/powerpc/ptrace.rst              |   1 +
>>   arch/powerpc/include/asm/hw_breakpoint.h      |  12 ++
>>   arch/powerpc/include/uapi/asm/ptrace.h        |   1 +
>>   arch/powerpc/kernel/Makefile                  |   3 +-
>>   arch/powerpc/kernel/hw_breakpoint.c           | 149 +---------------
>>   .../kernel/hw_breakpoint_constraints.c        | 162 ++++++++++++++++++
>>   arch/powerpc/kernel/process.c                 |  48 ++++++
>>   arch/powerpc/kernel/ptrace/ptrace-noadv.c     |   9 +-
>>   arch/powerpc/xmon/xmon.c                      |   1 +
>>   .../selftests/powerpc/ptrace/ptrace-hwbreak.c |  48 +++++-
>>   10 files changed, 282 insertions(+), 152 deletions(-)
>>   create mode 100644 arch/powerpc/kernel/hw_breakpoint_constraints.c
>> 
>
> Tested this patch set for:
> - SETHWDEBUG when CONFIG_HAVE_HW_BREAKPOINT=N = OK
> - Fix exception handling for CONFIG_HAVE_HW_BREAKPOINT=N = OK
> - Check for PPC_DEBUG_FEATURE_DATA_BP_ARCH_31 = OK
> - Fix quarword instruction handling on p10 predecessors = OK
> - Fix handling of vector instructions = OK
>
> Also tested for:
> - Set second watchpoint (P10 Mambo) = OK
> - Infinity loop on sc instruction = OK

Thanks.

I wasn't able to pick up your Tested-by tags as I'd already applied the
patches, but thanks for sending them anyway, they will live on in the
mailing list archives for eternity.

cheers