You can add options EARLY_PRINTF=sbi to get earlier debugging output.

In D37981#1374782, @jhb wrote:

I tripped over this again today when I had a panic in the callout code while holding the CC_LOCK spin mutex. First regression is that I can't get a dump in this case because the watchdog code now does too much work when called from the dump routines and tries to always invoke callout_* which does a nested panic. But now with this change, reset from DDB doesn't work either. What "saves" me in bhyve is that the recursive panics from reset eventually exhaust the stack resulting in a double fault (and I guess a triple fault?) that lets bhyve exit. I definitely would have expected reset to be a lighter-weight path that is more likely to work in tight spots than reboot, so I would prefer if reset was the /s path.

PMC class changes LGTM.

Thanks!

In D60072#1379055, @markj wrote:

I suspect that using pm_active to store hart IDs instead of CPU IDs is too clever and will cause more problems down the road. We should instead store CPU IDs there and make the SBI routines handle translation to hart IDs.

LGTM, with one question.

Needs rebase on top of D60004? (merged)

This one requires a bit more study on my part...

Thanks! I have a half-baked implementation of this and I encountered the same issue with OFW interface. There are useful functions implemented by libfdt which we cannot use, e.g. fdt_stringlist_get(). I wondered about what a change to the OFW interface would look like to accommodate something like this...

LGTM! Please update the change description and I can merge later today.

In D59992#1380497, @bscott_bunyatech.com.au wrote:

The code looks good and I've had it running in final form for a few days, and a few few weeks in a similar version.
Haven't really exercised the pinctrl function itself because it's very hard to know if it is doing the right thing without a shed load of diagnostics.
I can confirm that it doesn't seem to impact any existing GPIO functions from both userspace and other kernel drivers.

My question is: why does pmc_process_csw_stop_all() mess with the pps_cpustate fields in the first place? Why not leave it to this code, whose purpose is to handle this?

Needs rebase + refinement.

Include some missed items.

I do not feel able to review this, but I will merge today if I can.

Sure. I admit that I do not see why this is advantageous one way or the other. In either case two NMIs must be processed, no?

LGTM!

I think this is quite clean.

Looking pretty good!

@mmel will know for sure.

Good catch.

I am ready to commit this and have applied my suggested changes locally. Please confirm that you are happy with the copyright header text, and I will proceed.

Glad to see this moving forward. I only skimmed the overall structure, and in particular I am not confident about MSI code.

Note, the eqos(4) driver has an uncertain future. It is possible that we will move to supporting this hardware with the dwc(4) driver instead; see https://reviews.freebsd.org/D59191 if interested.

Generally LGTM, with some minor tweaks needed.