powerpc/watchpoint: Fix length calculation for unaligned target

Watchpoint match range is always doubleword(8 bytes) aligned on
powerpc. If the given range is crossing doubleword boundary, we need
to increase the length such that next doubleword also get
covered. Ex,

          address   len = 6 bytes
                |=========.
   |------------v--|------v--------|
   | | | | | | | | | | | | | | | | |
   |---------------|---------------|
    <---8 bytes--->

In such case, current code configures hw as:
  start_addr = address & ~HW_BREAKPOINT_ALIGN
  len = 8 bytes

And thus read/write in last 4 bytes of the given range is ignored.
Fix this by including next doubleword in the length.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191017093204.7511-3-ravi.bangoria@linux.ibm.com

diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 4a887e85a5f49d5107b5e7c9619b47338579ba00..ea91ac7f5a273d31da3c0bd675892008256130ec 100644 (file)
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -14,6 +14,7 @@ struct arch_hw_breakpoint {
        unsigned long   address;
        u16             type;
        u16             len; /* length of the target data symbol */
+       u16             hw_len; /* length programmed in hw */
 };
 
 /* Note: Don't change the the first 6 bits below as they are in the same order
@@ -73,6 +74,7 @@ static inline void hw_breakpoint_disable(void)
        brk.address = 0;
        brk.type = 0;
        brk.len = 0;
+       brk.hw_len = 0;
        if (ppc_breakpoint_available())
                __set_breakpoint(&brk);
 }

diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 5f66b95b68588a3a1eea28083154b9dd77c41030..cc14aa6c4a1bb41fe62524bbd5ea098f29ba878b 100644 (file)
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -30,10 +30,10 @@ int set_dawr(struct arch_hw_breakpoint *brk)
         * DAWR length is stored in field MDR bits 48:53.  Matches range in
         * doublewords (64 bits) baised by -1 eg. 0b000000=1DW and
         * 0b111111=64DW.
-        * brk->len is in bytes.
+        * brk->hw_len is in bytes.
         * This aligns up to double word size, shifts and does the bias.
         */
-       mrd = ((brk->len + 7) >> 3) - 1;
+       mrd = ((brk->hw_len + 7) >> 3) - 1;
        dawrx |= (mrd & 0x3f) << (63 - 53);
 
        if (ppc_md.set_dawr)
@@ -54,7 +54,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
                                    const char __user *user_buf,
                                    size_t count, loff_t *ppos)
 {
-       struct arch_hw_breakpoint null_brk = {0, 0, 0};
+       struct arch_hw_breakpoint null_brk = {0};
        size_t rc;
 
        /* Send error to user if they hypervisor won't allow us to write DAWR */

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 677041cb3c3e76f1aa87f7c832404aca903792b3..f36274d426ed20bdc2057c0e529d969f54932157 100644 (file)
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -126,6 +126,49 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
        return 0;
 }
 
+/*
+ * Watchpoint match range is always doubleword(8 bytes) aligned on
+ * powerpc. If the given range is crossing doubleword boundary, we
+ * need to increase the length such that next doubleword also get
+ * covered. Ex,
+ *
+ *          address   len = 6 bytes
+ *                |=========.
+ *   |------------v--|------v--------|
+ *   | | | | | | | | | | | | | | | | |
+ *   |---------------|---------------|
+ *    <---8 bytes--->
+ *
+ * In this case, we should configure hw as:
+ *   start_addr = address & ~HW_BREAKPOINT_ALIGN
+ *   len = 16 bytes
+ *
+ * @start_addr and @end_addr are inclusive.
+ */
+static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
+{
+       u16 max_len = DABR_MAX_LEN;
+       u16 hw_len;
+       unsigned long start_addr, end_addr;
+
+       start_addr = hw->address & ~HW_BREAKPOINT_ALIGN;
+       end_addr = (hw->address + hw->len - 1) | HW_BREAKPOINT_ALIGN;
+       hw_len = end_addr - start_addr + 1;
+
+       if (dawr_enabled()) {
+               max_len = DAWR_MAX_LEN;
+               /* DAWR region can't cross 512 bytes boundary */
+               if ((start_addr >> 9) != (end_addr >> 9))
+                       return -EINVAL;
+       }
+
+       if (hw_len > max_len)
+               return -EINVAL;
+
+       hw->hw_len = hw_len;
+       return 0;
+}
+
 /*
  * Validate the arch-specific HW Breakpoint register settings
  */
@@ -133,9 +176,9 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
                             const struct perf_event_attr *attr,
                             struct arch_hw_breakpoint *hw)
 {
-       int ret = -EINVAL, length_max;
+       int ret = -EINVAL;
 
-       if (!bp)
+       if (!bp || !attr->bp_len)
                return ret;
 
        hw->type = HW_BRK_TYPE_TRANSLATE;
@@ -155,26 +198,10 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
        hw->address = attr->bp_addr;
        hw->len = attr->bp_len;
 
-       /*
-        * Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8)
-        * and breakpoint addresses are aligned to nearest double-word
-        * HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
-        * 'symbolsize' should satisfy the check below.
-        */
        if (!ppc_breakpoint_available())
                return -ENODEV;
-       length_max = DABR_MAX_LEN; /* DABR */
-       if (dawr_enabled()) {
-               length_max = DAWR_MAX_LEN; /* 64 doublewords */
-               /* DAWR region can't cross 512 boundary */
-               if ((attr->bp_addr >> 9) !=
-                   ((attr->bp_addr + attr->bp_len - 1) >> 9))
-                       return -EINVAL;
-       }
-       if (hw->len >
-           (length_max - (hw->address & HW_BREAKPOINT_ALIGN)))
-               return -EINVAL;
-       return 0;
+
+       return hw_breakpoint_validate_len(hw);
 }
 
 /*

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 639ceae7da9d81de567b4c3d967bd8fd6b85a017..4df94b6e2f3229dc3030b5d33d1f79cbe15a679a 100644 (file)
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -715,6 +715,8 @@ static void set_debug_reg_defaults(struct thread_struct *thread)
 {
        thread->hw_brk.address = 0;
        thread->hw_brk.type = 0;
+       thread->hw_brk.len = 0;
+       thread->hw_brk.hw_len = 0;
        if (ppc_breakpoint_available())
                set_breakpoint(&thread->hw_brk);
 }
@@ -816,6 +818,7 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
                return false;
        if (a->len != b->len)
                return false;
+       /* no need to check hw_len. it's calculated from address and len */
        return true;
 }
 

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index eba39a5f62942cbe30369ab48561b764447e0691..c2dc93157b991b38baf48eb20fd54f1edd7de8ef 100644 (file)
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2426,6 +2426,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
        hw_brk.address = data & (~HW_BRK_TYPE_DABR);
        hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
        hw_brk.len = DABR_MAX_LEN;
+       hw_brk.hw_len = DABR_MAX_LEN;
        set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR);
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
        bp = thread->ptrace_bps[0];