The SynchronousRequest() function, programs the vring descriptor with the buffers pointed-by virtio-blk requests, status and memory that is referenced inside the request header.
The patch uses VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() function to map host address to device address and programs the vring descriptor with device addresses. Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> Cc: Jordan Justen <jordan.l.jus...@intel.com> Cc: Tom Lendacky <thomas.lenda...@amd.com> Cc: Laszlo Ersek <ler...@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.si...@amd.com> --- OvmfPkg/VirtioBlkDxe/VirtioBlk.h | 1 + OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 201 +++++++++++++++++--- 2 files changed, 180 insertions(+), 22 deletions(-) diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h index 6c402ca88ea4..9ec0b956b818 100644 --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h @@ -41,6 +41,7 @@ typedef struct { VRING Ring; // VirtioRingInit 2 EFI_BLOCK_IO_PROTOCOL BlockIo; // VirtioBlkInit 1 EFI_BLOCK_IO_MEDIA BlockIoMedia; // VirtioBlkInit 1 + VOID *RingMap; // VirtioRingMap 2 } VBLK_DEV; #define VIRTIO_BLK_FROM_BLOCK_IO(BlockIoPointer) \ diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c index bff15fe3add1..e23762743f75 100644 --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c @@ -232,7 +232,8 @@ VerifyReadWriteRequest ( @retval EFI_DEVICE_ERROR Failed to notify host side via VirtIo write, or unable to parse host response, or host response - is not VIRTIO_BLK_S_OK. + is not VIRTIO_BLK_S_OK or failed to map Buffer + for a bus master operation. **/ @@ -249,8 +250,16 @@ SynchronousRequest ( { UINT32 BlockSize; volatile VIRTIO_BLK_REQ Request; - volatile UINT8 HostStatus; + volatile UINT8 *HostStatus; + VOID *HostStatusBuffer; DESC_INDICES Indices; + VOID *RequestMapping; + VOID *StatusMapping; + VOID *BufferMapping; + EFI_PHYSICAL_ADDRESS BufferDeviceAddress; + EFI_PHYSICAL_ADDRESS HostStatusDeviceAddress; + EFI_PHYSICAL_ADDRESS RequestDeviceAddress; + EFI_STATUS Status, Ret; BlockSize = Dev->BlockIoMedia.BlockSize; @@ -278,9 +287,88 @@ SynchronousRequest ( VirtioPrepare (&Dev->Ring, &Indices); // + // Host status is bi-directional (we preset with a value and expect the device + // to update it). Allocate a host status buffer which can be mapped to + // access equally by both processor and the device. + // + Status = Dev->VirtIo->AllocateSharedPages ( + Dev->VirtIo, + EFI_SIZE_TO_PAGES (sizeof *HostStatus), + &HostStatusBuffer + ); + if (EFI_ERROR (Status)) { + return EFI_DEVICE_ERROR; + } + + // + // Map virtio-blk request header + // + Status = VirtioMapAllBytesInSharedBuffer ( + Dev->VirtIo, + VirtioOperationBusMasterRead, + (VOID *) &Request, + sizeof Request, + &RequestDeviceAddress, + &RequestMapping + ); + if (EFI_ERROR (Status)) { + Status = EFI_DEVICE_ERROR; + goto FreeHostStatusBuffer; + } + + // + // Map data buffer + // + if (BufferSize > 0) { + if (RequestIsWrite) { + Status = VirtioMapAllBytesInSharedBuffer ( + Dev->VirtIo, + VirtioOperationBusMasterRead, + (VOID *) Buffer, + BufferSize, + &BufferDeviceAddress, + &BufferMapping + ); + } else { + Status = VirtioMapAllBytesInSharedBuffer ( + Dev->VirtIo, + VirtioOperationBusMasterWrite, + (VOID *) Buffer, + BufferSize, + &BufferDeviceAddress, + &BufferMapping + ); + } + + if (EFI_ERROR (Status)) { + Status = EFI_DEVICE_ERROR; + goto UnmapRequestBuffer; + } + } + + // + // Map the Status Buffer with VirtioOperationBusMasterCommonBuffer so that + // both processor and device can access it. + // + Status = VirtioMapAllBytesInSharedBuffer ( + Dev->VirtIo, + VirtioOperationBusMasterCommonBuffer, + HostStatusBuffer, + sizeof *HostStatus, + &HostStatusDeviceAddress, + &StatusMapping + ); + if (EFI_ERROR (Status)) { + Status = EFI_DEVICE_ERROR; + goto UnmapDataBuffer; + } + + HostStatus = HostStatusBuffer; + + // // preset a host status for ourselves that we do not accept as success // - HostStatus = VIRTIO_BLK_S_IOERR; + *HostStatus = VIRTIO_BLK_S_IOERR; // // ensured by VirtioBlkInit() -- this predicate, in combination with the @@ -291,8 +379,13 @@ SynchronousRequest ( // // virtio-blk header in first desc // - VirtioAppendDesc (&Dev->Ring, (UINTN) &Request, sizeof Request, - VRING_DESC_F_NEXT, &Indices); + VirtioAppendDesc ( + &Dev->Ring, + RequestDeviceAddress, + sizeof Request, + VRING_DESC_F_NEXT, + &Indices + ); // // data buffer for read/write in second desc @@ -311,27 +404,62 @@ SynchronousRequest ( // // VRING_DESC_F_WRITE is interpreted from the host's point of view. // - VirtioAppendDesc (&Dev->Ring, (UINTN) Buffer, (UINT32) BufferSize, + VirtioAppendDesc ( + &Dev->Ring, + BufferDeviceAddress, + (UINT32) BufferSize, VRING_DESC_F_NEXT | (RequestIsWrite ? 0 : VRING_DESC_F_WRITE), - &Indices); + &Indices + ); } // // host status in last (second or third) desc // - VirtioAppendDesc (&Dev->Ring, (UINTN) &HostStatus, sizeof HostStatus, - VRING_DESC_F_WRITE, &Indices); + VirtioAppendDesc ( + &Dev->Ring, + HostStatusDeviceAddress, + sizeof *HostStatus, + VRING_DESC_F_WRITE, + &Indices + ); // // virtio-blk's only virtqueue is #0, called "requestq" (see Appendix D). // - if (VirtioFlush (Dev->VirtIo, 0, &Dev->Ring, &Indices, - NULL) == EFI_SUCCESS && - HostStatus == VIRTIO_BLK_S_OK) { - return EFI_SUCCESS; + Status = VirtioFlush (Dev->VirtIo, 0, &Dev->Ring, &Indices, NULL); + + // + // Unmap the HostStatus buffer before accessing it + // + Ret = Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, StatusMapping); + if (EFI_ERROR (Ret)) { + Status = EFI_DEVICE_ERROR; + } + + if (!EFI_ERROR (Status) && + *HostStatus == VIRTIO_BLK_S_OK) { + Status = EFI_SUCCESS; + } else { + Status = EFI_DEVICE_ERROR; } - return EFI_DEVICE_ERROR; +UnmapDataBuffer: + if (BufferSize > 0) { + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, BufferMapping); + } + +UnmapRequestBuffer: + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, RequestMapping); + +FreeHostStatusBuffer: + Dev->VirtIo->FreeSharedPages ( + Dev->VirtIo, + EFI_SIZE_TO_PAGES (sizeof *HostStatus), + HostStatusBuffer + ); + + return Status; } @@ -580,7 +708,8 @@ VirtioBlkDriverBindingSupported ( virtio-blk attributes the host provides. @return Error codes from VirtioRingInit() or - VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE(). + VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE or + VirtioRingMap(). **/ @@ -601,6 +730,7 @@ VirtioBlkInit ( UINT8 AlignmentOffset; UINT32 OptIoSize; UINT16 QueueSize; + UINT64 RingBaseShift; PhysicalBlockExp = 0; AlignmentOffset = 0; @@ -729,25 +859,42 @@ VirtioBlkInit ( } // + // If anything fails from here on, we must release the ring resources + // + Status = VirtioRingMap ( + Dev->VirtIo, + &Dev->Ring, + &RingBaseShift, + &Dev->RingMap + ); + 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, &Dev->Ring, 0); + Status = Dev->VirtIo->SetQueueAddress ( + Dev->VirtIo, + &Dev->Ring, + RingBaseShift + ); if (EFI_ERROR (Status)) { - goto ReleaseQueue; + goto UnmapQueue; } @@ -758,7 +905,7 @@ VirtioBlkInit ( Features &= ~(UINT64)VIRTIO_F_VERSION_1; Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features); if (EFI_ERROR (Status)) { - goto ReleaseQueue; + goto UnmapQueue; } } @@ -768,7 +915,7 @@ VirtioBlkInit ( NextDevStat |= VSTAT_DRIVER_OK; Status = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat); if (EFI_ERROR (Status)) { - goto ReleaseQueue; + goto UnmapQueue; } // @@ -811,6 +958,9 @@ VirtioBlkInit ( } return EFI_SUCCESS; +UnmapQueue: + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); + ReleaseQueue: VirtioRingUninit (Dev->VirtIo, &Dev->Ring); @@ -849,6 +999,7 @@ VirtioBlkUninit ( // Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); VirtioRingUninit (Dev->VirtIo, &Dev->Ring); SetMem (&Dev->BlockIo, sizeof Dev->BlockIo, 0x00); @@ -885,6 +1036,12 @@ VirtioBlkExitBoot ( // Dev = Context; Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); + + // + // Unmap the ring buffer so that hypervisor will not be able to get + // readable data after device is reset. + // + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); } /** -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel