[Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[Richard Henderson]

The branch target identification extension does not yet have
kernel or gcc support.  It should be in shape for developing
those though.

In system mode I do honor the GP bit from the page tables.
In user-only mode, there is a way to pretend that the GP bit
is always set.  Further linux-user changes will have to track
the ABI that gets developed.

In the meantime, provide a statically linable make check-tcg 
test for the functionality.

Changes since v2:
  * Rebase on master.
  * Incorporate feedback from Peter.


r~


Richard Henderson (12):
  target/arm: Introduce isar_feature_aa64_bti
  target/arm: Add PSTATE.BTYPE
  target/arm: Add BT and BTYPE to tb->flags
  exec: Add target-specific tlb bits to MemTxAttrs
  target/arm: Cache the GP bit for a page in MemTxAttrs
  target/arm: Default handling of BTYPE during translation
  target/arm: Reset btype for direct branches
  target/arm: Set btype for indirect branches
  target/arm: Add x-guarded-pages cpu property for user-only
  target/arm: Enable BTI for -cpu max
  linux-user/aarch64: Reset btype for syscalls and signals
  tests/tcg/aarch64: Add bti smoke test

 include/exec/memattrs.h           |  10 ++
 target/arm/cpu.h                  |  24 +++-
 target/arm/internals.h            |   6 +
 target/arm/translate.h            |   9 ++
 linux-user/aarch64/cpu_loop.c     |  15 +++
 target/arm/cpu64.c                |  22 ++++
 target/arm/helper.c               |  28 +++--
 target/arm/translate-a64.c        | 193 +++++++++++++++++++++++++++++-
 tests/tcg/aarch64/bti-1.c         |  61 ++++++++++
 tests/tcg/aarch64/bti-crt.inc.c   |  51 ++++++++
 tests/tcg/aarch64/Makefile.target |   7 +-
 11 files changed, 415 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/aarch64/bti-1.c
 create mode 100644 tests/tcg/aarch64/bti-crt.inc.c

-- 
2.17.2

[Qemu-devel] [PATCH v2 01/12] target/arm: Introduce isar_feature_aa64_bti

[Richard Henderson]

Also create field definitions for id_aa64pfr1 from ARMv8.5.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Include MTE and RAS_FRAC fields of AA64PFR1.
---
 target/arm/cpu.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ff81db420d..d7190f0712 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1672,6 +1672,11 @@ FIELD(ID_AA64PFR0, GIC, 24, 4)
 FIELD(ID_AA64PFR0, RAS, 28, 4)
 FIELD(ID_AA64PFR0, SVE, 32, 4)
 
+FIELD(ID_AA64PFR1, BT, 0, 4)
+FIELD(ID_AA64PFR1, SBSS, 4, 4)
+FIELD(ID_AA64PFR1, MTE, 8, 4)
+FIELD(ID_AA64PFR1, RAS_FRAC, 12, 4)
+
 FIELD(ID_AA64MMFR0, PARANGE, 0, 4)
 FIELD(ID_AA64MMFR0, ASIDBITS, 4, 4)
 FIELD(ID_AA64MMFR0, BIGEND, 8, 4)
@@ -3319,6 +3324,11 @@ static inline bool isar_feature_aa64_lor(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, LO) != 0;
 }
 
+static inline bool isar_feature_aa64_bti(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, BT) != 0;
+}
+
 /*
  * Forward to the above feature tests given an ARMCPU pointer.
  */
-- 
2.17.2

[Qemu-devel] [PATCH v2 02/12] target/arm: Add PSTATE.BTYPE

[Richard Henderson]

Place this in its own field within ENV, as that will
make it easier to reset from within TCG generated code.

With the change to pstate_read/write, exception entry
and return are automatically handled.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h           | 8 ++++++--
 target/arm/translate-a64.c | 3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d7190f0712..76e2f8fd42 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -234,6 +234,7 @@ typedef struct CPUARMState {
      *    semantics as for AArch32, as described in the comments on each field)
      *  nRW (also known as M[4]) is kept, inverted, in env->aarch64
      *  DAIF (exception masks) are kept in env->daif
+     *  BTYPE is kept in env->btype
      *  all other bits are stored in their correct places in env->pstate
      */
     uint32_t pstate;
@@ -263,6 +264,7 @@ typedef struct CPUARMState {
     uint32_t GE; /* cpsr[19:16] */
     uint32_t thumb; /* cpsr[5]. 0 = arm mode, 1 = thumb mode. */
     uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
+    uint32_t btype;  /* BTI branch type.  spsr[11:10].  */
     uint64_t daif; /* exception masks, in the bits they are in PSTATE */
 
     uint64_t elr_el[4]; /* AArch64 exception link regs  */
@@ -1197,6 +1199,7 @@ uint64_t get_pmceid(CPUARMState *env, unsigned which);
 #define PSTATE_I (1U << 7)
 #define PSTATE_A (1U << 8)
 #define PSTATE_D (1U << 9)
+#define PSTATE_BTYPE (3U << 10)
 #define PSTATE_IL (1U << 20)
 #define PSTATE_SS (1U << 21)
 #define PSTATE_V (1U << 28)
@@ -1205,7 +1208,7 @@ uint64_t get_pmceid(CPUARMState *env, unsigned which);
 #define PSTATE_N (1U << 31)
 #define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
 #define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
-#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
+#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF | PSTATE_BTYPE)
 /* Mode values for AArch64 */
 #define PSTATE_MODE_EL3h 13
 #define PSTATE_MODE_EL3t 12
@@ -1237,7 +1240,7 @@ static inline uint32_t pstate_read(CPUARMState *env)
     ZF = (env->ZF == 0);
     return (env->NF & 0x80000000) | (ZF << 30)
         | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
-        | env->pstate | env->daif;
+        | env->pstate | env->daif | (env->btype << 10);
 }
 
 static inline void pstate_write(CPUARMState *env, uint32_t val)
@@ -1247,6 +1250,7 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
     env->CF = (val >> 29) & 1;
     env->VF = (val << 3) & 0x80000000;
     env->daif = val & PSTATE_DAIF;
+    env->btype = (val >> 10) & 3;
     env->pstate = val & ~CACHED_PSTATE_BITS;
 }
 
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 4d28a27c3b..611279e98e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -163,6 +163,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
                 el,
                 psr & PSTATE_SP ? 'h' : 't');
 
