[PATCH 0/2] Quo vadis virtio-mmio?

[PATCH 0/2] Quo vadis virtio-mmio?

[Gerd Hoffmann]

virtio-mmio support in ovmf seems to be the unloved child.  The final
virto-1.0 specification was published five(!) years ago, nevertheless
the mmio transport doesn't support it yet ...

Some people argue that it has been obsoleted by virtio-pci.  Which is a
valid argument.  But IMHO isn't a good reason to just let virtio-mmio
bitrot.  We should either remove it from the tree, or support it.

So, opening the discussion with this little patch series.  It does the
latter and adds virtio 1.0 support.  For the mmio transport the
difference between 0.9.5 and 1.0 is rather small (when compared to the
pci transport), it is just a bunch of new registers for the changed
virtio queue initialization.  So the patch series is small too ...

take care,
  Gerd

Gerd Hoffmann (2):
  OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines
  OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.

 OvmfPkg/Include/IndustryStandard/Virtio10.h   | 12 +++++++
 .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  1 +
 .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++++++---
 .../VirtioMmioDeviceFunctions.c               | 31 +++++++++++++++++--
 4 files changed, 54 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH 1/2] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines

[Gerd Hoffmann]

Add defines for the config space offsets for virtio 1.0 mmio transport.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/Virtio10.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h b/OvmfPkg/Include/IndustryStandard/Virtio10.h
index 2c60be2b7c0c..a1712247e054 100644
--- a/OvmfPkg/Include/IndustryStandard/Virtio10.h
+++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
@@ -81,4 +81,16 @@ typedef struct {
 #define VIRTIO_F_VERSION_1      BIT32
 #define VIRTIO_F_IOMMU_PLATFORM BIT33
 
+//
+// MMIO VirtIo Header Offsets
+//
+#define VIRTIO_MMIO_OFFSET_QUEUE_READY                0x44
+#define VIRTIO_MMIO_OFFSET_QUEUE_DESC_LO              0x80
+#define VIRTIO_MMIO_OFFSET_QUEUE_DESC_HI              0x84
+#define VIRTIO_MMIO_OFFSET_QUEUE_AVAIL_LO             0x90
+#define VIRTIO_MMIO_OFFSET_QUEUE_AVAIL_HI             0x94
+#define VIRTIO_MMIO_OFFSET_QUEUE_USED_LO              0xa0
+#define VIRTIO_MMIO_OFFSET_QUEUE_USED_HI              0xa4
+#define VIRTIO_MMIO_OFFSET_CONFIG_GENERATION          0xfc
+
 #endif // _VIRTIO_1_0_H_
-- 
2.31.1

[PATCH 2/2] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.

[Gerd Hoffmann]

Add support for virtio 1.0 to the mmio transport.  virtio 0.9.5 uses
page size, page frame number and a fixed layout for the ring.  virtio
1.0 uses the physical addresses for base address, used bits and
available bits instead.

The ring layout is not changed, so a 0.9.5 compatible layout is used in
both 0.9.5 and 1.0 mode to to keep the code differences as small as
possible.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  1 +
 .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++++++---
 .../VirtioMmioDeviceFunctions.c               | 31 +++++++++++++++++--
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
index ab53b90d51c9..8b19996b716f 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
@@ -26,6 +26,7 @@
 
 typedef struct {
   UINT32                 Signature;
+  UINT32                 Version;
   VIRTIO_DEVICE_PROTOCOL VirtioDevice;
   PHYSICAL_ADDRESS       BaseAddress;
 } VIRTIO_MMIO_DEVICE;
diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c
index 6dbbba008c75..a8f78a50861b 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c
@@ -58,7 +58,6 @@ VirtioMmioInit (
   )
 {
   UINT32     MagicValue;
-  UINT32     Version;
 
   //
   // Initialize VirtIo Mmio Device
@@ -66,7 +65,6 @@ VirtioMmioInit (
   CopyMem (&Device->VirtioDevice, &mMmioDeviceProtocolTemplate,
         sizeof (VIRTIO_DEVICE_PROTOCOL));
   Device->BaseAddress = BaseAddress;
-  Device->VirtioDevice.Revision = VIRTIO_SPEC_REVISION (0, 9, 5);
   Device->VirtioDevice.SubSystemDeviceId =
           MmioRead32 (BaseAddress + VIRTIO_MMIO_OFFSET_DEVICE_ID);
 
@@ -78,8 +76,19 @@ VirtioMmioInit (
     return EFI_UNSUPPORTED;
   }
 
-  Version = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_VERSION);
-  if (Version != 1) {
+  Device->Version = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_VERSION);
+  switch (Device->Version) {
+  case 1:
+    DEBUG ((DEBUG_INFO, "%a virtio 0.9.5, id %d\n", __FUNCTION__,
+            Device->VirtioDevice.SubSystemDeviceId));
+    Device->VirtioDevice.Revision = VIRTIO_SPEC_REVISION (0, 9, 5);
+    break;
+  case 2:
+    DEBUG ((DEBUG_INFO, "%a virtio 1.0, id %d\n", __FUNCTION__,
+            Device->VirtioDevice.SubSystemDeviceId));
+    Device->VirtioDevice.Revision = VIRTIO_SPEC_REVISION (1, 0, 0);
+    break;
+  default:
     return EFI_UNSUPPORTED;
   }
 
diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
index b0d75fb1dd24..bf8523a6fb3b 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
@@ -151,7 +151,9 @@ VirtioMmioSetPageSize (
 
   Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
 
-  VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_PAGE_SIZE, PageSize);
+  if (Device->Version == 1) {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_PAGE_SIZE, PageSize);
+  }
 
   return EFI_SUCCESS;
 }
