Page MenuHomeFreeBSD

Improve MACHINE_ARCH handling for hard vs soft-float on RISC-V.
ClosedPublic

Authored by jhb on Apr 22 2020, 9:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 10, 5:24 AM
Unknown Object (File)
Mar 17 2024, 6:30 AM
Unknown Object (File)
Mar 17 2024, 6:30 AM
Unknown Object (File)
Mar 17 2024, 6:30 AM
Unknown Object (File)
Mar 17 2024, 6:30 AM
Unknown Object (File)
Mar 17 2024, 6:17 AM
Unknown Object (File)
Feb 16 2024, 5:53 AM
Unknown Object (File)
Dec 23 2023, 4:03 AM
Subscribers

Details

Summary

For userland, MACHINE_ARCH reflects the current ABI via preprocessor
directives. For the kernel, the hw.machine_arch sysctl uses the ELF
header flags of the current process to select the correct MACHINE_ARCH
value.

Test Plan
  • booted a GENERIC (really QEMU) kernel once with a hard-float world (sysctl hw.machine_arch returned "riscv64") and once with a soft-float world (sysctl hw.machine_arch returned "riscv64sf")

Diff Detail

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

Event Timeline

sys/kern/kern_mib.c
266 ↗(On Diff #70898)

It's a little bikeshed-y, so feel free to ignore, but this feels like something that should be machine specific.

Perhaps proc_machine_arch needs to be a __weak_symbol here, and optionally be defined in the machine-specific code.

sys/riscv/include/param.h
52 ↗(On Diff #70898)

Is there a plan for MACHINE_ARCHES? It doesn't seem to be used anywhere right now.

sys/riscv/include/param.h
52 ↗(On Diff #70898)

Sigh. Never mind, turns out I wasn't awake yet. It's introduced in D24542.

sys/kern/kern_mib.c
266 ↗(On Diff #70898)

Originally I wanted this to be an MD function, but as I thought about it, the amount of scaffolding you'd have to put in place would be much larger than the #ifdef's here. Sadly, MIPS doesn't encode soft vs hard float in its ELF flags, but in an ELF note, so I punted on trying to resolve this for MIPS. ARM does use ELF flags, so if we grew a real soft-float ARM target it would add another 4 line diff like this one. The ARM one is the only one I can realistically seeing us add in the future, so given the cost, I think doing a few MD #ifdef's inline is probably simpler and the most readable. (CheriBSD has a COMPAT_FREEBSD64 case that will go below, but that would be MI regardless).

I suppose we could add an optional function to sysentvec. That would also perhaps let us use that to handle freebsd32. Maybe I will try that. It will mean reworking the prior patch to add the framework.

I've also done a hard float qemu build to confirm this work. I have a softfloat build running, just as a sanity check and I'll report back when it's done.

sys/kern/kern_mib.c
266 ↗(On Diff #70898)

Makes sense.

There's one issue with this, on riscv64sf:

root@pkgbuilder:~ # sysctl hw.machine_arch
hw.machine_arch: riscv64sf
root@pkgbuilder:~ # cat Makefile
all:
        echo MACHINE_ARCH = ${MACHINE_ARCH}
root@pkgbuilder:~ # make
echo MACHINE_ARCH = riscv64
MACHINE_ARCH = riscv64

So, (b)make produces the wrong MACHINE_ARCH, which will break ports building.

Interesting, do we know how make chooses that? I guess it isn't using the sysctl but something else? If it is using MACHINE_ARCH directly then we may need to make the userland version conditional on the ABI.

In D24543#540114, @jhb wrote:

Interesting, do we know how make chooses that? I guess it isn't using the sysctl but something else? If it is using MACHINE_ARCH directly then we may need to make the userland version conditional on the ABI.

It's complicated, and I don't think I fully understand the idea behind all the options.

It's set in /usr/src/contrib/bmake/main.c:

const char *machine_arch = getenv("MACHINE_ARCH");
...
        if (!machine_arch) {
#if defined(MAKE_NATIVE) && defined(HAVE_SYSCTL) && defined(CTL_HW) && defined(HW_MACHINE_ARCH)
            static char machine_arch_buf[sizeof(utsname.machine)];
            int mib[2] = { CTL_HW, HW_MACHINE_ARCH };
            size_t len = sizeof(machine_arch_buf);

            if (sysctl(mib, __arraycount(mib), machine_arch_buf,
                    &len, NULL, 0) < 0) {
                (void)fprintf(stderr, "%s: sysctl failed (%s).\n", progname,
                    strerror(errno));
                exit(2);
            }

            machine_arch = machine_arch_buf;
#else
#ifndef MACHINE_ARCH
#ifdef MAKE_MACHINE_ARCH
            machine_arch = MAKE_MACHINE_ARCH;
#else
            machine_arch = "unknown";
#endif
#else
            machine_arch = MACHINE_ARCH;
#endif
#endif
        }

So, we can overrule it with an environment variable, but that's not really ideal. If the MACHINE_ARCH env var is not set (as is default) it depends on who we're built. It can use the sysctl, but it can also use either the MAKE_MACHINE_ARCH or MACHINE_ARCH define, depending on how it's built. I have to assume we end up in that last case and use the MACHINE_ARCH define from /usr/src/sys/riscv/include/param.h.
That's supported by a truss run over make, where I can see it sysctl()s for hw.machine but not for hw.machine_arch.

  • Make MACHINE_ARCH in userland reflect the current ABI.
jhb retitled this revision from Handle RISC-V hard vs soft-float ABI in hw.machine_arch via ELF header flags. to Improve MACHINE_ARCH handling for hard vs soft-float on RISC-V..Apr 24 2020, 7:22 PM
jhb edited the summary of this revision. (Show Details)

This one still returns the right output of hw.machine_arch but also fixes make -V MACHINE_ARCH

Other than what I think is a left over sys/elf.h include, this looks good.

sys/kern/kern_mib.c
48 ↗(On Diff #70956)

Why did you have to add elf if you did nothing else to this file?

This revision is now accepted and ready to land.Apr 24 2020, 8:40 PM

There's still something strange going on, because I can't build on the soft float image:

root@pkgbuilder:~ # cc -v t.c -o t
FreeBSD clang version 10.0.0 (git@github.com:llvm/llvm-project.git llvmorg-10.0.0-0-gd32170dbd5b)
Target: riscv64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin
 "/usr/bin/cc" -cc1 -triple riscv64-unknown-freebsd13.0 -emit-obj -mrelax-all -disable-free -main-file-name t.c -mrelocation-model static -mthread-model posix -mframe-pointer=all -fno-rounding-math -masm-verbose -mconstructor-aliases -munwind-tables -target-feature +m -target-feature +a -target-feature +f -target-feature +d -target-feature +c -target-feature +relax -target-abi lp64d -dwarf-column-info -fno-split-dwarf-inlining -debugger-tuning=gdb -v -resource-dir /usr/lib/clang/10.0.0 -fdebug-compilation-dir /root -ferror-limit 19 -fmessage-length 178 -fno-signed-char -fgnuc-version=4.2.1 -fobjc-runtime=gnustep -fdiagnostics-show-option -faddrsig -o /tmp/t-e0c74e.o -x c t.c
clang -cc1 version 10.0.0 based upon LLVM 10.0.0git default target riscv64-unknown-freebsd13.0
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/clang/10.0.0/include
 /usr/include
End of search list.
 "/usr/bin/ld" --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --enable-new-dtags -m elf64lriscv -o t /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/usr/lib /tmp/t-e0c74e.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/crtend.o /usr/lib/crtn.o
ld: error: /tmp/t-e0c74e.o: cannot link object files with different floating-point ABI
ld: error: t.c:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
cc: error: linker command failed with exit code 1 (use -v to see invocation)
In D24543#540703, @kp wrote:

There's still something strange going on, because I can't build on the soft float image:

root@pkgbuilder:~ # cc -v t.c -o t

'cc' is going to default to hard-float for RISC-V since that is what the RISC-V triple defaults to for clang. bsd.cpu.mk always adds an explicit -mabi option to set hard vs soft-float (and for MIPS we always add -msoft-float or -mhard-float in bsd.cpu.mk for similar reasons). I don't think clang supports a soft-float triple for RISC-V, so I don't think we can force the riscv64sf cc to default to soft-float by default (at least not via the trivial route of altering the default triple). That might be more of a question for @dim.

Note that you also need -mno-relax per the other warning you got. That's true of both hard and soft float and has always been true when using lld instead of ld.bfd for RISC-V.

sys/kern/kern_mib.c
48 ↗(On Diff #70956)

The earlier version that checked the ELF flags in this file needed it, I'll axe it.

In D24543#540964, @jhb wrote:
In D24543#540703, @kp wrote:

There's still something strange going on, because I can't build on the soft float image:

root@pkgbuilder:~ # cc -v t.c -o t

'cc' is going to default to hard-float for RISC-V since that is what the RISC-V triple defaults to for clang. bsd.cpu.mk always adds an explicit -mabi option to set hard vs soft-float (and for MIPS we always add -msoft-float or -mhard-float in bsd.cpu.mk for similar reasons). I don't think clang supports a soft-float triple for RISC-V, so I don't think we can force the riscv64sf cc to default to soft-float by default (at least not via the trivial route of altering the default triple). That might be more of a question for @dim.

Note that you also need -mno-relax per the other warning you got. That's true of both hard and soft float and has always been true when using lld instead of ld.bfd for RISC-V.

Thanks. I tried building ports (ports-mgmt/pkg) which failed so I tried the simple compile above. That failed too, so I assumed they failed for the same reason.
That's only somewhat true. It looks like the bootstrap process for the pkg build doesn't take the bsd.cpu.mk flags into account. I'll see if I can debug that, but this change looks good.