[edk2] [PATCH v2 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap()

Brijesh Singh posted 8 patches 7 years, 3 months ago
There is a newer version of this series
[edk2] [PATCH v2 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap()
Posted by Brijesh Singh 7 years, 3 months ago
When device is behind the IOMMU then driver need to pass the device
address when programing the bus master. The patch uses VirtioRingMap() to
map the VRING system physical address[es] to device address[es].

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/VirtioNetDxe/VirtioNet.h        |  7 ++-
 OvmfPkg/VirtioNetDxe/SnpInitialize.c    | 50 +++++++++++++++-----
 OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 10 ++--
 OvmfPkg/VirtioNetDxe/SnpShutdown.c      |  4 +-
 OvmfPkg/VirtioNetDxe/TechNotes.txt      |  1 +
 5 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 87a0f06e01a4..6762fc9d1d6e 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -82,10 +82,14 @@ typedef struct {
   EFI_HANDLE                  MacHandle;         // VirtioNetDriverBindingStart
 
   VRING                       RxRing;            // VirtioNetInitRing
+  VOID                        *RxRingMap;        // VirtioRingMap and
+                                                 // VirtioNetInitRing
   UINT8                       *RxBuf;            // VirtioNetInitRx
   UINT16                      RxLastUsed;        // VirtioNetInitRx
 
   VRING                       TxRing;            // VirtioNetInitRing
+  VOID                        *TxRingMap;        // VirtioRingMap and
+                                                 // VirtioNetInitRing
   UINT16                      TxMaxPending;      // VirtioNetInitTx
   UINT16                      TxCurPending;      // VirtioNetInitTx
   UINT16                      *TxFreeStack;      // VirtioNetInitTx
@@ -267,7 +271,8 @@ VOID
 EFIAPI
 VirtioNetUninitRing (
   IN OUT VNET_DEV *Dev,
-  IN OUT VRING    *Ring
+  IN OUT VRING    *Ring,
+  IN     VOID     *RingMap
   );
 
 //
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index 637c978709fd..8eabdbff6f5e 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -35,11 +35,13 @@
                            the network device.
   @param[out]    Ring      The virtio-ring inside the VNET_DEV structure,
                            corresponding to Selector.
+  @param[out]    Mapping   A resulting token to pass to VirtioNetUninitRing()
 
   @retval EFI_UNSUPPORTED  The queue size reported by the virtio-net device is
                            too small.
   @return                  Status codes from VIRTIO_CFG_WRITE(),
-                           VIRTIO_CFG_READ() and VirtioRingInit().
+                           VIRTIO_CFG_READ(), VirtioRingInit() and
+                           VirtioRingMap().
   @retval EFI_SUCCESS      Ring initialized.
 */
 
@@ -49,11 +51,14 @@ EFIAPI
 VirtioNetInitRing (
   IN OUT VNET_DEV *Dev,
   IN     UINT16   Selector,
-  OUT    VRING    *Ring
+  OUT    VRING    *Ring,
+  OUT    VOID     **Mapping
   )
 {
   EFI_STATUS Status;
   UINT16     QueueSize;
+  UINT64     RingBaseShift;
+  VOID       *MapInfo;
 
   //
   // step 4b -- allocate selected queue
@@ -80,29 +85,42 @@ VirtioNetInitRing (
   }
 
   //
+  // If anything fails from here on, we must release the ring resources.
+  //
+  Status = VirtioRingMap (Dev->VirtIo, Ring, &RingBaseShift, &MapInfo);
+  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 release the ring resources.
+  // 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 ReleaseQueue;
+    goto UnmapQueue;
   }
 
   Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
   //
   // step 4c -- report GPFN (guest-physical frame number) of queue
   //
-  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, 0);
+  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, RingBaseShift);
   if (EFI_ERROR (Status)) {
-    goto ReleaseQueue;
+    goto UnmapQueue;
   }
 
+  *Mapping = MapInfo;
+
   return EFI_SUCCESS;
 
+UnmapQueue:
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, MapInfo);
+
 ReleaseQueue:
   VirtioRingUninit (Dev->VirtIo, Ring);
 
