In D25531#576420, @jhb wrote:

FYI, I ran into this again and just updated qemu-devel to the latest (at the time) snapshot: 5.1.0 rc2 at D25991.

In D25935#575535, @ngie wrote:

LGTM.

I was a bit surprised to learn that this code relies on the jq utility from ports, by the way:

$ grep -r jq sbin/route/tests/
sbin/route/tests/utils.subr:    gateway=$(echo $result | jq -r ${query}.gateway)
sbin/route/tests/basic.sh:      atf_set require.progs jq
sbin/route/tests/basic.sh:      atf_set require.progs jq

I wish we had a tool capable of ingesting json in base :/...

Yeah, that'd be nice, and it'd make a lot of sense, considering the libxo-ification of a lot of tools.

I have complicated feelings about this.

Yeah, I don't see better options either. If there are no objections I'm going to commit this in a few days.

I've provided this as a point of discussion more than as a finished solution. I'm not sure if there's a better way to do this.

Thanks! That fixes the problem too.

In D25531#568751, @mhorne wrote:

After updating to a recent HEAD I discovered I can no longer boot inside QEMU. I know that the aforementioned bugs in the QEMU's PLIC are to blame, but I did not realize that they would cause a complete deadlock with this change applied. CI didn't catch it because the RISC-V testvm job has been broken for some time.

@kp, can you see that qemu-devel gets updated with the required fixes, or back out this change until the port is updated to 5.0? I will make sure that they get included with that update.

These are the required fixes by @jrtc27: aa4d30f and 5576582

In D25531#564692, @jrtc27 wrote:

Also, are we guaranteed to have the *post_ithread* hook run on the same hart? If not we'll complete for the wrong hart (though in practice all implementations I know of only track claims on a global basis so don't care which hart's claim/complete register you write to).

In D25531#564579, @kp wrote:

That reminds me: the qemu implementation has a bug that means it won't trigger interrupts when we re-enable them, leading to stalls (until you trigger another interrupt, e.g. by sending something on the console).

The following Qemu patch fixes that:

It appears that Qemu is aware of this, and a patch is either already in, or on its way in: https://lists.nongnu.org/archive/html/qemu-riscv/2020-06/msg00304.html
And upon re-reading that e-mail, it looks like you may already be aware of this too...

Add comments requested by mhorne

In D25531#564577, @jrtc27 wrote:

That's what the driver is currently doing in plic_pre_ithread and plic_post_ithread. The spec sucks and doesn't really say what claim/complete actually do, but at least the QEMU and Bluespec implementations of the PLIC will clear the IP bit and forbid the IP bit going high until completed, effectively masking the interrupt for _every_ target by masking on the _source_.

In D25531#564563, @jhb wrote:

FWIW, the general strategy on x86 and other platforms is to mask the interrupt in the PIC in the "pre_ithread" hook and later re-enable it in the PIC again in the "post_ithread" hook. Both "pre_ithread" and "post_filter" send an EOI as they are the only ones guaranteed to run synchronously on the CPU which received the interrupt.

@mhorne Do you want to commit or shall I?

In D25531#564411, @mhorne wrote:

Based on what the PLIC spec describes, reading/writing from the claim register is what distinguishes a claim and a completion. You are right that we should wait until after the ithread has done its work to signal completion of the interrupt.

Presumably you observed receiving the interrupt twice?

In D25531#564377, @jrtc27 wrote:

Are there not issues with a device driver disabling interrupts in its handler (either synchronous or asynchronous) for whatever reason and then that being clobbered by the plic_post_filter method? That already happened with pic_post_ithread... But we shouldn't need to disable/enable any more at all? An interrupt is suppressed the entire time we have it claimed, I assume the disable/enable code is there so we don't keep getting interrupts when using an ithread.

For those playing along at home, from 'Volume II: RISC-V Privileged Architectures V20190608-Priv-MSU-Ratified', '4.4.1 Addressing and Memory Protection', page 72: "The PTE format for Sv39 is shown in Figure 4.18. Bits 9–0 have the same meaning as for Sv32. Bits 63–54 are reserved for future use and must be zeroed by software for forward compatibility.".

In D25494#563540, @freqlabs wrote:

My current plan is to implement as much of the generally useful functionality of ifconfig in libifconfig as I can get away with. I think it would be good to start replacing bits of ifconfig with libifconfig, but I don't have an idea what a good timeline for that is, since I don't know if there is some reason it hasn't been started already.

Without immediate users for this it's probably a bit early to commit this, but I like the notion.
If nothing else it gives a bit more sample code of how we can wrap things in Lua, and I could also see use cases for base-system scripting where we may want something other than shell to do things in.

Add a comment to indicate this is a FreeBSD-specific change.

Change the LLVM default

Move the check outside of the lock.

Still happy on RISC-V.

That sounds good to me too.

In D25210#556612, @jrtc27 wrote:

That test doesn't show anything, we know the flag is required by default, and there's no easy way for Clang to know what your linker is at compile time (it doesn't get LDFLAGS normally, and you don't know what /usr/bin/ld is).

I run into this trying to build ports, but this is the simplest reproduction case:

With this patch python3 builds and runs on RISC-V.

In D25084#552683, @melifaro wrote:

Minor nit: any chance jails a and b can be named bridge_stp_a and bridge_stp_b to avoid potential clashes with other tests?

We no longer require jq

Looks sane to me (and passes regression tests), but I'm not the expert on this part of the stack,

Yeah, with these changes my CURRENT kernel / 12.1 user land machine starts bhyve just fine.

Add a comment explaining that the RX/TX descriptor limit must be a power of two.

Works for me (as in: I've tested this in my setup, and this works just fine).

Thanks!

It'd be nice (but it's not required for this to proceed) to have a test to exercise this, i.e. create a table with counters, send some traffic through, check that the counters incremented.

Fair enough. I have no immediate plans to dig deeper into this part of the code. I'd run into this a while back, and figured it made sense to remove unused arguments. I finally got around to this particular item on my todo list. If there's pending work that'll rely on it that's clearly not a good idea at this point.

In D24684#543497, @lwhsu wrote:

There are some non-rfc1918 IPs used in these cases. Are they necessary?