In D53910#1303910, @ziaee wrote:

In D53910#1303904, @jrtc27 wrote:

In D53910#1303893, @ziaee wrote:

In D53910#1303832, @jrtc27 wrote:

In D53910#1303533, @ziaee wrote:

Please understand that I am personally committed to seeing this website refresh land, however I am formally requesting changes proposed over email. I would like to propose the line objected to is replaced with "For over 30 years, this has been a grass roots project carried by passionate volunteers."

Firstly, for those of us not privy to this private email chain, what specifically is being objected to, and why?

The line "Check out the engagement driven by the FreeBSD Foundation on YouTube" is being objected to on the grounds that it is inappropriate for the "Community" box.

A social media presence dedicated to FreeBSD, by people who are involved in FreeBSD, that is an official FreeBSD-related organisation, and is engaging with people on the internet, is inappropriate for the "Community" section of the FreeBSD website how?..

The FreeBSD foundation is it's own entity, with it's own website. The Project is consists of a democratic system, which the Foundation exists completely separate and outside of.

In D53910#1303893, @ziaee wrote:

In D53910#1303832, @jrtc27 wrote:

In D53910#1303533, @ziaee wrote:

Please understand that I am personally committed to seeing this website refresh land, however I am formally requesting changes proposed over email. I would like to propose the line objected to is replaced with "For over 30 years, this has been a grass roots project carried by passionate volunteers."

Firstly, for those of us not privy to this private email chain, what specifically is being objected to, and why?

The line "Check out the engagement driven by the FreeBSD Foundation on YouTube" is being objected to on the grounds that it is inappropriate for the "Community" box.

The Vector spec does not require any ordering on the memory accesses performed, so a legal implementation could alternate which address faults.

In D53910#1303533, @ziaee wrote:

Please understand that I am personally committed to seeing this website refresh land, however I am formally requesting changes proposed over email. I would like to propose the line objected to is replaced with "For over 30 years, this has been a grass roots project carried by passionate volunteers."

But won’t we have built elftoolchain strip in that case?

In D56717#1299142, @asomers wrote:

In D56717#1299128, @jrtc27 wrote:

Should this not do the same as 9134ed157388f3e34374322a5de06449a031f1ec?

It's not the same thing. That older revision changed the default interactive installation to pkgbase. What my change is is for the non-interactive installations. That is, the installations that the CD will do if you put an /etc/installerconfig file on the CD.

Should this not do the same as 9134ed157388f3e34374322a5de06449a031f1ec?

Oh, I always forget which way round -h and -n are. -n is the one you want, as we accept both as meaning the same thing, but coreutils only accepts -n.

These probably want -h too? pkg.pkg less interesting, but PKG_ALTABI surely that can go quite wrong if not?

Oh well I knew *that* one worked, I've run into this before even I think and added the missing semicolon, assuming it was my mistake (which it was, even if GNU sed is more friendly)

And indeed we do already handle the newline case correctly:

Not sure if this is actually a bug. POSIX says:

In D56551#1296024, @kajetan.puchalski_arm.com wrote:

As in we'd change the definition of the hypctx_sysreg enum itself? And then do "something" to special case it in the function? And have a new hypctx_sys_reg_cap that returned uintcap_t *? That seems like it could work. Though I've tagged @markj on this review as he did the initial adaptation of the proposed bhyve/arm64 code to Morello.

Something like that, yes. You could probably even do some preprocessor voodoo and make hypctx_sys_reg and the read/write functions macros that instantiate to the current function for normal registers, and to your hypctx_sys_reg_cap with its different return type for the capability-enabled ones.
As I've mentioned previously, currently all these functions and macros get compiled away to bare mrs & msr instructions. The "return type" is just there so that the compiler finds the right instruction.

Also, you could get rid of the need for specific read/write functions if you had something like:

I wish.. The read/write functions are just cosmetic and don't do anything at the moment, but they'll be necessary to handle certain architectural edge cases that'll come up when implementing support for nested virt. I'm just adding them ahead of time.
The reason I'm trying to funnel all reads and writes through these functions is so that when corrections need to be made to register reads and writes they only get made in one place.

As with the lib32 change, this needs a clear justification for why this default is being changed

What's the justification for changing this long-standing default? I don't see one in the summary. I'm not saying it's the wrong thing to do, but it needs to be clearly stated.

In D56551#1295970, @kajetan.puchalski_arm.com wrote:

I would really like to not land this patch before it's clear there's an *easy* path to making this work on Morello.

What's the remaining concern in that regard, then? Looking at the cheribsd github repo it already does a bunch of #if __has_feature(capabilities).
For any arbitrary register or group thereof, you can just add an if into hypctx_sys_reg and use whatever storage you like. In the VNCR page, in a different page, in the hypctx struct, whatever works.
It's specifically meant to be flexible in cases where a register needs "special" handling and needs to be put through a dedicated function and the like.

And see for example D55860 as things we continue to find by using it in anger downstream. I think we are by far the biggest user to date of bhyve on any form of arm64.

In D56551#1295949, @kajetan.puchalski_arm.com wrote:

In D56551#1295942, @jrtc27 wrote:

In D56551#1295771, @andrew wrote:

It looks like the two capability registers this change affects are elr_el1 and vbar_el1. Both of these have at least 128 bits of space before the next register, so storage shouldn't be a problem for Morello.

sp_el0, tpidr_el0, tpidrro_el0, and tpidr_el1 could be split into a new capability array when they are managed in a later change.

Also sp_el1 and tpidr_el2 in the VNCR space (why is tpidr_el2 even in there? I don't know if I want to know...). I would really like to not land this patch before it's clear there's an *easy* path to making this work on Morello. Breaking bhyve on Morello would be really quite unfortunate and hostile towards CheriBSD.

Does bhyve currently work on Morello?

In D56551#1295771, @andrew wrote:

It looks like the two capability registers this change affects are elr_el1 and vbar_el1. Both of these have at least 128 bits of space before the next register, so storage shouldn't be a problem for Morello.

sp_el0, tpidr_el0, tpidrro_el0, and tpidr_el1 could be split into a new capability array when they are managed in a later change.

In D56551#1294476, @kajetan.puchalski_arm.com wrote:

In D56551#1294463, @jrtc27 wrote:

This is really quite an obnoxious change to deal with for Morello, where some of these registers are 128-bit capabilities not 64-bit integers...

Which ones specifically?
Do you just mean it's harder to deal with because without this you'd just expand the sizes of registers in hypctx, whereas the VNCR layout doesn't have the same flexibility?

This is really quite an obnoxious change to deal with for Morello, where some of these registers are 128-bit capabilities not 64-bit integers...

I now understand why this case is there and am contemplating unifying the two paths so this condition can just be deleted even on non-PowerPC (it's more interesting on CHERI where even our ET_EXEC will have relocations for capabilities, but not integer addresses...), but for now this should be sufficient.

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, or defer it to re-init), 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?