x86: Halt non-BSP CPUs on panic IPI_STOP

[cem]

Summary

We may need the BSP to reboot, but we don't need any AP CPU that isn't the
panic thread. Any CPU landing in this routine during panic isn't the panic
thread, so we can just detect !BSP && panic and shut down the logical core.

Test Plan

For some reason I thought we had already done something like this, but I'm not
seeing it.

[kib]

The hlt instruction there is a route in one way: only NMI or INIT IPI could unblock the core. Once I had a patch which allowed migrating the active thread in ddb from current cpu to arbitrary core, e.g. to look at the local state like APIC or MSRs not visible outside the CPU.

sys/x86/x86/mp_x86.c

1419 ↗

(On Diff #56519)

Operator is usually kept at the line which is broken.

I think the expression can be simplified as !IS_BSP(), on x86 boot cpu has cpuid 0.

[cem]

In D20019#430395, @kib wrote:

The hlt instruction there is a route in one way: only NMI or INIT IPI could unblock the core. Once I had a patch which allowed migrating the active thread in ddb from current cpu to arbitrary core, e.g. to look at the local state like APIC or MSRs not visible outside the CPU.

We could add a tunable to disable this behavior for such a patch, if you like.

sys/x86/x86/mp_x86.c

1419 ↗

(On Diff #56519)

Thanks, that does look better.

[kib]

In D20019#430453, @cem wrote:

In D20019#430395, @kib wrote:

The hlt instruction there is a route in one way: only NMI or INIT IPI could unblock the core. Once I had a patch which allowed migrating the active thread in ddb from current cpu to arbitrary core, e.g. to look at the local state like APIC or MSRs not visible outside the CPU.

We could add a tunable to disable this behavior for such a patch, if you like.

I do not think it is needed for now, it would only clog the patch.

[cem]

In D20019#430480, @kib wrote:

In D20019#430453, @cem wrote:

We could add a tunable to disable this behavior for such a patch, if you like.

I do not think it is needed for now, it would only clog the patch.

Ok, I will leave it out but fix the IS_BSP condition.

[cem]

Use more straightforward IS_BSP macro

[kib]

sys/x86/x86/mp_x86.c

1400 ↗

(On Diff #56559)

You do not need this variable anymore.

[cem]

sys/x86/x86/mp_x86.c

1400 ↗

(On Diff #56559)

Ah, thanks for catching that. Will fix before commit.

[avg]

A belated note that it is possible use monitor/mwait mechanism to get the power saving and inter-processor signaling at the same time.
I once had it implemented, but that code is quite out of date now. Also, it was interleaved with another, unrelated change.
Some bits:

• https://github.com/avg-I/freebsd/commit/c2acab4675b3f0e10bd17a79b49baa331a19bc92
• https://github.com/avg-I/freebsd/commit/b1abf62dfdfc0e1028a48e033f6de2f2b5f5722d

[cem]

In D20019#431068, @avg wrote:

A belated note that it is possible use monitor/mwait mechanism to get the power saving and inter-processor signaling at the same time.
I once had it implemented, but that code is quite out of date now. Also, it was interleaved with another, unrelated change.

Yes, I'm all for a version that uses that in preference (conditional on the monitor/mwait cpuid bit). It could even be used for the spin, albeit maybe increasing wakeup latency for fast IPI_STOPs (rendezvous operations? No, I guess that's a different IPI). Maybe there is no such issue as IPI_STOP needing low latency wakeup.

[markj]

In D20019#431206, @cem wrote:

In D20019#431068, @avg wrote:

A belated note that it is possible use monitor/mwait mechanism to get the power saving and inter-processor signaling at the same time.
I once had it implemented, but that code is quite out of date now. Also, it was interleaved with another, unrelated change.

Yes, I'm all for a version that uses that in preference (conditional on the monitor/mwait cpuid bit). It could even be used for the spin, albeit maybe increasing wakeup latency for fast IPI_STOPs (rendezvous operations? No, I guess that's a different IPI). Maybe there is no such issue as IPI_STOP needing low latency wakeup.

I thought Anton implemented that in OneFS...?

[cem]

In D20019#432700, @markj wrote:

In D20019#431206, @cem wrote:

Yes, I'm all for a version that uses that in preference (conditional on the monitor/mwait cpuid bit). It could even be used for the spin...

I thought Anton implemented that in OneFS...?

Yes, there is a version there. It doesn’t have the fallback behavior. I’d like to upstream it or at least start discussion. I had forgotten about it when I submitted this reiteration of the same basic idea — I was clearing out an old git tree from 2016 or 2015 to free some disk space and had forgotten the context.

[cem]

See https://reviews.freebsd.org/D20135