No support for ucontext.h posix API (looks like legacy)

I can't help but feel a lot of this belongs in packages rather than being big splats of hard-coded config that bsdinstall writes out that risks getting stale when ports gets updated

Did you simulate the UX of this happening? This looks like it's going to just silently report success and cause the next installer step to fail (or not, if there's already a partition table there?).

Don't we need to do a broadcast invalidate on fini (or remember the allocated VMIDs from the current epoch), otherwise we'll reallocate the same VMIDs if we kldload and start VMs? Also, don't leave the epoch as is whilst clearing the bitmap and next, that doesn't make sense, if the epoch is staying then so does the allocated set, and if it's being reset then so does the allocated set (along with invalidate).

In D55860#1278404, @andrew wrote:

Is there a reason to not just read the ID register in the vmm code?

One tiny missing space, otherwise LGTM, feel free to fix on commit

attempt to restore ints

In D55480#1269816, @kib wrote:

In D55480#1269558, @jrtc27 wrote:

In D55480#1269551, @kib wrote:

First, there are two unrelated changes. One for the in-kernel ELF image activator, another for the kernel linker.

Then, for the image activator, why the is_cheri checks needs to be done in MI code? We have brand matchers (header_supported/brand_supported), which get the pointer to the mapped ELF header. Why cannot it be done there, in <arch>/elf_machdep.c?

You could make the same argument for e_machine/EI_CLASS/EI_DATA. Like those, this is an MI concept, so it's in MI code rather than having MD boilerplate (that, I will note, also has some odd semantics when it comes to the bool-returning functions, though I don't remember off the top of my head what those were). The MD hooks are for MD things like "v1 or v2 of this architecture's psABI". Whereas this is part of ILP32-vs-LP64-vs-"L64PC128" (or whatever name gets given to it, if any), and corresponds to a specific instantiation of the imgact_elf.c template (Elf32 vs Elf64 vs "Elf64C")

Right now e_machine/class/data/version together form the arch identifier, it was not well thought and came from times where the scope of acceptance of the ELF was not envisioned. For instance, they reserved separate machine ids for 386 vs 486. Then they used different machine id for x86_64 instead of using 64bit class etc.

We do not distinguish arch-specific ISAs at imgact_elf. We do not determine the linux binaries there, it is the job of arch-specific brand. IMO we must not start doing that for cheri.

BTW, it is up to you that you want to identify chri binaries with some flags, but I am surprised that you do that instead of adding separate machine Ids since ISA is different.

Can you please include an appropriate description of the effects in the commit message?

In D55480#1269551, @kib wrote:

First, there are two unrelated changes. One for the in-kernel ELF image activator, another for the kernel linker.

Then, for the image activator, why the is_cheri checks needs to be done in MI code? We have brand matchers (header_supported/brand_supported), which get the pointer to the mapped ELF header. Why cannot it be done there, in <arch>/elf_machdep.c?

NB: It's also about to be in 14.4 via 829e479a0a37eb72023ce361f5b2379d82f8bc2a

update_uefi_bootentry assumes that the caller sets FREEBSD_BOOTNAME and
mntpt, which isn't the case anymore

Oh I see, the point is about what's passed up as the second argument, not what device the method is invoked on. I guess then my question becomes why is the DMA tag special? We don't do this for resources, and we don't do this for the bus space tag. Is that just because normally it's virtio_pci doing the resource allocation?

How does this differ from the default implementation specified in sys/kern/bus_if.m, namely bus_generic_get_dma_tag?

Use correct variable

So how are we dealing with /etc/fstab for existing systems?

Is this actually KBI or just internal KBI between kernel and vmm, which would always be in sync?

virtio_blk calls bus_dma_tag_set_iommu on PowerPC; would that not be needed for these new tags?

Do we still need the mb()s? I would expect bus_dmamap_sync to suffice.

IIRC GNU as and LLVM IAS disagree on what .weak and .glob[a]l mean. LLVM IAS doesn't let you mix them, but that's not really sensible, since one is binding and the other is visibility. We use .weak for linker sets but not .glob[a]l (I think used to use both) because one version of LLVM introduced that error, whilst I think GNU as wants what we used to have where we put both. I should probably raise it with upstream LLVM to point out that what they did doesn't make sense.

In D54964#1256922, @jhb wrote:

In D54964#1256826, @markj wrote:

In D54964#1256808, @jhb wrote:

I tried using a constraint of "m" and an operand modifier of "a" which I think is more what we want anyway (if you remove the & in the operand), but that gives me ICEs in both GCC 14 and GCC 15. This whole thing seems a bit gross. I realize we want the real symbol here and don't want it to resolve to 'foo@plt' or the like, but this whole thing feels a bit clunky. I guess we can't just do _SDT_ASM_WORD _SDT_PROBE_NAME(...) directly?

Sadly no. This doesn't work for function-scoped static variables, since there the variable name is not equal to that of the emitted symbol. And the DTRACE_PROBE* macros defined in sdt.h (and I think only used by ZFS) need this to work.

(Presumably on CHERI _SDT_WORD here is a .chericap and would really need the symbol name and not some goofy "make the compiler generate a constant address" thing anyway?

It expands to .chericap "%c0" with the operand defined as "i" (__unbounded_addressof(_SDT_PROBE_NAME(...)).

Humm, what what does the %c0 expand to? You can't really do .chericap 0x12345 and have it be meaningful at all. It has to be a symbol so that the assembler emits a static relocation that the linker can use to figure out the right bounds and permissions so it can construct a suitable relocation (either a .caprelocs / R_MORELLO_RELATIVE for a non-preemptible symbol, or a R_CHERI_CAPABILITY).

Ping? I went to update my board last night and noticed I still had this (with my inline suggested fix) as a local patch.

In D54969#1256900, @mhorne wrote:

Thanks a lot for handling this.

As I recall from previous experiments building with GCC, there was some missing atomic libcall, or something? Maybe this has resolved itself?

In D54966#1256752, @bz wrote:

In D54966#1256747, @jhb wrote:

I have no idea why clang fails to warn about this as I noted in the commit log. OTOH, if we want to make '%#b' a real thing, we need to document it in the manpage and fix both compilers to intentionally (vs accidentally) accept it.

I thought someone else was working on more %b for FreeBSD; or at least 64bit %b argument support. Was it @tuexen? I'd be happy to %#b supported :)

Effort: CHERI upstreaming or whatever it is seems like it would apply?

In D54246#1254644, @kib wrote:

In D54246#1254643, @adrian wrote:

ok, but on insistence from jrtc27 i added some debug logging here and it's never actually invoking syncicache in my tests.

This does not mean much. It only shows that the current binaries do not have this issue. But old binaries might have, then removing the hack breaks them.

In D54246#1254603, @adrian wrote:

This works for me on freebsd-16 on power8, I'm doing test buildworld -j32 on it right now!

In D54857#1254516, @doonbsd_gmail.com wrote:

Hold on. I just found one style problem. There must be a empty line after start curly brace when the function has single line body.

I send an updated patch.

In D54876#1254502, @imp wrote:

What are you using this for? I think it's ahoodchange, but I'm curios.

Well it is where it is because (a) asserting in vmmops_init is pointless with M_WAITOK as you say (b) vm_init is in a separate translation unit so in some world views should treat vmmops_init as a black box, and is therefore asserting that it never fails. And this pattern is copied from arm64, so we should keep the two consistent, whichever way it goes.

https://reviews.freebsd.org/P685 is a proposal for how to drastically reduce the amount of boilerplate here (even compared with the ifunc-only version). The wrapper vs ifunc difference is mostly hidden, with the only way it leaks out being that the macro needs to take argument types and names separately so it can use just the names in the call for the wrapper variant. That could be simplified back down in a future ifunc-only version. Built and test-booted arm64 GENERIC and riscv64 QEMU (with other patches to deal with the other issues here).

In D54831#1253758, @kib wrote:

In D54831#1253740, @jrtc27 wrote:

armv7 and riscv64 are tier 2, breaking them is not permitted. I have an idea for how to make them work though, without being too invasive.

ifunc can be emulated with the function pointer. This is of course breaks the microoptimizations real ifunc provides, but should keep the arches going until maintainers finally implement the proper solution.
I do not want to code that.

armv7 and riscv64 are tier 2, breaking them is not permitted. I have an idea for how to make them work though, without being too invasive.

Do we need to worry about $s in OBJROOT?

This probably needs more of an explanation for the issue with this pattern on CHERI than we had downstream

Our pkg is old enough to predate that support, but we're also not doing pkgbase yet, so presumably once it's mature enough upstream for us to adopt it downstream we'll first update pkg.

Won’t that break the dependencies list if clang is itself 32-bit? This also introduces a new hard-coding of the set of compat libraries rather than using _ALL_libcompats.