@@ -456,12 +474,22 @@ VirtioNetInitialize (
   //
   // step 4b, 4c -- allocate and report virtqueues
   //
-  Status = VirtioNetInitRing (Dev, VIRTIO_NET_Q_RX, &Dev->RxRing);
+  Status = VirtioNetInitRing (
+             Dev,
+             VIRTIO_NET_Q_RX,
+             &Dev->RxRing,
+             &Dev->RxRingMap
+             );
   if (EFI_ERROR (Status)) {
     goto DeviceFailed;
   }
 
-  Status = VirtioNetInitRing (Dev, VIRTIO_NET_Q_TX, &Dev->TxRing);
+  Status = VirtioNetInitRing (
+             Dev,
+             VIRTIO_NET_Q_TX,
+             &Dev->TxRing,
+             &Dev->TxRingMap
+             );
   if (EFI_ERROR (Status)) {
     goto ReleaseRxRing;
   }
@@ -510,10 +538,10 @@ AbortDevice:
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
 
 ReleaseTxRing:
-  VirtioNetUninitRing (Dev, &Dev->TxRing);
+  VirtioNetUninitRing (Dev, &Dev->TxRing, Dev->TxRingMap);
 
 ReleaseRxRing:
-  VirtioNetUninitRing (Dev, &Dev->RxRing);
+  VirtioNetUninitRing (Dev, &Dev->RxRing, Dev->RxRingMap);
 
 DeviceFailed:
   //
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index 5b75eabc7a6b..57c7395848bd 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -55,15 +55,19 @@ VirtioNetShutdownTx (
 /**
   Release TX and RX VRING resources.
 
-  @param[in,out] Dev   The VNET_DEV driver instance which was using the ring.
-  @param[in,out] Ring  The virtio ring to clean up.
+  @param[in,out] Dev       The VNET_DEV driver instance which was using
+                           the ring.
+  @param[in,out] Ring      The virtio ring to clean up.
+  @param[in]     RingMap   A token return from the VirtioRingMap()
 */
 VOID
 EFIAPI
 VirtioNetUninitRing (
   IN OUT VNET_DEV *Dev,
-  IN OUT VRING    *Ring
+  IN OUT VRING    *Ring,
+  IN     VOID     *RingMap
   )
 {
+  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RingMap);
   VirtioRingUninit (Dev->VirtIo, Ring);
 }
diff --git a/OvmfPkg/VirtioNetDxe/SnpShutdown.c b/OvmfPkg/VirtioNetDxe/SnpShutdown.c
index 432e0691d457..d8c11f20de61 100644
--- a/OvmfPkg/VirtioNetDxe/SnpShutdown.c
+++ b/OvmfPkg/VirtioNetDxe/SnpShutdown.c
@@ -67,8 +67,8 @@ VirtioNetShutdown (
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
   VirtioNetShutdownRx (Dev);
   VirtioNetShutdownTx (Dev);
-  VirtioNetUninitRing (Dev, &Dev->TxRing);
-  VirtioNetUninitRing (Dev, &Dev->RxRing);
+  VirtioNetUninitRing (Dev, &Dev->TxRing, Dev->TxRingMap);
+  VirtioNetUninitRing (Dev, &Dev->RxRing, Dev->RxRingMap);
 
   Dev->Snm.State = EfiSimpleNetworkStarted;
   Status = EFI_SUCCESS;
diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt
index 86b91f561495..0891e8210489 100644
--- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
+++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
@@ -72,6 +72,7 @@ faithfully indented) that implement the transition.
       VirtioRingInit           |  |   VirtioNetShutdownTx [SnpSharedHelpers.c]
     VirtioNetInitTx            |  |   VirtioNetUninitRing [SnpSharedHelpers.c]
     VirtioNetInitRx            |  |                       {Tx, Rx}
+                               |  |     VirtIo->UnmapSharedBuffer
                                |  |     VirtioRingUninit
                                v  |
                   +-----------------------------+
-- 
2.9.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap()
Posted by Laszlo Ersek 7 years, 3 months ago
On 09/11/17 14:16, Brijesh Singh wrote:
> When device is behind the IOMMU then driver need to pass the device
> address when programing the bus master. The patch uses VirtioRingMap() to
> map the VRING system physical address[es] to device address[es].
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  OvmfPkg/VirtioNetDxe/VirtioNet.h        |  7 ++-
>  OvmfPkg/VirtioNetDxe/SnpInitialize.c    | 50 +++++++++++++++-----
>  OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 10 ++--
>  OvmfPkg/VirtioNetDxe/SnpShutdown.c      |  4 +-
>  OvmfPkg/VirtioNetDxe/TechNotes.txt      |  1 +
>  5 files changed, 55 insertions(+), 17 deletions(-)
>
> diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
> index 87a0f06e01a4..6762fc9d1d6e 100644
> --- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
> +++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
> @@ -82,10 +82,14 @@ typedef struct {
>    EFI_HANDLE                  MacHandle;         // VirtioNetDriverBindingStart
>
>    VRING                       RxRing;            // VirtioNetInitRing
> +  VOID                        *RxRingMap;        // VirtioRingMap and
> +                                                 // VirtioNetInitRing
>    UINT8                       *RxBuf;            // VirtioNetInitRx
>    UINT16                      RxLastUsed;        // VirtioNetInitRx
>
>    VRING                       TxRing;            // VirtioNetInitRing
> +  VOID                        *TxRingMap;        // VirtioRingMap and
> +                                                 // VirtioNetInitRing
>    UINT16                      TxMaxPending;      // VirtioNetInitTx
>    UINT16                      TxCurPending;      // VirtioNetInitTx
>    UINT16                      *TxFreeStack;      // VirtioNetInitTx
> @@ -267,7 +271,8 @@ VOID
>  EFIAPI
>  VirtioNetUninitRing (
>    IN OUT VNET_DEV *Dev,
> -  IN OUT VRING    *Ring
> +  IN OUT VRING    *Ring,
> +  IN     VOID     *RingMap
>    );
>
>  //
> diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> index 637c978709fd..8eabdbff6f5e 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> @@ -35,11 +35,13 @@
>                             the network device.
>    @param[out]    Ring      The virtio-ring inside the VNET_DEV structure,
>                             corresponding to Selector.
> +  @param[out]    Mapping   A resulting token to pass to VirtioNetUninitRing()
>
>    @retval EFI_UNSUPPORTED  The queue size reported by the virtio-net device is
>                             too small.
>    @return                  Status codes from VIRTIO_CFG_WRITE(),
> -                           VIRTIO_CFG_READ() and VirtioRingInit().
> +                           VIRTIO_CFG_READ(), VirtioRingInit() and
> +                           VirtioRingMap().
>    @retval EFI_SUCCESS      Ring initialized.
>  */
>
> @@ -49,11 +51,14 @@ EFIAPI
>  VirtioNetInitRing (
>    IN OUT VNET_DEV *Dev,
>    IN     UINT16   Selector,
> -  OUT    VRING    *Ring
> +  OUT    VRING    *Ring,
> +  OUT    VOID     **Mapping
>    )
>  {
>    EFI_STATUS Status;
>    UINT16     QueueSize;
> +  UINT64     RingBaseShift;
> +  VOID       *MapInfo;
>
>    //
>    // step 4b -- allocate selected queue
> @@ -80,29 +85,42 @@ VirtioNetInitRing (
>    }
>
>    //
> +  // If anything fails from here on, we must release the ring resources.
> +  //
> +  Status = VirtioRingMap (Dev->VirtIo, Ring, &RingBaseShift, &MapInfo);
> +  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 release the ring resources.
> +  // 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 ReleaseQueue;
> +    goto UnmapQueue;
>    }
>
>    Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE);
>    if (EFI_ERROR (Status)) {
> -    goto ReleaseQueue;
> +    goto UnmapQueue;
>    }
>
>    //
>    // step 4c -- report GPFN (guest-physical frame number) of queue
>    //
> -  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, 0);
> +  Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, RingBaseShift);
>    if (EFI_ERROR (Status)) {
> -    goto ReleaseQueue;
> +    goto UnmapQueue;
>    }
>
> +  *Mapping = MapInfo;
> +
>    return EFI_SUCCESS;
>
> +UnmapQueue:
> +  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, MapInfo);
> +
>  ReleaseQueue:
>    VirtioRingUninit (Dev->VirtIo, Ring);
>
> @@ -456,12 +474,22 @@ VirtioNetInitialize (
>    //
>    // step 4b, 4c -- allocate and report virtqueues
>    //
> -  Status = VirtioNetInitRing (Dev, VIRTIO_NET_Q_RX, &Dev->RxRing);
> +  Status = VirtioNetInitRing (
> +             Dev,
> +             VIRTIO_NET_Q_RX,
> +             &Dev->RxRing,
> +             &Dev->RxRingMap
> +             );
>    if (EFI_ERROR (Status)) {
>      goto DeviceFailed;
>    }
>
> -  Status = VirtioNetInitRing (Dev, VIRTIO_NET_Q_TX, &Dev->TxRing);
> +  Status = VirtioNetInitRing (
> +             Dev,
> +             VIRTIO_NET_Q_TX,
> +             &Dev->TxRing,
> +             &Dev->TxRingMap
> +             );
>    if (EFI_ERROR (Status)) {
>      goto ReleaseRxRing;
>    }
> @@ -510,10 +538,10 @@ AbortDevice:
>    Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
>
>  ReleaseTxRing:
> -  VirtioNetUninitRing (Dev, &Dev->TxRing);
> +  VirtioNetUninitRing (Dev, &Dev->TxRing, Dev->TxRingMap);
>
>  ReleaseRxRing:
> -  VirtioNetUninitRing (Dev, &Dev->RxRing);
> +  VirtioNetUninitRing (Dev, &Dev->RxRing, Dev->RxRingMap);
>
>  DeviceFailed:
>    //
> diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> index 5b75eabc7a6b..57c7395848bd 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> @@ -55,15 +55,19 @@ VirtioNetShutdownTx (
>  /**
>    Release TX and RX VRING resources.
>
> -  @param[in,out] Dev   The VNET_DEV driver instance which was using the ring.
> -  @param[in,out] Ring  The virtio ring to clean up.
> +  @param[in,out] Dev       The VNET_DEV driver instance which was using
> +                           the ring.
> +  @param[in,out] Ring      The virtio ring to clean up.
> +  @param[in]     RingMap   A token return from the VirtioRingMap()
>  */
>  VOID
>  EFIAPI
>  VirtioNetUninitRing (
>    IN OUT VNET_DEV *Dev,
> -  IN OUT VRING    *Ring
> +  IN OUT VRING    *Ring,
> +  IN     VOID     *RingMap
>    )
>  {
> +  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RingMap);
>    VirtioRingUninit (Dev->VirtIo, Ring);
>  }
> diff --git a/OvmfPkg/VirtioNetDxe/SnpShutdown.c b/OvmfPkg/VirtioNetDxe/SnpShutdown.c
> index 432e0691d457..d8c11f20de61 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpShutdown.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpShutdown.c
> @@ -67,8 +67,8 @@ VirtioNetShutdown (
>    Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
>    VirtioNetShutdownRx (Dev);
>    VirtioNetShutdownTx (Dev);
> -  VirtioNetUninitRing (Dev, &Dev->TxRing);
> -  VirtioNetUninitRing (Dev, &Dev->RxRing);
> +  VirtioNetUninitRing (Dev, &Dev->TxRing, Dev->TxRingMap);
> +  VirtioNetUninitRing (Dev, &Dev->RxRing, Dev->RxRingMap);
>
>    Dev->Snm.State = EfiSimpleNetworkStarted;
>    Status = EFI_SUCCESS;
> diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt
> index 86b91f561495..0891e8210489 100644
> --- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
> +++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
> @@ -72,6 +72,7 @@ faithfully indented) that implement the transition.
>        VirtioRingInit           |  |   VirtioNetShutdownTx [SnpSharedHelpers.c]
>      VirtioNetInitTx            |  |   VirtioNetUninitRing [SnpSharedHelpers.c]
>      VirtioNetInitRx            |  |                       {Tx, Rx}
> +                               |  |     VirtIo->UnmapSharedBuffer
>                                 |  |     VirtioRingUninit
>                                 v  |
>                    +-----------------------------+
>

(1) The documentation udpate is not correct; please check my previous
review:

http://mid.mail-archive.com/04fefb16-23d5-f6df-0657-9d4c5e96ac57@redhat.com

In particular you forgot to add the VirtioRingMap function call on the
left side, under VirtioNetInitRing:

On 09/05/17 13:47, Laszlo Ersek wrote:
> (9d) modify the documentation again, to the following state:
>
>>                    +-------------------------+
>>                    | EfiSimpleNetworkStarted |
>>                    +-------------------------+
>>                                |  ^
>>   [SnpInitialize.c]            |  | [SnpShutdown.c]
>>   VirtioNetInitialize          |  | VirtioNetShutdown
>>     VirtioNetInitRing {Rx, Tx} |  |   VirtioNetShutdownRx [SnpSharedHelpers.c]
>>       VirtioRingInit           |  |   VirtioNetShutdownTx [SnpSharedHelpers.c]
>>       VirtioRingMap            |  |   VirtioNetUninitRing [SnpSharedHelpers.c]
>>     VirtioNetInitTx            |  |                       {Tx, Rx}
>>     VirtioNetInitRx            |  |     VirtIo->UnmapSharedBuffer
>>                                |  |     VirtioRingUninit
>>                                v  |
>>                   +-----------------------------+
>>                   | EfiSimpleNetworkInitialized |
>>                   +-----------------------------+

I think simply pasting the above into "TechNotes.txt" would be easiest.

The rest looks good!

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel