[PATCH v5 0/2] Implement erratum 766422

[PATCH v5 0/2] Implement erratum 766422

[Julien Grall]

This is fifth version of this patch series. It allows a guest kernel to
use THUMB set instructions on the processor affected by the errata 766422 (for
instance the Arndale board).

This patch series is divided in two patch:
    - The first patch implements a basic decoder to fill the ISS field
      of the hsr_dabt struct
    - The second patch implements the errata

Since the last version, only minor changes was made. See each patch, for the
changes.

Cheers,

Julien Grall (2):
  xen/arm: Start to implement an ARM decoder instruction
  xen/arm: erratum 766422: decode thumb store during data abort

 xen/arch/arm/Makefile                 |    1 +
 xen/arch/arm/decode.c                 |  165 +++++++++++++++++++++++++++++++++
 xen/arch/arm/decode.h                 |   49 ++++++++++
 xen/arch/arm/traps.c                  |   15 +++
 xen/include/asm-arm/arm32/processor.h |    4 +
 xen/include/asm-arm/arm64/processor.h |    2 +
 6 files changed, 236 insertions(+)
 create mode 100644 xen/arch/arm/decode.c
 create mode 100644 xen/arch/arm/decode.h

-- 
1.7.10.4

[PATCH v5 1/2] xen/arm: Start to implement an ARM decoder instruction

[Julien Grall]

Some errata on ARM processor requires to decode the instruction.
The decoder will, obviously, decode and fill the ISS fields of the hsr_dabt.

For the moment, the decoder only supports:
    - THUMB2 store instruction
    - THUMB single load/store instruction

Signed-off-by: Julien Grall <julien.grall@linaro.org>

---
    Changes in v5:
        - Fix typo in commit
        - Use gdprintk instead of printk
        - Remove TODOs
        - Add warning when it's an ARM instruction
    Changes in v4:
        - Add warning about the purpose of this function
        - Add helper to update ISS (register, sign, size) field of DABT
        - Improve decoding for THUMB 2 store instruction
        - Only decode thumb instruction if it's a 32-bit guest
---
 xen/arch/arm/Makefile |    1 +
 xen/arch/arm/decode.c |  165 +++++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/decode.h |   49 +++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 xen/arch/arm/decode.c
 create mode 100644 xen/arch/arm/decode.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 6e1208f..003ac84 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -30,6 +30,7 @@ obj-y += vtimer.o
 obj-y += vuart.o
 obj-y += hvm.o
 obj-y += device.o
