Re: [PULL v4 29/83] virtio: introduce virtio_queue_enable()

[Laszlo Ersek <lersek@redhat.com>]

On 11/09/22 07:59, Michael S. Tsirkin wrote:
> On Wed, Nov 09, 2022 at 01:52:01AM -0500, Michael S. Tsirkin wrote:
>> On Wed, Nov 09, 2022 at 11:31:23AM +0800, Jason Wang wrote:
>>> On Wed, Nov 9, 2022 at 3:43 AM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>>>
>>>> On Mon, 7 Nov 2022 at 18:10, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>
>>>>> From: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>>>>>
>>>>> Introduce the interface queue_enable() in VirtioDeviceClass and the
>>>>> fucntion virtio_queue_enable() in virtio, it can be called when
>>>>> VIRTIO_PCI_COMMON_Q_ENABLE is written and related virtqueue can be
>>>>> started. It only supports the devices of virtio 1 or later. The
>>>>> not-supported devices can only start the virtqueue when DRIVER_OK.
>>>>>
>>>>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>>>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>>> Message-Id: <20221017092558.111082-4-xuanzhuo@linux.alibaba.com>
>>>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>> ---
>>>>>  include/hw/virtio/virtio.h |  2 ++
>>>>>  hw/virtio/virtio.c         | 14 ++++++++++++++
>>>>>  2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>>> index 74d76c1dbc..b00b3fcf31 100644
>>>>> --- a/include/hw/virtio/virtio.h
>>>>> +++ b/include/hw/virtio/virtio.h
>>>>> @@ -149,6 +149,7 @@ struct VirtioDeviceClass {
>>>>>      void (*reset)(VirtIODevice *vdev);
>>>>>      void (*set_status)(VirtIODevice *vdev, uint8_t val);
>>>>>      void (*queue_reset)(VirtIODevice *vdev, uint32_t queue_index);
>>>>> +    void (*queue_enable)(VirtIODevice *vdev, uint32_t queue_index);
>>>>>      /* For transitional devices, this is a bitmap of features
>>>>>       * that are only exposed on the legacy interface but not
>>>>>       * the modern one.
>>>>> @@ -288,6 +289,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
>>>>>  int virtio_set_status(VirtIODevice *vdev, uint8_t val);
>>>>>  void virtio_reset(void *opaque);
>>>>>  void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
>>>>> +void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
>>>>>  void virtio_update_irq(VirtIODevice *vdev);
>>>>>  int virtio_set_features(VirtIODevice *vdev, uint64_t val);
>>>>>
>>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>>> index cf5f9ca387..9683b2e158 100644
>>>>> --- a/hw/virtio/virtio.c
>>>>> +++ b/hw/virtio/virtio.c
>>>>> @@ -2495,6 +2495,20 @@ void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
>>>>>      __virtio_queue_reset(vdev, queue_index);
>>>>>  }
>>>>>
>>>>> +void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
>>>>> +{
>>>>> +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>>>>> +
>>>>> +    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>>>>> +        error_report("queue_enable is only suppported in devices of virtio "
>>>>> +                     "1.0 or later.");
>>>>
>>>> Why is this triggering here? Maybe virtio_queue_enable() is called too
>>>> early. I have verified that the Linux guest driver sets VERSION_1. I
>>>> didn't check what SeaBIOS does.
>>>
>>> Looks like a bug, we should check device features here at least and it
>>> should be guest errors instead of error_report() here.
>>>
>>> Thanks
>>
>> But it's weird, queue enable is written before guest features?
>> How come?
>
> It's a bios bug:
>
>
>
>     vp_init_simple(&vdrive->vp, pci);
>     if (vp_find_vq(&vdrive->vp, 0, &vdrive->vq) < 0 ) {
>         dprintf(1, "fail to find vq for virtio-blk %pP\n", pci);
>         goto fail;
>     }
>
>     if (vdrive->vp.use_modern) {
>         struct vp_device *vp = &vdrive->vp;
>         u64 features = vp_get_features(vp);
>         u64 version1 = 1ull << VIRTIO_F_VERSION_1;
>         u64 iommu_platform = 1ull << VIRTIO_F_IOMMU_PLATFORM;
>         u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE;
>         if (!(features & version1)) {
>             dprintf(1, "modern device without virtio_1 feature bit: %pP\n", pci);
>             goto fail;
>         }
>
>         features = features & (version1 | iommu_platform | blk_size);
>         vp_set_features(vp, features);
>         status |= VIRTIO_CONFIG_S_FEATURES_OK;
>         vp_set_status(vp, status);
>
>
>
> Not good - does not match the spec. Here's what the spec says:
>
>
> The driver MUST follow this sequence to initialize a device:
> 1. Reset the device.
> 2. Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
> 3. Set the DRIVER status bit: the guest OS knows how to drive the device.
> 4. Read device feature bits, and write the subset of feature bits understood by the OS and driver to the
> device. During this step the driver MAY read (but MUST NOT write) the device-specific configuration
> fields to check that it can support the device before accepting it.
> 5. Set the FEATURES_OK status bit. The driver MUST NOT accept new feature bits after this step.
> 6. Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, the device does not
> support our subset of features and the device is unusable.
> 7. Perform device-specific setup, including discovery of virtqueues for the device, optional per-bus setup,
> reading and possibly writing the device’s virtio configuration space, and population of virtqueues.
> 8. Set the DRIVER_OK status bit. At this point the device is “live”.

Thanks for the Cc.

This should work properly in the edk2 (OVMF) guest drivers. When I
started work on the virtio-1.0 implementation, I noticed that the device
initialization sequence that guest drivers needed to follow had
*changed* from spec version 0.9.5 to spec version 1.0.

For example, in the virtio-rng driver, I addressed that with commit
0a781bdc7f87 ("OvmfPkg: VirtioRngDxe: adapt feature negotiation to
virtio-1.0", 2016-04-06):

  https://github.com/tianocore/edk2/commit/0a781bdc7f87

Therefore we have a larger context like this in edk2 (again the excerpt
is from the virtio-rng driver, but all drivers follow this pattern whose
devices can be either legacy or modern):

>   //
>   // Execute virtio-0.9.5, 2.2.1 Device Initialization Sequence.
>   //
>   NextDevStat = 0;             // step 1 -- reset device
>   Status      = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }

matches v1.0 spec step 1.

>
>   NextDevStat |= VSTAT_ACK;    // step 2 -- acknowledge device presence
>   Status       = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }

matches v1.0 spec step 2.

>
>   NextDevStat |= VSTAT_DRIVER; // step 3 -- we know how to drive it
>   Status       = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }

matches v1.0 spec step 3.

>
>   //
>   // Set Page Size - MMIO VirtIo Specific
>   //
>   Status = Dev->VirtIo->SetPageSize (Dev->VirtIo, EFI_PAGE_SIZE);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // step 4a -- retrieve and validate features
>   //
>   Status = Dev->VirtIo->GetDeviceFeatures (Dev->VirtIo, &Features);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }

matches v1.0 spec step 4, the "read" part

>
>   Features &= VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM;
>
>   //
>   // In virtio-1.0, feature negotiation is expected to complete before queue
>   // discovery, and the device can also reject the selected set of features.
>   //
>   if (Dev->VirtIo->Revision >= VIRTIO_SPEC_REVISION (1, 0, 0)) {
>     Status = Virtio10WriteFeatures (Dev->VirtIo, Features, &NextDevStat);
>     if (EFI_ERROR (Status)) {
>       goto Failed;
>     }
>   }

This is skipped for virtio-0.9.5. For virtio-1.0,
Virtio10WriteFeatures() does the following:

>   Status = VirtIo->SetGuestFeatures (VirtIo, Features);
>   if (EFI_ERROR (Status)) {
>     return Status;
>   }

Covers v1.0 spec step 4, the rest of it.

>
>   *DeviceStatus |= VSTAT_FEATURES_OK;
>   Status         = VirtIo->SetDeviceStatus (VirtIo, *DeviceStatus);
>   if (EFI_ERROR (Status)) {
>     return Status;
>   }

Covers v1.0 spec step 5.

>
>   Status = VirtIo->GetDeviceStatus (VirtIo, DeviceStatus);
>   if (EFI_ERROR (Status)) {
>     return Status;
>   }
>
>   if ((*DeviceStatus & VSTAT_FEATURES_OK) == 0) {
>     Status = EFI_UNSUPPORTED;
>   }

Covers v1.0 spec step 6.

OK, return to the previous call frame:

>
>   //
>   // step 4b -- allocate request virtqueue, just use #0
>   //
>   Status = Dev->VirtIo->SetQueueSel (Dev->VirtIo, 0);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   Status = Dev->VirtIo->GetQueueNumMax (Dev->VirtIo, &QueueSize);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // VirtioRngGetRNG() uses one descriptor
>   //
>   if (QueueSize < 1) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   Status = VirtioRingInit (Dev->VirtIo, QueueSize, &Dev->Ring);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // If anything fails from here on, we must release the ring resources.
>   //
>   Status = VirtioRingMap (
>              Dev->VirtIo,
>              &Dev->Ring,
>              &RingBaseShift,
>              &Dev->RingMap
>              );
>   if (EFI_ERROR (Status)) {
>     goto ReleaseQueue;
>   }
>
>   //
>   // Additional steps for MMIO: align the queue appropriately, and set the
>   // size. If anything fails from here on, we must unmap the ring resources.
>   //
>   Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize);
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   //
>   // step 4c -- Report GPFN (guest-physical frame number) of queue.
>   //
>   Status = Dev->VirtIo->SetQueueAddress (
>                           Dev->VirtIo,
>                           &Dev->Ring,
>                           RingBaseShift
>                           );
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }

These cover v1.0 spec step 7.

>
>   //
>   // step 5 -- Report understood features and guest-tuneables.
>   //
>   if (Dev->VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0)) {
>     Features &= ~(UINT64)(VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM);
>     Status    = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features);
>     if (EFI_ERROR (Status)) {
>       goto UnmapQueue;
>     }
>   }

This is exclusive to virtio-0.9.5; the "step 5" reference in the comment
pertains to that spec version. In virtio-0.9.5, this is the location
(just before setting DRIVER_OK) where the guest driver has to report its
desired features (and the device has no means to reject any feature set
that is otherwise a subset of the host feature set).

>
>   //
>   // step 6 -- initialization complete
>   //
>   NextDevStat |= VSTAT_DRIVER_OK;
>   Status       = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }

This covers v1.0 spec step 8 (and v0.9.5 spec step 6, as the comment
shows).

Now, in drivers whose devices are virtio-1.0-only, such as
virtio-gpu-pci, vhost-user-fs-pci, this "selectivity" is not there. The
virtio-0.9.5 branch is simply eliminated, and the virtio-1.0 logic is
unconditional. Additionally, in those drivers, the "step" references in
the code comments match the v1.0 bullet list. For example, in the
virtio-fs driver:

> EFI_STATUS
> VirtioFsInit (
>   IN OUT VIRTIO_FS  *VirtioFs
>   )
> {
>   UINT8             NextDevStat;
>   EFI_STATUS        Status;
>   UINT64            Features;
>   VIRTIO_FS_CONFIG  Config;
>   UINTN             Idx;
>   UINT64            RingBaseShift;
>
>   //
>   // Execute virtio-v1.1-cs01-87fa6b5d8155, 3.1.1 Driver Requirements: Device
>   // Initialization.
>   //
>   // 1. Reset the device.
>   //
>   NextDevStat = 0;
>   Status      = VirtioFs->Virtio->SetDeviceStatus (VirtioFs->Virtio, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 2. Set the ACKNOWLEDGE status bit [...]
>   //
>   NextDevStat |= VSTAT_ACK;
>   Status       = VirtioFs->Virtio->SetDeviceStatus (VirtioFs->Virtio, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 3. Set the DRIVER status bit [...]
>   //
>   NextDevStat |= VSTAT_DRIVER;
>   Status       = VirtioFs->Virtio->SetDeviceStatus (VirtioFs->Virtio, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 4. Read device feature bits...
>   //
>   Status = VirtioFs->Virtio->GetDeviceFeatures (VirtioFs->Virtio, &Features);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   if ((Features & VIRTIO_F_VERSION_1) == 0) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   //
>   // No device-specific feature bits have been defined in file "virtio-fs.tex"
>   // of the virtio spec at <https://github.com/oasis-tcs/virtio-spec.git>, as
>   // of commit 87fa6b5d8155.
>   //
>   Features &= VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM;
>
>   //
>   // ... and write the subset of feature bits understood by the [...] driver to
>   // the device. [...]
>   // 5. Set the FEATURES_OK status bit.
>   // 6. Re-read device status to ensure the FEATURES_OK bit is still set [...]
>   //
>   Status = Virtio10WriteFeatures (VirtioFs->Virtio, Features, &NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 7. Perform device-specific setup, including discovery of virtqueues for
>   // the device, [...] reading [...] the device's virtio configuration space
>   //
>   Status = VirtioFsReadConfig (VirtioFs->Virtio, &Config);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 7.a. Convert the filesystem label from UTF-8 to UCS-2. Only labels with
>   // printable ASCII code points (U+0020 through U+007E) are supported.
>   // NUL-terminate at either the terminator we find, or right after the
>   // original label.
>   //
>   for (Idx = 0; Idx < VIRTIO_FS_TAG_BYTES && Config.Tag[Idx] != '\0'; Idx++) {
>     if ((Config.Tag[Idx] < 0x20) || (Config.Tag[Idx] > 0x7E)) {
>       Status = EFI_UNSUPPORTED;
>       goto Failed;
>     }
>
>     VirtioFs->Label[Idx] = Config.Tag[Idx];
>   }
>
>   VirtioFs->Label[Idx] = L'\0';
>
>   //
>   // 7.b. We need one queue for sending normal priority requests.
>   //
>   if (Config.NumReqQueues < 1) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   //
>   // 7.c. Fetch and remember the number of descriptors we can place on the
>   // queue at once. We'll need two descriptors per request, as a minimum --
>   // request header, response header.
>   //
>   Status = VirtioFs->Virtio->SetQueueSel (
>                                VirtioFs->Virtio,
>                                VIRTIO_FS_REQUEST_QUEUE
>                                );
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   Status = VirtioFs->Virtio->GetQueueNumMax (
>                                VirtioFs->Virtio,
>                                &VirtioFs->QueueSize
>                                );
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   if (VirtioFs->QueueSize < 2) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   //
>   // 7.d. [...] population of virtqueues [...]
>   //
>   Status = VirtioRingInit (
>              VirtioFs->Virtio,
>              VirtioFs->QueueSize,
>              &VirtioFs->Ring
>              );
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   Status = VirtioRingMap (
>              VirtioFs->Virtio,
>              &VirtioFs->Ring,
>              &RingBaseShift,
>              &VirtioFs->RingMap
>              );
>   if (EFI_ERROR (Status)) {
>     goto ReleaseQueue;
>   }
>
>   Status = VirtioFs->Virtio->SetQueueAddress (
>                                VirtioFs->Virtio,
>                                &VirtioFs->Ring,
>                                RingBaseShift
>                                );
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   //
>   // 8. Set the DRIVER_OK status bit.
>   //
>   NextDevStat |= VSTAT_DRIVER_OK;
>   Status       = VirtioFs->Virtio->SetDeviceStatus (VirtioFs->Virtio, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   return EFI_SUCCESS;
>
> UnmapQueue:
>   VirtioFs->Virtio->UnmapSharedBuffer (VirtioFs->Virtio, VirtioFs->RingMap);
>
> ReleaseQueue:
>   VirtioRingUninit (VirtioFs->Virtio, &VirtioFs->Ring);
>
> Failed:
>   //
>   // If any of these steps go irrecoverably wrong, the driver SHOULD set the
>   // FAILED status bit to indicate that it has given up on the device (it can
>   // reset the device later to restart if desired). [...]
>   //
>   // Virtio access failure here should not mask the original error.
>   //
>   NextDevStat |= VSTAT_FAILED;
>   VirtioFs->Virtio->SetDeviceStatus (VirtioFs->Virtio, NextDevStat);
>
>   return Status;
> }

In the virtio-gpu driver:

> EFI_STATUS
> VirtioGpuInit (
>   IN OUT VGPU_DEV  *VgpuDev
>   )
> {
>   UINT8       NextDevStat;
>   EFI_STATUS  Status;
>   UINT64      Features;
>   UINT16      QueueSize;
>   UINT64      RingBaseShift;
>
>   //
>   // Execute virtio-v1.0-cs04, 3.1.1 Driver Requirements: Device
>   // Initialization.
>   //
>   // 1. Reset the device.
>   //
>   NextDevStat = 0;
>   Status      = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 2. Set the ACKNOWLEDGE status bit [...]
>   //
>   NextDevStat |= VSTAT_ACK;
>   Status       = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 3. Set the DRIVER status bit [...]
>   //
>   NextDevStat |= VSTAT_DRIVER;
>   Status       = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 4. Read device feature bits...
>   //
>   Status = VgpuDev->VirtIo->GetDeviceFeatures (VgpuDev->VirtIo, &Features);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   if ((Features & VIRTIO_F_VERSION_1) == 0) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   //
>   // We only want the most basic 2D features.
>   //
>   Features &= VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM;
>
>   //
>   // ... and write the subset of feature bits understood by the [...] driver to
>   // the device. [...]
>   // 5. Set the FEATURES_OK status bit.
>   // 6. Re-read device status to ensure the FEATURES_OK bit is still set [...]
>   //
>   Status = Virtio10WriteFeatures (VgpuDev->VirtIo, Features, &NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // 7. Perform device-specific setup, including discovery of virtqueues for
>   // the device [...]
>   //
>   Status = VgpuDev->VirtIo->SetQueueSel (
>                               VgpuDev->VirtIo,
>                               VIRTIO_GPU_CONTROL_QUEUE
>                               );
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   Status = VgpuDev->VirtIo->GetQueueNumMax (VgpuDev->VirtIo, &QueueSize);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // We implement each VirtIo GPU command that we use with two descriptors:
>   // request, response.
>   //
>   if (QueueSize < 2) {
>     Status = EFI_UNSUPPORTED;
>     goto Failed;
>   }
>
>   //
>   // [...] population of virtqueues [...]
>   //
>   Status = VirtioRingInit (VgpuDev->VirtIo, QueueSize, &VgpuDev->Ring);
>   if (EFI_ERROR (Status)) {
>     goto Failed;
>   }
>
>   //
>   // If anything fails from here on, we have to release the ring.
>   //
>   Status = VirtioRingMap (
>              VgpuDev->VirtIo,
>              &VgpuDev->Ring,
>              &RingBaseShift,
>              &VgpuDev->RingMap
>              );
>   if (EFI_ERROR (Status)) {
>     goto ReleaseQueue;
>   }
>
>   //
>   // If anything fails from here on, we have to unmap the ring.
>   //
>   Status = VgpuDev->VirtIo->SetQueueAddress (
>                               VgpuDev->VirtIo,
>                               &VgpuDev->Ring,
>                               RingBaseShift
>                               );
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   //
>   // 8. Set the DRIVER_OK status bit.
>   //
>   NextDevStat |= VSTAT_DRIVER_OK;
>   Status       = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
>   if (EFI_ERROR (Status)) {
>     goto UnmapQueue;
>   }
>
>   return EFI_SUCCESS;
>
> UnmapQueue:
>   VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
>
> ReleaseQueue:
>   VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring);
>
> Failed:
>   //
>   // If any of these steps go irrecoverably wrong, the driver SHOULD set the
>   // FAILED status bit to indicate that it has given up on the device (it can
>   // reset the device later to restart if desired). [...]
>   //
>   // VirtIo access failure here should not mask the original error.
>   //
>   NextDevStat |= VSTAT_FAILED;
>   VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
>
>   return Status;
> }

Laszlo