Yeah, sorry, I was actually thinking of a WIP one that isn't yet finished: https://paste.fbsd.dev/7kvf/ -- the prototype for this one was a pass with claude for the fun of it, and I haven't had time yet to sanity check what it did or what I asked it to do. I was going to switch USBSTS to stdatomic rather than putting it under any lock in particular, but I'm not sure that's actually the sensible thing to do.

I note that I have D59779 that was about to land; it might make sense to rebase this onto that one and use the finer-grained event mutex? I think both the original author of that one and I overlooked this one.

Cc'ing @jhb specifically, as he may be better equipped to answer re: appetite for features in support of legacy BIOS.

In D60413#1386055, @wanpengqian_gmail.com wrote:

My takeaway: this is mainly a correctness fix, with a modest efficiency win on top.

[...]

What's your takeaway on the results from your testing? I don't have a well-informed perspective here, but that feels like a modest but respectable enough win to make it worth doing (particularly when we finish getting USB device passthrough and can then support a wider variety of devices and usage patterns).

In D60274#1385664, @wanpengqian_gmail.com wrote:

Thanks for the review. Agreed on all three points. I'm new to the process here, so two questions on how you'd like them delivered:

1. EHB handling: I'd follow the spec (and QEMU's model): when EHB is already set, only post the event to the ring and don't touch IP/EINT or raise an interrupt; when a write to ERDP clears EHB and the ring is not empty, set IP/EHB again and raise. IMOD pacing (and the IMODI reset value) would stay for a separate change. Would you prefer the EHB part as a separate parent revision with this INTx change rebased on top, or folded into this revision as one commit?

2. The IMAN_IP/IMAN_IE rename touches sys/dev/usb/controller/xhcireg.h, which xhci(4) uses too. I'm happy to do it as a follow-up revision once this one lands, if that works for you.

The pci_msi_enabled() check in pci_xhci_deassert_interrupt() will be in the next update.

This is a pre-existing problem, but it might be nice to rername these at some point to IMAN_IP and IMAN_IE to line up witih the spec.

Thanks!

Dag-Erling might also have opinions here

Yes, this is perfectly sensible.

Switch gears and just control the release and install dependencies. Convert .WAIT into .ORDER and dcument the pain point that requires it, but call it out of scope for the moment as long as we don't break re@ builds.

In D60036#1377648, @ziaee wrote:

Can you also update release(7) with the new knobs?

In D60036#1377406, @cperciva wrote:

When you get to cloudware, my sense is that it's more likely someone will run e.g. make ec2ami-desktop-ufs rather than make cloudware-release.

Perhaps a better solution would be to have the relevant Makefile logic included unconditionally and have the WITH_* flags only control which targets are dependencies of release ?

In D60003#1376600, @jamie wrote:

It also means that I can go back to setting the state to invalid after the mac_prison_destroy call, and restore the comment that says "ideally we call this prior to any final state invalidation to be safe." But is that the right thing to do? I'm uncomfortable with the idea of a prison that has pr_ref == 0 and yet has pr_state == something valid. I think it still might be best to call it invalid, so if the mac handlers do anything that shouldn't be done to an invalid prison, they stand a greater chance of (properly) panicking.

Can you submit this one upstream to https://github.com/libecc/libecc, please?

Flip usbsts write and pci_xhci_assert_interrupt() call

Fix portsc reset in pci_xhci_deinit_port

This doesn't seem like it particularly violates any bhyve principles, so I may proceed with commtting this within the next week or so with no objection otherwise from bhyve folks

Extend locking to ERSTBA handling and pci_xhci_reset()

CC @bnovkov, IIRC he has an AMD machine with ControlArea outside of the device range that should be functional after this.

The first set of patches breaking this up starts at D59779

Manpage revision, incorporate some of polyduekes' feedback

Rebase, small revisions to the manpage, and a variety of cosmetic fixes

Taking this to address some feedback

Fixed one indentation issue

Restore handling of XHCI_TRB_TYPE_RESET_EP to be sure that we don't retain data
that should be dropped according to the spec.

It probably makes sense for this to ditch d_close entirely in favor of devfs_set_cdevpriv(9). This is how we typically get reliable semantics. @kib has a writeup of why d_close is shit for this somewhere, but I always have to go trolling through mail history to find it

A shlib interface where we'll dlopen and discover commands that way could also be neat, to facilitate things that want to operate in C using libzfs/libbe. The interface would take the subcommand args, libbe handle and roll with it.

In D59388#1370038, @markj wrote:

Oh? Presumably the current userboot protocol would be maintained, or do you have another idea in mind there?

Unprivileged bhyve doesn't work with bhyveload today, as that needs the VM to be created and executed by a single process. I'd still use userboot, I just want to have all of the functionality in one executable. I also want to be able to boot a kernel directly without a bootloader. I think we'll want to support different "flavours" of vCPU initialization in order to support these different usecases.

In D59388#1369907, @markj wrote:

Just to make sure I understand: we are saying that one can't expect to load and boot a new FreeBSD kernel with an old bhyveload+userboot.so? Or is it old bhyveload and newer userboot.so? I see that one reason for this is that userboot.so might not recognize some ZFS features used in a newer VM image. Are there other reasons?

In D59388#1368872, @michaelo wrote:

@kevans Anything holding your off applying it?

I did a cursory pass over verbs in base to see if we /might/ want to expand this to others / pass it through to mp->ctlreq, but it doesn't seem very likely except maybe in getactive?

In D59592#1367544, @des wrote:

In D59592#1367478, @imp wrote:

The only concern I have with the kernel enforcing write is what happens if we boot old binary, new kernel (which is quite typical). Would we have situations where we can't configure the system enough to build/install a new binary.

We could add some logic in g_ctl_ioctl() that sets FWRITE if the caller is the superuser, and then remove it in a year or two? Or make it conditional on COMPAT_FREEBSD15?

For posterity, I think we agreed that I would rebase this forward since @aokblast is quite busy these days, and I'm going to see if there are functional pieces I can split this further into. (after Euro)

In D59572#1366987, @seuros wrote:

In D59572#1366964, @adrian wrote:

Right; we can fix those but after this commit. This commit doesn't change their values, it just adds a level of indirection to them.

I was testing the world , that why i pushed the version bump. I reverted the version bump.

I note that there's a second copy of all of this in auditdistd, and we probably want to do similar things.

No idea why I would have done this, I've done plenty of atf-c... thanks!

I'm not that familiar with hastd, but this looks straightforward and correct to me.

review feedback: go with the first suggestion, and leave pluton_wait_reply() to
always poll for a reply at least once. The timeout == 0 case shouldn't really
happen in practice today, but it's worth defining the semantics now.