x86: Fix instruction breakpoint encoding

Lengths and types of breakpoints are encoded in a half byte
into CPU registers. However when we extract these values
and store them, we add a high half byte part to them: 0x40 to the
length and 0x80 to the type.
When that gets reloaded to the CPU registers, the high part
is masked.

While making the instruction breakpoints available for perf,
I zapped that high part on instruction breakpoint encoding
and that broke the arch -> generic translation used by ptrace
instruction breakpoints. Writing dr7 to set an inst breakpoint
was then failing.

There is no apparent reason for these high parts so we could get
rid of them altogether. That's an invasive change though so let's
do that later and for now fix the problem by restoring that inst
breakpoint high part encoding in this sole patch.

Reported-by: Kelvie Wong <kelvie@ieee.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Prasad <prasad@linux.vnet.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Cc: Will Deacon <will.deacon@arm.com>

diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 528a11e8d3e35f64fea90202d6f196d77d48e708..824ca07860d012cdcc9c46886500d056fdb2aeed 100644 (file)
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -20,7 +20,7 @@ struct arch_hw_breakpoint {
 #include <linux/list.h>
 
 /* Available HW breakpoint length encodings */
-#define X86_BREAKPOINT_LEN_X           0x00
+#define X86_BREAKPOINT_LEN_X           0x40
 #define X86_BREAKPOINT_LEN_1           0x40
 #define X86_BREAKPOINT_LEN_2           0x44
 #define X86_BREAKPOINT_LEN_4           0x4c

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index a474ec37c32f84df372d39eac5730532d60d0228..ff15c9dcc25de8be8144fd4d15f1dfd3069314e4 100644 (file)
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -206,11 +206,27 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
 int arch_bp_generic_fields(int x86_len, int x86_type,
                           int *gen_len, int *gen_type)
 {
-       /* Len */
-       switch (x86_len) {
-       case X86_BREAKPOINT_LEN_X:
+       /* Type */
+       switch (x86_type) {
+       case X86_BREAKPOINT_EXECUTE:
+               if (x86_len != X86_BREAKPOINT_LEN_X)
+                       return -EINVAL;
+
+               *gen_type = HW_BREAKPOINT_X;
                *gen_len = sizeof(long);
+               return 0;
+       case X86_BREAKPOINT_WRITE:
+               *gen_type = HW_BREAKPOINT_W;
                break;
+       case X86_BREAKPOINT_RW:
+               *gen_type = HW_BREAKPOINT_W | HW_BREAKPOINT_R;
+               break;
+       default:
+               return -EINVAL;
+       }
+
+       /* Len */
+       switch (x86_len) {
        case X86_BREAKPOINT_LEN_1:
                *gen_len = HW_BREAKPOINT_LEN_1;
                break;
@@ -229,21 +245,6 @@ int arch_bp_generic_fields(int x86_len, int x86_type,
                return -EINVAL;
        }
 
-       /* Type */
-       switch (x86_type) {
-       case X86_BREAKPOINT_EXECUTE:
-               *gen_type = HW_BREAKPOINT_X;
-               break;
-       case X86_BREAKPOINT_WRITE:
-               *gen_type = HW_BREAKPOINT_W;
-               break;
-       case X86_BREAKPOINT_RW:
-               *gen_type = HW_BREAKPOINT_W | HW_BREAKPOINT_R;
-               break;
-       default:
-               return -EINVAL;
-       }
-
        return 0;
 }
 
@@ -316,9 +317,6 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
        ret = -EINVAL;
 
        switch (info->len) {
-       case X86_BREAKPOINT_LEN_X:
-               align = sizeof(long) -1;
-               break;
        case X86_BREAKPOINT_LEN_1:
                align = 0;
                break;