Date: Mon, 15 May 2023 10:59:04 -0500 From: Kyle Evans <kevans@freebsd.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Kyle Evans <kevans@freebsd.org>, "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: <CACNAnaFde1dky00b9pgBm7UBBizgLkSGnHBnbZ9Yq6OD0HsZqA@mail.gmail.com> In-Reply-To: <B6F0D20E-7A0A-41B5-878E-D22DF15FED99@freebsd.org> References: <202305151542.34FFgN7D071754@gitrepo.freebsd.org> <B6F0D20E-7A0A-41B5-878E-D22DF15FED99@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, May 15, 2023 at 10:49=E2=80=AFAM Jessica Clarke <jrtc27@freebsd.org= > wrote: > > On 15 May 2023, at 16:42, Kyle Evans <kevans@FreeBSD.org> wrote: > > > > The branch main has been updated by kevans: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D4b500174dd2d1e16dcb87c22= 364f724472c82edc > > > > 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 > > > > arm64: emulate swp/swpb instructions > > > > Add another undefined instruction handler for compat32 and watch out= for > > SWP/SWPB instructions. > > > > SWP/SWPB were deprecated in ARMv6 and declared obsolete in ARMv7, bu= t > > this implementation is motivated by some proprietary software that s= till > > uses SWP/SWPB. Because it's deprecated, emulation is pushed back beh= ind > > a sysctl that defaults to OFF in GENERIC so that it doesn't potentia= lly > > adversely affect package builds; it's unknown whether software may t= est > > for a functional swp/swpb instruction with the desire of using it la= ter, > > 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). > > > > The EMUL_SWP config option may be used to enable emulation by defaul= t in > > environments where emulation is desired and won't really be turned o= ff. > > > > 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(+) > > > > diff --git a/sys/arm64/arm64/freebsd32_machdep.c b/sys/arm64/arm64/free= bsd32_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, "s= truct siginfo32 size incorrect" > > > > extern void freebsd32_sendsig(sig_t catcher, ksiginfo_t *ksi, sigset_t = *mask); > > > > +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 mac= hine > > * 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> > > > > +#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> > > > > +#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 d= ata"); > > > > +#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); > > } > > > > +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 checki= ng > > + * 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? > Good shout, fixed in b68588618b43, thanks. Clear lack of consideration on my part, and I'm guessing they were ints in an earlier version of NetBSD's implementation. > Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaFde1dky00b9pgBm7UBBizgLkSGnHBnbZ9Yq6OD0HsZqA>