-
Notifications
You must be signed in to change notification settings - Fork 107
Refine the VirtioDevice trait #10
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
Conversation
Refine the VirtioDevice trait to better support virtio backends. Also introduce VirtioQueueConfig and VirtioDeviceConfig to manage virtio device configuration information. Signed-off-by: Xingjun Liu <[email protected]> Signed-off-by: Liu Jiang <[email protected]>
|
@andreeaflorescu you may want to have a look here. |
|
Yap, I'll take a look next week once I am back home. |
andreeaflorescu
left a comment
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 was also working on a design for the virtio devices, but it's not really complete. Should we sync and see how we can merge them?
| } | ||
|
|
||
| /// Get all irqfds associated with vritio queues. | ||
| pub fn get_vring_irqfds(&self) -> Vec<&EventFd> { |
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.
Is there a usecase where both LegacyIrq and MsiIrq are supported at the same time? It looks like the implementation of the DeviceConfig is wildly different depending on the interrupt system. Each function here is converted in a match on the type of interrupt.
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.
Per PCI spec, a device will work in Legacy mode after reset, and may switch to/from PCI MSI/MSI-x mode when OS programs the interrupt configuration registers. So it's a must for a PCI device to support both legacy and MSI/MSIx interrupts.
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.
In this case, can we have VirtioDeviceConfig as a trait as well? What we have here can be the implementation of the trait and in Firecracker's case we can implement our own Device Configuration which should be much simpler.
|
On Mon, Nov 04, 2019 at 04:20:48AM -0800, Andreea Florescu wrote:
andreeaflorescu commented on this pull request.
> use std::sync::Arc;
+use vm_device::interrupt::{InterruptIndex, InterruptSourceGroup, InterruptSourceType};
We have to think this through. Right now it looks like a dependency on vm-device from vm-virtio will introduce a cyclic dependency between those two. vm-virtio device depends on vm-device because it needs interrupts and vm-device depends on vm-virtio because the DeviceManager manages devices including VirtioDevices.
I think DeviceManager or the whole vm-device crate should not have any
dependency against vm-virtio. DeviceManager will manage devices
implementing a set of vm-device defined traits (like e.g. the IO one).
Cheers,
Samuel.
|
|
On Tue, Nov 05, 2019 at 01:33:25AM -0800, Andreea Florescu wrote:
> use std::sync::Arc;
+use vm_device::interrupt::{InterruptIndex, InterruptSourceGroup, InterruptSourceType};
Ok, I'll look again at the vm-device crate. What PR should I look at to understand the design of interrupts?
|
|
This has been moved to vm-device, so close it. |

This is an RFC PR, which depends on rust-vmm/vm-device#9 and rust-vmm/vm-device#11
It refines the VirtioDevice trait to better support virtio backends.
Also introduce VirtioQueueConfig and VirtioDeviceConfig to manage
virtio device configuration information.
Signed-off-by: Xingjun Liu [email protected]
Signed-off-by: Liu Jiang [email protected]