Implement 32-bit atomic_fcmpset() in userland for armv4/v5.
ClosedPublic
Actions

Authored by jhb on Apr 20 2018, 9:21 PM.

Details

Reviewers

Group Reviewers

Commits

rS332892: Implement 32-bit atomic_fcmpset() in userland for armv4/v5.
rS332891: Fix some harmless type mismatches in the ARM atomic_cmpset implementations.

Summary

Add an implementation of atomic_fcmpset_32() using RAS for armv4/v5. This fixes recent world breakage due to use of atomic_fcmpset() in userland.
While here, be more careful to not expose wrapper macros for 64-bit atomic_*cmpset to userland for armv4/v5 as only 32-bit cmpset is implemented.

Test Plan

arm world now builds
I have no way to test this, and this is my first time attempting to do arm assembly, so could definitely use testing

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Apr 20 2018, 9:21 PM

Herald added a subscriber: andrew. · View Herald TranscriptApr 20 2018, 9:21 PM

Harbormaster completed remote builds in B16243: Diff 41703.Apr 20 2018, 9:21 PM

This is all goodness. C casting rules masked a lot of these sins.
I think RAS fcmpset_32 is good, but my ARM assembler is a bit rusty so take that with a grain of salt.

sys/arm/include/atomic-v4.h
118 ↗	(On Diff #41703)	good change, but don't think this fixes anything due to C casting rules at the end.
152 ↗	(On Diff #41703)	ouch. again, 0 vs 1 the casting rules wouldn't matter, but it's still a good change.

This revision is now accepted and ready to land.Apr 20 2018, 10:02 PM

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

The code looks correct, sans all the type changes. I'd recommend committing the type changes separately from fcmpset change.

Closed by commit rS332891: Fix some harmless type mismatches in the ARM atomic_cmpset implementations. (authored by jhb). · Explain WhyApr 23 2018, 4:50 PM

This revision was automatically updated to reflect the committed changes.

In D15147#319396, @jmg wrote:

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

The code looks correct, sans all the type changes. I'd recommend committing the type changes separately from fcmpset change.

The BBB has an armv7 CPU so it uses atomic-v6.h, not this file. Only a few specific armv4/v5 boards are supported by FreeBSD and they aren't the common BBB, ${FRUIT}Pi type boards which are all based on armv6 (original RPi) or armv7/v8.

In D15147#319660, @jhb wrote:

In D15147#319396, @jmg wrote:

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

The code looks correct, sans all the type changes. I'd recommend committing the type changes separately from fcmpset change.

The BBB has an armv7 CPU so it uses atomic-v6.h, not this file. Only a few specific armv4/v5 boards are supported by FreeBSD and they aren't the common BBB, ${FRUIT}Pi type boards which are all based on armv6 (original RPi) or armv7/v8.

ahh, stupid arm4 vs armv7 BS, it's been too long. I do have a Avila Gateworks board. But I thought there was discussion about dropping it.
http://www.gateworks.com/product/item/avila-gw2348-4-network-processor
and it is ARMv5: https://en.wikipedia.org/wiki/XScale#IXP_network_processor

armeb is in danger, since there were almost no responses to my call for users.
armv4,5 is less in danger since there were responses. There's users, but the floor for FreeBSD is rising and may rise above what they can provide.