public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/4] add support for virtio-mmio 1.0
@ 2021-08-17  9:23 Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 1/4] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines Gerd Hoffmann
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2021-08-17  9:23 UTC (permalink / raw)
  To: devel; +Cc: Gerd Hoffmann

This little series adds virtio 1.0 support for the virtio-mmio
transport.  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 ...

v2 changes:
 - Added review tags for patches #1 + #2.
 - Add a patch to make sure we have a valid QueueNum.
 - Add a patch to support all 64 virtio 1.0 feature bits.
v3 changes:
 - Add #defines for virtio-mmio version field.

take care,
  Gerd

Gerd Hoffmann (4):
  OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines
  OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.
  OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0
  OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 feature bit handling

 OvmfPkg/Include/IndustryStandard/Virtio10.h   | 12 ++++
 .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  5 ++
 .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++--
 .../VirtioMmioDeviceFunctions.c               | 71 ++++++++++++++++---
 4 files changed, 92 insertions(+), 13 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/4] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines
  2021-08-17  9:23 [PATCH v3 0/4] add support for virtio-mmio 1.0 Gerd Hoffmann
@ 2021-08-17  9:23 ` Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support Gerd Hoffmann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2021-08-17  9:23 UTC (permalink / raw)
  To: devel; +Cc: Gerd Hoffmann, Philippe Mathieu-Daude

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

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.
  2021-08-17  9:23 [PATCH v3 0/4] add support for virtio-mmio 1.0 Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 1/4] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines Gerd Hoffmann
@ 2021-08-17  9:23 ` Gerd Hoffmann
  2021-08-17  9:55   ` Philippe Mathieu-Daudé
  2021-08-17  9:23 ` [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0 Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 4/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 feature bit handling Gerd Hoffmann
  3 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2021-08-17  9:23 UTC (permalink / raw)
  To: devel; +Cc: Gerd Hoffmann, Philippe Mathieu-Daude

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>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  4 +++
 .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++++++---
 .../VirtioMmioDeviceFunctions.c               | 31 +++++++++++++++++--
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
index ab53b90d51c9..0c2f99633c46 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
@@ -23,9 +23,13 @@
 #include <Library/MemoryAllocationLib.h>
 
 #define VIRTIO_MMIO_DEVICE_SIGNATURE  SIGNATURE_32 ('V', 'M', 'I', 'O')
+#define VIRTIO_MMIO_DEVICE_VERSION_0_95  1
+#define VIRTIO_MMIO_DEVICE_VERSION_1_00  2
+
 
 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..a97ef9352d0f 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 VIRTIO_MMIO_DEVICE_VERSION_0_95:
+    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 VIRTIO_MMIO_DEVICE_VERSION_1_00:
+    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..a0ee8e5f3c86 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 == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    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 == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0
  2021-08-17  9:23 [PATCH v3 0/4] add support for virtio-mmio 1.0 Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 1/4] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines Gerd Hoffmann
  2021-08-17  9:23 ` [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support Gerd Hoffmann
@ 2021-08-17  9:23 ` Gerd Hoffmann
  2021-08-17  9:48   ` [edk2-devel] " Philippe Mathieu-Daudé
  2021-08-17  9:23 ` [PATCH v3 4/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 feature bit handling Gerd Hoffmann
  3 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2021-08-17  9:23 UTC (permalink / raw)
  To: devel; +Cc: Gerd Hoffmann

Use QueueNumMax as QueueNum default for drivers which do not
explicitly call VIRTIO_DEVICE_PROTOCOL->SetQueueSize().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../Library/VirtioMmioDeviceLib/VirtioMmioDevice.h   |  1 +
 .../VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c  | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
index 0c2f99633c46..5ad951f4154a 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
@@ -30,6 +30,7 @@
 typedef struct {
   UINT32                 Signature;
   UINT32                 Version;
+  UINT16                 QueueNum;
   VIRTIO_DEVICE_PROTOCOL VirtioDevice;
   PHYSICAL_ADDRESS       BaseAddress;
 } VIRTIO_MMIO_DEVICE;
diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
index a0ee8e5f3c86..b9ee407eddc4 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
@@ -83,7 +83,11 @@ VirtioMmioSetQueueSize (
 
   Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
 
-  VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, QueueSize);
+  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    Device->QueueNum = QueueSize;
+  } else {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, QueueSize);
+  }
 
   return EFI_SUCCESS;
 }
