Add IntrPin::import_state and migrate LPC UART pin states
#669
Conversation
|
Clippy is now complaining about some code that didn't change in this branch, must be lints added in the latest Rust version. |
Currently, interrupt pin state for userspace-emulated devices, like the LPC UART, is accounted for both by the kernel in Bhyve and userspace in Propolis. When userspace wishes to assert an IRQ for such a device, it records the assertion of the IRQ line in userspace and then calls the ioctl that asserts the IRQ in kernelspace. However, when such devices are migrated, only the kernelspace Bhyve state is migrated, and any userspace-tracked pin assertion counts are not. This means that these states may become out of sync during a migration, potentially resulting in a lost interrupt. This branch introduces a mechanism for migrating userspace's understanding of pin counts, in addition to migrating kernelspace's. Because incrementing the userspace-tracked pin counts through the IntrPin::assert or IntrPin::set_state methods may generate an ioctl that changes the kernelspace state of that interrupt pin, we cannot just call those methods while importing userspace pin assertion counts. Instead, I've introduced a new import_state method to the IntrPin trait, which is intended to be called only during imports. This method sets the userspace-tracked pin state, the same as set_state, but will not perform an ioctl call. This way, we can import the userspace state independently of the kernelspace state, without triggering additional interrupts that don't actually exist. I've modified the LPC UART to use this new mechanism. I've manually tested this change by running a migration between two lab machines while mashing keys into the serial console, and the console does not appear to get stuck. Fixes #390
| @@ -27,6 +27,8 @@ pub trait IntrPin: Send + Sync + 'static { | |||
| self.deassert(); | |||
| } | |||
| } | |||
|
|
|||
| fn import_state(&self, is_asserted: bool); | |||
There was a problem hiding this comment.
I know the rest of this trait is missing doc comments, but for something like this which is less obvious in its use, a good explanation would be useful for both consumers and implementers.
There was a problem hiding this comment.
How do you feel about the comment from 1412f9c? I'd love any suggestions on how to describe this more clearly.
There was a problem hiding this comment.
I suppose I would focus more on the edge not being visible to the consumer. In the case of FuncPin, we're not emitting guest-sourced events to the greater hypervisor (propolis-server). Including a specific example of the kernel-vmm-managed state being left unchanged as well would probably also be appropriate.
This method is intended for use when importing a guest.
I'd probably move this up in the description, since it's key info about the motivations here.
There was a problem hiding this comment.
Okay, I gave it another pass. Hopefully this one isn't too wordy.
Currently, interrupt pin state for userspace-emulated devices, like the
LPC UART, is accounted for both by the kernel in Bhyve and userspace in
Propolis. When userspace wishes to assert an IRQ for such a device, it
records the assertion of the IRQ line in userspace and then calls the
ioctlthat asserts the IRQ in kernelspace. However, when such devicesare migrated, only the kernelspace Bhyve state is migrated, and any
userspace-tracked pin assertion counts are not. This means that these
states may become out of sync during a migration, potentially resulting
in a lost interrupt.
This branch introduces a mechanism for migrating userspace's
understanding of pin counts, in addition to migrating kernelspace's.
Because incrementing the userspace-tracked pin counts through the
IntrPin::assertorIntrPin::set_statemethods may generate anioctlthat changes the kernelspace state of that interrupt pin, wecannot just call those methods while importing userspace pin assertion
counts. Instead, I've introduced a new
import_statemethod to theIntrPintrait, which is intended to be called only during imports.This method sets the userspace-tracked pin state, the same as
set_state, but will not perform anioctlcall. This way, we canimport the userspace state independently of the kernelspace state,
without triggering additional interrupts that don't actually exist. I've
modified the LPC UART to use this new mechanism.
I've manually tested this change by running a migration between two lab
machines while mashing keys into the serial console, and the console
does not appear to get stuck.