Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v6 0/8] powerpc/watchpoint: Bug fixes plus new feature
 flag
To:     Ravi Bangoria <ravi.bangoria@linux.ibm.com>, mpe@ellerman.id.au,
        christophe.leroy@c-s.fr
Cc:     mikey@neuling.org, jniethe5@gmail.com, pedromfc@linux.ibm.com,
        linux-kernel@vger.kernel.org, paulus@samba.org,
        rogealve@linux.ibm.com, naveen.n.rao@linux.vnet.ibm.com,
        linuxppc-dev@lists.ozlabs.org
References: <20200902042945.129369-1-ravi.bangoria@linux.ibm.com>
From:   Rogerio Alves <rcardoso@linux.ibm.com>
Message-ID: <6927523d-de63-910a-e789-5fab424c7eb9@linux.ibm.com>
Date:   Thu, 17 Sep 2020 10:24:54 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200902042945.129369-1-ravi.bangoria@linux.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


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