Page MenuHomeFreeBSD

riscv: implement kernel ifunc resolution
AcceptedPublic

Authored by mhorne on Tue, Feb 3, 3:47 PM.

Details

Reviewers
kib
jrtc27
imp
Group Reviewers
riscv
Summary

This completes the set of architectures.

It boots with the latest scheduler changes.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 70374
Build 67257: arc lint + arc unit

Event Timeline

mhorne requested review of this revision.Tue, Feb 3, 3:47 PM
imp added inline comments.
sys/riscv/include/ifunc.h
33

Isn't it about time to retire this too?

sys/riscv/riscv/machdep.c
627

why'd you move this?

This revision is now accepted and ready to land.Tue, Feb 3, 4:43 PM
sys/riscv/riscv/machdep.c
627

IMHO, sched_instance_select(), and even more the now uncommented link_elf_ireloc(), were misplaced for RISC V and have no real business in parse_metadata(). Elf relocs presumably must happen after the CPU has been identified, as selection functions need that info to choose the most performant variant given what the CPU reports. This is actually what we are doing in amd64. I'd also bet that sched_instance_select() needs to be after init_static_kenv() to work as intended, so basically after parse_metadata().

sys/riscv/include/ifunc.h
33

The flag is new. We now have the choice to retain it for some future use or remove it outright. I will post a follow-up tomorrow.

sys/riscv/riscv/machdep.c
627

Yes, as @olce says it can be run a little later, and after identify_cpu(0) makes much more information available to the resolvers. I'll note the move in the commit message.

kib added inline comments.
sys/riscv/include/ifunc.h
33

Do you mean, removing its only use from sched_shim.c?
I do not object, but do we need to remove __DEFINE_SHIM() from sched_shim.c as well?

sys/riscv/include/ifunc.h
33

__DEFINE_SHIM can be simplified to not separate argument types and names (and I think can then be made variadic as there's no clever splicing needed) but still has value in reducing duplication

sys/riscv/riscv/elf_machdep.c
506

Should we not be passing hwcap and 0s like userspace? Otherwise any use of DEFINE_UIFUNC will have junk in the registers.