+obj-y += decode.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
new file mode 100644
index 0000000..abe9f26
--- /dev/null
+++ b/xen/arch/arm/decode.c
@@ -0,0 +1,165 @@
+/*
+ * xen/arch/arm/decode.c
+ *
+ * Instruction decoder
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (C) 2013 Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/types.h>
+#include <xen/sched.h>
+#include <asm/current.h>
+#include <asm/guest_access.h>
+#include <xen/lib.h>
+
+#include "decode.h"
+
+static void update_dabt(struct hsr_dabt *dabt, int reg,
+                        uint8_t size, bool_t sign)
+{
+    dabt->reg = reg;
+    dabt->size = size;
+    dabt->sign = sign;
+}
+
+static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1)
+{
+    uint16_t hw2;
+    int rc;
+    uint16_t rt;
+
+    rc = raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2));
+    if ( rc )
+        return rc;
+
+    rt = (hw2 >> 12) & 0x7;
+
+    switch ( (hw1 >> 9) & 0xf )
+    {
+    case 12:
+    {
+        bool_t sign = !!(hw1 & (1 << 8));
+        bool_t load = !!(hw1 & (1 << 4));
+
+        if ( (hw1 & 0x0110) == 0x0100 )
+            /* NEON instruction */
+            goto bad_thumb2;
+
+        if ( (hw1 & 0x0070) == 0x0070 )
+            /* Undefined opcodes */
+            goto bad_thumb2;
+
+        /* Store/Load single data item */
+        if ( rt == 15 )
+            /* XXX: Rt == 15 is only invalid for store instruction */
+            goto bad_thumb2;
+
+        if ( !load && sign )
+            /* Store instruction doesn't support sign extension */
+            goto bad_thumb2;
+
+        update_dabt(dabt, rt, (hw1 >> 5) & 3, sign);
+
+        break;
+    }
+    default:
+        goto bad_thumb2;
+    }
+
+    return 0;
+
+bad_thumb2:
+    gdprintk(XENLOG_ERR, "unhandled THUMB2 instruction 0x%x%x\n", hw1, hw2);
+
+    return 1;
+}
+
+static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
+{
+    uint16_t instr;
+    int rc;
+
+    rc = raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr));
+    if ( rc )
+        return rc;
+
+    switch ( instr >> 12 )
+    {
+    case 5:
+    {
+        /* Load/Store register */
+        uint16_t opB = (instr >> 9) & 0x7;
+        int reg = instr & 7;
+
+        switch ( opB & 0x3 )
+        {
+        case 0: /* Non-signed word */
+            update_dabt(dabt, reg, 2, 0);
+            break;
+        case 1: /* Non-signed halfword */
+            update_dabt(dabt, reg, 1, 0);
+            break;
+        case 2: /* Non-signed byte */
+            update_dabt(dabt, reg, 0, 0);
+            break;
+        case 3: /* Signed byte */
+            update_dabt(dabt, reg, 0, 1);
+            break;
+        }
+
+        break;
+    }
+    case 6:
+        /* Load/Store word immediate offset */
+        update_dabt(dabt, instr & 7, 2, 0);
+        break;
+    case 7:
+        /* Load/Store byte immediate offset */
+        update_dabt(dabt, instr & 7, 0, 0);
+        break;
+    case 8:
+        /* Load/Store halfword immediate offset */
+        update_dabt(dabt, instr & 7, 1, 0);
+        break;
+    case 9:
+        /* Load/Store word sp offset */
+        update_dabt(dabt, (instr >> 8) & 7, 2, 0);
+        break;
+    case 14:
+        if ( instr & (1 << 11) )
+            return decode_thumb2(pc, dabt, instr);
+        goto bad_thumb;
+    case 15:
+        return decode_thumb2(pc, dabt, instr);
+    default:
+        goto bad_thumb;
+    }
+
+    return 0;
+
+bad_thumb:
+    gdprintk(XENLOG_ERR, "unhandled THUMB instruction 0x%x\n", instr);
+    return 1;
+}
+
+int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt *dabt)
+{
+    if ( is_pv32_domain(current->domain) && regs->cpsr & PSR_THUMB )
+        return decode_thumb(regs->pc, dabt);
+
+    /* TODO: Handle ARM instruction */
+    gdprintk(XENLOG_ERR, "unhandled ARM instruction\n");
+
+    return 1;
+}
diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
new file mode 100644
index 0000000..4613763
--- /dev/null
+++ b/xen/arch/arm/decode.h
@@ -0,0 +1,49 @@
+/*
+ * xen/arch/arm/decode.h
+ *
+ * Instruction decoder
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (C) 2013 Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ARCH_ARM_DECODE_H_
+#define __ARCH_ARM_DECODE_H_
+
+#include <asm/regs.h>
+#include <asm/processor.h>
+
+/**
+ * Decode an instruction from pc
+ * /!\ This function is not intended to fully decode an instruction. It
+ * considers that the instruction is valid.
+ *
+ * This function will get:
+ *  - The transfer register
+ *  - Sign bit
+ *  - Size
+ */
+
+int decode_instruction(const struct cpu_user_regs *regs,
+                       struct hsr_dabt *dabt);
+
+#endif /* __ARCH_ARM_DECODE_H_ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.10.4

