On Thu, 29 Aug 2019 18:50:27 +0200
Sergio Lopez <s...@redhat.com> wrote:
> Implement the modern (v2) personality, according to the VirtIO 1.0
> specification.
>
> Support for v2 among guests is not as widespread as it'd be
> desirable. While the Linux driver has had it for a while, support is
> missing, at least, from Tianocore EDK II, NetBSD and FreeBSD.
>
> For this reason, the v2 personality is disabled, keeping the legacy
> behavior as default. Machine types willing to use v2, can enable it
> using MachineClass's compat_props.
>
> Signed-off-by: Sergio Lopez <s...@redhat.com>
> ---
> Changelog:
>
> v2:
> - Switch from RFC to PATCH.
> - Avoid the modern vs. legacy dichotomy. Use legacy or non-legacy
> instead. (Andrea Bolognani, Cornelia Huck)
> - Include the register offset in the warning messages. (Stefan
> Hajnoczi)
> - Fix device endianness for the non-legacy mode. (Michael S. Tsirkin)
> - Honor the specs in VIRTIO_MMIO_QUEUE_READY. (Michael S. Tsirkin)
> ---
> hw/virtio/virtio-mmio.c | 296 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 279 insertions(+), 17 deletions(-)
>
> @@ -146,28 +163,51 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr
> offset, unsigned size)
> case VIRTIO_MMIO_MAGIC_VALUE:
> return VIRT_MAGIC;
> case VIRTIO_MMIO_VERSION:
> - return VIRT_VERSION;
> + if (proxy->legacy) {
> + return VIRT_VERSION_LEGACY;
> + } else {
> + return VIRT_VERSION;
> + }
> case VIRTIO_MMIO_DEVICE_ID:
> return vdev->device_id;
> case VIRTIO_MMIO_VENDOR_ID:
> return VIRT_VENDOR;
> case VIRTIO_MMIO_DEVICE_FEATURES:
> - if (proxy->host_features_sel) {
> - return 0;
> - }
> - return vdev->host_features;
> + return vdev->host_features >> (32 * proxy->host_features_sel);
Hm... I think you want to return 0 for host_features_sel > 0 on legacy
devices.
Also, there's VirtIODeviceClass->legacy_features, which probably should
be masked out for non-legacy devices?
(...)
> @@ -229,17 +275,33 @@ static void virtio_mmio_write(void *opaque, hwaddr
> offset, uint64_t value,
> }
> switch (offset) {
> case VIRTIO_MMIO_DEVICE_FEATURES_SEL:
> - proxy->host_features_sel = value;
> + if (value) {
> + proxy->host_features_sel = 1;
> + } else {
> + proxy->host_features_sel = 0;
> + }
> break;
> case VIRTIO_MMIO_DRIVER_FEATURES:
> - if (!proxy->guest_features_sel) {
> + if (!proxy->legacy) {
> + proxy->guest_features[proxy->guest_features_sel] = value;
> + } else if (!proxy->guest_features_sel) {
> virtio_set_features(vdev, value);
If the guest tries to set something !0 for guest_features_sel > 0 on a
legacy device, should that be logged as a guest bug?
> }
> break;
(...)
Otherwise, looks good to me.
