Page MenuHomeFreeBSD

arm32 support for inline ffs, fls
ClosedPublic

Authored by dougm on May 26 2019, 2:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 21, 3:01 PM
Unknown Object (File)
Thu, Dec 5, 2:44 PM
Unknown Object (File)
Sat, Nov 30, 2:23 AM
Unknown Object (File)
Thu, Nov 28, 5:51 AM
Unknown Object (File)
Thu, Nov 28, 5:51 AM
Unknown Object (File)
Thu, Nov 28, 5:51 AM
Unknown Object (File)
Thu, Nov 28, 5:51 AM
Unknown Object (File)
Thu, Nov 28, 5:51 AM

Details

Summary

Implement the ffs and fls functions, and their longer counterparts, in
cpufunc, in terms of __builtin_ffs and the like, for arm32
architectures, and use those, rather than the simple libkern
implementations, in building arm32 kernels.

Test Plan

Built arm.armv6 and arm.armv7 kernels with this change in place and found evidence of the inline arm implementation of ffs in examining the pmap.o binary with objdump:

4fb0:       e6ff0f36        rbit    r0, r6
4fb4:       e16f5f10        clz     r5, r0
4fb8:       e1850009        orr     r0, r5, r9

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

So why not do this for all architectures ? Is the problem that old gcc on ppc/sparc64/mips does not support __builtins required ?

In D20412#440654, @kib wrote:

So why not do this for all architectures ? Is the problem that old gcc on ppc/sparc64/mips does not support __builtins required ?

I tried a powerpc build with essentially the same patch:
linking kernel.full
watchdog.o: In function `flsll':
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:305: undefined reference to `clzdi2'
subr_blist.o: In function `ffsll':
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:276: undefined reference to `
ffsdi2'
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:276: undefined reference to `ffsdi2'
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:276: undefined reference to `
ffsdi2'
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:276: undefined reference to `ffsdi2'
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:276: undefined reference to `
ffsdi2'
tcp_lro.o: In function `flsll':
/local/obj/freebsd/base/head/powerpc.powerpc/sys/GENERIC/./machine/cpufunc.h:305: undefined reference to `__clzdi2'

  • Error code 1

So there must be something else necessary for powerpc.

I don't mind trying on other architectures, but that doesn't mean that there must be one giant patch that fixes all architectures at once.

__clzdi2() and other functions are indeed libgcc symbols implementing the builtins, so at least gcc cannot properly inline the operation.

Was arm patch tested on some arm machines ?

No testing on an actual arm machine has been performed.

No testing on an actual arm machine has been performed.

You can try to solicit help on arm@ and current@. I suppose just compiling new kernel and use the swap should do it ?

Change patch so that it affects arm6 and arm7, but not arm4 or arm5.

Fix a keyword error in the conf file.

Looks fine to me, but I agree that we should get some testing before the change is committed.

This revision is now accepted and ready to land.May 27 2019, 6:46 PM

So you can commit with both mentor' approvals after testing.

manu added a subscriber: manu.

LGTM, I'll try to find to time to test today on armv7 and armv6.

Build a new kernel with both files patched for an RPi 1 (armv6, 1176jzf) and booted it.
Nothing broke. The RPi is back to normal service. LGTM.

It's ok on armv7 (imx6 board)

Looks good.

Per kib's recommendation, I tested on imx6 by forcing swap usage (mount tempfs as /tmp and dd lots of data to a file in /tmp), top showed lots of swap usage going on...

Swap: 3072M Total, 643M Used, 2429M Free, 20% Inuse, 9608K In, 3072K Out

On reading the file back from /tmp, it contained the expected data.

This revision was automatically updated to reflect the committed changes.