+    if (cpu_isar_feature(aa64_bti, cpu)) {
+        cpu_fprintf(f, "  BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
+    }
     if (!(flags & CPU_DUMP_FPU)) {
         cpu_fprintf(f, "\n");
         return;
-- 
2.17.2

[Qemu-devel] [PATCH v2 03/12] target/arm: Add BT and BTYPE to tb->flags

[Richard Henderson]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h           |  2 ++
 target/arm/translate.h     |  4 ++++
 target/arm/helper.c        | 22 +++++++++++++++-------
 target/arm/translate-a64.c |  2 ++
 4 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 76e2f8fd42..e18f823419 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3043,6 +3043,8 @@ FIELD(TBFLAG_A64, TBII, 0, 2)
 FIELD(TBFLAG_A64, SVEEXC_EL, 2, 2)
 FIELD(TBFLAG_A64, ZCR_LEN, 4, 4)
 FIELD(TBFLAG_A64, PAUTH_ACTIVE, 8, 1)
+FIELD(TBFLAG_A64, BT, 9, 1)
+FIELD(TBFLAG_A64, BTYPE, 10, 2)
 
 static inline bool bswap_code(bool sctlr_b)
 {
diff --git a/target/arm/translate.h b/target/arm/translate.h
index bb37d35741..3d5e8bacac 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -69,6 +69,10 @@ typedef struct DisasContext {
     bool ss_same_el;
     /* True if v8.3-PAuth is active.  */
     bool pauth_active;
+    /* True with v8.5-BTI and SCTLR_ELx.BT* set.  */
+    bool bt;
+    /* A copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.  */
+    uint8_t btype;
     /* Bottom two bits of XScale c15_cpar coprocessor access control reg */
     int c15_cpar;
     /* TCG op of the current insn_start.  */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 92666e5208..6efe88a157 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13607,6 +13607,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
 
     if (is_a64(env)) {
         ARMCPU *cpu = arm_env_get_cpu(env);
+        uint64_t sctlr;
 
         *pc = env->pc;
         flags = FIELD_DP32(flags, TBFLAG_ANY, AARCH64_STATE, 1);
@@ -13651,6 +13652,12 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
             flags = FIELD_DP32(flags, TBFLAG_A64, ZCR_LEN, zcr_len);
         }
 
+        if (current_el == 0) {
+            /* FIXME: ARMv8.1-VHE S2 translation regime.  */
+            sctlr = env->cp15.sctlr_el[1];
+        } else {
+            sctlr = env->cp15.sctlr_el[current_el];
+        }
         if (cpu_isar_feature(aa64_pauth, cpu)) {
             /*
              * In order to save space in flags, we record only whether
@@ -13658,17 +13665,18 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
              * a nop, or "active" when some action must be performed.
              * The decision of which action to take is left to a helper.
              */
-            uint64_t sctlr;
-            if (current_el == 0) {
-                /* FIXME: ARMv8.1-VHE S2 translation regime.  */
-                sctlr = env->cp15.sctlr_el[1];
-            } else {
-                sctlr = env->cp15.sctlr_el[current_el];
-            }
             if (sctlr & (SCTLR_EnIA | SCTLR_EnIB | SCTLR_EnDA | SCTLR_EnDB)) {
                 flags = FIELD_DP32(flags, TBFLAG_A64, PAUTH_ACTIVE, 1);
             }
         }
+
+        if (cpu_isar_feature(aa64_bti, cpu)) {
+            /* Note that SCTLR_EL[23].BT == SCTLR_BT1.  */
+            if (sctlr & (current_el == 0 ? SCTLR_BT0 : SCTLR_BT1)) {
+                flags = FIELD_DP32(flags, TBFLAG_A64, BT, 1);
+            }
+            flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
+        }
     } else {
         *pc = env->regs[15];
         flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 611279e98e..9ce523e2ff 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13801,6 +13801,8 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase,
     dc->sve_excp_el = FIELD_EX32(tb_flags, TBFLAG_A64, SVEEXC_EL);
     dc->sve_len = (FIELD_EX32(tb_flags, TBFLAG_A64, ZCR_LEN) + 1) * 16;
     dc->pauth_active = FIELD_EX32(tb_flags, TBFLAG_A64, PAUTH_ACTIVE);
+    dc->bt = FIELD_EX32(tb_flags, TBFLAG_A64, BT);
+    dc->btype = FIELD_EX32(tb_flags, TBFLAG_A64, BTYPE);
     dc->vec_len = 0;
     dc->vec_stride = 0;
     dc->cp_regs = arm_cpu->cp_regs;
-- 
2.17.2

[Qemu-devel] [PATCH v2 04/12] exec: Add target-specific tlb bits to MemTxAttrs

[Richard Henderson]

These bits can be used to cache target-specific data in cputlb
read from the page tables.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/memattrs.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index d4a1642098..d4a3477d71 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -37,6 +37,16 @@ typedef struct MemTxAttrs {
     unsigned int user:1;
     /* Requester ID (for MSI for example) */
     unsigned int requester_id:16;
+    /*
+     * The following are target-specific page-table bits.  These are not
+     * related to actual memory transactions at all.  However, this structure
+     * is part of the tlb_fill interface, cached in the cputlb structure,
+     * and has unused bits.  These fields will be read by target-specific
+     * helpers using env->iotlb[mmu_idx][tlb_index()].attrs.target_tlb_bitN.
+     */
+    unsigned int target_tlb_bit0 : 1;
+    unsigned int target_tlb_bit1 : 1;
+    unsigned int target_tlb_bit2 : 1;
 } MemTxAttrs;
 
 /* Bus masters which don't specify any attributes will get this,
-- 
2.17.2

[Qemu-devel] [PATCH v2 05/12] target/arm: Cache the GP bit for a page in MemTxAttrs

[Richard Henderson]

Caching the bit means that we will not have to re-walk the
page tables to look up the bit during translation.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6efe88a157..70277222da 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10457,6 +10457,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
     bool ttbr1_valid;
     uint64_t descaddrmask;
     bool aarch64 = arm_el_is_aa64(env, el);
+    bool guarded = false;
 
     /* TODO:
      * This code does not handle the different format TCR for VTCR_EL2.
@@ -10629,6 +10630,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         }
         /* Merge in attributes from table descriptors */
         attrs |= nstable << 3; /* NS */
+        guarded |= extract64(descriptor, 50, 1);  /* GP */
         if (param.hpd) {
             /* HPD disables all the table attributes except NSTable.  */
             break;
@@ -10674,6 +10676,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
          */
         txattrs->secure = false;
     }
+    /* When in aarch64 mode, and BTI is enabled, remember GP in the IOTLB.  */
+    if (aarch64 && guarded && cpu_isar_feature(aa64_bti, cpu)) {
+        txattrs->target_tlb_bit0 = true;
+    }
 
     if (cacheattrs != NULL) {
         if (mmu_idx == ARMMMUIdx_S2NS) {
-- 
2.17.2

[Qemu-devel] [PATCH v2 06/12] target/arm: Default handling of BTYPE during translation

[Richard Henderson]

The branch target exception for guarded pages has high priority,
and only 8 instructions are valid for that case.  Perform this
check before doing any other decode.

Clear BTYPE after all insns that neither set BTYPE nor exit via
exception (DISAS_NORETURN).

Not yet handled are insns that exit via DISAS_NORETURN for some
other reason, like direct branches.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Change octal constants to binary.
---
 target/arm/internals.h     |   6 ++
 target/arm/translate.h     |   9 ++-
 target/arm/translate-a64.c | 139 +++++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+), 2 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index a6fd4582b2..d01a3f9f44 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -268,6 +268,7 @@ enum arm_exception_class {
     EC_FPIDTRAP               = 0x08,
     EC_PACTRAP                = 0x09,
     EC_CP14RRTTRAP            = 0x0c,
+    EC_BTITRAP                = 0x0d,
     EC_ILLEGALSTATE           = 0x0e,
     EC_AA32_SVC               = 0x11,
     EC_AA32_HVC               = 0x12,
@@ -439,6 +440,11 @@ static inline uint32_t syn_pactrap(void)
     return EC_PACTRAP << ARM_EL_EC_SHIFT;
 }
 
+static inline uint32_t syn_btitrap(int btype)
+{
+    return (EC_BTITRAP << ARM_EL_EC_SHIFT) | btype;
+}
+
 static inline uint32_t syn_insn_abort(int same_el, int ea, int s1ptw, int fsc)
 {
     return (EC_INSNABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 3d5e8bacac..f73939d7b4 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -71,8 +71,13 @@ typedef struct DisasContext {
     bool pauth_active;
     /* True with v8.5-BTI and SCTLR_ELx.BT* set.  */
     bool bt;
-    /* A copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.  */
-    uint8_t btype;
+    /*
+     * >= 0, a copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.
+     *  < 0, set by the current instruction.
+     */
+    int8_t btype;
+    /* True if this page is guarded.  */
+    bool guarded_page;
     /* Bottom two bits of XScale c15_cpar coprocessor access control reg */
     int c15_cpar;
     /* TCG op of the current insn_start.  */
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 9ce523e2ff..bb64a47c0f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -128,6 +128,16 @@ static inline int get_a64_user_mem_index(DisasContext *s)
     return arm_to_core_mmu_idx(useridx);
 }
 
+static void reset_btype(DisasContext *s)
+{
+    if (s->btype != 0) {
+        TCGv_i32 zero = tcg_const_i32(0);
+        tcg_gen_st_i32(zero, cpu_env, offsetof(CPUARMState, btype));
+        tcg_temp_free_i32(zero);
+        s->btype = 0;
+    }
+}
+
 void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
                             fprintf_function cpu_fprintf, int flags)
 {
@@ -13717,6 +13727,90 @@ static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
     }
 }
 
+/**
+ * is_guarded_page:
+ * @env: The cpu environment
+ * @s: The DisasContext
+ *
+ * Return true if the page is guarded.
+ */
+static bool is_guarded_page(CPUARMState *env, DisasContext *s)
+{
+#ifdef CONFIG_USER_ONLY
+    return false;  /* FIXME */
+#else
+    uint64_t addr = s->base.pc_first;
+    int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
+    unsigned int index = tlb_index(env, mmu_idx, addr);
+    CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+
+    /*
+     * We test this immediately after reading an insn, which means
+     * that any normal page must be in the TLB.  The only exception
+     * would be for executing from flash or device memory, which
+     * does not retain the TLB entry.
+     *
+     * FIXME: Assume false for those, for now.  We could use
+     * arm_cpu_get_phys_page_attrs_debug to re-read the page
+     * table entry even for that case.
+     */
+    return (tlb_hit(entry->addr_code, addr) &&
+            env->iotlb[mmu_idx][index].attrs.target_tlb_bit0);
+#endif
+}
+
+/**
+ * btype_destination_ok:
+ * @insn: The instruction at the branch destination
+ * @bt: SCTLR_ELx.BT
+ * @btype: PSTATE.BTYPE, and is non-zero
+ *
+ * On a guarded page, there are a limited number of insns
+ * that may be present at the branch target:
+ *   - branch target identifiers,
+ *   - paciasp, pacibsp,
+ *   - BRK insn
+ *   - HLT insn
+ * Anything else causes a Branch Target Exception.
+ *
+ * Return true if the branch is compatible, false to raise BTITRAP.
+ */
+static bool btype_destination_ok(uint32_t insn, bool bt, int btype)
+{
+    if ((insn & 0xfffff01fu) == 0xd503201fu) {
+        /* HINT space */
+        switch (extract32(insn, 5, 7)) {
+        case 0b011001: /* PACIASP */
+        case 0b011011: /* PACIBSP */
+            /*
+             * If SCTLR_ELx.BT, then PACI*SP are not compatible
+             * with btype == 3.  Otherwise all btype are ok.
+             */
+            return !bt || btype != 3;
+        case 0b100000: /* BTI */
+            /* Not compatible with any btype.  */
+            return false;
+        case 0b100010: /* BTI c */
+            /* Not compatible with btype == 3 */
+            return btype != 3;
+        case 0b100100: /* BTI j */
+            /* Not compatible with btype == 2 */
+            return btype != 2;
+        case 0b100110: /* BTI jc */
+            /* Compatible with any btype.  */
+            return true;
+        }
+    } else {
+        switch (insn & 0xffe0001fu) {
+        case 0xd4200000u: /* BRK */
+        case 0xd4400000u: /* HLT */
+            /* Give priority to the breakpoint exception.  */
+            return true;
+        }
+    }
+    return false;
+}
+
 /* C3.1 A64 instruction index by encoding */
 static void disas_a64_insn(CPUARMState *env, DisasContext *s)
 {
@@ -13728,6 +13822,43 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s)
 
     s->fp_access_checked = false;
 
+    if (dc_isar_feature(aa64_bti, s)) {
+        if (s->base.num_insns == 1) {
+            /*
+             * At the first insn of the TB, compute s->guarded_page.
+             * We delayed computing this until successfully reading
+             * the first insn of the TB, above.  This (mostly) ensures
+             * that the softmmu tlb entry has been populated, and the
+             * page table GP bit is available.
+             *
+             * Note that we need to compute this even if btype == 0,
+             * because this value is used for BR instructions later
+             * where ENV is not available.
+             */
+            s->guarded_page = is_guarded_page(env, s);
+
+            /* First insn can have btype set to non-zero.  */
+            tcg_debug_assert(s->btype >= 0);
+
+            /*
+             * Note that the Branch Target Exception has fairly high
+             * priority -- below debugging exceptions but above most
+             * everything else.  This allows us to handle this now
+             * instead of waiting until the insn is otherwise decoded.
+             */
+            if (s->btype != 0
+                && s->guarded_page
+                && !btype_destination_ok(insn, s->bt, s->btype)) {
+                gen_exception_insn(s, 4, EXCP_UDEF, syn_btitrap(s->btype),
+                                   default_exception_el(s));
+                return;
+            }
+        } else {
+            /* Not the first insn: btype must be 0.  */
+            tcg_debug_assert(s->btype == 0);
+        }
+    }
+
     switch (extract32(insn, 25, 4)) {
     case 0x0: case 0x1: case 0x3: /* UNALLOCATED */
         unallocated_encoding(s);
@@ -13764,6 +13895,14 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s)
 
     /* if we allocated any temporaries, free them here */
     free_tmp_a64(s);
+
+    /*
+     * After execution of most insns, btype is reset to 0.
+     * Note that we set btype == -1 when the insn sets btype.
+     */
+    if (s->btype > 0 && s->base.is_jmp != DISAS_NORETURN) {
+        reset_btype(s);
+    }
 }
 
 static void aarch64_tr_init_disas_context(DisasContextBase *dcbase,
-- 
2.17.2

[Qemu-devel] [PATCH v2 07/12] target/arm: Reset btype for direct branches

[Richard Henderson]

This is all of the non-exception cases of DISAS_NORETURN.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Do not reset byte for syscalls
---
 target/arm/translate-a64.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index bb64a47c0f..dbac09743c 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1362,6 +1362,7 @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
     }
 
     /* B Branch / BL Branch with link */
+    reset_btype(s);
     gen_goto_tb(s, 0, addr);
 }
 
@@ -1386,6 +1387,7 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
     tcg_cmp = read_cpu_reg(s, rt, sf);
     label_match = gen_new_label();
 
+    reset_btype(s);
     tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ,
                         tcg_cmp, 0, label_match);
 
@@ -1415,6 +1417,8 @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn)
     tcg_cmp = tcg_temp_new_i64();
     tcg_gen_andi_i64(tcg_cmp, cpu_reg(s, rt), (1ULL << bit_pos));
     label_match = gen_new_label();
+
+    reset_btype(s);
     tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ,
                         tcg_cmp, 0, label_match);
     tcg_temp_free_i64(tcg_cmp);
