When device is behind the IOMMU, driver is require to pass the device
address of transmit buffer for the bus master operations.
The patch uses VirtioMapAllBytesInSharedBuffer() to map transmit buffer
system physical address to the device address.
Since the transmit buffers are returned back to caller in
VirtioNetGetStatus() hence we use OrderCollection library interface to
save the host to device address mapping. After the buffer is succesfully
transmited we do reverse lookup in OrderCollection data structure to get
the host address for the transmitted 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 | 19 +++
OvmfPkg/VirtioNetDxe/SnpGetStatus.c | 30 +++-
OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 157 ++++++++++++++++++++
OvmfPkg/VirtioNetDxe/SnpTransmit.c | 37 ++++-
5 files changed, 232 insertions(+), 12 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 @@ [LibraryClasses]
DebugLib
DevicePathLib
MemoryAllocationLib
+ OrderedCollectionLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
index 3efe95a735d8..d931969af795 100644
--- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
+++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
@@ -269,6 +269,25 @@ VirtioNetShutdownTx (
IN OUT VNET_DEV *Dev
);
+EFI_STATUS
+EFIAPI
+VirtioMapTxBuf (
+ IN VNET_DEV *Dev,
+ IN UINT16 Index,
+ IN EFI_PHYSICAL_ADDRESS HostAddress,
+ IN UINTN NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
+ );
+
+EFI_STATUS
+EFIAPI
+VirtioUnmapTxBuf (
+ IN VNET_DEV *Dev,
+ IN UINT16 Index,
+ OUT EFI_PHYSICAL_ADDRESS *HostAddress,
+ IN EFI_PHYSICAL_ADDRESS DeviceAddress
+ );
+
//
// event callbacks
//
diff --git a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
index 694940ea1d97..10ca26de6d7a 100644
--- a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
+++ b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
@@ -5,6 +5,7 @@
Copyright (C) 2013, Red Hat, Inc.
Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+ 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
@@ -47,7 +48,8 @@
@retval EFI_INVALID_PARAMETER One or more of the parameters has an
unsupported value.
@retval EFI_DEVICE_ERROR The command could not be sent to the network
- interface.
+ interface or failed to map TxBuf to bus master
+ address.
@retval EFI_UNSUPPORTED This function is not supported by the network
interface.
@@ -126,8 +128,10 @@ VirtioNetGetStatus (
*TxBuf = NULL;
}
else {
- UINT16 UsedElemIdx;
- UINT32 DescIdx;
+ UINT16 UsedElemIdx;
+ UINT32 DescIdx;
+ EFI_PHYSICAL_ADDRESS BufferAddress;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
//
// fetch the first descriptor among those that the hypervisor reports
@@ -141,9 +145,27 @@ VirtioNetGetStatus (
ASSERT (DescIdx < (UINT32) (2 * Dev->TxMaxPending - 1));
//
+ // Ring descriptor contains the device address for the caller buffer.
+ // Lets unmap the device address and find its corresponding system
+ // physical address.
+ //
+ DeviceAddress = Dev->TxRing.Desc[DescIdx + 1].Addr;
+ Status = VirtioUnmapTxBuf (
+ Dev,
+ DescIdx + 1,
+ &BufferAddress,
+ DeviceAddress
+ );
+ if (EFI_ERROR (Status)) {
+ Status = EFI_DEVICE_ERROR;
+ goto Exit;
+ }
+
+ //
+ //
// report buffer address to caller that has been enqueued by caller
//
- *TxBuf = (VOID *)(UINTN) Dev->TxRing.Desc[DescIdx + 1].Addr;
+ *TxBuf = (VOID *)(UINTN) BufferAddress;
//
// now this descriptor can be used again to enqueue a transmit buffer
diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
index aeb9e6605f0d..b91ddd3e4ede 100644
--- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
+++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
@@ -14,10 +14,25 @@
**/
+#include <Library/BaseLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/OrderedCollectionLib.h>
#include "VirtioNet.h"
+typedef struct {
+ UINT16 Value;
+} USER_KEY;
+
+typedef struct {
+ USER_KEY Key;
+ EFI_PHYSICAL_ADDRESS HostAddress;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
+ VOID *BufMap;
+} USER_STRUCT;
+
+STATIC ORDERED_COLLECTION *Collection;
+
/**
Release RX and TX resources on the boundary of the
EfiSimpleNetworkInitialized state.
@@ -63,3 +78,145 @@ VirtioNetShutdownTx (
FreePool (Dev->TxFreeStack);
}
+
+STATIC
+INTN
+EFIAPI
+KeyCompare (
+ IN CONST VOID *StandaloneKey,
+ IN CONST VOID *UserStruct
+ )
+{
+ CONST USER_KEY *CmpKey;
+ CONST USER_STRUCT *CmpStruct;
+
+ CmpKey = StandaloneKey;
+ CmpStruct = UserStruct;
+
+ return CmpKey->Value < CmpStruct->Key.Value ? -1 :
+ CmpKey->Value > CmpStruct->Key.Value ? 1 :
+ 0;
+}
+
+STATIC
+INTN
+EFIAPI
+UserStructCompare (
+ IN CONST VOID *UserStruct1,
+ IN CONST VOID *UserStruct2
+ )
+{
+ CONST USER_STRUCT *CmpStruct1;
+
+ CmpStruct1 = UserStruct1;
+
+ return KeyCompare (&CmpStruct1->Key, UserStruct2);
+}
+
+EFI_STATUS
+EFIAPI
+VirtioMapTxBuf (
+ IN VNET_DEV *Dev,
+ IN UINT16 Index,
+ IN EFI_PHYSICAL_ADDRESS HostAddress,
+ IN UINTN NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
+ )
+{
+ EFI_STATUS Status;
+ ORDERED_COLLECTION_ENTRY *Entry;
+ USER_STRUCT *UserStruct;
+ EFI_PHYSICAL_ADDRESS Address;
+ VOID *Mapping;
+
+ //
+ // If ordered collection is not initialized then initialize it.
+ //
+ if (Collection == NULL) {
+ Collection = OrderedCollectionInit (UserStructCompare, KeyCompare);
+ if (Collection == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ }
+
+ UserStruct = AllocatePool (sizeof (*UserStruct));
+ if (UserStruct == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = VirtioMapAllBytesInSharedBuffer (
+ Dev->VirtIo,
+ VirtioOperationBusMasterRead,
+ (VOID *) (UINTN) HostAddress,
+ NumberOfBytes,
+ &Address,
+ &Mapping
+ );
+ if (EFI_ERROR (Status)) {
+ goto ReleaseUserStruct;
+ }
+
+ UserStruct->Key.Value = Index;
+ UserStruct->HostAddress = HostAddress;
+ UserStruct->DeviceAddress = Address;
+ UserStruct->BufMap = Mapping;
+
+ Status = OrderedCollectionInsert (Collection, &Entry, UserStruct);
+ switch (Status) {
+ case RETURN_OUT_OF_RESOURCES:
+ Status = EFI_OUT_OF_RESOURCES;
+ goto UnmapBuffer;
+ case RETURN_ALREADY_STARTED:
+ Status = EFI_INVALID_PARAMETER;
+ goto UnmapBuffer;
+ default:
+ ASSERT (Status == RETURN_SUCCESS);
+ break;
+ }
+
+ ASSERT (OrderedCollectionUserStruct (Entry) == UserStruct);
+
+ *DeviceAddress = Address;
+ return EFI_SUCCESS;
+
+UnmapBuffer:
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Mapping);
+
+ReleaseUserStruct:
+ FreePool (UserStruct);
+ return Status;
+}
+
+EFI_STATUS
+EFIAPI
+VirtioUnmapTxBuf (
+ IN VNET_DEV *Dev,
+ IN UINT16 Index,
+ OUT EFI_PHYSICAL_ADDRESS *HostAddress,
+ IN EFI_PHYSICAL_ADDRESS DeviceAddress
+ )
+{
+ USER_KEY StandaloneKey;
+ ORDERED_COLLECTION_ENTRY *Entry;
+ USER_STRUCT *UserStruct;
+ VOID *Ptr;
+ EFI_STATUS Status;
+
+ StandaloneKey.Value = Index;
+ Entry = OrderedCollectionFind (Collection, &StandaloneKey);
+ if (Entry == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ OrderedCollectionDelete (Collection, Entry, &Ptr);
+
+ UserStruct = Ptr;
+ ASSERT (UserStruct->DeviceAddress == DeviceAddress);
+ ASSERT (UserStruct->Key.Value == Index);
+
+ *HostAddress = UserStruct->HostAddress;
+ Status = Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, UserStruct->BufMap);
+ FreePool (UserStruct);
+
+ return Status;
+}
diff --git a/OvmfPkg/VirtioNetDxe/SnpTransmit.c b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
index 7ca40d5d0650..1bd6e0d70d7c 100644
--- a/OvmfPkg/VirtioNetDxe/SnpTransmit.c
+++ b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
@@ -55,7 +55,8 @@
@retval EFI_INVALID_PARAMETER One or more of the parameters has an
unsupported value.
@retval EFI_DEVICE_ERROR The command could not be sent to the network
- interface.
+ interface or failed to map the Buffer to
+ bus master address.
@retval EFI_UNSUPPORTED This function is not supported by the network
interface.
@@ -73,11 +74,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 +146,29 @@ VirtioNetTransmit (
}
//
- // virtio-0.9.5, 2.4.1 Supplying Buffers to The Device
+ // Get the available descriptor
//
DescIdx = Dev->TxFreeStack[Dev->TxCurPending++];
- Dev->TxRing.Desc[DescIdx + 1].Addr = (UINTN) Buffer;
+
+ //
+ // Map the transmit buffer system physical address to device address.
+ //
+ Status = VirtioMapTxBuf (
+ Dev,
+ DescIdx + 1,
+ (EFI_PHYSICAL_ADDRESS) (UINTN) 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
+ //
+ Dev->TxRing.Desc[DescIdx + 1].Addr = DeviceAddress;
Dev->TxRing.Desc[DescIdx + 1].Len = (UINT32) BufferSize;
//
--
2.7.4
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 09/01/17 13:24, Brijesh Singh wrote:
> When device is behind the IOMMU, driver is require to pass the device
> address of transmit buffer for the bus master operations.
>
> The patch uses VirtioMapAllBytesInSharedBuffer() to map transmit buffer
> system physical address to the device address.
>
> Since the transmit buffers are returned back to caller in
> VirtioNetGetStatus() hence we use OrderCollection library interface to
> save the host to device address mapping. After the buffer is succesfully
> transmited we do reverse lookup in OrderCollection data structure to get
> the host address for the transmitted 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 | 19 +++
> OvmfPkg/VirtioNetDxe/SnpGetStatus.c | 30 +++-
> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 157 ++++++++++++++++++++
> OvmfPkg/VirtioNetDxe/SnpTransmit.c | 37 ++++-
> 5 files changed, 232 insertions(+), 12 deletions(-)
I have some preliminary comments for this patch. This is by no means a
full review.
(1) Using "SnpSharedHelpers.c" for this kind of functionality is
appropriate, but then please add a comment block between
- the current group of functions (VirtioNetShutdownRx,
VirtioNetShutdownTx, and the upcoming VirtioNetUninitRing),
- and between these other data types and functions.
The current comment block only covers the first group of functions.
(2) Please use more sensible names for the data structures and their
fields. I'm not sure just yet if I'm going to agree with the current
usage, but USER_STRUCT and "Value" are not good names. Please add
documentation to the fields.
(3) The "HostAddress" field and parameter can be / should be (VOID*).
(4) You don't need to define a structure for USER_KEY if it has only one
field.
(5) The ordered collection should be initialized separately, in
VirtioNetInitTx().
(6) Currently OrderedCollectionUninit() is not called anywhere. The
collection should be torn down in VirtioNetShutdownTx().
Beware, tearing down the transmit mappings takes more work than just
calling OrderedCollectionUninit(). You'll have to iterate over the
structure, remove each node, destroy (unmap + free) each shared buffer,
free each user structure, and finally call OrderedCollectionUninit().
Generally it looks like this,
for (Entry = OrderedCollectionMin (Collection); Entry != NULL;
Entry = Entry2) {
Entry2 = OrderedCollectionNext (Entry);
OrderedCollectionDelete (Collection, Entry, &UserStruct);
//
// now unmap and free the shared buffer identified by UserStruct
//
// finally, free UserStruct itself
//
}
OrderedCollectionUninit (Collection);
(7) We'll have to document the new model in "TechNotes.txt".
(8) VirtioOperationBusMasterRead is not appropriate for mappings that
remain in existence after we return from an async protocol member
function, such as SNP.Transmit(). Namely, if ExitBootServices() is
called next, we have to tear down all the mappings (outgoing buffers)
*without* releasing memory, and BusMasterRead is not OK for that. Only
CommonBuffer is.
(9) Which makes me realize that VirtioNetExitBoot() is not updated in
this patch at all. It should basically do the same as (5), except nodes
from the collection should not be deleted, shared buffers should only be
unmapped, not freed, and user structures should not be deleted.
Something like this:
for (Entry = OrderedCollectionMin (Collection);
Entry != NULL;
Entry = OrderedCollectionNext (Entry)) {
UserStruct = OrderedCollectionUserStruct (Entry);
//
// now unmap the shared buffer identified by UserStruct
//
}
(10) This patch is going to be large (even larger than now); can we
split it up? First, update the documentation. Second, just introduce the
data structure and the helper functions. Third, update the current code
to set up the data structure, call the helpers whenever appropriate, and
tear down the data structure.
(11) Regarding points (8) and (9), i.e., unmapping at
ExitBootServices(), have you been following the related discussion in:
[edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
buffers at ExitBootServices()
?
Let's return to this patch when we reach it with the gradual advancement
that I'm requesting.
Thank you,
Laszlo
> 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 @@ [LibraryClasses]
> DebugLib
> DevicePathLib
> MemoryAllocationLib
> + OrderedCollectionLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> UefiLib
> diff --git a/OvmfPkg/VirtioNetDxe/VirtioNet.h b/OvmfPkg/VirtioNetDxe/VirtioNet.h
> index 3efe95a735d8..d931969af795 100644
> --- a/OvmfPkg/VirtioNetDxe/VirtioNet.h
> +++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h
> @@ -269,6 +269,25 @@ VirtioNetShutdownTx (
> IN OUT VNET_DEV *Dev
> );
>
> +EFI_STATUS
> +EFIAPI
> +VirtioMapTxBuf (
> + IN VNET_DEV *Dev,
> + IN UINT16 Index,
> + IN EFI_PHYSICAL_ADDRESS HostAddress,
> + IN UINTN NumberOfBytes,
> + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
> + );
> +
> +EFI_STATUS
> +EFIAPI
> +VirtioUnmapTxBuf (
> + IN VNET_DEV *Dev,
> + IN UINT16 Index,
> + OUT EFI_PHYSICAL_ADDRESS *HostAddress,
> + IN EFI_PHYSICAL_ADDRESS DeviceAddress
> + );
> +
> //
> // event callbacks
> //
> diff --git a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> index 694940ea1d97..10ca26de6d7a 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c
> @@ -5,6 +5,7 @@
>
> Copyright (C) 2013, Red Hat, Inc.
> Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> + 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
> @@ -47,7 +48,8 @@
> @retval EFI_INVALID_PARAMETER One or more of the parameters has an
> unsupported value.
> @retval EFI_DEVICE_ERROR The command could not be sent to the network
> - interface.
> + interface or failed to map TxBuf to bus master
> + address.
> @retval EFI_UNSUPPORTED This function is not supported by the network
> interface.
>
> @@ -126,8 +128,10 @@ VirtioNetGetStatus (
> *TxBuf = NULL;
> }
> else {
> - UINT16 UsedElemIdx;
> - UINT32 DescIdx;
> + UINT16 UsedElemIdx;
> + UINT32 DescIdx;
> + EFI_PHYSICAL_ADDRESS BufferAddress;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
>
> //
> // fetch the first descriptor among those that the hypervisor reports
> @@ -141,9 +145,27 @@ VirtioNetGetStatus (
> ASSERT (DescIdx < (UINT32) (2 * Dev->TxMaxPending - 1));
>
> //
> + // Ring descriptor contains the device address for the caller buffer.
> + // Lets unmap the device address and find its corresponding system
> + // physical address.
> + //
> + DeviceAddress = Dev->TxRing.Desc[DescIdx + 1].Addr;
> + Status = VirtioUnmapTxBuf (
> + Dev,
> + DescIdx + 1,
> + &BufferAddress,
> + DeviceAddress
> + );
> + if (EFI_ERROR (Status)) {
> + Status = EFI_DEVICE_ERROR;
> + goto Exit;
> + }
> +
> + //
> + //
> // report buffer address to caller that has been enqueued by caller
> //
> - *TxBuf = (VOID *)(UINTN) Dev->TxRing.Desc[DescIdx + 1].Addr;
> + *TxBuf = (VOID *)(UINTN) BufferAddress;
>
> //
> // now this descriptor can be used again to enqueue a transmit buffer
> diff --git a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> index aeb9e6605f0d..b91ddd3e4ede 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c
> @@ -14,10 +14,25 @@
>
> **/
>
> +#include <Library/BaseLib.h>
> #include <Library/MemoryAllocationLib.h>
> +#include <Library/OrderedCollectionLib.h>
>
> #include "VirtioNet.h"
>
> +typedef struct {
> + UINT16 Value;
> +} USER_KEY;
> +
> +typedef struct {
> + USER_KEY Key;
> + EFI_PHYSICAL_ADDRESS HostAddress;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
> + VOID *BufMap;
> +} USER_STRUCT;
> +
> +STATIC ORDERED_COLLECTION *Collection;
> +
> /**
> Release RX and TX resources on the boundary of the
> EfiSimpleNetworkInitialized state.
> @@ -63,3 +78,145 @@ VirtioNetShutdownTx (
>
> FreePool (Dev->TxFreeStack);
> }
> +
> +STATIC
> +INTN
> +EFIAPI
> +KeyCompare (
> + IN CONST VOID *StandaloneKey,
> + IN CONST VOID *UserStruct
> + )
> +{
> + CONST USER_KEY *CmpKey;
> + CONST USER_STRUCT *CmpStruct;
> +
> + CmpKey = StandaloneKey;
> + CmpStruct = UserStruct;
> +
> + return CmpKey->Value < CmpStruct->Key.Value ? -1 :
> + CmpKey->Value > CmpStruct->Key.Value ? 1 :
> + 0;
> +}
> +
> +STATIC
> +INTN
> +EFIAPI
> +UserStructCompare (
> + IN CONST VOID *UserStruct1,
> + IN CONST VOID *UserStruct2
> + )
> +{
> + CONST USER_STRUCT *CmpStruct1;
> +
> + CmpStruct1 = UserStruct1;
> +
> + return KeyCompare (&CmpStruct1->Key, UserStruct2);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +VirtioMapTxBuf (
> + IN VNET_DEV *Dev,
> + IN UINT16 Index,
> + IN EFI_PHYSICAL_ADDRESS HostAddress,
> + IN UINTN NumberOfBytes,
> + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress
> + )
> +{
> + EFI_STATUS Status;
> + ORDERED_COLLECTION_ENTRY *Entry;
> + USER_STRUCT *UserStruct;
> + EFI_PHYSICAL_ADDRESS Address;
> + VOID *Mapping;
> +
> + //
> + // If ordered collection is not initialized then initialize it.
> + //
> + if (Collection == NULL) {
> + Collection = OrderedCollectionInit (UserStructCompare, KeyCompare);
> + if (Collection == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> + }
> +
> + UserStruct = AllocatePool (sizeof (*UserStruct));
> + if (UserStruct == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Status = VirtioMapAllBytesInSharedBuffer (
> + Dev->VirtIo,
> + VirtioOperationBusMasterRead,
> + (VOID *) (UINTN) HostAddress,
> + NumberOfBytes,
> + &Address,
> + &Mapping
> + );
> + if (EFI_ERROR (Status)) {
> + goto ReleaseUserStruct;
> + }
> +
> + UserStruct->Key.Value = Index;
> + UserStruct->HostAddress = HostAddress;
> + UserStruct->DeviceAddress = Address;
> + UserStruct->BufMap = Mapping;
> +
> + Status = OrderedCollectionInsert (Collection, &Entry, UserStruct);
> + switch (Status) {
> + case RETURN_OUT_OF_RESOURCES:
> + Status = EFI_OUT_OF_RESOURCES;
> + goto UnmapBuffer;
> + case RETURN_ALREADY_STARTED:
> + Status = EFI_INVALID_PARAMETER;
> + goto UnmapBuffer;
> + default:
> + ASSERT (Status == RETURN_SUCCESS);
> + break;
> + }
> +
> + ASSERT (OrderedCollectionUserStruct (Entry) == UserStruct);
> +
> + *DeviceAddress = Address;
> + return EFI_SUCCESS;
> +
> +UnmapBuffer:
> + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Mapping);
> +
> +ReleaseUserStruct:
> + FreePool (UserStruct);
> + return Status;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +VirtioUnmapTxBuf (
> + IN VNET_DEV *Dev,
> + IN UINT16 Index,
> + OUT EFI_PHYSICAL_ADDRESS *HostAddress,
> + IN EFI_PHYSICAL_ADDRESS DeviceAddress
> + )
> +{
> + USER_KEY StandaloneKey;
> + ORDERED_COLLECTION_ENTRY *Entry;
> + USER_STRUCT *UserStruct;
> + VOID *Ptr;
> + EFI_STATUS Status;
> +
> + StandaloneKey.Value = Index;
> + Entry = OrderedCollectionFind (Collection, &StandaloneKey);
> + if (Entry == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + OrderedCollectionDelete (Collection, Entry, &Ptr);
> +
> + UserStruct = Ptr;
> + ASSERT (UserStruct->DeviceAddress == DeviceAddress);
> + ASSERT (UserStruct->Key.Value == Index);
> +
> + *HostAddress = UserStruct->HostAddress;
> + Status = Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, UserStruct->BufMap);
> + FreePool (UserStruct);
> +
> + return Status;
> +}
> diff --git a/OvmfPkg/VirtioNetDxe/SnpTransmit.c b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> index 7ca40d5d0650..1bd6e0d70d7c 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpTransmit.c
> @@ -55,7 +55,8 @@
> @retval EFI_INVALID_PARAMETER One or more of the parameters has an
> unsupported value.
> @retval EFI_DEVICE_ERROR The command could not be sent to the network
> - interface.
> + interface or failed to map the Buffer to
> + bus master address.
> @retval EFI_UNSUPPORTED This function is not supported by the network
> interface.
>
> @@ -73,11 +74,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 +146,29 @@ VirtioNetTransmit (
> }
>
> //
> - // virtio-0.9.5, 2.4.1 Supplying Buffers to The Device
> + // Get the available descriptor
> //
> DescIdx = Dev->TxFreeStack[Dev->TxCurPending++];
> - Dev->TxRing.Desc[DescIdx + 1].Addr = (UINTN) Buffer;
> +
> + //
> + // Map the transmit buffer system physical address to device address.
> + //
> + Status = VirtioMapTxBuf (
> + Dev,
> + DescIdx + 1,
> + (EFI_PHYSICAL_ADDRESS) (UINTN) 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
> + //
> + Dev->TxRing.Desc[DescIdx + 1].Addr = DeviceAddress;
> Dev->TxRing.Desc[DescIdx + 1].Len = (UINT32) BufferSize;
>
> //
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 09/05/17 14:41, Laszlo Ersek wrote: > On 09/01/17 13:24, Brijesh Singh wrote: >> When device is behind the IOMMU, driver is require to pass the device >> address of transmit buffer for the bus master operations. >> >> The patch uses VirtioMapAllBytesInSharedBuffer() to map transmit buffer >> system physical address to the device address. >> >> Since the transmit buffers are returned back to caller in >> VirtioNetGetStatus() hence we use OrderCollection library interface to >> save the host to device address mapping. After the buffer is succesfully >> transmited we do reverse lookup in OrderCollection data structure to get >> the host address for the transmitted 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 | 19 +++ >> OvmfPkg/VirtioNetDxe/SnpGetStatus.c | 30 +++- >> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 157 ++++++++++++++++++++ >> OvmfPkg/VirtioNetDxe/SnpTransmit.c | 37 ++++- >> 5 files changed, 232 insertions(+), 12 deletions(-) > > I have some preliminary comments for this patch. This is by no means a > full review. (12) The helper functions should be called VirtioNet*, not Virtio*. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 09/05/17 14:41, Laszlo Ersek wrote: > On 09/01/17 13:24, Brijesh Singh wrote: >> When device is behind the IOMMU, driver is require to pass the device >> address of transmit buffer for the bus master operations. >> >> The patch uses VirtioMapAllBytesInSharedBuffer() to map transmit buffer >> system physical address to the device address. >> >> Since the transmit buffers are returned back to caller in >> VirtioNetGetStatus() hence we use OrderCollection library interface to >> save the host to device address mapping. After the buffer is succesfully >> transmited we do reverse lookup in OrderCollection data structure to get >> the host address for the transmitted 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 | 19 +++ >> OvmfPkg/VirtioNetDxe/SnpGetStatus.c | 30 +++- >> OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 157 ++++++++++++++++++++ >> OvmfPkg/VirtioNetDxe/SnpTransmit.c | 37 ++++- >> 5 files changed, 232 insertions(+), 12 deletions(-) > (7) We'll have to document the new model in "TechNotes.txt". I've now re-read the final section of the text file, section Virtio internals -- Tx I propose the following updates. (Here I'm providing a diff, not a desired "end status", like I did for the diagram earlier. A diff is harder to interpret for a diagram, but easy for plain text.) Please verify that the proposed documentation updates match the logic that you've implemented: > diff --git a/OvmfPkg/VirtioNetDxe/TechNotes.txt b/OvmfPkg/VirtioNetDxe/TechNotes.txt > index 9c1dfe6a773e..7a7b3071abba 100644 > --- a/OvmfPkg/VirtioNetDxe/TechNotes.txt > +++ b/OvmfPkg/VirtioNetDxe/TechNotes.txt > @@ -310,10 +310,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: > > @@ -336,9 +340,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 Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.