On 2/8/23 10:05 AM, Mark Johnston wrote:
On Sun, Jan 15, 2023 at 10:46:21AM -0500, Mark Johnston wrote:
On Wed, Nov 23, 2022 at 08:00:16AM +0000, Corvin Köhne wrote:
The branch main has been updated by corvink:
URL:
https://cgit.FreeBSD.org/src/commit/?id=7c326ab5bb9aced8dcbc2465ac1c9ff8df2ba46b
commit 7c326ab5bb9aced8dcbc2465ac1c9ff8df2ba46b
Author: Corvin Köhne <[email protected]>
AuthorDate: 2022-11-21 14:00:04 +0000
Commit: Corvin Köhne <[email protected]>
CommitDate: 2022-11-23 08:00:04 +0000
vmm: don't lock a mtx in the icr_low write handler
x2apic accesses are handled by a wrmsr exit. This handler is called in a
critical section. So, we can't lock a mtx in the icr_low handler.
Reported by: kp, pho
Tested by: kp, pho
Approved by: manu (mentor)
Fixes: c0f35dbf19c3c8825bd2b321d8efd582807d1940 vmm: Use
a cpuset_t for vCPUs waiting for STARTUP IPIs.
MFC after: 1 week
MFC with: c0f35dbf19c3c8825bd2b321d8efd582807d1940
Sponsored by: Beckhoff Automation GmbH & Co. KG
Differential Revision: https://reviews.freebsd.org/D37452
---
Hi Corvin,
This seems to break AP startup when using bhyve/libvmmapi from FreeBSD
13.1 on a kernel built from main. It looks like the commit somehow
regresses commit 769b884e2e2, but I'm not sure yet exactly how.
I debugged this further and am not quite sure how to fix the problem,
which isn't specific to this commit after all. I'll try to describe it
here.
Suppose we're booting a VM with 2 vCPUs. When the BSP raises the INIT
IPI to start AP 1, vlapic_icrlo_write_handler() looks up the destination
vCPU with vlapic_calcdest(), which only returns active vCPUs. However,
old bhyve executables activate APs (i.e., call vm_activate_cpu())
lazily, only upon receiving a VM_EXITCODE_SPINUP_AP message. Thus,
vm_handle_ipi() simply doesn't doesn't do anything since "dmask" is
empty, so APs don't boot up.
To further complicate things, new vmm.ko allocates vCPUs lazily. New
bhyve executables call vm_activate_cpu() for all vCPUs before running
the BSP, but as I said above, old bhyve executables do not. So merely
fixing "dmask" in vlapic_icrlo_write_handler() to include
not-yet-activated vCPUs doesn't work, and we can't allocate a new vCPU
in that context. In general it seems that we want an INIT IPI to
trigger allocation of a vcpu structure to preserve compatibility with
old bhyve, but I don't see a good way to implement that.
I would quite like to fix this since I make heavy use of 13.1-RELEASE
bhyve+jails on a kernel running main. I believe bhyve from stable/13 is
unaffected, but 13.2 is not yet released. Any suggestions would be
appreciated.
Hmm, I thought I had fixed this by using the bitmask of started CPUs
rather than requiring the vCPU to be allocated. I was definitely testing
an old bhyve binary from head against the vCPU branch while working on it
and remember hitting this exact case, but I thought I had fixed it. Oh,
hmm, my fix was the commit quoted above (769b884e2e2).
--
John Baldwin
