-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generalize interrupt controllers and implement support for the in-kernel GICv3 in HVF #280
Conversation
Creating this one as a draft since it depends on #256. I'll rebase it once that one gets merged. |
0e13014
to
10fc897
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some very minor comments/questions, none of which are blocking or need to be addressed immediately.
serial: Arc<Mutex<devices::legacy::Serial>>, | ||
) -> Result<()> { | ||
if self.irq > self.last_irq { | ||
return Err(Error::IrqsExhausted); | ||
} | ||
|
||
if let Some(intc) = intc { | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not blocking, but why does setting the serial interrupt controller and irq line need to be in a separate scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures the Mutex
we acquire with serial.lock().unwrap()
is dropped as soon as possible. We could also do an explicit drop(serial)
, but this looks cleaner and it's less prone to errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. Thanks!
if let Some(boot_sender) = boot_sender { | ||
boot_senders.insert(vcpu.get_mpidr(), boot_sender); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you do this above when checking the cpu index to avoid the if let
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. This needs to happen after the Vcpu
object has been created (to be able to obtain its mpidr
). And we need to have created both boot_sender
and boot_receiver
before creating the Vcpu
, so I think this is the only option (but I'm open to suggestions! ;-)).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes... not sure how I missed that detail
So far we didn't have a general way for creating an managing IRQ chips. On KVM systems, the in-kernel chip was implicitly created by vstate, while on HVF the userspace gicv3 was created explicitly. In this change we generalize IRQ management behind IrqChip, and we make the creation of the IRQ chip explicit for every platform. This is a pretty large commit, but all these changes must be done in an atomic way. Signed-off-by: Sergio Lopez <slp@redhat.com>
With HVF, we generate MPIDR by taking Vcpu.id and setting Aff1 to that value by doing a left-shift, as this is what the GIC expects to be found in the distributor. But, in vstate::Vcpu::configure_aarch64, we were using the Vcpu.id without left-shifting it. For consistency, let's generate MPIDR for the Vcpu once and use it everywhere. Signed-off-by: Sergio Lopez <slp@redhat.com>
Re-generate them from SDK 15.0. Signed-off-by: Sergio Lopez <slp@redhat.com>
macOS 15 extended Hypervisor.framework with an in-kernel HVF GICv3 device. Using it reduces the cost of GIC operations and enables us to use nested virt. Add support for it and enable it automatically when the functions are found in HVF. Signed-off-by: Sergio Lopez <slp@redhat.com>
@jakecorrenti if you have a minute, please let me know if I have addressed all your comments. I'd like to get this one and #288 for the next release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just re-tested chroot_vm and boot_efi examples. Don't seem to be any issues on my end.
This series tackles a much needed and long due overhaul of the interrupt system to normalize it between architectures and hypervisors. Then, on top of that, it implements support for the in-kernel GICv3 that's present in Hypervisor.framework, as included in macOS Sequoia.