@@ -1441,6 +1445,7 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn)
     addr = s->pc + sextract32(insn, 5, 19) * 4 - 4;
     cond = extract32(insn, 0, 4);
 
+    reset_btype(s);
     if (cond < 0x0e) {
         /* genuinely conditional branches */
         TCGLabel *label_match = gen_new_label();
@@ -1605,6 +1610,7 @@ static void handle_sync(DisasContext *s, uint32_t insn,
          * a self-modified code correctly and also to take
          * any pending interrupts immediately.
          */
+        reset_btype(s);
         gen_goto_tb(s, 0, s->pc);
         return;
     default:
-- 
2.17.2

[Qemu-devel] [PATCH v2 08/12] target/arm: Set btype for indirect branches

[Richard Henderson]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index dbac09743c..89cc54dbed 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -138,6 +138,19 @@ static void reset_btype(DisasContext *s)
     }
 }
 
+static void set_btype(DisasContext *s, int val)
+{
+    TCGv_i32 tcg_val;
+
+    /* BTYPE is a 2-bit field, and 0 should be done with reset_btype.  */
+    tcg_debug_assert(val >= 1 && val <= 3);
+
+    tcg_val = tcg_const_i32(val);
+    tcg_gen_st_i32(tcg_val, cpu_env, offsetof(CPUARMState, btype));
+    tcg_temp_free_i32(tcg_val);
+    s->btype = -1;
+}
+
 void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
                             fprintf_function cpu_fprintf, int flags)
 {
@@ -1982,6 +1995,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
 static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
 {
     unsigned int opc, op2, op3, rn, op4;
+    unsigned btype_mod = 2;   /* 0: BR, 1: BLR, 2: other */
     TCGv_i64 dst;
     TCGv_i64 modifier;
 
@@ -1999,6 +2013,7 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
     case 0: /* BR */
     case 1: /* BLR */
     case 2: /* RET */
+        btype_mod = opc;
         switch (op3) {
         case 0:
             /* BR, BLR, RET */
@@ -2042,7 +2057,6 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
         default:
             goto do_unallocated;
         }
-
         gen_a64_set_pc(s, dst);
         /* BLR also needs to load return address */
         if (opc == 1) {
@@ -2058,6 +2072,7 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
         if (op3 != 2 || op3 != 3) {
             goto do_unallocated;
         }
+        btype_mod = opc & 1;
         if (s->pauth_active) {
             dst = new_tmp_a64(s);
             modifier = cpu_reg_sp(s, op4);
@@ -2141,6 +2156,26 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
         return;
     }
 
+    switch (btype_mod) {
+    case 0: /* BR */
+        if (dc_isar_feature(aa64_bti, s)) {
+            /* BR to {x16,x17} or !guard -> 1, else 3.  */
+            set_btype(s, rn == 16 || rn == 17 || !s->guarded_page ? 1 : 3);
+        }
+        break;
+
+    case 1: /* BLR */
+        if (dc_isar_feature(aa64_bti, s)) {
+            /* BLR sets BTYPE to 2, regardless of source guarded page.  */
+            set_btype(s, 2);
+        }
+        break;
+
+    default: /* RET or none of the above.  */
+        /* BTYPE will be set to 0 by normal end-of-insn processing.  */
+        break;
+    }
+
     s->base.is_jmp = DISAS_JUMP;
 }
 
-- 
2.17.2

[Qemu-devel] [PATCH v2 09/12] target/arm: Add x-guarded-pages cpu property for user-only

[Richard Henderson]

While waiting for a proper userland ABI, allow static test
cases to be written assuming that GP is set for all pages.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Renamed the property with x- prefix
---
 target/arm/cpu.h           |  4 ++++
 target/arm/cpu64.c         | 18 ++++++++++++++++++
 target/arm/translate-a64.c |  8 +++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e18f823419..8c9eb519ef 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -895,6 +895,10 @@ struct ARMCPU {
      */
     bool cfgend;
 
+#ifdef CONFIG_USER_ONLY
+    bool guarded_pages;
+#endif
+
     QLIST_HEAD(, ARMELChangeHook) pre_el_change_hooks;
     QLIST_HEAD(, ARMELChangeHook) el_change_hooks;
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index e9bc461c36..a563f7e74d 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -311,6 +311,18 @@ static void cpu_max_set_packey(Object *obj, Visitor *v, const char *name,
     }
     error_propagate(errp, err);
 }
+
+static bool aarch64_cpu_get_guarded_pages(Object *obj, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    return cpu->guarded_pages;
+}
+
+static void aarch64_cpu_set_guarded_pages(Object *obj, bool val, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    cpu->guarded_pages = val;
+}
 #endif
 
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
@@ -416,6 +428,12 @@ static void aarch64_max_initfn(Object *obj)
             cpu->env.cp15.sctlr_el[1] |= SCTLR_EnIA | SCTLR_EnIB;
             cpu->env.cp15.sctlr_el[1] |= SCTLR_EnDA | SCTLR_EnDB;
         }
+
+        object_property_add_bool(obj, "x-guarded-pages",
+                                 aarch64_cpu_get_guarded_pages,
+                                 aarch64_cpu_set_guarded_pages, NULL);
+        object_property_set_description(obj, "x-guarded-pages",
+            "Set on/off GuardPage bit for all pages", NULL);
 #endif
 
         cpu->sve_max_vq = ARM_MAX_VQ;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 89cc54dbed..a1adb8cde0 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13778,7 +13778,13 @@ static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
 static bool is_guarded_page(CPUARMState *env, DisasContext *s)
 {
 #ifdef CONFIG_USER_ONLY
-    return false;  /* FIXME */
+    /*
+     * FIXME: What is the userland ABI for this?
+     * For the moment this is controlled by an attribute:
+     *   -cpu max,guarded_pages=on.
+     */
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    return cpu->guarded_pages;
 #else
     uint64_t addr = s->base.pc_first;
     int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
-- 
2.17.2

[Qemu-devel] [PATCH v2 10/12] target/arm: Enable BTI for -cpu max

[Richard Henderson]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu64.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index a563f7e74d..9d2276fe53 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -368,6 +368,10 @@ static void aarch64_max_initfn(Object *obj)
         t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
         cpu->isar.id_aa64pfr0 = t;
 
+        t = cpu->isar.id_aa64pfr1;
+        t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
+        cpu->isar.id_aa64pfr1 = t;
+
         t = cpu->isar.id_aa64mmfr1;
         t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */
         t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);
-- 
2.17.2

[Qemu-devel] [PATCH v2 11/12] linux-user/aarch64: Reset btype for syscalls and signals

[Richard Henderson]

The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
so we need to make sure that the value is 0 before clone,
fork, or syscall return.

The value of btype for signals is defined, but it does not make
sense for a SIGILL handler to enter with the btype set as for
the indirect branch that caused the SIGILL.

Clearing the value early means that btype is zero within the pstate
saved into the signal frame, and so is also zero on (normal) signal
return, but also allows the signal handler to adjust the value as
seen after the sigcontext restore.

This last is a guess at a future kernel's user-space ABI.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/cpu_loop.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 65d815f030..51ea9961ba 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -83,8 +83,19 @@ void cpu_loop(CPUARMState *env)
         cpu_exec_end(cs);
         process_queued_cpu_work(cs);
 
+        /*
+         * The state of BTYPE on syscall and interrupt entry is CONSTRAINED
+         * UNPREDICTABLE.  The real kernel will need to tidy this up as well.
+         * Do this before syscalls and signals, so that the value is correct
+         * both within signal handlers, and on return from syscall (especially
+         * clone & fork) and from signal handlers.
+         *
+         * The SIGILL signal handler, for BTITrap, can see the failing BTYPE
+         * within the ESR value in the signal frame.
+         */
         switch (trapnr) {
         case EXCP_SWI:
+            env->btype = 0;
             ret = do_syscall(env,
                              env->xregs[8],
                              env->xregs[0],
@@ -104,6 +115,7 @@ void cpu_loop(CPUARMState *env)
             /* just indicate that signals should be handled asap */
             break;
         case EXCP_UDEF:
+            env->btype = 0;
             info.si_signo = TARGET_SIGILL;
             info.si_errno = 0;
             info.si_code = TARGET_ILL_ILLOPN;
@@ -112,6 +124,7 @@ void cpu_loop(CPUARMState *env)
             break;
         case EXCP_PREFETCH_ABORT:
         case EXCP_DATA_ABORT:
+            env->btype = 0;
             info.si_signo = TARGET_SIGSEGV;
             info.si_errno = 0;
             /* XXX: check env->error_code */
@@ -121,12 +134,14 @@ void cpu_loop(CPUARMState *env)
             break;
         case EXCP_DEBUG:
         case EXCP_BKPT:
+            env->btype = 0;
             info.si_signo = TARGET_SIGTRAP;
             info.si_errno = 0;
             info.si_code = TARGET_TRAP_BRKPT;
             queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
             break;
         case EXCP_SEMIHOST:
+            env->btype = 0;
             env->xregs[0] = do_arm_semihosting(env);
             break;
         case EXCP_YIELD:
-- 
2.17.2

[Qemu-devel] [PATCH v2 12/12] tests/tcg/aarch64: Add bti smoke test

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/aarch64/bti-1.c         | 61 +++++++++++++++++++++++++++++++
 tests/tcg/aarch64/bti-crt.inc.c   | 51 ++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  7 +++-
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/bti-1.c
 create mode 100644 tests/tcg/aarch64/bti-crt.inc.c

diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c
new file mode 100644
index 0000000000..fa8a521a47
--- /dev/null
+++ b/tests/tcg/aarch64/bti-1.c
@@ -0,0 +1,61 @@
+/*
+ * Branch target identification, basic notskip cases.
+ */
+
+#include "bti-crt.inc.c"
+
+static void skip2_sigill(int sig, siginfo_t *info, ucontext_t *uc)
+{
+    uc->uc_mcontext.pc += 8;
+}
+
+#define NOP       "nop"
+#define BTI_N     "hint #32"
+#define BTI_C     "hint #34"
+#define BTI_J     "hint #36"
+#define BTI_JC    "hint #38"
+
+#define BTYPE_1(DEST) \
+    asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x16")
+
+#define BTYPE_2(DEST) \
+    asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x16", "x30")
+
+#define BTYPE_3(DEST) \
+    asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x15")
+
+#define TEST(WHICH, DEST, EXPECT) \
+    do { WHICH(DEST); fail += skipped ^ EXPECT; } while (0)
+
+
+int main()
+{
+    int fail = 0;
+    int skipped;
+
+    /* Signal-like with SA_SIGINFO.  */
+    signal_info(SIGILL, skip2_sigill);
+
+    TEST(BTYPE_1, NOP, 1);
+    TEST(BTYPE_1, BTI_N, 1);
+    TEST(BTYPE_1, BTI_C, 0);
+    TEST(BTYPE_1, BTI_J, 0);
+    TEST(BTYPE_1, BTI_JC, 0);
+
+    TEST(BTYPE_2, NOP, 1);
+    TEST(BTYPE_2, BTI_N, 1);
+    TEST(BTYPE_2, BTI_C, 0);
+    TEST(BTYPE_2, BTI_J, 1);
+    TEST(BTYPE_2, BTI_JC, 0);
+
+    TEST(BTYPE_3, NOP, 1);
+    TEST(BTYPE_3, BTI_N, 1);
+    TEST(BTYPE_3, BTI_C, 1);
+    TEST(BTYPE_3, BTI_J, 0);
+    TEST(BTYPE_3, BTI_JC, 0);
+
+    return fail;
+}
diff --git a/tests/tcg/aarch64/bti-crt.inc.c b/tests/tcg/aarch64/bti-crt.inc.c
new file mode 100644
index 0000000000..ef7831ad76
--- /dev/null
+++ b/tests/tcg/aarch64/bti-crt.inc.c
@@ -0,0 +1,51 @@
+/*
+ * Minimal user-environment for testing BTI.
+ *
+ * Normal libc is not built with BTI support enabled, and so could
+ * generate a BTI TRAP before ever reaching main.
+ */
+
+#include <stdlib.h>
+#include <signal.h>
+#include <ucontext.h>
+#include <asm/unistd.h>
+
+int main(void);
+
+void _start(void)
+{
+    exit(main());
+}
+
+void exit(int ret)
+{
+    register int x0 __asm__("x0") = ret;
+    register int x8 __asm__("x8") = __NR_exit;
+
+    asm volatile("svc #0" : : "r"(x0), "r"(x8));
+    __builtin_unreachable();
+}
+
+/*
+ * Irritatingly, the user API struct sigaction does not match the
+ * kernel API struct sigaction.  So for simplicity, isolate the
+ * kernel ABI here, and make this act like signal.
+ */
+void signal_info(int sig, void (*fn)(int, siginfo_t *, ucontext_t *))
+{
+    struct kernel_sigaction {
+        void (*handler)(int, siginfo_t *, ucontext_t *);
+        unsigned long flags;
+        unsigned long restorer;
+        unsigned long mask;
+    } sa = { fn, SA_SIGINFO, 0, 0 };
+
+    register int x0 __asm__("x0") = sig;
+    register void *x1 __asm__("x1") = &sa;
+    register void *x2 __asm__("x2") = 0;
+    register int x3 __asm__("x3") = sizeof(unsigned long);
+    register int x8 __asm__("x8") = __NR_rt_sigaction;
+
+    asm volatile("svc #0"
+                 : : "r"(x0), "r"(x1), "r"(x2), "r"(x3), "r"(x8) : "memory");
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 08c45b8470..b529a8c9c9 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -8,10 +8,15 @@ VPATH 		+= $(AARCH64_SRC)
 # we don't build any of the ARM tests
 AARCH64_TESTS=$(filter-out $(ARM_TESTS), $(TESTS))
 AARCH64_TESTS+=fcvt
-TESTS:=$(AARCH64_TESTS)
 
 fcvt: LDFLAGS+=-lm
 
 run-fcvt: fcvt
 	$(call run-test,$<,$(QEMU) $<, "$< on $(TARGET_NAME)")
 	$(call diff-out,$<,$(AARCH64_SRC)/fcvt.ref)
+
+AARCH64_TESTS += bti-1
+bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
+run-bti-1: QEMU += -cpu max,x-guarded-pages=on
+
+TESTS:=$(AARCH64_TESTS)
-- 
2.17.2

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128223118.5255-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI
Type: series
Message-id: 20190128223118.5255-1-richard.henderson@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
18c0b24bcd tests/tcg/aarch64: Add bti smoke test
d5315467cf linux-user/aarch64: Reset btype for syscalls and signals
ba139e0144 target/arm: Enable BTI for -cpu max
1a387cb4b4 target/arm: Add x-guarded-pages cpu property for user-only
b3fbe9ad27 target/arm: Set btype for indirect branches
905ade3305 target/arm: Reset btype for direct branches
57cafb41c4 target/arm: Default handling of BTYPE during translation
1c6561c9a8 target/arm: Cache the GP bit for a page in MemTxAttrs
9ee071294c exec: Add target-specific tlb bits to MemTxAttrs
79116bebaa target/arm: Add BT and BTYPE to tb->flags
6fd0b311a5 target/arm: Add PSTATE.BTYPE
59b3d084bc target/arm: Introduce isar_feature_aa64_bti

=== OUTPUT BEGIN ===
1/12 Checking commit 59b3d084bc01 (target/arm: Introduce isar_feature_aa64_bti)
2/12 Checking commit 6fd0b311a5a6 (target/arm: Add PSTATE.BTYPE)
3/12 Checking commit 79116bebaace (target/arm: Add BT and BTYPE to tb->flags)
4/12 Checking commit 9ee071294c1b (exec: Add target-specific tlb bits to MemTxAttrs)
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: include/exec/memattrs.h:47:
+    unsigned int target_tlb_bit0 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: include/exec/memattrs.h:48:
+    unsigned int target_tlb_bit1 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: include/exec/memattrs.h:49:
+    unsigned int target_tlb_bit2 : 1;
                                  ^

total: 3 errors, 0 warnings, 16 lines checked

Patch 4/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/12 Checking commit 1c6561c9a819 (target/arm: Cache the GP bit for a page in MemTxAttrs)
6/12 Checking commit 57cafb41c482 (target/arm: Default handling of BTYPE during translation)
WARNING: Block comments use a leading /* on a separate line
#70: FILE: target/arm/translate-a64.c:13730:
+/**

ERROR: return is not a function, parentheses are not required
#97: FILE: target/arm/translate-a64.c:13757:
+    return (tlb_hit(entry->addr_code, addr) &&

WARNING: Block comments use a leading /* on a separate line
#102: FILE: target/arm/translate-a64.c:13762:
+/**

total: 1 errors, 2 warnings, 196 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/12 Checking commit 905ade330525 (target/arm: Reset btype for direct branches)
8/12 Checking commit b3fbe9ad27be (target/arm: Set btype for indirect branches)
9/12 Checking commit 1a387cb4b4bf (target/arm: Add x-guarded-pages cpu property for user-only)
10/12 Checking commit ba139e01449c (target/arm: Enable BTI for -cpu max)
11/12 Checking commit d5315467cf35 (linux-user/aarch64: Reset btype for syscalls and signals)
12/12 Checking commit 18c0b24bcd26 (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#116: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 128 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190128223118.5255-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128223118.5255-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI
Type: series
Message-id: 20190128223118.5255-1-richard.henderson@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190128223118.5255-1-richard.henderson@linaro.org -> patchew/20190128223118.5255-1-richard.henderson@linaro.org
Switched to a new branch 'test'
c6a612538e tests/tcg/aarch64: Add bti smoke test
d2aa2b0d2c linux-user/aarch64: Reset btype for syscalls and signals
b986e6c3f5 target/arm: Enable BTI for -cpu max
e1caea7ca5 target/arm: Add x-guarded-pages cpu property for user-only
4f7a8dd8bb target/arm: Set btype for indirect branches
3c57957c9f target/arm: Reset btype for direct branches
7ea1157e0b target/arm: Default handling of BTYPE during translation
6ac89e22c2 target/arm: Cache the GP bit for a page in MemTxAttrs
09f79d5766 exec: Add target-specific tlb bits to MemTxAttrs
9ad3acceed target/arm: Add BT and BTYPE to tb->flags
920eae0614 target/arm: Add PSTATE.BTYPE
feb08a6fee target/arm: Introduce isar_feature_aa64_bti

=== OUTPUT BEGIN ===
1/12 Checking commit feb08a6fee4a (target/arm: Introduce isar_feature_aa64_bti)
2/12 Checking commit 920eae0614cb (target/arm: Add PSTATE.BTYPE)
3/12 Checking commit 9ad3acceedf5 (target/arm: Add BT and BTYPE to tb->flags)
4/12 Checking commit 09f79d576620 (exec: Add target-specific tlb bits to MemTxAttrs)
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: include/exec/memattrs.h:47:
+    unsigned int target_tlb_bit0 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: include/exec/memattrs.h:48:
+    unsigned int target_tlb_bit1 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: include/exec/memattrs.h:49:
+    unsigned int target_tlb_bit2 : 1;
                                  ^

total: 3 errors, 0 warnings, 16 lines checked

Patch 4/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/12 Checking commit 6ac89e22c243 (target/arm: Cache the GP bit for a page in MemTxAttrs)
6/12 Checking commit 7ea1157e0b93 (target/arm: Default handling of BTYPE during translation)
ERROR: return is not a function, parentheses are not required
#97: FILE: target/arm/translate-a64.c:13757:
+    return (tlb_hit(entry->addr_code, addr) &&

total: 1 errors, 0 warnings, 196 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/12 Checking commit 3c57957c9f30 (target/arm: Reset btype for direct branches)
8/12 Checking commit 4f7a8dd8bbd3 (target/arm: Set btype for indirect branches)
9/12 Checking commit e1caea7ca5ad (target/arm: Add x-guarded-pages cpu property for user-only)
10/12 Checking commit b986e6c3f551 (target/arm: Enable BTI for -cpu max)
11/12 Checking commit d2aa2b0d2cec (linux-user/aarch64: Reset btype for syscalls and signals)
12/12 Checking commit c6a612538e4c (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#116: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 128 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190128223118.5255-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128223118.5255-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190128223118.5255-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20190128223118.5255-1-richard.henderson@linaro.org -> patchew/20190128223118.5255-1-richard.henderson@linaro.org
Switched to a new branch 'test'
c6a6125 tests/tcg/aarch64: Add bti smoke test
d2aa2b0 linux-user/aarch64: Reset btype for syscalls and signals
b986e6c target/arm: Enable BTI for -cpu max
e1caea7 target/arm: Add x-guarded-pages cpu property for user-only
4f7a8dd target/arm: Set btype for indirect branches
3c57957 target/arm: Reset btype for direct branches
7ea1157 target/arm: Default handling of BTYPE during translation
6ac89e2 target/arm: Cache the GP bit for a page in MemTxAttrs
09f79d5 exec: Add target-specific tlb bits to MemTxAttrs
9ad3acc target/arm: Add BT and BTYPE to tb->flags
920eae0 target/arm: Add PSTATE.BTYPE
feb08a6 target/arm: Introduce isar_feature_aa64_bti

=== OUTPUT BEGIN ===
1/12 Checking commit feb08a6fee4a (target/arm: Introduce isar_feature_aa64_bti)
2/12 Checking commit 920eae0614cb (target/arm: Add PSTATE.BTYPE)
3/12 Checking commit 9ad3acceedf5 (target/arm: Add BT and BTYPE to tb->flags)
4/12 Checking commit 09f79d576620 (exec: Add target-specific tlb bits to MemTxAttrs)
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: include/exec/memattrs.h:47:
+    unsigned int target_tlb_bit0 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: include/exec/memattrs.h:48:
+    unsigned int target_tlb_bit1 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: include/exec/memattrs.h:49:
+    unsigned int target_tlb_bit2 : 1;
                                  ^

total: 3 errors, 0 warnings, 16 lines checked

Patch 4/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/12 Checking commit 6ac89e22c243 (target/arm: Cache the GP bit for a page in MemTxAttrs)
6/12 Checking commit 7ea1157e0b93 (target/arm: Default handling of BTYPE during translation)
ERROR: return is not a function, parentheses are not required
#97: FILE: target/arm/translate-a64.c:13757:
+    return (tlb_hit(entry->addr_code, addr) &&

total: 1 errors, 0 warnings, 196 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/12 Checking commit 3c57957c9f30 (target/arm: Reset btype for direct branches)
8/12 Checking commit 4f7a8dd8bbd3 (target/arm: Set btype for indirect branches)
9/12 Checking commit e1caea7ca5ad (target/arm: Add x-guarded-pages cpu property for user-only)
10/12 Checking commit b986e6c3f551 (target/arm: Enable BTI for -cpu max)
11/12 Checking commit d2aa2b0d2cec (linux-user/aarch64: Reset btype for syscalls and signals)
12/12 Checking commit c6a612538e4c (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#116: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 128 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190128223118.5255-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128223118.5255-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI
Type: series
Message-id: 20190128223118.5255-1-richard.henderson@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190128223118.5255-1-richard.henderson@linaro.org -> patchew/20190128223118.5255-1-richard.henderson@linaro.org
Switched to a new branch 'test'
edf251e560 tests/tcg/aarch64: Add bti smoke test
7a7646284f linux-user/aarch64: Reset btype for syscalls and signals
59d6dcfdca target/arm: Enable BTI for -cpu max
f7377fcdc1 target/arm: Add x-guarded-pages cpu property for user-only
23690ae82f target/arm: Set btype for indirect branches
10de2a2461 target/arm: Reset btype for direct branches
a12bad9666 target/arm: Default handling of BTYPE during translation
d0f08030ef target/arm: Cache the GP bit for a page in MemTxAttrs
b226128ce3 exec: Add target-specific tlb bits to MemTxAttrs
7eab305810 target/arm: Add BT and BTYPE to tb->flags
49f36a5c20 target/arm: Add PSTATE.BTYPE
ef5152464e target/arm: Introduce isar_feature_aa64_bti

=== OUTPUT BEGIN ===
1/12 Checking commit ef5152464e73 (target/arm: Introduce isar_feature_aa64_bti)
2/12 Checking commit 49f36a5c2086 (target/arm: Add PSTATE.BTYPE)
3/12 Checking commit 7eab30581026 (target/arm: Add BT and BTYPE to tb->flags)
4/12 Checking commit b226128ce320 (exec: Add target-specific tlb bits to MemTxAttrs)
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: include/exec/memattrs.h:47:
+    unsigned int target_tlb_bit0 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: include/exec/memattrs.h:48:
+    unsigned int target_tlb_bit1 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: include/exec/memattrs.h:49:
+    unsigned int target_tlb_bit2 : 1;
                                  ^

total: 3 errors, 0 warnings, 16 lines checked

Patch 4/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/12 Checking commit d0f08030ef1d (target/arm: Cache the GP bit for a page in MemTxAttrs)
6/12 Checking commit a12bad9666e5 (target/arm: Default handling of BTYPE during translation)
ERROR: return is not a function, parentheses are not required
#97: FILE: target/arm/translate-a64.c:13757:
+    return (tlb_hit(entry->addr_code, addr) &&

total: 1 errors, 0 warnings, 196 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/12 Checking commit 10de2a246194 (target/arm: Reset btype for direct branches)
8/12 Checking commit 23690ae82f20 (target/arm: Set btype for indirect branches)
9/12 Checking commit f7377fcdc121 (target/arm: Add x-guarded-pages cpu property for user-only)
10/12 Checking commit 59d6dcfdca71 (target/arm: Enable BTI for -cpu max)
11/12 Checking commit 7a7646284f29 (linux-user/aarch64: Reset btype for syscalls and signals)
12/12 Checking commit edf251e5609e (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#116: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 128 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190128223118.5255-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128223118.5255-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI
Message-id: 20190128223118.5255-1-richard.henderson@linaro.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20190128223118.5255-1-richard.henderson@linaro.org -> patchew/20190128223118.5255-1-richard.henderson@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 'd4e7d7ac663fcb55f1b93575445fcbca372f17a7'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out '9b7ab2fa020341dee8bf9df6c9cf40003e0136df'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out '441a84d3a642a10b948369c63f32367e8ff6395b'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
edf251e tests/tcg/aarch64: Add bti smoke test
7a76462 linux-user/aarch64: Reset btype for syscalls and signals
59d6dcf target/arm: Enable BTI for -cpu max
f7377fc target/arm: Add x-guarded-pages cpu property for user-only
23690ae target/arm: Set btype for indirect branches
10de2a2 target/arm: Reset btype for direct branches
a12bad9 target/arm: Default handling of BTYPE during translation
d0f0803 target/arm: Cache the GP bit for a page in MemTxAttrs
b226128 exec: Add target-specific tlb bits to MemTxAttrs
7eab305 target/arm: Add BT and BTYPE to tb->flags
49f36a5 target/arm: Add PSTATE.BTYPE
ef51524 target/arm: Introduce isar_feature_aa64_bti

=== OUTPUT BEGIN ===
1/12 Checking commit ef5152464e73 (target/arm: Introduce isar_feature_aa64_bti)
2/12 Checking commit 49f36a5c2086 (target/arm: Add PSTATE.BTYPE)
3/12 Checking commit 7eab30581026 (target/arm: Add BT and BTYPE to tb->flags)
4/12 Checking commit b226128ce320 (exec: Add target-specific tlb bits to MemTxAttrs)
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: include/exec/memattrs.h:47:
+    unsigned int target_tlb_bit0 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: include/exec/memattrs.h:48:
+    unsigned int target_tlb_bit1 : 1;
                                  ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: include/exec/memattrs.h:49:
+    unsigned int target_tlb_bit2 : 1;
                                  ^

total: 3 errors, 0 warnings, 16 lines checked

Patch 4/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/12 Checking commit d0f08030ef1d (target/arm: Cache the GP bit for a page in MemTxAttrs)
6/12 Checking commit a12bad9666e5 (target/arm: Default handling of BTYPE during translation)
ERROR: return is not a function, parentheses are not required
#97: FILE: target/arm/translate-a64.c:13757:
+    return (tlb_hit(entry->addr_code, addr) &&

total: 1 errors, 0 warnings, 196 lines checked

Patch 6/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/12 Checking commit 10de2a246194 (target/arm: Reset btype for direct branches)
8/12 Checking commit 23690ae82f20 (target/arm: Set btype for indirect branches)
9/12 Checking commit f7377fcdc121 (target/arm: Add x-guarded-pages cpu property for user-only)
10/12 Checking commit 59d6dcfdca71 (target/arm: Enable BTI for -cpu max)
11/12 Checking commit 7a7646284f29 (linux-user/aarch64: Reset btype for syscalls and signals)
12/12 Checking commit edf251e5609e (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#116: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 128 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190128223118.5255-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 04/12] exec: Add target-specific tlb bits to MemTxAttrs

[Peter Maydell]

On Mon, 28 Jan 2019 at 22:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These bits can be used to cache target-specific data in cputlb
> read from the page tables.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/memattrs.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> index d4a1642098..d4a3477d71 100644
> --- a/include/exec/memattrs.h
> +++ b/include/exec/memattrs.h
> @@ -37,6 +37,16 @@ typedef struct MemTxAttrs {
>      unsigned int user:1;
>      /* Requester ID (for MSI for example) */
>      unsigned int requester_id:16;
> +    /*
> +     * The following are target-specific page-table bits.  These are not
> +     * related to actual memory transactions at all.  However, this structure
> +     * is part of the tlb_fill interface, cached in the cputlb structure,
> +     * and has unused bits.  These fields will be read by target-specific
> +     * helpers using env->iotlb[mmu_idx][tlb_index()].attrs.target_tlb_bitN.
> +     */
> +    unsigned int target_tlb_bit0 : 1;
> +    unsigned int target_tlb_bit1 : 1;
> +    unsigned int target_tlb_bit2 : 1;
>  } MemTxAttrs;
>
>  /* Bus masters which don't specify any attributes will get this,

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 05/12] target/arm: Cache the GP bit for a page in MemTxAttrs

[Peter Maydell]

On Mon, 28 Jan 2019 at 22:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Caching the bit means that we will not have to re-walk the
> page tables to look up the bit during translation.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 6efe88a157..70277222da 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10457,6 +10457,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>      bool ttbr1_valid;
>      uint64_t descaddrmask;
>      bool aarch64 = arm_el_is_aa64(env, el);
> +    bool guarded = false;
>
>      /* TODO:
>       * This code does not handle the different format TCR for VTCR_EL2.
> @@ -10629,6 +10630,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          }
>          /* Merge in attributes from table descriptors */
>          attrs |= nstable << 3; /* NS */
> +        guarded |= extract64(descriptor, 50, 1);  /* GP */

Should just be "guarded =", as per previous discussion.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 07/12] target/arm: Reset btype for direct branches

[Peter Maydell]

On Mon, 28 Jan 2019 at 22:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is all of the non-exception cases of DISAS_NORETURN.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Do not reset byte for syscalls
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 05/12] target/arm: Cache the GP bit for a page in MemTxAttrs

[Richard Henderson]

On 2/4/19 11:41 AM, Peter Maydell wrote:
> On Mon, 28 Jan 2019 at 22:31, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Caching the bit means that we will not have to re-walk the
>> page tables to look up the bit during translation.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/arm/helper.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 6efe88a157..70277222da 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -10457,6 +10457,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>>      bool ttbr1_valid;
>>      uint64_t descaddrmask;
>>      bool aarch64 = arm_el_is_aa64(env, el);
>> +    bool guarded = false;
>>
>>      /* TODO:
>>       * This code does not handle the different format TCR for VTCR_EL2.
>> @@ -10629,6 +10630,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>>          }
>>          /* Merge in attributes from table descriptors */
>>          attrs |= nstable << 3; /* NS */
>> +        guarded |= extract64(descriptor, 50, 1);  /* GP */
> 
> Should just be "guarded =", as per previous discussion.
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Agreed.  Do you need a re-spin for this?


r~

Re: [Qemu-devel] [PATCH v2 11/12] linux-user/aarch64: Reset btype for syscalls and signals

[Peter Maydell]

On Mon, 28 Jan 2019 at 22:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
> so we need to make sure that the value is 0 before clone,
> fork, or syscall return.
>
> The value of btype for signals is defined, but it does not make
> sense for a SIGILL handler to enter with the btype set as for
> the indirect branch that caused the SIGILL.
>
> Clearing the value early means that btype is zero within the pstate
> saved into the signal frame, and so is also zero on (normal) signal
> return, but also allows the signal handler to adjust the value as
> seen after the sigcontext restore.
>
> This last is a guess at a future kernel's user-space ABI.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/cpu_loop.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 65d815f030..51ea9961ba 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -83,8 +83,19 @@ void cpu_loop(CPUARMState *env)
>          cpu_exec_end(cs);
>          process_queued_cpu_work(cs);
>
> +        /*
> +         * The state of BTYPE on syscall and interrupt entry is CONSTRAINED
> +         * UNPREDICTABLE.  The real kernel will need to tidy this up as well.
> +         * Do this before syscalls and signals, so that the value is correct
> +         * both within signal handlers, and on return from syscall (especially
> +         * clone & fork) and from signal handlers.
> +         *
> +         * The SIGILL signal handler, for BTITrap, can see the failing BTYPE
> +         * within the ESR value in the signal frame.
> +         */
>          switch (trapnr) {
>          case EXCP_SWI:
> +            env->btype = 0;
>              ret = do_syscall(env,
>                               env->xregs[8],
>                               env->xregs[0],

If the idea is to give a particular value on return from
the syscall and on entry to a signal handler, shouldn't we be
setting it after the do_syscall() call returns, and in the
signal handler entry path ?

> @@ -104,6 +115,7 @@ void cpu_loop(CPUARMState *env)
>              /* just indicate that signals should be handled asap */
>              break;
>          case EXCP_UDEF:
> +            env->btype = 0;
>              info.si_signo = TARGET_SIGILL;
>              info.si_errno = 0;
>              info.si_code = TARGET_ILL_ILLOPN;
> @@ -112,6 +124,7 @@ void cpu_loop(CPUARMState *env)
>              break;
>          case EXCP_PREFETCH_ABORT:
>          case EXCP_DATA_ABORT:
> +            env->btype = 0;
>              info.si_signo = TARGET_SIGSEGV;
>              info.si_errno = 0;
>              /* XXX: check env->error_code */

> @@ -121,12 +134,14 @@ void cpu_loop(CPUARMState *env)
>              break;
>          case EXCP_DEBUG:
>          case EXCP_BKPT:
> +            env->btype = 0;
>              info.si_signo = TARGET_SIGTRAP;
>              info.si_errno = 0;
>              info.si_code = TARGET_TRAP_BRKPT;
>              queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
>              break;
>          case EXCP_SEMIHOST:
> +            env->btype = 0;

Leaving btype alone rather than clearing it here would be
consistent with how we handle semihosting in system emulation,
right ?

>              env->xregs[0] = do_arm_semihosting(env);
>              break;
>          case EXCP_YIELD:
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 11/12] linux-user/aarch64: Reset btype for syscalls and signals

[Richard Henderson]

On 2/4/19 12:02 PM, Peter Maydell wrote:
> On Mon, 28 Jan 2019 at 22:31, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
>> so we need to make sure that the value is 0 before clone,
>> fork, or syscall return.
>>
>> The value of btype for signals is defined, but it does not make
>> sense for a SIGILL handler to enter with the btype set as for
>> the indirect branch that caused the SIGILL.
>>
>> Clearing the value early means that btype is zero within the pstate
>> saved into the signal frame, and so is also zero on (normal) signal
>> return, but also allows the signal handler to adjust the value as
>> seen after the sigcontext restore.
>>
>> This last is a guess at a future kernel's user-space ABI.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  linux-user/aarch64/cpu_loop.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
>> index 65d815f030..51ea9961ba 100644
>> --- a/linux-user/aarch64/cpu_loop.c
>> +++ b/linux-user/aarch64/cpu_loop.c
>> @@ -83,8 +83,19 @@ void cpu_loop(CPUARMState *env)
>>          cpu_exec_end(cs);
>>          process_queued_cpu_work(cs);
>>
>> +        /*
>> +         * The state of BTYPE on syscall and interrupt entry is CONSTRAINED
>> +         * UNPREDICTABLE.  The real kernel will need to tidy this up as well.
>> +         * Do this before syscalls and signals, so that the value is correct
>> +         * both within signal handlers, and on return from syscall (especially
>> +         * clone & fork) and from signal handlers.
>> +         *
>> +         * The SIGILL signal handler, for BTITrap, can see the failing BTYPE
>> +         * within the ESR value in the signal frame.
>> +         */
>>          switch (trapnr) {
>>          case EXCP_SWI:
>> +            env->btype = 0;
>>              ret = do_syscall(env,
>>                               env->xregs[8],
>>                               env->xregs[0],
> 
> If the idea is to give a particular value on return from
> the syscall and on entry to a signal handler, shouldn't we be
> setting it after the do_syscall() call returns, and in the
> signal handler entry path ?
> 
>> @@ -104,6 +115,7 @@ void cpu_loop(CPUARMState *env)
>>              /* just indicate that signals should be handled asap */
>>              break;
>>          case EXCP_UDEF:
>> +            env->btype = 0;
>>              info.si_signo = TARGET_SIGILL;
>>              info.si_errno = 0;
>>              info.si_code = TARGET_ILL_ILLOPN;
>> @@ -112,6 +124,7 @@ void cpu_loop(CPUARMState *env)
>>              break;
>>          case EXCP_PREFETCH_ABORT:
>>          case EXCP_DATA_ABORT:
>> +            env->btype = 0;
>>              info.si_signo = TARGET_SIGSEGV;
>>              info.si_errno = 0;
>>              /* XXX: check env->error_code */
> 
>> @@ -121,12 +134,14 @@ void cpu_loop(CPUARMState *env)
>>              break;
>>          case EXCP_DEBUG:
>>          case EXCP_BKPT:
>> +            env->btype = 0;
>>              info.si_signo = TARGET_SIGTRAP;
>>              info.si_errno = 0;
>>              info.si_code = TARGET_TRAP_BRKPT;
>>              queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
>>              break;
>>          case EXCP_SEMIHOST:
>> +            env->btype = 0;
> 
> Leaving btype alone rather than clearing it here would be
> consistent with how we handle semihosting in system emulation,
> right ?

Er.. yes.  I sort of forgot we had semi-hosting for aa64.


r~

Re: [Qemu-devel] [PATCH v2 00/12] target/arm: Implement ARMv8.5-BTI

[Peter Maydell]

On Mon, 28 Jan 2019 at 22:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The branch target identification extension does not yet have
> kernel or gcc support.  It should be in shape for developing
> those though.
>
> In system mode I do honor the GP bit from the page tables.
> In user-only mode, there is a way to pretend that the GP bit
> is always set.  Further linux-user changes will have to track
> the ABI that gets developed.
>
> In the meantime, provide a statically linable make check-tcg
> test for the functionality.
>
> Changes since v2:
>   * Rebase on master.
>   * Incorporate feedback from Peter.

I've applied patches 1-8 and 10 to target-arm.next, which gives
us the system emulation part. The linux-user support I've given
some review comments on.

thanks
-- PMM