On 09/11/17 14:16, 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 <[email protected]> > Cc: Jordan Justen <[email protected]> > Cc: Tom Lendacky <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh <[email protected]> > --- > 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 7d5f33b01dc8..3f48bcc6b67c 100644 > --- a/OvmfPkg/VirtioNetDxe/VirtioNet.h > +++ b/OvmfPkg/VirtioNetDxe/VirtioNet.h > @@ -96,7 +96,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 > EFI_PHYSICAL_ADDRESS RxBufDeviceBase; // VirtioNetInitRx > } VNET_DEV; > diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c > b/OvmfPkg/VirtioNetDxe/SnpInitialize.c > index 54c808c501bf..6cedb406a172 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..2ec3dc385a9f 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)), > + (VOID *) Dev->TxSharedReq > + ); > + > FreePool (Dev->TxFreeStack); > } > >
(1) Regarding the last argument of FreeSharedPages(), from my previous review at [email protected]">http://mid.mail-archive.com/[email protected] you missed: On 09/06/17 11:11, Laszlo Ersek wrote: > (18) The (VOID*) cast is unneeded. The rest is good. Thanks! Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
