arm64: add tests for swp/swpb emulation
ClosedPublic
Actions

Authored by kevans on Apr 18 2023, 6:24 PM.

Details

Reviewers

imp
mmel

Group Reviewers

arm64
Klara

Commits

rGccb59683b983: arm64: add tests for swp/swpb emulation

Summary

One test is suitable to be hooked up to the build, so I've done this
here. The other test lives in tools/regression because failure is a
bit more subjective -- generally, one runs it for some unbounded amount
of time and observe if it eventually exits because two threads acquired
the same mutex.

Sponsored by: Stormshield
Sponsored by: Klara, Inc.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 51069
Build 47960: arc lint + arc unit

Event Timeline

kevans created this revision.Apr 18 2023, 6:24 PM

Herald added subscribers: emaste, andrew, imp. · View Herald TranscriptApr 18 2023, 6:24 PM

kevans requested review of this revision.Apr 18 2023, 6:24 PM

Harbormaster completed remote builds in B51013: Diff 120583.Apr 18 2023, 6:24 PM

kevans added a parent revision: D39667: arm64: emulate swp/swpb instructions.Apr 18 2023, 6:24 PM

andrew added inline comments.Apr 20 2023, 9:21 AM

tests/sys/compat32/Makefile
3–4	Should this be `MACHINE` or `MACHINE_CPUARCH`? I've preferred the latter in userspace, but given this is for a kernel test the former might be better.

imp requested changes to this revision.Apr 20 2023, 3:35 PM

imp added inline comments.

tests/sys/compat32/Makefile
3–4	It should be MACHINE_ARCH I'd think, with the corresponding renames below. I think MACHINE_ARCH because it's userland (and it's about binaries executing, which are tied to MACHINE_ARCH, not MACHINE). And MACHINE_ARCH instead of MACHINE_CPUARCH because we'd want to do this possibly for powerpc64, but not powerpc64le, since we don't have a powerpcle.

This revision now requires changes to proceed.Apr 20 2023, 3:35 PM

MACHINE -> MACHINE_ARCH

Harbormaster completed remote builds in B51069: Diff 120745.Apr 20 2023, 5:43 PM

kevans added inline comments.Apr 20 2023, 5:47 PM

tests/sys/compat32/Makefile
3–4	I also changed the dir name in tools/regression/compat32 to be consistent, although we won't be hooking that up with any make infrastructure at the compat32 level most likely.

Hmm, the SWP/SWPB instruction is obsolete in ARMv7 ISA (in favor of atomic operations). It not supported/implemented on ARMv7 SMP cpus and arm32 kernel doesn't support it at all. IMHO we should be consistent at least between arm32 and compat32 on aarch64.

https://developer.arm.com/documentation/ddi0406/c/System-Level-Architecture/System-Control-Registers-in-a-VMSA-implementation/VMSA-System-control-registers-descriptions--in-register-order/ID-ISAR4--Instruction-Set-Attribute-Register-4--VMSA?lang=en

In D39668#904485, @mmel wrote:

Hmm, the SWP/SWPB instruction is obsolete in ARMv7 ISA (in favor of atomic operations). It not supported/implemented on ARMv7 SMP cpus and arm32 kernel doesn't support it at all. IMHO we should be consistent at least between arm32 and compat32 on aarch64.

https://developer.arm.com/documentation/ddi0406/c/System-Level-Architecture/System-Control-Registers-in-a-VMSA-implementation/VMSA-System-control-registers-descriptions--in-register-order/ID-ISAR4--Instruction-Set-Attribute-Register-4--VMSA?lang=en

This is the only missing piece these folks need to build/run some specific software they have w/ compat32 on aarch64, though... it'd be really nice not to block that just because of this relatively minor detail.

I see. So in this case can we enable swp emulation using sysctl, disabled by default? I'm worried that this may cause bad behavior for ports build for arm32 - the port may detect swp during build. A bit paranoid, I know.

Also, can you put comment briefly on why we need to emulate deprecated, unimplemented (not enabled) on real 32bit hw ? It doesn't make sense without context. Thanks.

sebastien.bini_stormshield.eu added a subscriber: sebastien.bini_stormshield.eu.Apr 25 2023, 9:23 AM

Also check that the kernel was built with COMPAT_FREEBSD32
Use the new sysctl to turn swp emulation on as needed, make sure we can't run these tests in parallel in normal kyua runs.

Harbormaster completed remote builds in B51195: Diff 121039.Apr 25 2023, 8:13 PM

In D39668#905298, @mmel wrote:

I see. So in this case can we enable swp emulation using sysctl, disabled by default? I'm worried that this may cause bad behavior for ports build for arm32 - the port may detect swp during build. A bit paranoid, I know.

I agree; I added a compat.arm.emul_swp sysctl.

Also, can you put comment briefly on why we need to emulate deprecated, unimplemented (not enabled) on real 32bit hw ? It doesn't make sense without context. Thanks.

I put a mention in the commit message of D39667 (the implementation), let me know if you'd prefer to see it elsewhere as well.

More than enough, thanks. And sorry for starting this in the wrong review.

for the makefiles

This revision is now accepted and ready to land.May 9 2023, 6:05 PM

Closed by commit rGccb59683b983: arm64: add tests for swp/swpb emulation (authored by kevans). · Explain WhyMay 15 2023, 3:42 PM

This revision was automatically updated to reflect the committed changes.

kevans added a commit: rGccb59683b983: arm64: add tests for swp/swpb emulation.