* [PATCH v3 1/8] OvmfPkg/VirtioNetDxe: add helper VirtioNetUninitRing()
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap() Brijesh Singh
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
Consolidate the virtio VRING resource cleanup into VirtioNetUninitRing().
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioNetDxe/VirtioNet.h | 7 +++++++
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 4 ++--
OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 16 ++++++++++++++++
OvmfPkg/VirtioNetDxe/SnpShutdown.c | 4 ++--
OvmfPkg/VirtioNetDxe/TechNotes.txt | 5 +++--
5 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 710859bc6115..87a0f06e01a4 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -263,6 +263,13 @@ VirtioNetShutdownTx (
IN OUT VNET_DEV *Dev
);
+VOID
+EFIAPI
+VirtioNetUninitRing (
+ IN OUT VNET_DEV *Dev,
+ IN OUT VRING *Ring
+ );
+
//
// event callbacks
//
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index 0ecfe044a977..637c978709fd 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -510,10 +510,10 @@ AbortDevice:
Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
ReleaseTxRing:
- VirtioRingUninit (Dev->VirtIo, &Dev->TxRing);
+ VirtioNetUninitRing (Dev, &Dev->TxRing);
ReleaseRxRing:
- VirtioRingUninit (Dev->VirtIo, &Dev->RxRing);
+ VirtioNetUninitRing (Dev, &Dev->RxRing);
DeviceFailed:
//
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index 9fedb72fdbd4..5b75eabc7a6b 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -51,3 +51,19 @@ VirtioNetShutdownTx (
{
FreePool (Dev->TxFreeStack);
}
+
+/**
+ 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.
+*/
+VOID
+EFIAPI
+VirtioNetUninitRing (
+ IN OUT VNET_DEV *Dev,
+ IN OUT VRING *Ring
+ )
+{
+ VirtioRingUninit (Dev->VirtIo, Ring);
+}
diff --git a/OvmfPkg/VirtioNetDxe/SnpShutdown.c b/OvmfPkg/VirtioNetDxe/SnpShutdown.c
index 5e84191fbbdd..432e0691d457 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);
- VirtioRingUninit (Dev->VirtIo, &Dev->TxRing);
- VirtioRingUninit (Dev->VirtIo, &Dev->RxRing);
+ VirtioNetUninitRing (Dev, &Dev->TxRing);
+ VirtioNetUninitRing (Dev, &Dev->RxRing);
Dev->Snm.State = EfiSimpleNetworkStarted;
Status = EFI_SUCCESS;
diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt
index 9c1dfe6a773e..86b91f561495 100644
--- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
+++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
@@ -70,8 +70,9 @@ faithfully indented) that implement the transition.
VirtioNetInitialize | | VirtioNetShutdown
VirtioNetInitRing {Rx, Tx} | | VirtioNetShutdownRx [SnpSharedHelpers.c]
VirtioRingInit | | VirtioNetShutdownTx [SnpSharedHelpers.c]
- VirtioNetInitTx | | VirtioRingUninit {Tx, Rx}
- VirtioNetInitRx | |
+ VirtioNetInitTx | | VirtioNetUninitRing [SnpSharedHelpers.c]
+ VirtioNetInitRx | | {Tx, Rx}
+ | | VirtioRingUninit
v |
+-----------------------------+
| EfiSimpleNetworkInitialized |
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap()
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 1/8] OvmfPkg/VirtioNetDxe: add helper VirtioNetUninitRing() Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 18:57 ` Laszlo Ersek
2017-09-14 11:08 ` [PATCH v3 3/8] OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages() Brijesh Singh
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
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 | 5 +-
5 files changed, 57 insertions(+), 19 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..37250b14a98c 100644
--- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
+++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
@@ -70,8 +70,9 @@ faithfully indented) that implement the transition.
VirtioNetInitialize | | VirtioNetShutdown
VirtioNetInitRing {Rx, Tx} | | VirtioNetShutdownRx [SnpSharedHelpers.c]
VirtioRingInit | | VirtioNetShutdownTx [SnpSharedHelpers.c]
- VirtioNetInitTx | | VirtioNetUninitRing [SnpSharedHelpers.c]
- VirtioNetInitRx | | {Tx, Rx}
+ VirtioRingMap | | VirtioNetUninitRing [SnpSharedHelpers.c]
+ VirtioNetInitTx | | {Tx, Rx}
+ VirtioNetInitRx | | VirtIo->UnmapSharedBuffer
| | VirtioRingUninit
v |
+-----------------------------+
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap()
2017-09-14 11:08 ` [PATCH v3 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap() Brijesh Singh
@ 2017-09-14 18:57 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2017-09-14 18:57 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/17 13:08, 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 | 5 +-
> 5 files changed, 57 insertions(+), 19 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/8] OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages()
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 1/8] OvmfPkg/VirtioNetDxe: add helper VirtioNetUninitRing() Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 2/8] OvmfPkg/VirtioNetDxe: map VRINGs using VirtioRingMap() Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 19:04 ` Laszlo Ersek
2017-09-14 11:08 ` [PATCH v3 4/8] OvmfPkg/VirtioNetDxe: dynamically alloc transmit header Brijesh Singh
` (4 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
When device is behind the IOMMU, VirtioNetDxe is required to use the
device address in bus master operations. RxBuf is allocated using
AllocatePool() which returns the system physical address.
The patch uses VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() to allocate
the RxBuf and map with VirtioMapAllBytesInSharedBuffer() so that we can
obtain the device address for RxBuf.
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 | 4 +
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 78 +++++++++++++++-----
OvmfPkg/VirtioNetDxe/SnpReceive.c | 5 +-
OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 7 +-
OvmfPkg/VirtioNetDxe/TechNotes.txt | 2 +-
5 files changed, 75 insertions(+), 21 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 6762fc9d1d6e..995593f4b236 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -4,6 +4,7 @@
Protocol instances for virtio-net devices.
Copyright (C) 2013, Red Hat, Inc.
+ Copyright (c) 2017, AMD Inc, All rights reserved.<BR>
This program and the accompanying materials are licensed and made available
under the terms and conditions of the BSD License which accompanies this
@@ -86,6 +87,9 @@ typedef struct {
// VirtioNetInitRing
UINT8 *RxBuf; // VirtioNetInitRx
UINT16 RxLastUsed; // VirtioNetInitRx
+ UINTN RxBufNrPages; // VirtioNetInitRx
+ EFI_PHYSICAL_ADDRESS RxBufDeviceBase; // VirtioNetInitRx
+ VOID *RxBufMap; // VirtioNetInitRx
VRING TxRing; // VirtioNetInitRing
VOID *TxRingMap; // VirtioRingMap and
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index 8eabdbff6f5e..b739875d4c01 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -242,8 +242,9 @@ VirtioNetInitTx (
@param[in,out] Dev The VNET_DEV driver instance about to enter the
EfiSimpleNetworkInitialized state.
- @retval EFI_OUT_OF_RESOURCES Failed to allocate RX destination area.
- @return Status codes from VIRTIO_CFG_WRITE().
+ @return Status codes from VIRTIO_CFG_WRITE() or
+ VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages or
+ VirtioMapAllBytesInSharedBuffer().
@retval EFI_SUCCESS RX setup successful. The device is live and may
already be writing to the receive area.
*/
@@ -255,13 +256,15 @@ VirtioNetInitRx (
IN OUT VNET_DEV *Dev
)
{
- EFI_STATUS Status;
- UINTN VirtioNetReqSize;
- UINTN RxBufSize;
- UINT16 RxAlwaysPending;
- UINTN PktIdx;
- UINT16 DescIdx;
- UINT8 *RxPtr;
+ EFI_STATUS Status;
+ UINTN VirtioNetReqSize;
+ UINTN RxBufSize;
+ UINT16 RxAlwaysPending;
+ UINTN PktIdx;
+ UINT16 DescIdx;
+ UINTN NumBytes;
+ EFI_PHYSICAL_ADDRESS RxBufDeviceAddress;
+ VOID *RxBuffer;
//
// In VirtIo 1.0, the NumBuffers field is mandatory. In 0.9.5, it depends on
@@ -286,11 +289,39 @@ VirtioNetInitRx (
//
RxAlwaysPending = (UINT16) MIN (Dev->RxRing.QueueSize / 2, VNET_MAX_PENDING);
- Dev->RxBuf = AllocatePool (RxAlwaysPending * RxBufSize);
- if (Dev->RxBuf == NULL) {
- return EFI_OUT_OF_RESOURCES;
+ //
+ // The RxBuf is shared between guest and hypervisor, use
+ // AllocateSharedPages() to allocate this memory region and map it with
+ // BusMasterCommonBuffer so that it can be accessed by both guest and
+ // hypervisor.
+ //
+ NumBytes = RxAlwaysPending * RxBufSize;
+ Dev->RxBufNrPages = EFI_SIZE_TO_PAGES (NumBytes);
+ Status = Dev->VirtIo->AllocateSharedPages (
+ Dev->VirtIo,
+ Dev->RxBufNrPages,
+ &RxBuffer
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
}
+ ZeroMem (RxBuffer, NumBytes);
+
+ Status = VirtioMapAllBytesInSharedBuffer (
+ Dev->VirtIo,
+ VirtioOperationBusMasterCommonBuffer,
+ RxBuffer,
+ NumBytes,
+ &Dev->RxBufDeviceBase,
+ &Dev->RxBufMap
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeSharedBuffer;
+ }
+
+ Dev->RxBuf = RxBuffer;
+
//
// virtio-0.9.5, 2.4.2 Receiving Used Buffers From the Device
//
@@ -310,7 +341,7 @@ VirtioNetInitRx (
// link each chain into (from) the available ring as well
//
DescIdx = 0;
- RxPtr = Dev->RxBuf;
+ RxBufDeviceAddress = Dev->RxBufDeviceBase;
for (PktIdx = 0; PktIdx < RxAlwaysPending; ++PktIdx) {
//
// virtio-0.9.5, 2.4.1.2 Updating the Available Ring
@@ -321,16 +352,16 @@ VirtioNetInitRx (
//
// virtio-0.9.5, 2.4.1.1 Placing Buffers into the Descriptor Table
//
- Dev->RxRing.Desc[DescIdx].Addr = (UINTN) RxPtr;
+ Dev->RxRing.Desc[DescIdx].Addr = RxBufDeviceAddress;
Dev->RxRing.Desc[DescIdx].Len = (UINT32) VirtioNetReqSize;
Dev->RxRing.Desc[DescIdx].Flags = VRING_DESC_F_WRITE | VRING_DESC_F_NEXT;
Dev->RxRing.Desc[DescIdx].Next = (UINT16) (DescIdx + 1);
- RxPtr += Dev->RxRing.Desc[DescIdx++].Len;
+ RxBufDeviceAddress += Dev->RxRing.Desc[DescIdx++].Len;
- Dev->RxRing.Desc[DescIdx].Addr = (UINTN) RxPtr;
+ Dev->RxRing.Desc[DescIdx].Addr = RxBufDeviceAddress;
Dev->RxRing.Desc[DescIdx].Len = (UINT32) (RxBufSize - VirtioNetReqSize);
Dev->RxRing.Desc[DescIdx].Flags = VRING_DESC_F_WRITE;
- RxPtr += Dev->RxRing.Desc[DescIdx++].Len;
+ RxBufDeviceAddress += Dev->RxRing.Desc[DescIdx++].Len;
}
//
@@ -351,10 +382,21 @@ VirtioNetInitRx (
Status = Dev->VirtIo->SetQueueNotify (Dev->VirtIo, VIRTIO_NET_Q_RX);
if (EFI_ERROR (Status)) {
Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
- FreePool (Dev->RxBuf);
+ goto UnmapSharedBuffer;
}
return Status;
+
+UnmapSharedBuffer:
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RxBufMap);
+
+FreeSharedBuffer:
+ Dev->VirtIo->FreeSharedPages (
+ Dev->VirtIo,
+ Dev->RxBufNrPages,
+ RxBuffer
+ );
+ return Status;
}
diff --git a/OvmfPkg/VirtioNetDxe/SnpReceive.c b/OvmfPkg/VirtioNetDxe/SnpReceive.c
index 99abd7ebe454..c42489636ea0 100644
--- a/OvmfPkg/VirtioNetDxe/SnpReceive.c
+++ b/OvmfPkg/VirtioNetDxe/SnpReceive.c
@@ -82,6 +82,7 @@ VirtioNetReceive (
UINT8 *RxPtr;
UINT16 AvailIdx;
EFI_STATUS NotifyStatus;
+ UINTN RxBufOffset;
if (This == NULL || BufferSize == NULL || Buffer == NULL) {
return EFI_INVALID_PARAMETER;
@@ -143,7 +144,9 @@ VirtioNetReceive (
*HeaderSize = Dev->Snm.MediaHeaderSize;
}
- RxPtr = (UINT8 *)(UINTN) Dev->RxRing.Desc[DescIdx + 1].Addr;
+ RxBufOffset = (UINTN)(Dev->RxRing.Desc[DescIdx + 1].Addr -
+ Dev->RxBufDeviceBase);
+ RxPtr = Dev->RxBuf + RxBufOffset;
CopyMem (Buffer, RxPtr, RxLen);
if (DestAddr != NULL) {
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index 57c7395848bd..ee4f9ed36ecd 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -39,7 +39,12 @@ VirtioNetShutdownRx (
IN OUT VNET_DEV *Dev
)
{
- FreePool (Dev->RxBuf);
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RxBufMap);
+ Dev->VirtIo->FreeSharedPages (
+ Dev->VirtIo,
+ Dev->RxBufNrPages,
+ Dev->RxBuf
+ );
}
diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt
index 37250b14a98c..fedbaee07ec7 100644
--- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
+++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
@@ -247,7 +247,7 @@ In VirtioNetInitRx, the guest allocates the fixed size Receive Destination
Area, which accommodates all packets delivered asynchronously by the host. To
each packet, a slice of this area is dedicated; each slice is further
subdivided into virtio-net request header and network packet data. The
-(guest-physical) addresses of these sub-slices are denoted with A2, A3, A4 and
+(device-physical) addresses of these sub-slices are denoted with A2, A3, A4 and
so on. Importantly, an even-subscript "A" always belongs to a virtio-net
request header, while an odd-subscript "A" always belongs to a packet
sub-slice.
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/8] OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages()
2017-09-14 11:08 ` [PATCH v3 3/8] OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages() Brijesh Singh
@ 2017-09-14 19:04 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2017-09-14 19:04 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/17 13:08, Brijesh Singh wrote:
> When device is behind the IOMMU, VirtioNetDxe is required to use the
> device address in bus master operations. RxBuf is allocated using
> AllocatePool() which returns the system physical address.
>
> The patch uses VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() to allocate
> the RxBuf and map with VirtioMapAllBytesInSharedBuffer() so that we can
> obtain the device address for RxBuf.
>
> 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 | 4 +
> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 78 +++++++++++++++-----
> OvmfPkg/VirtioNetDxe/SnpReceive.c | 5 +-
> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 7 +-
> OvmfPkg/VirtioNetDxe/TechNotes.txt | 2 +-
> 5 files changed, 75 insertions(+), 21 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 4/8] OvmfPkg/VirtioNetDxe: dynamically alloc transmit header
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
` (2 preceding siblings ...)
2017-09-14 11:08 ` [PATCH v3 3/8] OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages() Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 19:12 ` Laszlo Ersek
2017-09-14 11:08 ` [PATCH v3 5/8] OvmfPkg/VirtioNetDxe: update TechNotes Brijesh Singh
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
Each network packet is submitted for transmission by pushing the head
descriptor of a two-part descriptor chain to the Available Ring of the
TX queue. VirtioNetInitTx() sets up the the descriptor chains for all
queueable packets in advance, and points all the head descriptors to the
same shared, never modified, VIRTIO_1_0_NET_REQ header object (or its
initial VIRTIO_NET_REQ sub-object, dependent on virtio version).
VirtioNetInitTx() currently uses the header object's system physical
address for populating the head descriptors.
When device is behind the IOMMU, VirtioNet driver is required to provide
the device address of VIRTIO_1_0_NET_REQ header. In this patch we
dynamically allocate the header using AllocateSharedPages() and map with
BusMasterCommonBuffer so that header can be accessed by both processor
and the device.
We map the header object for CommonBuffer operation because, in order to
stick with the current code order, we populate the head descriptors with
the header's device address first, and fill in the header itself second.
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 | 3 +-
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 64 +++++++++++++++++---
OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 7 +++
3 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 995593f4b236..027f75993e8e 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -97,7 +97,8 @@ typedef struct {
UINT16 TxMaxPending; // VirtioNetInitTx
UINT16 TxCurPending; // VirtioNetInitTx
UINT16 *TxFreeStack; // VirtioNetInitTx
- VIRTIO_1_0_NET_REQ TxSharedReq; // VirtioNetInitTx
+ VIRTIO_1_0_NET_REQ *TxSharedReq; // VirtioNetInitTx
+ VOID *TxSharedReqMap; // VirtioNetInitTx
UINT16 TxLastUsed; // VirtioNetInitTx
} VNET_DEV;
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index b739875d4c01..9621f936d2cb 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -18,6 +18,7 @@
**/
#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/UefiBootServicesTableLib.h>
@@ -147,6 +148,9 @@ ReleaseQueue:
@retval EFI_OUT_OF_RESOURCES Failed to allocate the stack to track the heads
of free descriptor chains.
+ @return Status codes from VIRTIO_DEVICE_PROTOCOL.
+ AllocateSharedPages() or
+ VirtioMapAllBytesInSharedBuffer()
@retval EFI_SUCCESS TX setup successful.
*/
@@ -157,8 +161,11 @@ VirtioNetInitTx (
IN OUT VNET_DEV *Dev
)
{
- UINTN TxSharedReqSize;
- UINTN PktIdx;
+ UINTN TxSharedReqSize;
+ UINTN PktIdx;
+ EFI_STATUS Status;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
+ VOID *TxSharedReqBuffer;
Dev->TxMaxPending = (UINT16) MIN (Dev->TxRing.QueueSize / 2,
VNET_MAX_PENDING);
@@ -170,12 +177,42 @@ VirtioNetInitTx (
}
//
+ // Allocate TxSharedReq header and map with BusMasterCommonBuffer so that it
+ // can be accessed equally by both processor and device.
+ //
+ Status = Dev->VirtIo->AllocateSharedPages (
+ Dev->VirtIo,
+ EFI_SIZE_TO_PAGES (sizeof *Dev->TxSharedReq),
+ &TxSharedReqBuffer
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeTxFreeStack;
+ }
+
+ ZeroMem (TxSharedReqBuffer, sizeof *Dev->TxSharedReq);
+
+ Status = VirtioMapAllBytesInSharedBuffer (
+ Dev->VirtIo,
+ VirtioOperationBusMasterCommonBuffer,
+ TxSharedReqBuffer,
+ sizeof *(Dev->TxSharedReq),
+ &DeviceAddress,
+ &Dev->TxSharedReqMap
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeTxSharedReqBuffer;
+ }
+
+ Dev->TxSharedReq = TxSharedReqBuffer;
+
+
+ //
// In VirtIo 1.0, the NumBuffers field is mandatory. In 0.9.5, it depends on
// VIRTIO_NET_F_MRG_RXBUF, which we never negotiate.
//
TxSharedReqSize = (Dev->VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0)) ?
- sizeof Dev->TxSharedReq.V0_9_5 :
- sizeof Dev->TxSharedReq;
+ sizeof (Dev->TxSharedReq->V0_9_5) :
+ sizeof *Dev->TxSharedReq;
for (PktIdx = 0; PktIdx < Dev->TxMaxPending; ++PktIdx) {
UINT16 DescIdx;
@@ -187,7 +224,7 @@ VirtioNetInitTx (
// For each possibly pending packet, lay out the descriptor for the common
// (unmodified by the host) virtio-net request header.
//
- Dev->TxRing.Desc[DescIdx].Addr = (UINTN) &Dev->TxSharedReq;
+ Dev->TxRing.Desc[DescIdx].Addr = DeviceAddress;
Dev->TxRing.Desc[DescIdx].Len = (UINT32) TxSharedReqSize;
Dev->TxRing.Desc[DescIdx].Flags = VRING_DESC_F_NEXT;
Dev->TxRing.Desc[DescIdx].Next = (UINT16) (DescIdx + 1);
@@ -202,13 +239,13 @@ VirtioNetInitTx (
//
// virtio-0.9.5, Appendix C, Packet Transmission
//
- Dev->TxSharedReq.V0_9_5.Flags = 0;
- Dev->TxSharedReq.V0_9_5.GsoType = VIRTIO_NET_HDR_GSO_NONE;
+ Dev->TxSharedReq->V0_9_5.Flags = 0;
+ Dev->TxSharedReq->V0_9_5.GsoType = VIRTIO_NET_HDR_GSO_NONE;
//
// For VirtIo 1.0 only -- the field exists, but it is unused
//
- Dev->TxSharedReq.NumBuffers = 0;
+ Dev->TxSharedReq->NumBuffers = 0;
//
// virtio-0.9.5, 2.4.2 Receiving Used Buffers From the Device
@@ -223,6 +260,17 @@ VirtioNetInitTx (
*Dev->TxRing.Avail.Flags = (UINT16) VRING_AVAIL_F_NO_INTERRUPT;
return EFI_SUCCESS;
+
+FreeTxSharedReqBuffer:
+ Dev->VirtIo->FreeSharedPages (
+ Dev->VirtIo,
+ EFI_SIZE_TO_PAGES (sizeof *(Dev->TxSharedReq)),
+ TxSharedReqBuffer
+ );
+FreeTxFreeStack:
+ FreePool (Dev->TxFreeStack);
+
+ return Status;
}
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index ee4f9ed36ecd..2fce8142d554 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -54,6 +54,13 @@ VirtioNetShutdownTx (
IN OUT VNET_DEV *Dev
)
{
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->TxSharedReqMap);
+ Dev->VirtIo->FreeSharedPages (
+ Dev->VirtIo,
+ EFI_SIZE_TO_PAGES (sizeof *(Dev->TxSharedReq)),
+ Dev->TxSharedReq
+ );
+
FreePool (Dev->TxFreeStack);
}
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 4/8] OvmfPkg/VirtioNetDxe: dynamically alloc transmit header
2017-09-14 11:08 ` [PATCH v3 4/8] OvmfPkg/VirtioNetDxe: dynamically alloc transmit header Brijesh Singh
@ 2017-09-14 19:12 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2017-09-14 19:12 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/17 13:08, Brijesh Singh wrote:
> Each network packet is submitted for transmission by pushing the head
> descriptor of a two-part descriptor chain to the Available Ring of the
> TX queue. VirtioNetInitTx() sets up the the descriptor chains for all
> queueable packets in advance, and points all the head descriptors to the
> same shared, never modified, VIRTIO_1_0_NET_REQ header object (or its
> initial VIRTIO_NET_REQ sub-object, dependent on virtio version).
> VirtioNetInitTx() currently uses the header object's system physical
> address for populating the head descriptors.
>
> When device is behind the IOMMU, VirtioNet driver is required to provide
> the device address of VIRTIO_1_0_NET_REQ header. In this patch we
> dynamically allocate the header using AllocateSharedPages() and map with
> BusMasterCommonBuffer so that header can be accessed by both processor
> and the device.
>
> We map the header object for CommonBuffer operation because, in order to
> stick with the current code order, we populate the head descriptors with
> the header's device address first, and fill in the header itself second.
>
> 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 | 3 +-
> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 64 +++++++++++++++++---
> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 7 +++
> 3 files changed, 65 insertions(+), 9 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 5/8] OvmfPkg/VirtioNetDxe: update TechNotes
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
` (3 preceding siblings ...)
2017-09-14 11:08 ` [PATCH v3 4/8] OvmfPkg/VirtioNetDxe: dynamically alloc transmit header Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 6/8] OvmfPkg/VirtioNetDxe: add Tx packet map/unmap helper functions Brijesh Singh
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
In next patches we will update Virtio transmit to use the device-mapped
address of the caller-supplied packet. The patch documents the new model.
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioNetDxe/TechNotes.txt | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt
index fedbaee07ec7..40a22f66dc22 100644
--- a/OvmfPkg/VirtioNetDxe/TechNotes.txt
+++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt
@@ -312,10 +312,14 @@ in the following:
that is shared by all of the head descriptors. This virtio-net request header
is never modified by the host.
-- Each tail descriptor is re-pointed to the caller-supplied packet buffer
- whenever VirtioNetTransmit places the corresponding head descriptor on the
- Available Ring. The caller is responsible to hang on to the unmodified buffer
- until it is reported transmitted by VirtioNetGetStatus.
+- Each tail descriptor is re-pointed to the device-mapped address of the
+ caller-supplied packet buffer whenever VirtioNetTransmit places the
+ corresponding head descriptor on the Available Ring. A reverse mapping, from
+ the device-mapped address to the caller-supplied packet address, is saved in
+ an associative data structure that belongs to the driver instance.
+
+- Per spec, the caller is responsible to hang on to the unmodified packet
+ buffer until it is reported transmitted by VirtioNetGetStatus.
Steps of packet transmission:
@@ -338,9 +342,11 @@ Steps of packet transmission:
- Client code calls VirtioNetGetStatus. In case the Used Ring is empty, the
function reports no Tx completion. Otherwise, a head descriptor's index is
consumed from the Used Ring and recycled to the private stack. The client
- code's original packet buffer address is fetched from the tail descriptor
- (where it has been stored at VirtioNetTransmit time) and returned to the
- caller.
+ code's original packet buffer address is calculated by fetching the
+ device-mapped address from the tail descriptor (where it has been stored at
+ VirtioNetTransmit time), and by looking up the device-mapped address in the
+ associative data structure. The reverse-mapped packet buffer address is
+ returned to the caller.
- The Len field of the Used Ring Element is not checked. The host is assumed to
have transmitted the entire packet -- VirtioNetTransmit had forced it below
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 6/8] OvmfPkg/VirtioNetDxe: add Tx packet map/unmap helper functions
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
` (4 preceding siblings ...)
2017-09-14 11:08 ` [PATCH v3 5/8] OvmfPkg/VirtioNetDxe: update TechNotes Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 19:56 ` Laszlo Ersek
2017-09-14 11:08 ` [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address Brijesh Singh
2017-09-14 11:08 ` [PATCH v3 8/8] OvmfPkg/VirtioNetDxe: negotiate VIRTIO_F_IOMMU_PLATFORM Brijesh Singh
7 siblings, 1 reply; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
When device is behind IOMMU, driver is require to pass the device address
of TxBuf in the Tx VRING. The patch adds helper functions and data
structure to map and unmap the TxBuf system physical address to a device
address.
Since the TxBuf is returned back to caller from VirtioNetGetStatus() hence
we use OrderedCollection interface to save the TxBuf system physical to
device address mapping. After the TxBuf is succesfully transmitted
VirtioNetUnmapTxBuf() does the reverse lookup in OrderedCollection data
structure to get the system physical address of TxBuf for a given device
address.
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.inf | 1 +
OvmfPkg/VirtioNetDxe/VirtioNet.h | 38 ++++
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 18 +-
OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 224 ++++++++++++++++++++
4 files changed, 279 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.inf b/OvmfPkg/VirtioNetDxe/VirtioNet.inf
index a855ad4ac154..9ff6d87e6190 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.inf
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.inf
@@ -49,6 +49,7 @@
DebugLib
DevicePathLib
MemoryAllocationLib
+ OrderedCollectionLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 027f75993e8e..3fc88cfb790e 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -26,6 +26,7 @@
#include <Protocol/DevicePath.h>
#include <Protocol/DriverBinding.h>
#include <Protocol/SimpleNetwork.h>
+#include <Library/OrderedCollectionLib.h>
#define VNET_SIG SIGNATURE_32 ('V', 'N', 'E', 'T')
@@ -100,6 +101,7 @@ typedef struct {
VIRTIO_1_0_NET_REQ *TxSharedReq; // VirtioNetInitTx
VOID *TxSharedReqMap; // VirtioNetInitTx
UINT16 TxLastUsed; // VirtioNetInitTx
+ ORDERED_COLLECTION *TxBufCollection; // VirtioNetInitTx
} VNET_DEV;
@@ -281,6 +283,42 @@ VirtioNetUninitRing (
);
//
+// utility functions to map caller-supplied Tx buffer system physical address
+// to a device address and vice versa
+//
+EFI_STATUS
+EFIAPI
+VirtioNetMapTxBuf (
+ IN VNET_DEV *Dev,
+ IN VOID *Buffer,
+ IN UINTN NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
+ );
+
+EFI_STATUS
+EFIAPI
+VirtioNetUnmapTxBuf (
+ IN VNET_DEV *Dev,
+ OUT VOID **Buffer,
+ IN EFI_PHYSICAL_ADDRESS DeviceAddress
+ );
+
+INTN
+EFIAPI
+VirtioNetTxBufMapInfoCompare (
+ IN CONST VOID *UserStruct1,
+ IN CONST VOID *UserStruct2
+ );
+
+INTN
+EFIAPI
+VirtioNetTxBufDeviceAddressCompare (
+ IN CONST VOID *StandaloneKey,
+ IN CONST VOID *UserStruct
+ );
+
+
+//
// event callbacks
//
VOID
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index 9621f936d2cb..ffb3deefe00c 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -147,7 +147,8 @@ ReleaseQueue:
EfiSimpleNetworkInitialized state.
@retval EFI_OUT_OF_RESOURCES Failed to allocate the stack to track the heads
- of free descriptor chains.
+ of free descriptor chains or failed to init
+ TxBufCollection.
@return Status codes from VIRTIO_DEVICE_PROTOCOL.
AllocateSharedPages() or
VirtioMapAllBytesInSharedBuffer()
@@ -176,6 +177,15 @@ VirtioNetInitTx (
return EFI_OUT_OF_RESOURCES;
}
+ Dev->TxBufCollection = OrderedCollectionInit (
+ VirtioNetTxBufMapInfoCompare,
+ VirtioNetTxBufDeviceAddressCompare
+ );
+ if (Dev->TxBufCollection == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeTxFreeStack;
+ }
+
//
// Allocate TxSharedReq header and map with BusMasterCommonBuffer so that it
// can be accessed equally by both processor and device.
@@ -186,7 +196,7 @@ VirtioNetInitTx (
&TxSharedReqBuffer
);
if (EFI_ERROR (Status)) {
- goto FreeTxFreeStack;
+ goto UninitTxBufCollection;
}
ZeroMem (TxSharedReqBuffer, sizeof *Dev->TxSharedReq);
@@ -267,6 +277,10 @@ FreeTxSharedReqBuffer:
EFI_SIZE_TO_PAGES (sizeof *(Dev->TxSharedReq)),
TxSharedReqBuffer
);
+
+UninitTxBufCollection:
+ OrderedCollectionUninit (Dev->TxBufCollection);
+
FreeTxFreeStack:
FreePool (Dev->TxFreeStack);
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index 2fce8142d554..18dbf1812541 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -18,6 +18,16 @@
#include "VirtioNet.h"
+//
+// The user structure for the ordered collection that will track the mapping
+// info of the packets queued in TxRing
+//
+typedef struct {
+ VOID *Buffer;
+ EFI_PHYSICAL_ADDRESS DeviceAddress; // lookup key for reverse mapping
+ VOID *BufMap;
+} TX_BUF_MAP_INFO;
+
/**
Release RX and TX resources on the boundary of the
EfiSimpleNetworkInitialized state.
@@ -54,6 +64,10 @@ VirtioNetShutdownTx (
IN OUT VNET_DEV *Dev
)
{
+ ORDERED_COLLECTION_ENTRY *Entry, *Entry2;
+ TX_BUF_MAP_INFO *TxBufMapInfo;
+ VOID *UserStruct;
+
Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->TxSharedReqMap);
Dev->VirtIo->FreeSharedPages (
Dev->VirtIo,
@@ -61,6 +75,17 @@ VirtioNetShutdownTx (
Dev->TxSharedReq
);
+ for (Entry = OrderedCollectionMin (Dev->TxBufCollection);
+ Entry != NULL;
+ Entry = Entry2) {
+ Entry2 = OrderedCollectionNext (Entry);
+ OrderedCollectionDelete (Dev->TxBufCollection, Entry, &UserStruct);
+ TxBufMapInfo = UserStruct;
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, TxBufMapInfo->BufMap);
+ FreePool (TxBufMapInfo);
+ }
+ OrderedCollectionUninit (Dev->TxBufCollection);
+
FreePool (Dev->TxFreeStack);
}
@@ -83,3 +108,202 @@ VirtioNetUninitRing (
Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RingMap);
VirtioRingUninit (Dev->VirtIo, Ring);
}
+
+
+/**
+ Map Caller-supplied TxBuf buffer to the device-mapped address
+
+ @param[in] Dev The VNET_DEV driver instance which wants to
+ map the Tx packet.
+ @param[in] Buffer The system physical address of TxBuf
+ @param[in] NumberOfBytes Number of bytes to map
+ @param[out] DeviceAddress The resulting device address for the bus
+ master access.
+
+ @retval EFI_OUT_OF_RESOURCES The request could not be completed due to
+ a lack of resources.
+ @return Status codes from
+ VirtioMapAllBytesInSharedBuffer()
+ @retval EFI_SUCCESS Caller-supplied buffer is succesfully mapped.
+*/
+EFI_STATUS
+EFIAPI
+VirtioNetMapTxBuf (
+ IN VNET_DEV *Dev,
+ IN VOID *Buffer,
+ IN UINTN NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
+ )
+{
+ EFI_STATUS Status;
+ TX_BUF_MAP_INFO *TxBufMapInfo;
+ EFI_PHYSICAL_ADDRESS Address;
+ VOID *Mapping;
+
+ TxBufMapInfo = AllocatePool (sizeof (*TxBufMapInfo));
+ if (TxBufMapInfo == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = VirtioMapAllBytesInSharedBuffer (
+ Dev->VirtIo,
+ VirtioOperationBusMasterRead,
+ Buffer,
+ NumberOfBytes,
+ &Address,
+ &Mapping
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeTxBufMapInfo;
+ }
+
+ TxBufMapInfo->Buffer = Buffer;
+ TxBufMapInfo->DeviceAddress = Address;
+ TxBufMapInfo->BufMap = Mapping;
+
+ Status = OrderedCollectionInsert (
+ Dev->TxBufCollection,
+ NULL,
+ TxBufMapInfo
+ );
+ switch (Status) {
+ case EFI_OUT_OF_RESOURCES:
+ goto UnmapTxBuf;
+ case EFI_ALREADY_STARTED:
+ //
+ // This should never happen: it implies
+ //
+ // - an identity-mapping VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer()
+ // implementation -- which is fine,
+ //
+ // - and an SNP client that queues multiple instances of the exact same
+ // buffer address with SNP.Transmit() -- which is undefined behavior,
+ // based on the TxBuf language in UEFI-2.7,
+ // EFI_SIMPLE_NETWORK.GetStatus().
+ //
+ ASSERT (FALSE);
+ Status = EFI_INVALID_PARAMETER;
+ goto UnmapTxBuf;
+ default:
+ ASSERT_EFI_ERROR (Status);
+ break;
+ }
+
+ *DeviceAddress = Address;
+ return EFI_SUCCESS;
+
+UnmapTxBuf:
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Mapping);
+
+FreeTxBufMapInfo:
+ FreePool (TxBufMapInfo);
+ return Status;
+}
+
+/**
+ Unmap (aka reverse mapping) device mapped TxBuf buffer to the system
+ physical address
+
+ @param[in] Dev The VNET_DEV driver instance which wants to
+ reverse- and unmap the Tx packet.
+ @param[out] Buffer The system physical address of TxBuf
+ @param[in] DeviceAddress The device address for the TxBuf
+
+ @retval EFI_INVALID_PARAMETER The DeviceAddress is not mapped
+ @retval EFI_SUCCESS The TxBuf at DeviceAddress has been unmapped,
+ and Buffer has been set to TxBuf's system
+ physical address.
+
+*/
+EFI_STATUS
+EFIAPI
+VirtioNetUnmapTxBuf (
+ IN VNET_DEV *Dev,
+ OUT VOID **Buffer,
+ IN EFI_PHYSICAL_ADDRESS DeviceAddress
+ )
+{
+ ORDERED_COLLECTION_ENTRY *Entry;
+ TX_BUF_MAP_INFO *TxBufMapInfo;
+ VOID *UserStruct;
+
+ Entry = OrderedCollectionFind (Dev->TxBufCollection, &DeviceAddress);
+ if (Entry == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ OrderedCollectionDelete (Dev->TxBufCollection, Entry, &UserStruct);
+
+ TxBufMapInfo = UserStruct;
+
+ *Buffer = TxBufMapInfo->Buffer;
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, TxBufMapInfo->BufMap);
+ FreePool (TxBufMapInfo);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Comparator function for two TX_BUF_MAP_INFO objects.
+
+ @param[in] UserStruct1 Pointer to the first TX_BUF_MAP_INFO object.
+
+ @param[in] UserStruct2 Pointer to the second TX_BUF_MAP_INFO object.
+
+ @retval <0 If UserStruct1 compares less than UserStruct2.
+
+ @retval 0 If UserStruct1 compares equal to UserStruct2.
+
+ @retval >0 If UserStruct1 compares greater than UserStruct2.
+*/
+INTN
+EFIAPI
+VirtioNetTxBufMapInfoCompare (
+ IN CONST VOID *UserStruct1,
+ IN CONST VOID *UserStruct2
+ )
+{
+ CONST TX_BUF_MAP_INFO *MapInfo1;
+ CONST TX_BUF_MAP_INFO *MapInfo2;
+
+ MapInfo1 = UserStruct1;
+ MapInfo2 = UserStruct2;
+
+ return MapInfo1->DeviceAddress < MapInfo2->DeviceAddress ? -1 :
+ MapInfo1->DeviceAddress > MapInfo2->DeviceAddress ? 1 :
+ 0;
+}
+
+/**
+ Compare a standalone DeviceAddress against a TX_BUF_MAP_INFO object
+ containing an embedded DeviceAddress.
+
+ @param[in] StandaloneKey Pointer to DeviceAddress, which has type
+ EFI_PHYSICAL_ADDRESS.
+
+ @param[in] UserStruct Pointer to the TX_BUF_MAP_INFO object with the
+ embedded DeviceAddress.
+
+ @retval <0 If StandaloneKey compares less than UserStruct's key.
+
+ @retval 0 If StandaloneKey compares equal to UserStruct's key.
+
+ @retval >0 If StandaloneKey compares greater than UserStruct's key.
+**/
+INTN
+EFIAPI
+VirtioNetTxBufDeviceAddressCompare (
+ IN CONST VOID *StandaloneKey,
+ IN CONST VOID *UserStruct
+ )
+{
+ CONST EFI_PHYSICAL_ADDRESS *DeviceAddress;
+ CONST TX_BUF_MAP_INFO *MapInfo;
+
+ DeviceAddress = StandaloneKey;
+ MapInfo = UserStruct;
+
+ return *DeviceAddress < MapInfo->DeviceAddress ? -1 :
+ *DeviceAddress > MapInfo->DeviceAddress ? 1 :
+ 0;
+}
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 6/8] OvmfPkg/VirtioNetDxe: add Tx packet map/unmap helper functions
2017-09-14 11:08 ` [PATCH v3 6/8] OvmfPkg/VirtioNetDxe: add Tx packet map/unmap helper functions Brijesh Singh
@ 2017-09-14 19:56 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2017-09-14 19:56 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/17 13:08, Brijesh Singh wrote:
> When device is behind IOMMU, driver is require to pass the device address
> of TxBuf in the Tx VRING. The patch adds helper functions and data
> structure to map and unmap the TxBuf system physical address to a device
> address.
>
> Since the TxBuf is returned back to caller from VirtioNetGetStatus() hence
> we use OrderedCollection interface to save the TxBuf system physical to
> device address mapping. After the TxBuf is succesfully transmitted
> VirtioNetUnmapTxBuf() does the reverse lookup in OrderedCollection data
> structure to get the system physical address of TxBuf for a given device
> address.
>
> 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.inf | 1 +
> OvmfPkg/VirtioNetDxe/VirtioNet.h | 38 ++++
> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 18 +-
> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 224 ++++++++++++++++++++
> 4 files changed, 279 insertions(+), 2 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
` (5 preceding siblings ...)
2017-09-14 11:08 ` [PATCH v3 6/8] OvmfPkg/VirtioNetDxe: add Tx packet map/unmap helper functions Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
2017-09-14 20:58 ` Laszlo Ersek
2017-09-14 11:08 ` [PATCH v3 8/8] OvmfPkg/VirtioNetDxe: negotiate VIRTIO_F_IOMMU_PLATFORM Brijesh Singh
7 siblings, 1 reply; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
When device is behind the IOMMU, driver is require to pass the device
address of caller-supplied transmit buffer for the bus master operations.
The patch uses VirtioNetMapTxBuf() to map caller-supplied Tx packet to a
device-address and enqueue the device address in VRING for transfer and
perform the reverse mapping when transfer is completed so that we can
return the caller-supplied buffer.
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/SnpGetStatus.c | 41 +++++++++++++++-----
OvmfPkg/VirtioNetDxe/SnpTransmit.c | 27 ++++++++++---
2 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
index 694940ea1d97..f817b98801fa 100644
--- a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
+++ b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
@@ -61,11 +61,15 @@ VirtioNetGetStatus (
OUT VOID **TxBuf OPTIONAL
)
{
- VNET_DEV *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT16 RxCurUsed;
- UINT16 TxCurUsed;
+ VNET_DEV *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT16 RxCurUsed;
+ UINT16 TxCurUsed;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
+ UINT32 LocalInterruptStatus;
+
+ LocalInterruptStatus = *InterruptStatus;
if (This == NULL) {
return EFI_INVALID_PARAMETER;
@@ -113,11 +117,11 @@ VirtioNetGetStatus (
//
*InterruptStatus = 0;
if (Dev->RxLastUsed != RxCurUsed) {
- *InterruptStatus |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
+ LocalInterruptStatus |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
}
if (Dev->TxLastUsed != TxCurUsed) {
ASSERT (Dev->TxCurPending > 0);
- *InterruptStatus |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
+ LocalInterruptStatus |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
}
}
@@ -141,17 +145,36 @@ VirtioNetGetStatus (
ASSERT (DescIdx < (UINT32) (2 * Dev->TxMaxPending - 1));
//
- // report buffer address to caller that has been enqueued by caller
+ // get the device address that has been enqueued for the caller's
+ // transmit buffer
//
- *TxBuf = (VOID *)(UINTN) Dev->TxRing.Desc[DescIdx + 1].Addr;
+ DeviceAddress = Dev->TxRing.Desc[DescIdx + 1].Addr;
//
// now this descriptor can be used again to enqueue a transmit buffer
//
Dev->TxFreeStack[--Dev->TxCurPending] = (UINT16) DescIdx;
+
+ //
+ // Unmap the device address and perform the reverse mapping to find the
+ // caller buffer address.
+ //
+ Status = VirtioNetUnmapTxBuf (
+ Dev,
+ TxBuf,
+ DeviceAddress
+ );
+ if (EFI_ERROR (Status)) {
+ //
+ // VirtioNetUnmapTxBuf should never fail, if we have reached here
+ // that means our internal state has been corrupted
+ //
+ ASSERT (FALSE);
+ }
}
}
+ *InterruptStatus = LocalInterruptStatus;
Status = EFI_SUCCESS;
Exit:
diff --git a/OvmfPkg/VirtioNetDxe/SnpTransmit.c b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
index 7ca40d5d0650..b39226e138b9 100644
--- a/OvmfPkg/VirtioNetDxe/SnpTransmit.c
+++ b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
@@ -73,11 +73,12 @@ VirtioNetTransmit (
IN UINT16 *Protocol OPTIONAL
)
{
- VNET_DEV *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT16 DescIdx;
- UINT16 AvailIdx;
+ VNET_DEV *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT16 DescIdx;
+ UINT16 AvailIdx;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
if (This == NULL || BufferSize == 0 || Buffer == NULL) {
return EFI_INVALID_PARAMETER;
@@ -144,10 +145,24 @@ VirtioNetTransmit (
}
//
+ // Map the transmit buffer system physical address to device address.
+ //
+ Status = VirtioNetMapTxBuf (
+ Dev,
+ Buffer,
+ BufferSize,
+ &DeviceAddress
+ );
+ if (EFI_ERROR (Status)) {
+ Status = EFI_DEVICE_ERROR;
+ goto Exit;
+ }
+
+ //
// virtio-0.9.5, 2.4.1 Supplying Buffers to The Device
//
DescIdx = Dev->TxFreeStack[Dev->TxCurPending++];
- Dev->TxRing.Desc[DescIdx + 1].Addr = (UINTN) Buffer;
+ Dev->TxRing.Desc[DescIdx + 1].Addr = DeviceAddress;
Dev->TxRing.Desc[DescIdx + 1].Len = (UINT32) BufferSize;
//
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address
2017-09-14 11:08 ` [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address Brijesh Singh
@ 2017-09-14 20:58 ` Laszlo Ersek
2017-09-14 21:01 ` Brijesh Singh
0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2017-09-14 20:58 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/17 13:08, Brijesh Singh wrote:
> When device is behind the IOMMU, driver is require to pass the device
> address of caller-supplied transmit buffer for the bus master operations.
>
> The patch uses VirtioNetMapTxBuf() to map caller-supplied Tx packet to a
> device-address and enqueue the device address in VRING for transfer and
> perform the reverse mapping when transfer is completed so that we can
> return the caller-supplied buffer.
>
> 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/SnpGetStatus.c | 41 +++++++++++++++-----
> OvmfPkg/VirtioNetDxe/SnpTransmit.c | 27 ++++++++++---
> 2 files changed, 53 insertions(+), 15 deletions(-)
>
> diff --git a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> index 694940ea1d97..f817b98801fa 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> @@ -61,11 +61,15 @@ VirtioNetGetStatus (
> OUT VOID **TxBuf OPTIONAL
> )
> {
> - VNET_DEV *Dev;
> - EFI_TPL OldTpl;
> - EFI_STATUS Status;
> - UINT16 RxCurUsed;
> - UINT16 TxCurUsed;
> + VNET_DEV *Dev;
> + EFI_TPL OldTpl;
> + EFI_STATUS Status;
> + UINT16 RxCurUsed;
> + UINT16 TxCurUsed;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
> + UINT32 LocalInterruptStatus;
> +
> + LocalInterruptStatus = *InterruptStatus;
>
> if (This == NULL) {
> return EFI_INVALID_PARAMETER;
> @@ -113,11 +117,11 @@ VirtioNetGetStatus (
> //
> *InterruptStatus = 0;
> if (Dev->RxLastUsed != RxCurUsed) {
> - *InterruptStatus |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> + LocalInterruptStatus |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> }
> if (Dev->TxLastUsed != TxCurUsed) {
> ASSERT (Dev->TxCurPending > 0);
> - *InterruptStatus |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
> + LocalInterruptStatus |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
> }
> }
>
I must have been unclear in my v2 review, sorry about that. My point was
that, given the changes that I requested in points v2/3 and v2/4, the
handling of "InterruptStatus" was fine as it was.
(1) Therefore please undo the v3 "InterruptStatus" changes -- the
"LocalInterruptStatus" variable is unnecessary.
> @@ -141,17 +145,36 @@ VirtioNetGetStatus (
> ASSERT (DescIdx < (UINT32) (2 * Dev->TxMaxPending - 1));
>
> //
> - // report buffer address to caller that has been enqueued by caller
> + // get the device address that has been enqueued for the caller's
> + // transmit buffer
> //
> - *TxBuf = (VOID *)(UINTN) Dev->TxRing.Desc[DescIdx + 1].Addr;
> + DeviceAddress = Dev->TxRing.Desc[DescIdx + 1].Addr;
>
> //
> // now this descriptor can be used again to enqueue a transmit buffer
> //
> Dev->TxFreeStack[--Dev->TxCurPending] = (UINT16) DescIdx;
> +
> + //
> + // Unmap the device address and perform the reverse mapping to find the
> + // caller buffer address.
> + //
> + Status = VirtioNetUnmapTxBuf (
> + Dev,
> + TxBuf,
> + DeviceAddress
> + );
> + if (EFI_ERROR (Status)) {
> + //
> + // VirtioNetUnmapTxBuf should never fail, if we have reached here
> + // that means our internal state has been corrupted
> + //
> + ASSERT (FALSE);
The rest of VirtioNetGetStatus() looks good, but here's the other
misunderstanding:
(2) Please *keep* the ASSERT() that you are adding above, and below it,
*add back* what you had in v2:
Status = EFI_DEVICE_ERROR;
goto Exit;
The rest looks good.
Can you submit v4 soon please? :)
Thanks,
Laszlo
> + }
> }
> }
>
> + *InterruptStatus = LocalInterruptStatus;
> Status = EFI_SUCCESS;
>
> Exit:
> diff --git a/OvmfPkg/VirtioNetDxe/SnpTransmit.c b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> index 7ca40d5d0650..b39226e138b9 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> @@ -73,11 +73,12 @@ VirtioNetTransmit (
> IN UINT16 *Protocol OPTIONAL
> )
> {
> - VNET_DEV *Dev;
> - EFI_TPL OldTpl;
> - EFI_STATUS Status;
> - UINT16 DescIdx;
> - UINT16 AvailIdx;
> + VNET_DEV *Dev;
> + EFI_TPL OldTpl;
> + EFI_STATUS Status;
> + UINT16 DescIdx;
> + UINT16 AvailIdx;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
>
> if (This == NULL || BufferSize == 0 || Buffer == NULL) {
> return EFI_INVALID_PARAMETER;
> @@ -144,10 +145,24 @@ VirtioNetTransmit (
> }
>
> //
> + // Map the transmit buffer system physical address to device address.
> + //
> + Status = VirtioNetMapTxBuf (
> + Dev,
> + Buffer,
> + BufferSize,
> + &DeviceAddress
> + );
> + if (EFI_ERROR (Status)) {
> + Status = EFI_DEVICE_ERROR;
> + goto Exit;
> + }
> +
> + //
> // virtio-0.9.5, 2.4.1 Supplying Buffers to The Device
> //
> DescIdx = Dev->TxFreeStack[Dev->TxCurPending++];
> - Dev->TxRing.Desc[DescIdx + 1].Addr = (UINTN) Buffer;
> + Dev->TxRing.Desc[DescIdx + 1].Addr = DeviceAddress;
> Dev->TxRing.Desc[DescIdx + 1].Len = (UINT32) BufferSize;
>
> //
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address
2017-09-14 20:58 ` Laszlo Ersek
@ 2017-09-14 21:01 ` Brijesh Singh
0 siblings, 0 replies; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 21:01 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel
Cc: brijesh.singh, Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 09/14/2017 03:58 PM, Laszlo Ersek wrote:
...>
> The rest of VirtioNetGetStatus() looks good, but here's the other
> misunderstanding:
>
> (2) Please *keep* the ASSERT() that you are adding above, and below it,
> *add back* what you had in v2:
>
> Status = EFI_DEVICE_ERROR;
> goto Exit;
>
> The rest looks good.
>
> Can you submit v4 soon please? :)
>
Yes, I should be submitting very soon :)
-Brijesh
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 8/8] OvmfPkg/VirtioNetDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
2017-09-14 11:08 [PATCH v3 0/8] OvmfPkg/VirtioNetDxe: map host address to device address Brijesh Singh
` (6 preceding siblings ...)
2017-09-14 11:08 ` [PATCH v3 7/8] OvmfPkg/VirtioNetDxe: map caller-supplied Tx packet to device-address Brijesh Singh
@ 2017-09-14 11:08 ` Brijesh Singh
7 siblings, 0 replies; 15+ messages in thread
From: Brijesh Singh @ 2017-09-14 11:08 UTC (permalink / raw)
To: edk2-devel
Cc: Brijesh Singh, Ard Biesheuvel, Jordan Justen, Tom Lendacky,
Laszlo Ersek
VirtioNetDxe driver has been updated to use IOMMU-like member functions
from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to
device address. We do not need to do anything special when
VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with
VIRTIO_F_VERSION_1.
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index ffb3deefe00c..9dff04ce6071 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -562,7 +562,8 @@ VirtioNetInitialize (
ASSERT (Dev->Snm.MediaPresentSupported ==
!!(Features & VIRTIO_NET_F_STATUS));
- Features &= VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_F_VERSION_1;
+ Features &= VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_F_VERSION_1 |
+ VIRTIO_F_IOMMU_PLATFORM;
//
// In virtio-1.0, feature negotiation is expected to complete before queue
@@ -602,7 +603,7 @@ VirtioNetInitialize (
// step 5 -- keep only the features we want
//
if (Dev->VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0)) {
- Features &= ~(UINT64)VIRTIO_F_VERSION_1;
+ Features &= ~(UINT64)(VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM);
Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features);
if (EFI_ERROR (Status)) {
goto ReleaseTxRing;
--
2.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread