Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 May 2023 16:49:45 +0100
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        Kyle Evans <kevans@FreeBSD.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: 4b500174dd2d - main - arm64: emulate swp/swpb instructions
Message-ID:  <B6F0D20E-7A0A-41B5-878E-D22DF15FED99@freebsd.org>
In-Reply-To: <202305151542.34FFgN7D071754@gitrepo.freebsd.org>
References:  <202305151542.34FFgN7D071754@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 15 May 2023, at 16:42, Kyle Evans <kevans@FreeBSD.org> wrote:
>=20
> The branch main has been updated by kevans:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D4b500174dd2d1e16dcb87c22364f7244=
72c82edc
>=20
> commit 4b500174dd2d1e16dcb87c22364f724472c82edc
> Author:     Kyle Evans <kevans@FreeBSD.org>
> AuthorDate: 2023-05-15 15:42:10 +0000
> Commit:     Kyle Evans <kevans@FreeBSD.org>
> CommitDate: 2023-05-15 15:42:10 +0000
>=20
>    arm64: emulate swp/swpb instructions
>=20
>    Add another undefined instruction handler for compat32 and watch =
out for
>    SWP/SWPB instructions.
>=20
>    SWP/SWPB were deprecated in ARMv6 and declared obsolete in ARMv7, =
but
>    this implementation is motivated by some proprietary software that =
still
>    uses SWP/SWPB. Because it's deprecated, emulation is pushed back =
behind
>    a sysctl that defaults to OFF in GENERIC so that it doesn't =
potentially
>    adversely affect package builds; it's unknown whether software may =
test
>    for a functional swp/swpb instruction with the desire of using it =
later,
>    so we err on the side of caution to ensure we don't end up with =
swp/swpb
>    use in freebsd/arm packages (which are built on aarch64).
>=20
>    The EMUL_SWP config option may be used to enable emulation by =
default in
>    environments where emulation is desired and won't really be turned =
off.
>=20
>    Reviewed by:    andrew, mmel (both earlier version)
>    Sponsored by:   Stormshield
>    Sponsored by:   Klara, Inc.
>    Differential Revision:  https://reviews.freebsd.org/D39667
> ---
> sys/arm64/arm64/freebsd32_machdep.c |   4 +
> sys/arm64/arm64/undefined.c         | 162 =
++++++++++++++++++++++++++++++++++++
> sys/conf/options.arm64              |   2 +
> 3 files changed, 168 insertions(+)
>=20
> diff --git a/sys/arm64/arm64/freebsd32_machdep.c =
b/sys/arm64/arm64/freebsd32_machdep.c
> index 5fadef74df87..44f35f1b2abf 100644
> --- a/sys/arm64/arm64/freebsd32_machdep.c
> +++ b/sys/arm64/arm64/freebsd32_machdep.c
> @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
> #include <sys/mutex.h>
> #include <sys/syscallsubr.h>
> #include <sys/ktr.h>
> +#include <sys/sysctl.h>
> #include <sys/sysent.h>
> #include <sys/sysproto.h>
> #include <machine/armreg.h>
> @@ -55,6 +56,9 @@ _Static_assert(sizeof(struct siginfo32) =3D=3D 64, =
"struct siginfo32 size incorrect"
>=20
> extern void freebsd32_sendsig(sig_t catcher, ksiginfo_t *ksi, sigset_t =
*mask);
>=20
> +SYSCTL_NODE(_compat, OID_AUTO, arm, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
> +    "32-bit mode");
> +
> /*
>  * The first two fields of a ucontext_t are the signal mask and the =
machine
>  * context.  The next field is uc_link; we want to avoid destroying =
the link
> diff --git a/sys/arm64/arm64/undefined.c b/sys/arm64/arm64/undefined.c
> index 1feb242db060..7f436aaef6e5 100644
> --- a/sys/arm64/arm64/undefined.c
> +++ b/sys/arm64/arm64/undefined.c
> @@ -39,14 +39,47 @@ __FBSDID("$FreeBSD$");
> #include <sys/queue.h>
> #include <sys/signal.h>
> #include <sys/signalvar.h>
> +#include <sys/sysctl.h>
> #include <sys/sysent.h>
>=20
> +#include <machine/atomic.h>
> #include <machine/frame.h>
> +#define _MD_WANT_SWAPWORD
> +#include <machine/md_var.h>
> +#include <machine/pcb.h>
> #include <machine/undefined.h>
> #include <machine/vmparam.h>
>=20
> +#include <vm/vm.h>
> +#include <vm/vm_extern.h>
> +
> +/* Low bit masked off */
> +#define INSN_COND(insn) ((insn >> 28) & ~0x1)
> +#define INSN_COND_INVERTED(insn) ((insn >> 28) & 0x1)
> +#define INSN_COND_EQ 0x00 /* NE */
> +#define INSN_COND_CS 0x02 /* CC */
> +#define INSN_COND_MI 0x04 /* PL */
> +#define INSN_COND_VS 0x06 /* VC */
> +#define INSN_COND_HI 0x08 /* LS */
> +#define INSN_COND_GE 0x0a /* LT */
> +#define INSN_COND_GT 0x0c /* LE */
> +#define INSN_COND_AL 0x0e /* Always */
> +
> MALLOC_DEFINE(M_UNDEF, "undefhandler", "Undefined instruction handler =
data");
>=20
> +#ifdef COMPAT_FREEBSD32
> +#ifndef EMUL_SWP
> +#define EMUL_SWP 0
> +#endif
> +
> +SYSCTL_DECL(_compat_arm);
> +
> +static bool compat32_emul_swp =3D EMUL_SWP;
> +SYSCTL_BOOL(_compat_arm, OID_AUTO, emul_swp,
> +    CTLFLAG_RWTUN | CTLFLAG_MPSAFE, &compat32_emul_swp, 0,
> +    "Enable SWP/SWPB emulation");
> +#endif
> +
> struct undef_handler {
> LIST_ENTRY(undef_handler) uh_link;
> undef_handler_t uh_handler;
> @@ -88,6 +121,54 @@ id_aa64mmfr2_handler(vm_offset_t va, uint32_t =
insn, struct trapframe *frame,
> return (0);
> }
>=20
> +static bool
> +arm_cond_match(uint32_t insn, struct trapframe *frame)
> +{
> + uint64_t spsr;
> + uint32_t cond;
> + bool invert;
> + bool match;
> +
> + /*
> + * Generally based on the function of the same name in NetBSD, though
> + * condition bits left in their original position rather than =
shifting
> + * over the low bit that indicates inversion for quicker sanity =
checking
> + * against spec.
> + */
> + spsr =3D frame->tf_spsr;
> + cond =3D INSN_COND(insn);
> + invert =3D INSN_COND_INVERTED(insn);
> +
> + switch (cond) {
> + case INSN_COND_EQ:
> + match =3D (spsr & PSR_Z) !=3D 0;
> + break;
> + case INSN_COND_CS:
> + match =3D (spsr & PSR_C) !=3D 0;
> + break;
> + case INSN_COND_MI:
> + match =3D (spsr & PSR_N) !=3D 0;
> + break;
> + case INSN_COND_VS:
> + match =3D (spsr & PSR_V) !=3D 0;
> + break;
> + case INSN_COND_HI:
> + match =3D (spsr & (PSR_C | PSR_Z)) =3D=3D PSR_C;
> + break;
> + case INSN_COND_GE:
> + match =3D (!(spsr & PSR_N) =3D=3D !(spsr & PSR_V));
> + break;
> + case INSN_COND_GT:
> + match =3D !(spsr & PSR_Z) && (!(spsr & PSR_N) =3D=3D !(spsr & =
PSR_V));
> + break;
> + case INSN_COND_AL:
> + match =3D true;
> + break;
> + }
> +
> + return (!match !=3D !invert);

This is equivalent to match !=3D invert?

Jess

> +}
> +
> #ifdef COMPAT_FREEBSD32
> /* arm32 GDB breakpoints */
> #define GDB_BREAKPOINT 0xe6000011
> @@ -113,6 +194,86 @@ gdb_trapper(vm_offset_t va, uint32_t insn, struct =
trapframe *frame,
> }
> return 0;
> }
> +
> +static int
> +swp_emulate(vm_offset_t va, uint32_t insn, struct trapframe *frame,
> +    uint32_t esr)
> +{
> + ksiginfo_t ksi;
> + struct thread *td;
> + vm_offset_t vaddr;
> + uint64_t *regs;
> + uint32_t val;
> + int attempts, error, Rn, Rd, Rm;
> + bool is_swpb;
> +
> + td =3D curthread;
> +
> + /*
> + * swp, swpb only; there are no Thumb swp/swpb instructions so we can
> + * safely bail out if we're in Thumb mode.
> + */
> + if (!compat32_emul_swp || !SV_PROC_FLAG(td->td_proc, SV_ILP32) ||
> +    (frame->tf_spsr & PSR_T) !=3D 0)
> + return (0);
> + else if ((insn & 0x0fb00ff0) !=3D 0x01000090)
> + return (0);
> + else if (!arm_cond_match(insn, frame))
> + goto next; /* Handled, but does nothing */
> +
> + Rn =3D (insn & 0xf0000) >> 16;
> + Rd =3D (insn & 0xf000) >> 12;
> + Rm =3D (insn & 0xf);
> +
> + regs =3D frame->tf_x;
> + vaddr =3D regs[Rn] & 0xffffffff;
> + val =3D regs[Rm];
> +
> + /* Enforce alignment for swp. */
> + is_swpb =3D (insn & 0x00400000) !=3D 0;
> + if (!is_swpb && (vaddr & 3) !=3D 0)
> + goto fault;
> +
> + attempts =3D 0;
> +
> + do {
> + if (is_swpb) {
> + uint8_t bval;
> +
> + bval =3D val;
> + error =3D swapueword8((void *)vaddr, &bval);
> + val =3D bval;
> + } else {
> + error =3D swapueword32((void *)vaddr, &val);
> + }
> +
> + if (error =3D=3D -1)
> + goto fault;
> +
> + /*
> + * Avoid potential DoS, e.g., on CPUs that don't implement
> + * global monitors.
> + */
> + if (error !=3D 0 && (++attempts % 5) =3D=3D 0)
> + maybe_yield();
> + } while (error !=3D 0);
> +
> + regs[Rd] =3D val;
> +
> +next:
> + /* No thumb SWP/SWPB */
> + frame->tf_elr +=3D 4; //INSN_SIZE;
> +
> + return (1);
> +fault:
> + ksiginfo_init_trap(&ksi);
> + ksi.ksi_signo =3D SIGSEGV;
> + ksi.ksi_code =3D SEGV_MAPERR;
> + ksi.ksi_addr =3D (void *)va;
> + trapsignal(td, &ksi);
> +
> + return (1);
> +}
> #endif
>=20
> void
> @@ -125,6 +286,7 @@ undef_init(void)
> install_undef_handler(false, id_aa64mmfr2_handler);
> #ifdef COMPAT_FREEBSD32
> install_undef_handler(true, gdb_trapper);
> + install_undef_handler(true, swp_emulate);
> #endif
> }
>=20
> diff --git a/sys/conf/options.arm64 b/sys/conf/options.arm64
> index 26c7c87e49e2..0d2a5f177754 100644
> --- a/sys/conf/options.arm64
> +++ b/sys/conf/options.arm64
> @@ -14,6 +14,8 @@ PERTHREAD_SSP opt_global.h
>=20
> # Binary compatibility
> COMPAT_FREEBSD32 opt_global.h
> +# Emulate SWP/SWPB for COMPAT_FREEBSD32
> +EMUL_SWP opt_global.h
>=20
> # EFI Runtime services support
> EFIRT opt_efirt.h




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B6F0D20E-7A0A-41B5-878E-D22DF15FED99>