@@ -171,6 +175,10 @@ VirtioMmioSetQueueSel (
 
   VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_SEL, Sel);
 
+  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    Device->QueueNum = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM_MAX) & 0xFFFF;
+  }
+
   return EFI_SUCCESS;
 }
 
@@ -193,6 +201,8 @@ VirtioMmioSetQueueAddress (
     VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
       (UINT32)((UINTN)Ring->Base >> EFI_PAGE_SHIFT));
   } else {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, Device->QueueNum);
+
     Address = (UINT64)Ring->Base;
     VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_DESC_LO,
                       (UINT32)Address);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 4/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 feature bit handling
  2021-08-17  9:23 [PATCH v3 0/4] add support for virtio-mmio 1.0 Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2021-08-17  9:23 ` [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0 Gerd Hoffmann
@ 2021-08-17  9:23 ` Gerd Hoffmann
  3 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2021-08-17  9:23 UTC (permalink / raw)
  To: devel; +Cc: Gerd Hoffmann

virtio 1.0 has 64 feature bits instead of 32.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../VirtioMmioDeviceFunctions.c               | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
index b9ee407eddc4..7215e0414c24 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
@@ -20,6 +20,7 @@ VirtioMmioGetDeviceFeatures (
   )
 {
   VIRTIO_MMIO_DEVICE *Device;
+  UINT32  LowBits, HighBits;
 
   if (DeviceFeatures == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -27,7 +28,15 @@ VirtioMmioGetDeviceFeatures (
 
   Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
 
-  *DeviceFeatures = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES);
+  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    *DeviceFeatures = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES);
+  } else {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES_SEL, 0);
+    LowBits = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES_SEL, 1);
+    HighBits = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_HOST_FEATURES);
+    *DeviceFeatures = LShiftU64(HighBits, 32) | LowBits;
+  }
 
   return EFI_SUCCESS;
 }
@@ -238,11 +247,20 @@ VirtioMmioSetGuestFeatures (
 
   Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
 
-  if (Features > MAX_UINT32) {
-    return EFI_UNSUPPORTED;
+  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
+    if (Features > MAX_UINT32) {
+      return EFI_UNSUPPORTED;
+    }
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES,
+                      (UINT32)Features);
+  } else {
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES_SEL, 0);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES,
+                      (UINT32)Features);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES_SEL, 1);
+    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES,
+                      (UINT32)RShiftU64(Features, 32));
   }
