Page MenuHomeFreeBSD

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb created this revision.Apr 20 2018, 9:21 PM
imp accepted this revision.Apr 20 2018, 10:02 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
jmg added a subscriber: jmg.Apr 22 2018, 6:27 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.

This revision was automatically updated to reflect the committed changes.
jhb added a comment.Apr 23 2018, 4:58 PM
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.

jmg added a comment.Apr 23 2018, 10:34 PM
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

imp added a comment.Apr 23 2018, 10:42 PM

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.