[PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
Mark Rutland
mark.rutland at arm.com
Fri Feb 3 09:27:00 PST 2023
On Fri, Feb 03, 2023 at 06:08:08PM +0800, Ruan Jinjie wrote:
>
>
> 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,
Well that certainly sounds wrong!
When you say "in qemu", so you mean in a KVM VM, a TCG VM, or usermode
emulation?
Can you tell me exactly how you're invoking qemu?
Can you give an example of the IABT and DABT you see?
> 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")
FWIW, these encodings look correc to me.
Thanks,
Mark.
>
> 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:
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list