-  VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_FEATURES,
-    (UINT32)Features);
 
   return EFI_SUCCESS;
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0
  2021-08-17  9:23 ` [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0 Gerd Hoffmann
@ 2021-08-17  9:48   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-17  9:48 UTC (permalink / raw)
  To: devel, kraxel

On 8/17/21 11:23 AM, Gerd Hoffmann wrote:
> Use QueueNumMax as QueueNum default for drivers which do not
> explicitly call VIRTIO_DEVICE_PROTOCOL->SetQueueSize().
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../Library/VirtioMmioDeviceLib/VirtioMmioDevice.h   |  1 +
>  .../VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c  | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> index 0c2f99633c46..5ad951f4154a 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> @@ -30,6 +30,7 @@
>  typedef struct {
>    UINT32                 Signature;
>    UINT32                 Version;
> +  UINT16                 QueueNum;
>    VIRTIO_DEVICE_PROTOCOL VirtioDevice;
>    PHYSICAL_ADDRESS       BaseAddress;
>  } VIRTIO_MMIO_DEVICE;
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> index a0ee8e5f3c86..b9ee407eddc4 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> @@ -83,7 +83,11 @@ VirtioMmioSetQueueSize (
>  
>    Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
>  
> -  VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, QueueSize);
> +  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
> +    Device->QueueNum = QueueSize;
> +  } else {
> +    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, QueueSize);
> +  }
>  
>    return EFI_SUCCESS;
>  }
> @@ -171,6 +175,10 @@ VirtioMmioSetQueueSel (
>  
>    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_SEL, Sel);
>  
> +  if (Device->Version == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
> +    Device->QueueNum = VIRTIO_CFG_READ (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM_MAX) & 0xFFFF;
> +  }
> +
>    return EFI_SUCCESS;
>  }
>  
> @@ -193,6 +201,8 @@ VirtioMmioSetQueueAddress (
>      VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
>        (UINT32)((UINTN)Ring->Base >> EFI_PAGE_SHIFT));
>    } else {
> +    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_NUM, Device->QueueNum);

This looks like a bugfix in the previous patch... Going to revisit it.

>      Address = (UINT64)Ring->Base;
>      VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_DESC_LO,
>                        (UINT32)Address);
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support.
  2021-08-17  9:23 ` [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support Gerd Hoffmann
@ 2021-08-17  9:55   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-17  9:55 UTC (permalink / raw)
  To: Gerd Hoffmann, devel

Hi Gerd,

On 8/17/21 11:23 AM, Gerd Hoffmann wrote:
> 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>
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
>  .../VirtioMmioDeviceLib/VirtioMmioDevice.h    |  4 +++
>  .../VirtioMmioDeviceLib/VirtioMmioDevice.c    | 17 +++++++---
>  .../VirtioMmioDeviceFunctions.c               | 31 +++++++++++++++++--
>  3 files changed, 45 insertions(+), 7 deletions(-)
> 
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> index ab53b90d51c9..0c2f99633c46 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> @@ -23,9 +23,13 @@
>  #include <Library/MemoryAllocationLib.h>
>  
>  #define VIRTIO_MMIO_DEVICE_SIGNATURE  SIGNATURE_32 ('V', 'M', 'I', 'O')
> +#define VIRTIO_MMIO_DEVICE_VERSION_0_95  1

After looking at the next patch, I wonder if it wouldn't be simpler
to add/use VIRTIO_MMIO_DEVICE_VERSION_0_95 in a preliminary patch
(2/5), returning EFI_UNSUPPORTED when not v0.95.

Next patch (3/5) add QueueNum for v0.95 (returning EFI_UNSUPPORTED
for != v0.95)

Then finally (4/5) add v1.00.

Current 4/4 becomes 5/5.

> +#define VIRTIO_MMIO_DEVICE_VERSION_1_00  2

So remove from this patch,

>  
>  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..a97ef9352d0f 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 VIRTIO_MMIO_DEVICE_VERSION_0_95:
> +    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;

Remove from here <--

> +  case VIRTIO_MMIO_DEVICE_VERSION_1_00:
> +    DEBUG ((DEBUG_INFO, "%a virtio 1.0, id %d\n", __FUNCTION__,
> +            Device->VirtioDevice.SubSystemDeviceId));
> +    Device->VirtioDevice.Revision = VIRTIO_SPEC_REVISION (1, 0, 0);
> +    break;

--> to here (moved to 4/5).

> +  default:
>      return EFI_UNSUPPORTED;
>    }
>  
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> index b0d75fb1dd24..a0ee8e5f3c86 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 == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
> +    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_GUEST_PAGE_SIZE, PageSize);

     else
         EFI_UNSUPPORTED
> +  }
>  
>    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 == VIRTIO_MMIO_DEVICE_VERSION_0_95) {
> +    VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
> +      (UINT32)((UINTN)Ring->Base >> EFI_PAGE_SHIFT));
> +  } else {

       EFI_UNSUPPORTED for now,

> +    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);

^ this moved to 4/5.

> +  }
>  
>    return EFI_SUCCESS;
>  }
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-17  9:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-17  9:23 [PATCH v3 0/4] add support for virtio-mmio 1.0 Gerd Hoffmann
2021-08-17  9:23 ` [PATCH v3 1/4] OvmfPkg/Virtio10: Add virtio-mmio 1.0 defines Gerd Hoffmann
2021-08-17  9:23 ` [PATCH v3 2/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 support Gerd Hoffmann
2021-08-17  9:55   ` Philippe Mathieu-Daudé
2021-08-17  9:23 ` [PATCH v3 3/4] OvmfPkg/VirtioMmioDeviceLib: Add default QueueNum for virtio 1.0 Gerd Hoffmann
2021-08-17  9:48   ` [edk2-devel] " Philippe Mathieu-Daudé
2021-08-17  9:23 ` [PATCH v3 4/4] OvmfPkg/VirtioMmioDeviceLib: Add virtio 1.0 feature bit handling Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox