[PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
Ruan Jinjie
ruanjinjie at huawei.com
Fri Feb 3 02:08:08 PST 2023
On 2022/10/19 22:41, Mark Rutland wrote:
> Support for deprecated instructions can be enabled or disabled at
> runtime. To handle this, the code in armv8_deprecated.c registers and
> unregisters undef_hooks, and makes cross CPU calls to configure HW
> support. This is rather complicated, and the synchronization required to
> make this safe ends up serializing the handling of instructions which
> have been trapped.
>
> This patch simplifies the deprecated instruction handling by removing
> the dynamic registration and unregistration, and changing the trap
> handling code to determine whether a handler should be invoked. This
> removes the need for dynamic list management, and simplifies the locking
> requirements, making it possible to handle trapped instructions entirely
> in parallel.
>
> Where changing the emulation state requires a cross-call, this is
> serialized by locally disabling interrupts, ensuring that the CPU is not
> left in an inconsistent state.
>
> To simplify sysctl management, each insn_emulation is given a separate
> sysctl table, permitting these to be registered separately. The core
> sysctl code will iterate over all of these when walking sysfs.
>
> I've tested this with userspace programs which use each of the
> deprecated instructions, and I've concurrently modified the support
> level for each of the features back-and-forth between HW and emulated to
> check that there are no spurious SIGILLs sent to userspace when the
> support level is changed.
Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
double problem. However, I can not trigger the emulation of these
deprecated instructions in qemu with userspace program as below, but
cause IABT and DABT, so that I can not verify the correctness of the
patch.Can you give me some help to test the emulation of these
deprecated instructions?
#include<stdio.h>
#define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
: : "r" (0) : "memory")
#define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
: : "r" (0) : "memory")
#define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
: : "r" (0) : "memory")
int main(){
isb();
dsb();
dmb();
return 0;
}
>
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: James Morse <james.morse at arm.com>
> Cc: Joey Gouly <joey.gouly at arm.com>
> Cc: Peter Zijlstra <peterz at infradead.org>
> Cc: Will Deacon <will at kernel.org>
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> ---
> arch/arm64/include/asm/traps.h | 19 +-
> arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
> arch/arm64/kernel/traps.c | 40 +---
> 3 files changed, 149 insertions(+), 192 deletions(-)
>
> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
> index 6e5826470bea..1f361e2da516 100644
> --- a/arch/arm64/include/asm/traps.h
> +++ b/arch/arm64/include/asm/traps.h
> @@ -13,17 +13,16 @@
>
> struct pt_regs;
>
> -struct undef_hook {
> - struct list_head node;
> - u32 instr_mask;
> - u32 instr_val;
> - u64 pstate_mask;
> - u64 pstate_val;
> - int (*fn)(struct pt_regs *regs, u32 instr);
> -};
> +#ifdef CONFIG_ARMV8_DEPRECATED
> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
> +#else
> +static inline bool
> +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> +{
> + return false;
> +}
> +#endif /* CONFIG_ARMV8_DEPRECATED */
>
> -void register_undef_hook(struct undef_hook *hook);
> -void unregister_undef_hook(struct undef_hook *hook);
> void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
> void arm64_notify_segfault(unsigned long addr);
> void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 7f2ce49dbf97..ed0788cf6bbb 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -38,17 +38,24 @@ enum insn_emulation_mode {
> enum legacy_insn_status {
> INSN_DEPRECATED,
> INSN_OBSOLETE,
> + INSN_UNAVAILABLE,
> };
>
> struct insn_emulation {
> const char *name;
> - struct list_head node;
> enum legacy_insn_status status;
> - struct undef_hook *hooks;
> + bool (*try_emulate)(struct pt_regs *regs,
> + u32 insn);
> int (*set_hw_mode)(bool enable);
> +
> int current_mode;
> int min;
> int max;
> +
> + /*
> + * sysctl for this emulation + a sentinal entry.
> + */
> + struct ctl_table sysctl[2];
> };
>
> #define ARM_OPCODE_CONDTEST_FAIL 0
> @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
> return ARM_OPCODE_CONDTEST_UNCOND;
> }
>
> +#ifdef CONFIG_SWP_EMULATION
> /*
> * Implement emulation of the SWP/SWPB instructions using load-exclusive and
> * store-exclusive.
> @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
> return 0;
> }
>
> -/*
> - * Only emulate SWP/SWPB executed in ARM state/User mode.
> - * The kernel must be SWP free and SWP{B} does not exist in Thumb.
> - */
> -static struct undef_hook swp_hooks[] = {
> - {
> - .instr_mask = 0x0fb00ff0,
> - .instr_val = 0x01000090,
> - .pstate_mask = PSR_AA32_MODE_MASK,
> - .pstate_val = PSR_AA32_MODE_USR,
> - .fn = swp_handler
> - },
> - { }
> -};
> +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
> +{
> + /* SWP{B} only exists in ARM state and does not exist in Thumb */
> + if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> + return false;
> +
> + if ((insn & 0x0fb00ff0) != 0x01000090)
> + return false;
> +
> + return swp_handler(regs, insn) == 0;
> +}
>
> static struct insn_emulation insn_swp = {
> .name = "swp",
> .status = INSN_OBSOLETE,
> - .hooks = swp_hooks,
> + .try_emulate = try_emulate_swp,
> .set_hw_mode = NULL,
> };
> +#endif /* CONFIG_SWP_EMULATION */
>
> +#ifdef CONFIG_CP15_BARRIER_EMULATION
> static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
> {
> perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
> @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
> return 0;
> }
>
> -static struct undef_hook cp15_barrier_hooks[] = {
> - {
> - .instr_mask = 0x0fff0fdf,
> - .instr_val = 0x0e070f9a,
> - .pstate_mask = PSR_AA32_MODE_MASK,
> - .pstate_val = PSR_AA32_MODE_USR,
> - .fn = cp15barrier_handler,
> - },
> - {
> - .instr_mask = 0x0fff0fff,
> - .instr_val = 0x0e070f95,
> - .pstate_mask = PSR_AA32_MODE_MASK,
> - .pstate_val = PSR_AA32_MODE_USR,
> - .fn = cp15barrier_handler,
> - },
> - { }
> -};
> +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
> +{
> + if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> + return false;
> +
> + if ((insn & 0x0fff0fdf) == 0x0e070f9a)
> + return cp15barrier_handler(regs, insn) == 0;
> +
> + if ((insn & 0x0fff0fff) == 0x0e070f95)
> + return cp15barrier_handler(regs, insn) == 0;
> +
> + return false;
> +}
>
> static struct insn_emulation insn_cp15_barrier = {
> .name = "cp15_barrier",
> .status = INSN_DEPRECATED,
> - .hooks = cp15_barrier_hooks,
> + .try_emulate = try_emulate_cp15_barrier,
> .set_hw_mode = cp15_barrier_set_hw_mode,
> };
> +#endif /* CONFIG_CP15_BARRIER_EMULATION */
>
> +#ifdef CONFIG_SETEND_EMULATION
> static int setend_set_hw_mode(bool enable)
> {
> if (!cpu_supports_mixed_endian_el0())
> @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
> return rc;
> }
>
> -static struct undef_hook setend_hooks[] = {
> - {
> - .instr_mask = 0xfffffdff,
> - .instr_val = 0xf1010000,
> - .pstate_mask = PSR_AA32_MODE_MASK,
> - .pstate_val = PSR_AA32_MODE_USR,
> - .fn = a32_setend_handler,
> - },
> - {
> - /* Thumb mode */
> - .instr_mask = 0xfffffff7,
> - .instr_val = 0x0000b650,
> - .pstate_mask = (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
> - .pstate_val = (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
> - .fn = t16_setend_handler,
> - },
> - {}
> -};
> +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
> +{
> + if (compat_thumb_mode(regs) &&
> + (insn & 0xfffffff7) == 0x0000b650)
> + return t16_setend_handler(regs, insn) == 0;
> +
> + if (compat_user_mode(regs) &&
> + (insn & 0xfffffdff) == 0xf1010000)
> + return a32_setend_handler(regs, insn) == 0;
> +
> + return false;
> +}
>
> static struct insn_emulation insn_setend = {
> .name = "setend",
> .status = INSN_DEPRECATED,
> - .hooks = setend_hooks,
> + .try_emulate = try_emulate_setend,
> .set_hw_mode = setend_set_hw_mode,
> };
> +#endif /* CONFIG_SETEND_EMULATION */
> +
> +static struct insn_emulation *insn_emulations[] = {
> +#ifdef CONFIG_SWP_EMULATION
> + &insn_swp,
> +#endif
> +#ifdef CONFIG_CP15_BARRIER_EMULATION
> + &insn_cp15_barrier,
> +#endif
> +#ifdef CONFIG_SETEND_EMULATION
> + &insn_setend,
> +#endif
> +};
>
> -static LIST_HEAD(insn_emulation);
> -static int nr_insn_emulated __initdata;
> -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
> static DEFINE_MUTEX(insn_emulation_mutex);
>
> -static void register_emulation_hooks(struct insn_emulation *insn)
> -{
> - struct undef_hook *hook;
> -
> - BUG_ON(!insn->hooks);
> -
> - for (hook = insn->hooks; hook->instr_mask; hook++)
> - register_undef_hook(hook);
> -
> - pr_notice("Registered %s emulation handler\n", insn->name);
> -}
> -
> -static void remove_emulation_hooks(struct insn_emulation *insn)
> -{
> - struct undef_hook *hook;
> -
> - BUG_ON(!insn->hooks);
> -
> - for (hook = insn->hooks; hook->instr_mask; hook++)
> - unregister_undef_hook(hook);
> -
> - pr_notice("Removed %s emulation handler\n", insn->name);
> -}
> -
> static void enable_insn_hw_mode(void *data)
> {
> struct insn_emulation *insn = (struct insn_emulation *)data;
> @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
> {
> int rc = 0;
> unsigned long flags;
> - struct insn_emulation *insn;
>
> - raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> - list_for_each_entry(insn, &insn_emulation, node) {
> - bool enable = (insn->current_mode == INSN_HW);
> + /*
> + * Disable IRQs to serialize against an IPI from
> + * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
> + * recent enablement state if the two race with one another.
> + */
> + local_irq_save(flags);
> + for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> + struct insn_emulation *insn = insn_emulations[i];
> + bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
> if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
> pr_warn("CPU[%u] cannot support the emulation of %s",
> cpu, insn->name);
> rc = -EINVAL;
> }
> }
> - raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> + local_irq_restore(flags);
> +
> return rc;
> }
>
> @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> case INSN_UNDEF: /* Nothing to be done */
> break;
> case INSN_EMULATE:
> - remove_emulation_hooks(insn);
> break;
> case INSN_HW:
> if (!run_all_cpu_set_hw_mode(insn, false))
> @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> case INSN_UNDEF:
> break;
> case INSN_EMULATE:
> - register_emulation_hooks(insn);
> break;
> case INSN_HW:
> ret = run_all_cpu_set_hw_mode(insn, true);
> @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> return ret;
> }
>
> -static void __init register_insn_emulation(struct insn_emulation *insn)
> -{
> - unsigned long flags;
> -
> - insn->min = INSN_UNDEF;
> -
> - switch (insn->status) {
> - case INSN_DEPRECATED:
> - insn->current_mode = INSN_EMULATE;
> - /* Disable the HW mode if it was turned on at early boot time */
> - run_all_cpu_set_hw_mode(insn, false);
> - insn->max = INSN_HW;
> - break;
> - case INSN_OBSOLETE:
> - insn->current_mode = INSN_UNDEF;
> - insn->max = INSN_EMULATE;
> - break;
> - }
> -
> - raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> - list_add(&insn->node, &insn_emulation);
> - nr_insn_emulated++;
> - raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> -
> - /* Register any handlers if required */
> - update_insn_emulation_mode(insn, INSN_UNDEF);
> -}
> -
> static int emulation_proc_handler(struct ctl_table *table, int write,
> void *buffer, size_t *lenp,
> loff_t *ppos)
> @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> ret = update_insn_emulation_mode(insn, prev_mode);
> if (ret) {
> /* Mode change failed, revert to previous mode. */
> - insn->current_mode = prev_mode;
> + WRITE_ONCE(insn->current_mode, prev_mode);
> update_insn_emulation_mode(insn, INSN_UNDEF);
> }
> ret:
> @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> return ret;
> }
>
> -static void __init register_insn_emulation_sysctl(void)
> +static void __init register_insn_emulation(struct insn_emulation *insn)
> {
> - unsigned long flags;
> - int i = 0;
> - struct insn_emulation *insn;
> - struct ctl_table *insns_sysctl, *sysctl;
> + struct ctl_table *sysctl;
>
> - insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
> - GFP_KERNEL);
> - if (!insns_sysctl)
> - return;
> + insn->min = INSN_UNDEF;
>
> - raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> - list_for_each_entry(insn, &insn_emulation, node) {
> - sysctl = &insns_sysctl[i];
> + switch (insn->status) {
> + case INSN_DEPRECATED:
> + insn->current_mode = INSN_EMULATE;
> + /* Disable the HW mode if it was turned on at early boot time */
> + run_all_cpu_set_hw_mode(insn, false);
> + insn->max = INSN_HW;
> + break;
> + case INSN_OBSOLETE:
> + insn->current_mode = INSN_UNDEF;
> + insn->max = INSN_EMULATE;
> + break;
> + case INSN_UNAVAILABLE:
> + insn->current_mode = INSN_UNDEF;
> + insn->max = INSN_UNDEF;
> + break;
> + }
> +
> + /* Program the HW if required */
> + update_insn_emulation_mode(insn, INSN_UNDEF);
> +
> + if (insn->status != INSN_UNAVAILABLE) {
> + sysctl = &insn->sysctl[0];
>
> sysctl->mode = 0644;
> sysctl->maxlen = sizeof(int);
> @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
> sysctl->extra1 = &insn->min;
> sysctl->extra2 = &insn->max;
> sysctl->proc_handler = emulation_proc_handler;
> - i++;
> +
> + register_sysctl("abi", sysctl);
> + }
> +}
> +
> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> +{
> + for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> + struct insn_emulation *ie = insn_emulations[i];
> +
> + if (ie->status == INSN_UNAVAILABLE)
> + continue;
> +
> + /*
> + * A trap may race with the mode being changed
> + * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
> + * avoid a spurious UNDEF.
> + */
> + if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
> + continue;
> +
> + if (ie->try_emulate(regs, insn))
> + return true;
> }
> - raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>
> - register_sysctl("abi", insns_sysctl);
> + return false;
> }
>
> /*
> @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
> */
> static int __init armv8_deprecated_init(void)
> {
> - if (IS_ENABLED(CONFIG_SWP_EMULATION))
> - register_insn_emulation(&insn_swp);
> +#ifdef CONFIG_SETEND_EMULATION
> + if (!system_supports_mixed_endian_el0()) {
> + insn_setend.status = INSN_UNAVAILABLE;
> + pr_info("setend instruction emulation is not supported on this system\n");
> + }
>
> - if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
> - register_insn_emulation(&insn_cp15_barrier);
> +#endif
> + for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> + struct insn_emulation *ie = insn_emulations[i];
>
> - if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
> - if (system_supports_mixed_endian_el0())
> - register_insn_emulation(&insn_setend);
> - else
> - pr_info("setend instruction emulation is not supported on this system\n");
> + if (ie->status == INSN_UNAVAILABLE)
> + continue;
> +
> + register_insn_emulation(ie);
> }
>
> cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
> "arm64/isndep:starting",
> run_all_insn_set_hw_mode, NULL);
> - register_insn_emulation_sysctl();
> -
> return 0;
> }
>
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 96eaf1aaec12..4c0caa589e12 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
> regs->pstate &= ~PSR_BTYPE_MASK;
> }
>
> -static LIST_HEAD(undef_hook);
> -static DEFINE_RAW_SPINLOCK(undef_lock);
> -
> -void register_undef_hook(struct undef_hook *hook)
> -{
> - unsigned long flags;
> -
> - raw_spin_lock_irqsave(&undef_lock, flags);
> - list_add(&hook->node, &undef_hook);
> - raw_spin_unlock_irqrestore(&undef_lock, flags);
> -}
> -
> -void unregister_undef_hook(struct undef_hook *hook)
> -{
> - unsigned long flags;
> -
> - raw_spin_lock_irqsave(&undef_lock, flags);
> - list_del(&hook->node);
> - raw_spin_unlock_irqrestore(&undef_lock, flags);
> -}
> -
> static int user_insn_read(struct pt_regs *regs, u32 *insnp)
> {
> u32 instr;
> @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
> return 0;
> }
>
> -static int call_undef_hook(struct pt_regs *regs, u32 instr)
> -{
> - struct undef_hook *hook;
> - unsigned long flags;
> - int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
> -
> - raw_spin_lock_irqsave(&undef_lock, flags);
> - list_for_each_entry(hook, &undef_hook, node)
> - if ((instr & hook->instr_mask) == hook->instr_val &&
> - (regs->pstate & hook->pstate_mask) == hook->pstate_val)
> - fn = hook->fn;
> -
> - raw_spin_unlock_irqrestore(&undef_lock, flags);
> -
> - return fn ? fn(regs, instr) : 1;
> -}
> -
> void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
> {
> const char *desc;
> @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
> if (try_emulate_mrs(regs, insn))
> return;
>
> - if (call_undef_hook(regs, insn) == 0)
> + if (try_emulate_armv8_deprecated(regs, insn))
> return;
>
> out_err:
More information about the linux-arm-kernel
mailing list