virtio-device: extend VirtioDeviceActions Trait #291
Conversation
joaopeixoto13
requested review from
alexandruag,
andreeaflorescu,
jiangliu,
slp,
stsquad,
epilys and
stefano-garzarella
as code owners
joaopeixoto13
force-pushed
the
virtio-actions
branch
from
c22649d
to
b801a26
Compare
|
Update: I already have a raw implementation that allows me to verify the PR to use the VirtioDeviceActions trait to accommodate not only VirtIO backends but also Vhost and Vhost-user backends. In this case, it was tested and proven to work for: VirtIO-Net and VirtIO-Block, Vhost-user-fs, Vhost-user-vsock and Vhost-vsock. |
| let config_space = &self.borrow().config_space; | ||
| let config_len = config_space.len(); | ||
| if offset >= config_len { | ||
| error!("Failed to read from config space"); | ||
| return; | ||
| } | ||
|
|
||
| // TODO: Are partial reads ok? | ||
| let end = cmp::min(offset.saturating_add(data.len()), config_len); | ||
| let read_len = end - offset; | ||
| // Cannot fail because the lengths are identical and we do bounds checking beforehand. | ||
| data[..read_len].copy_from_slice(&config_space[offset..end]) | ||
| <Self as VirtioDeviceActions>::read_config(self, offset, data) | ||
| } |
There was a problem hiding this comment.
Now read_config is delegated to VirtioDeviceActions, but that means T must implement it now, whereas it did it not have to before. Or did I miss something? It seems like you're taking away functionality here, not adding.
There was a problem hiding this comment.
@epilys You are right. I have already placed the default implementations within the VirtioDeviceActions trait. Only the devices that need to invoke some additional logic must implement it. For example, for the vhost-user devices and when the driver intends to read from or write to the device configuration space, if the protocol feature CONFIG is negotiated, dedicated logic is necessary to handle these operations (e.g. GET_CONFIG/SET_CONFIG requests).
joaopeixoto13
force-pushed
the
virtio-actions
branch
from
b801a26
to
1140db0
Compare
joaopeixoto13
force-pushed
the
virtio-actions
branch
from
bf3f970
to
9d00018
Compare
joaopeixoto13
force-pushed
the
virtio-actions
branch
3 times, most recently
from
a722cf2
to
506262a
Compare
This commit enhances the VirtioDeviceActions Trait to accommodate Vhost and Vhost-User devices effectively. It introduces four new methods to the VirtioDeviceActions trait to facilitate seamless interaction with these devices: - `read_config` and `write_config`: These methods are invoked when the driver intends to read from or write to the device configuration space. Given that the device configuration space can be managed by various handlers outside of the VMM, such as vhost-user when the protocol feature CONFIG is negotiated, dedicated logic is necessary to handle these operations (e.g. GET_CONFIG/SET_CONFIG requests). - `negotiate_driver_features`: This method is called when the driver finishes the negotiation of the device features with the frontend device (selecting page 0). This method is crucial when the device handler is implemented outside of the VMM since the frontend device needs to negotiate the features with the backend device. Otherwise, the device will not be prepared to support, for example, multiple queues and configuration space reads and writes. - `interrupt_status`: When the driver requires reading the interrupt status from the device, this method is invoked. Since the responsibility for managing interrupt status lies with the frontend device, specialized logic is needed to update the interrupt status appropriately (Used Buffer Notification or Configuration Change Notification). If the device is implemented within the VMM, the interrupt status is direct management and updating by the device. Signed-off-by: João Peixoto <joaopeixotooficial@gmail.com>
joaopeixoto13
force-pushed
the
virtio-actions
branch
from
506262a
to
8b2214b
Compare
Sorry, I thought I was in the latest version. I believe everything is now in order. |
Summary of the PR
This PR enhances the VirtioDeviceActions Trait to allow the VMM to leverage the VirtIO MMIO implementation for not only VirtIO devices but also for vhost and vhost-user devices.
Problem:
Solution:
read_configandwrite_configmethods to allow the VMM to instead of executing MMIO writes and reads within the vm-virtio workspace, using the set_config and get_config methods provided by the Rust-VMM vhost workspace.negotiate_driver_featuresmethod to allow the VMM to exchange the driver and protocol features with the backend device.interrupt_statusmethod to allow the VMM to, for example, read the configuration space from the backend device and check for any changes. If the device configuration space has changed, the VMM may update the interrupt status register by setting bit 1. Conversely, if no changes are detected, the VMM may set the bit 0, signaling a used buffer notification. This applies to the backends that do not use the VHOST_USER_BACKEND_CONFIG_CHANGE_MSG message to notify about configuration changes (which I think is the case for all Rust-VMM vhost-user backends). Otherwise, the VMM can simply handle the message and update the interrupt status register accordingly.Validation:
I already have a raw implementation that allows me to verify this PR.
In this case, it was tested and proven to work for: VirtIO-Net, VirtIO-Block, VirtIO-Console, Vhost-user-fs, Vhost-user-vsock and Vhost-vsock.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.