Don't use IOVEC_ADVANCE

Fix simple_attr as well

I found another place I need to update (simple_attr), will update the review later today with those changes as well.

This also makes me appreciate struct uio.

This would be an alternate fix instead of D55816. Note that I have compiled this, but have no good way to test it so would appreciate some testing.

To be fair, I think the cast is required in the one place the function is used, so I think it is fine to make the function match the one use case.

Given the check for interrupts being disabled is in the new hooks, I wonder if the name should reflect that? I also think spelling out user vs kernel would be more readable? Maybe trap_check_intr_user or something? Not that the name matters tremendously.

I guess you plan to make these ifuncs for FRED?

When committing, you can just use Matt's name as the commit author (you will still be the committer). I agree with Jess on reformatting the macros.

Great that we already use bus_space. I feared the worst when I saw this assignment fly by in the earlier diff.

In particular, is the invariant you are tripping over QCMD_STAILQ_CHECK_TAIL?

Bug reporter confirmed this fixes support for multiple ports.

The error of having assert() invoke code that is required (such that the code will just flat out break if you build with NDEBUG) still needs fixing. Siva's version fixes that bigger bug. I think It's fine if buf_to_iov() returns size_t, but I think the -1 is still useful from iov_to_buf to avoid ambiguity for a return value of 0. reallocf() is also a good fix.

Sigh, this is kind of terrible. Code that is required should never be invoked as part of assert(). Your change is correct, but I think style-wise it is better to declare bufsize at the start of the function instead of creating a nested block.

I actually find it really convenient that the log files are in .CURDIR.

Sample output available at https://reviews.freebsd.org/P704

Maybe also note in the commit log that this adds support for reading from non-regular files such as FIFOs?

Some wording nits and suggestions.

I agree we can drop support for the GPLv2 binutils.

This is actually not correct, please see my other stack of reviews. We are adding this node twice for some PCI buses, and this just ensures we add it twice for every bus. In particular, see the description in D55563.

In D55562#1273431, @obiwac wrote:

In D55562#1271617, @ngie wrote:

Hmm... atopcase(4) still panicked with a similar issue (once I figured out that I needed to add ig4 to my KERNCONF). This may have also occurred because my Mac had low battery and the hardware sends out events at the hardware level to OSes to suspend the host (there's always a chance that this is a bug with atopcase(4)).

This is probably because of the acpi_get_flags() call in acpi_wake_{sleep,run}_prep(). We should be checking if we have ACPI_IVAR_FLAGS in those functions too, or be doing something like D55609.

I think the former is the better solution, because otherwise we're still creating the sysctls even if the driver doesn't support this ivar.

Nice testing harness!

These all look good to me.

I also don't think anyone would know what "cdialog" is, but we all know what dialog is. Do we want to move dpv and friends to ports first? Maybe Cy can create new git repos for dpv + libdpv + libfigpar and a port to install them?

I would maybe tweak the commit log a bit though:

I've merged the ccp(4) changes if you want to land the rest of this.

I suspect the complexity wasn't to avoid 4 bytes per thread but more because TLS wasn't reliably working on all of our architectures when this was first written (but I could be wrong). :)

I would perhaps tweak the commit title to be something like "virtio pci: Enable device models on platforms IOMMUs" And then maybe in the first paragraph after the title use:

Thanks for putting up with all my nits