[PATCH v5 2/2] xen/arm: erratum 766422: decode thumb store during data abort

[Julien Grall]

From the errata document:

When a non-secure non-hypervisor memory operation instruction generates a
stage2 page table translation fault, a trap to the hypervisor will be triggered.
For an architecturally defined subset of instructions, the Hypervisor Syndrome
Register (HSR) will have the Instruction Syndrome Valid (ISV) bit set to 1’b1,
and the Rt field should reflect the source register (for stores) or destination
register for loads.
On Cortex-A15, for Thumb and ThumbEE stores, the Rt value may be incorrect
and should not be used, even if the ISV bit is set. All loads, and all ARM
instruction set loads and stores, will have the correct Rt value if the ISV
bit is set.

To avoid this issue, Xen needs to decode thumb store instruction and update
the transfer register.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v5:
        - Add gdprintk when the instruction decoding fails
        - s/errata/erratum in patch and commit message
    Changes in v3:
        - Use unlikely to check if the processor is affected by the errata
        - Move the decoder in another patch and use it
    Changes in v2:
        - Only decode the instruction on affected processor
        - Handle ARM 32-bit instruction in read_instruction
---
 xen/arch/arm/traps.c                  |   15 +++++++++++++++
 xen/include/asm-arm/arm32/processor.h |    4 ++++
 xen/include/asm-arm/arm64/processor.h |    2 ++
 3 files changed, 21 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6b5fa51..a30797c 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -36,6 +36,7 @@
 #include <asm/cpregs.h>
 #include <asm/psci.h>
 
+#include "decode.h"
 #include "io.h"
 #include "vtimer.h"
 #include <asm/gic.h>
@@ -1336,6 +1337,20 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     if ( !dabt.valid )
         goto bad_data_abort;
 
+    /*
+     * Erratum 766422: Thumb store translation fault to Hypervisor may
+     * not have correct HSR Rt value.
+     */
+    if ( cpu_has_erratum_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
+    {
+        rc = decode_instruction(regs, &info.dabt);
+        if ( rc )
+        {
+            gdprintk(XENLOG_ERR, "Unable to decode instruction\n");
+            goto bad_data_abort;
+        }
+    }
+
     if (handle_mmio(&info))
     {
         advance_pc(regs, hsr);
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index b266252..d1b89d0 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -111,6 +111,10 @@ struct cpu_user_regs
 #define READ_SYSREG(R...)       READ_SYSREG32(R)
 #define WRITE_SYSREG(V, R...)   WRITE_SYSREG32(V, R)
 
+/* Erratum 766422: only Cortex A15 r0p4 is affected */
+#define cpu_has_erratum_766422()                             \
+    (unlikely(current_cpu_data.midr.bits == 0x410fc0f4))
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ARM_ARM32_PROCESSOR_H */
diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
index c8d4609..5bf0867 100644
--- a/xen/include/asm-arm/arm64/processor.h
+++ b/xen/include/asm-arm/arm64/processor.h
@@ -104,6 +104,8 @@ struct cpu_user_regs
 #define READ_SYSREG(name)     READ_SYSREG64(name)
 #define WRITE_SYSREG(v, name) WRITE_SYSREG64(v, name)
 
+#define cpu_has_erratum_766422() 0
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ARM_ARM64_PROCESSOR_H */
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v5 0/2] Implement erratum 766422

[Ian Campbell]

On Thu, 2013-08-08 at 13:56 +0100, Julien Grall wrote:
> This is fifth version of this patch series. It allows a guest kernel to
> use THUMB set instructions on the processor affected by the errata 766422 (for
> instance the Arndale board).
> 
> This patch series is divided in two patch:
>     - The first patch implements a basic decoder to fill the ISS field
>       of the hsr_dabt struct
>     - The second patch implements the errata
> 
> Since the last version, only minor changes was made. See each patch, for the
> changes.

I acked the first and applied both. Thanks.