@@ -181,13 +183,36 @@ VirtioMmioSetQueueAddress (
   )
 {
   VIRTIO_MMIO_DEVICE *Device;
+  UINT64 Address;
 
   ASSERT (RingBaseShift == 0);
 
   Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
 
-  VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
-    (UINT32)((UINTN)Ring->Base >> EFI_PAGE_SHIFT));
+  if (Device->Version == 1) {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
+      (UINT32)((UINTN)Ring->Base >> EFI_PAGE_SHIFT));
+  } else {
+    Address = (UINT64)Ring->Base;
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_DESC_LO,
+                      (UINT32)Address);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_DESC_HI,
+                      (UINT32)RShiftU64(Address, 32));
+
+    Address = (UINT64)Ring->Avail.Flags;
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_AVAIL_LO,
+                      (UINT32)Address);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_AVAIL_HI,
+                      (UINT32)RShiftU64(Address, 32));
+
+    Address = (UINT64)Ring->Used.Flags;
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_USED_LO,
+                      (UINT32)Address);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_USED_HI,
+                      (UINT32)RShiftU64(Address, 32));
+
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_READY, 1);
+  }
 
   return EFI_SUCCESS;
 }
-- 
2.31.1

Re: [edk2-devel] [PATCH 0/2] Quo vadis virtio-mmio?

[Philippe Mathieu-Daudé]

On 6/9/21 7:05 AM, Gerd Hoffmann wrote:
> virtio-mmio support in ovmf seems to be the unloved child.  The final
> virto-1.0 specification was published five(!) years ago, nevertheless
> the mmio transport doesn't support it yet ...
> 
> Some people argue that it has been obsoleted by virtio-pci.  Which is a
> valid argument.  But IMHO isn't a good reason to just let virtio-mmio
> bitrot.  We should either remove it from the tree, or support it.
> 
> So, opening the discussion with this little patch series.  It does the
> latter and adds virtio 1.0 support.  For the mmio transport the
> difference between 0.9.5 and 1.0 is rather small (when compared to the
> pci transport), it is just a bunch of new registers for the changed
> virtio queue initialization.  So the patch series is small too ...
> 
> take care,
>   Gerd
> 
> Gerd Hoffmann (2):
>   OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines
>   OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.
> 
>  OvmfPkg/Include/IndustryStandard/Virtio10.h   | 12 +++++++
>  .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  1 +
>  .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++++++---
>  .../VirtioMmioDeviceFunctions.c               | 31 +++++++++++++++++--
>  4 files changed, 54 insertions(+), 7 deletions(-)

Series:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>