These rings are shared memory buffers between host and device in which
a cyclic buffer is managed to send request descriptors from host to
device and receive completion descriptors from device to host.
Note that because device may be constrained by IOMMU or guest may be run
under AMD SEV, we make sure to map these rings to device by using
PciIo->Map().
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
OvmfPkg/PvScsiDxe/PvScsi.c | 235 +++++++++++++++++++++++++++++++++++++
OvmfPkg/PvScsiDxe/PvScsi.h | 17 +++
2 files changed, 252 insertions(+)
diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index fb2407d2adb2..c3f5d38f3d30 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -16,6 +16,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/PciIo.h>
+#include <Protocol/PciRootBridgeIo.h>
#include "PvScsi.h"
@@ -396,6 +397,209 @@ PvScsiSetPCIAttributes (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+PvScsiAllocatePages (
+ IN PVSCSI_DEV *Dev,
+ IN UINTN Pages,
+ IN OUT VOID **HostAddress
+ )
+{
+ return Dev->PciIo->AllocateBuffer (
+ Dev->PciIo,
+ AllocateAnyPages,
+ EfiBootServicesData,
+ Pages,
+ HostAddress,
+ EFI_PCI_ATTRIBUTE_MEMORY_CACHED
+ );
+}
+
+STATIC
+VOID
+PvScsiFreePages (
+ IN PVSCSI_DEV *Dev,
+ IN UINTN Pages,
+ IN VOID *HostAddress
+ )
+{
+ Dev->PciIo->FreeBuffer (
+ Dev->PciIo,
+ Pages,
+ HostAddress
+ );
+}
+
+STATIC
+EFI_STATUS
+PvScsiMapBuffer (
+ IN PVSCSI_DEV *Dev,
+ IN EFI_PCI_IO_PROTOCOL_OPERATION PciIoOperation,
+ IN VOID *HostAddress,
+ IN UINTN NumberOfBytes,
+ OUT PVSCSI_DMA_DESC *DmaDesc
+ )
+{
+ EFI_STATUS Status;
+ UINTN BytesMapped;
+
+ BytesMapped = NumberOfBytes;
+ Status = Dev->PciIo->Map (
+ Dev->PciIo,
+ PciIoOperation,
+ HostAddress,
+ &BytesMapped,
+ &DmaDesc->DeviceAddress,
+ &DmaDesc->Mapping
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (BytesMapped != NumberOfBytes) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Unmap;
+ }
+
+ return EFI_SUCCESS;
+
+Unmap:
+ Dev->PciIo->Unmap (Dev->PciIo, DmaDesc->Mapping);
+ DmaDesc->Mapping = NULL;
+
+ return Status;
+}
+
+STATIC
+VOID
+PvScsiUnmapBuffer (
+ IN PVSCSI_DEV *Dev,
+ IN OUT PVSCSI_DMA_DESC *DmaDesc)
+{
+ Dev->PciIo->Unmap (Dev->PciIo, DmaDesc->Mapping);
+}
+
+STATIC
+EFI_STATUS
+PvScsiAllocateSharedPages (
+ IN PVSCSI_DEV *Dev,
+ IN UINTN Pages,
+ IN EFI_PCI_IO_PROTOCOL_OPERATION PciIoOperation,
+ OUT VOID **HostAddress,
+ OUT PVSCSI_DMA_DESC *DmaDesc
+ )
+{
+ EFI_STATUS Status;
+
+ *HostAddress = NULL;
+ DmaDesc->Mapping = NULL;
+
+ Status = PvScsiAllocatePages (Dev, Pages, HostAddress);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = PvScsiMapBuffer (
+ Dev,
+ PciIoOperation,
+ *HostAddress,
+ Pages * EFI_PAGE_SIZE,
+ DmaDesc
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreePages;
+ }
+
+ return EFI_SUCCESS;
+
+FreePages:
+ PvScsiFreePages (Dev, Pages, *HostAddress);
+ *HostAddress = NULL;
+
+ return Status;
+}
+
+STATIC
+VOID
+PvScsiFreeSharedPages (
+ IN PVSCSI_DEV *Dev,
+ IN UINTN Pages,
+ IN OUT VOID **HostAddress,
+ IN OUT PVSCSI_DMA_DESC *DmaDesc
+ )
+{
+ if (*HostAddress) {
+ if (DmaDesc->Mapping) {
+ PvScsiUnmapBuffer (Dev, DmaDesc);
+ DmaDesc->Mapping = NULL;
+ }
+
+ PvScsiFreePages (Dev, Pages, *HostAddress);
+ *HostAddress = NULL;
+ }
+}
+
+STATIC
+EFI_STATUS
+PvScsiInitRings (
+ IN OUT PVSCSI_DEV *Dev
+ )
+{
+ EFI_STATUS Status;
+ PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
+
+ Status = PvScsiAllocateSharedPages (
+ Dev,
+ 1,
+ EfiPciIoOperationBusMasterCommonBuffer,
+ (VOID **)&Dev->RingDesc.RingState,
+ &Dev->RingDesc.RingStateDmaDesc
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ ZeroMem (Dev->RingDesc.RingState, EFI_PAGE_SIZE);
+
+ Status = PvScsiAllocateSharedPages (
+ Dev,
+ 1,
+ EfiPciIoOperationBusMasterCommonBuffer,
+ (VOID **)&Dev->RingDesc.RingReqs,
+ &Dev->RingDesc.RingReqsDmaDesc
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ ZeroMem (Dev->RingDesc.RingReqs, EFI_PAGE_SIZE);
+
+ Status = PvScsiAllocateSharedPages (
+ Dev,
+ 1,
+ EfiPciIoOperationBusMasterCommonBuffer,
+ (VOID **)&Dev->RingDesc.RingCmps,
+ &Dev->RingDesc.RingCmpsDmaDesc
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
+
+ ZeroMem (&Cmd, sizeof Cmd);
+ Cmd.ReqRingNumPages = 1;
+ Cmd.CmpRingNumPages = 1;
+ Cmd.RingsStatePPN =
+ ((UINT64) Dev->RingDesc.RingStateDmaDesc.DeviceAddress) >>
+ EFI_PAGE_SHIFT;
+ Cmd.ReqRingPPNs[0] =
+ ((UINT64) Dev->RingDesc.RingReqsDmaDesc.DeviceAddress) >>
+ EFI_PAGE_SHIFT;
+ Cmd.CmpRingPPNs[0] =
+ ((UINT64) Dev->RingDesc.RingCmpsDmaDesc.DeviceAddress) >>
+ EFI_PAGE_SHIFT;
+
+ return PvScsiWriteCmdDesc(Dev, PVSCSI_CMD_SETUP_RINGS, &Cmd, sizeof Cmd);
+}
+
STATIC
EFI_STATUS
PvScsiInit (
@@ -425,6 +629,15 @@ PvScsiInit (
if (EFI_ERROR (Status)) {
return Status;
}
+
+ //
+ // Init PVSCSI rings
+ //
+ Status = PvScsiInitRings (Dev);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
//
// Populate the exported interface's attributes
//
@@ -463,6 +676,28 @@ PvScsiUninit (
IN OUT PVSCSI_DEV *Dev
)
{
+ //
+ // Free PVSCSI rings
+ //
+ PvScsiFreeSharedPages (
+ Dev,
+ 1,
+ (VOID **)&Dev->RingDesc.RingCmps,
+ &Dev->RingDesc.RingCmpsDmaDesc
+ );
+ PvScsiFreeSharedPages (
+ Dev,
+ 1,
+ (VOID **)&Dev->RingDesc.RingReqs,
+ &Dev->RingDesc.RingReqsDmaDesc
+ );
+ PvScsiFreeSharedPages (
+ Dev,
+ 1,
+ (VOID **)&Dev->RingDesc.RingState,
+ &Dev->RingDesc.RingStateDmaDesc
+ );
+
//
// Restore PCI Attributes
//
diff --git a/OvmfPkg/PvScsiDxe/PvScsi.h b/OvmfPkg/PvScsiDxe/PvScsi.h
index 5f611dbbc98c..6d23b6e1eccf 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.h
+++ b/OvmfPkg/PvScsiDxe/PvScsi.h
@@ -15,12 +15,29 @@
#include <Library/DebugLib.h>
#include <Protocol/ScsiPassThruExt.h>
+typedef struct {
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
+ VOID *Mapping;
+} PVSCSI_DMA_DESC;
+
+typedef struct {
+ PVSCSI_RINGS_STATE *RingState;
+ PVSCSI_DMA_DESC RingStateDmaDesc;
+
+ PVSCSI_RING_REQ_DESC *RingReqs;
+ PVSCSI_DMA_DESC RingReqsDmaDesc;
+
+ PVSCSI_RING_CMP_DESC *RingCmps;
+ PVSCSI_DMA_DESC RingCmpsDmaDesc;
+} PVSCSI_RING_DESC;
+
#define PVSCSI_SIG SIGNATURE_32 ('P', 'S', 'C', 'S')
typedef struct {
UINT32 Signature;
EFI_PCI_IO_PROTOCOL *PciIo;
UINT64 OriginalPciAttributes;
+ PVSCSI_RING_DESC RingDesc;
UINT8 MaxTarget;
UINT8 MaxLun;
EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55879): https://edk2.groups.io/g/devel/message/55879
Mute This Topic: https://groups.io/mt/72001281/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
really trivial style comments, for now (I intend to look at this in more
detail in v2):
On 03/16/20 16:01, Liran Alon wrote:
> These rings are shared memory buffers between host and device in which
> a cyclic buffer is managed to send request descriptors from host to
> device and receive completion descriptors from device to host.
>
> Note that because device may be constrained by IOMMU or guest may be run
> under AMD SEV, we make sure to map these rings to device by using
> PciIo->Map().
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
> OvmfPkg/PvScsiDxe/PvScsi.c | 235 +++++++++++++++++++++++++++++++++++++
> OvmfPkg/PvScsiDxe/PvScsi.h | 17 +++
> 2 files changed, 252 insertions(+)
>
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index fb2407d2adb2..c3f5d38f3d30 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -16,6 +16,7 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
> #include <Protocol/PciIo.h>
> +#include <Protocol/PciRootBridgeIo.h>
>
> #include "PvScsi.h"
>
> @@ -396,6 +397,209 @@ PvScsiSetPCIAttributes (
> return EFI_SUCCESS;
> }
>
> +STATIC
> +EFI_STATUS
> +PvScsiAllocatePages (
> + IN PVSCSI_DEV *Dev,
> + IN UINTN Pages,
> + IN OUT VOID **HostAddress
> + )
> +{
> + return Dev->PciIo->AllocateBuffer (
> + Dev->PciIo,
> + AllocateAnyPages,
> + EfiBootServicesData,
> + Pages,
> + HostAddress,
> + EFI_PCI_ATTRIBUTE_MEMORY_CACHED
> + );
> +}
> +
> +STATIC
> +VOID
> +PvScsiFreePages (
> + IN PVSCSI_DEV *Dev,
> + IN UINTN Pages,
> + IN VOID *HostAddress
> + )
> +{
> + Dev->PciIo->FreeBuffer (
> + Dev->PciIo,
> + Pages,
> + HostAddress
> + );
> +}
> +
> +STATIC
> +EFI_STATUS
> +PvScsiMapBuffer (
> + IN PVSCSI_DEV *Dev,
> + IN EFI_PCI_IO_PROTOCOL_OPERATION PciIoOperation,
> + IN VOID *HostAddress,
> + IN UINTN NumberOfBytes,
> + OUT PVSCSI_DMA_DESC *DmaDesc
> + )
> +{
> + EFI_STATUS Status;
> + UINTN BytesMapped;
> +
> + BytesMapped = NumberOfBytes;
> + Status = Dev->PciIo->Map (
> + Dev->PciIo,
> + PciIoOperation,
> + HostAddress,
> + &BytesMapped,
> + &DmaDesc->DeviceAddress,
> + &DmaDesc->Mapping
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (BytesMapped != NumberOfBytes) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Unmap;
> + }
> +
> + return EFI_SUCCESS;
> +
> +Unmap:
> + Dev->PciIo->Unmap (Dev->PciIo, DmaDesc->Mapping);
> + DmaDesc->Mapping = NULL;
> +
> + return Status;
> +}
> +
> +STATIC
> +VOID
> +PvScsiUnmapBuffer (
> + IN PVSCSI_DEV *Dev,
> + IN OUT PVSCSI_DMA_DESC *DmaDesc)
> +{
> + Dev->PciIo->Unmap (Dev->PciIo, DmaDesc->Mapping);
> +}
> +
> +STATIC
> +EFI_STATUS
> +PvScsiAllocateSharedPages (
> + IN PVSCSI_DEV *Dev,
> + IN UINTN Pages,
> + IN EFI_PCI_IO_PROTOCOL_OPERATION PciIoOperation,
> + OUT VOID **HostAddress,
> + OUT PVSCSI_DMA_DESC *DmaDesc
> + )
> +{
> + EFI_STATUS Status;
> +
> + *HostAddress = NULL;
> + DmaDesc->Mapping = NULL;
> +
> + Status = PvScsiAllocatePages (Dev, Pages, HostAddress);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = PvScsiMapBuffer (
> + Dev,
> + PciIoOperation,
> + *HostAddress,
> + Pages * EFI_PAGE_SIZE,
(1) Please use EFI_PAGES_TO_SIZE(); it's more idiomatic. (The argument
should have type UINTN -- "Pages" is already UINTN, so that's good.)
> + DmaDesc
> + );
> + if (EFI_ERROR (Status)) {
> + goto FreePages;
> + }
> +
> + return EFI_SUCCESS;
> +
> +FreePages:
> + PvScsiFreePages (Dev, Pages, *HostAddress);
> + *HostAddress = NULL;
> +
> + return Status;
> +}
> +
> +STATIC
> +VOID
> +PvScsiFreeSharedPages (
> + IN PVSCSI_DEV *Dev,
> + IN UINTN Pages,
> + IN OUT VOID **HostAddress,
> + IN OUT PVSCSI_DMA_DESC *DmaDesc
> + )
> +{
> + if (*HostAddress) {
> + if (DmaDesc->Mapping) {
> + PvScsiUnmapBuffer (Dev, DmaDesc);
> + DmaDesc->Mapping = NULL;
> + }
> +
> + PvScsiFreePages (Dev, Pages, *HostAddress);
> + *HostAddress = NULL;
> + }
> +}
> +
> +STATIC
> +EFI_STATUS
> +PvScsiInitRings (
> + IN OUT PVSCSI_DEV *Dev
> + )
> +{
> + EFI_STATUS Status;
> + PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
> +
> + Status = PvScsiAllocateSharedPages (
> + Dev,
> + 1,
> + EfiPciIoOperationBusMasterCommonBuffer,
> + (VOID **)&Dev->RingDesc.RingState,
> + &Dev->RingDesc.RingStateDmaDesc
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + ZeroMem (Dev->RingDesc.RingState, EFI_PAGE_SIZE);
> +
> + Status = PvScsiAllocateSharedPages (
> + Dev,
> + 1,
> + EfiPciIoOperationBusMasterCommonBuffer,
> + (VOID **)&Dev->RingDesc.RingReqs,
> + &Dev->RingDesc.RingReqsDmaDesc
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + ZeroMem (Dev->RingDesc.RingReqs, EFI_PAGE_SIZE);
> +
> + Status = PvScsiAllocateSharedPages (
> + Dev,
> + 1,
> + EfiPciIoOperationBusMasterCommonBuffer,
> + (VOID **)&Dev->RingDesc.RingCmps,
> + &Dev->RingDesc.RingCmpsDmaDesc
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
> +
> + ZeroMem (&Cmd, sizeof Cmd);
> + Cmd.ReqRingNumPages = 1;
> + Cmd.CmpRingNumPages = 1;
> + Cmd.RingsStatePPN =
> + ((UINT64) Dev->RingDesc.RingStateDmaDesc.DeviceAddress) >>
> + EFI_PAGE_SHIFT;
> + Cmd.ReqRingPPNs[0] =
> + ((UINT64) Dev->RingDesc.RingReqsDmaDesc.DeviceAddress) >>
> + EFI_PAGE_SHIFT;
> + Cmd.CmpRingPPNs[0] =
> + ((UINT64) Dev->RingDesc.RingCmpsDmaDesc.DeviceAddress) >>
> + EFI_PAGE_SHIFT;
(2) Edk2 allows the << and >> operators when the LHS operand is not
wider than UINTN. For shifting UINT64, please call the LShiftU64() and
RShiftU64() functions from BaseLib.
> +
> + return PvScsiWriteCmdDesc(Dev, PVSCSI_CMD_SETUP_RINGS, &Cmd, sizeof Cmd);
> +}
> +
> STATIC
> EFI_STATUS
> PvScsiInit (
> @@ -425,6 +629,15 @@ PvScsiInit (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + //
> + // Init PVSCSI rings
> + //
> + Status = PvScsiInitRings (Dev);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
(3) Again, we should jump to "RestorePciAttributes" here.
(More careful review postponed to v2.)
Thanks
Laszlo
> //
> // Populate the exported interface's attributes
> //
> @@ -463,6 +676,28 @@ PvScsiUninit (
> IN OUT PVSCSI_DEV *Dev
> )
> {
> + //
> + // Free PVSCSI rings
> + //
> + PvScsiFreeSharedPages (
> + Dev,
> + 1,
> + (VOID **)&Dev->RingDesc.RingCmps,
> + &Dev->RingDesc.RingCmpsDmaDesc
> + );
> + PvScsiFreeSharedPages (
> + Dev,
> + 1,
> + (VOID **)&Dev->RingDesc.RingReqs,
> + &Dev->RingDesc.RingReqsDmaDesc
> + );
> + PvScsiFreeSharedPages (
> + Dev,
> + 1,
> + (VOID **)&Dev->RingDesc.RingState,
> + &Dev->RingDesc.RingStateDmaDesc
> + );
> +
> //
> // Restore PCI Attributes
> //
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.h b/OvmfPkg/PvScsiDxe/PvScsi.h
> index 5f611dbbc98c..6d23b6e1eccf 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.h
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.h
> @@ -15,12 +15,29 @@
> #include <Library/DebugLib.h>
> #include <Protocol/ScsiPassThruExt.h>
>
> +typedef struct {
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
> + VOID *Mapping;
> +} PVSCSI_DMA_DESC;
> +
> +typedef struct {
> + PVSCSI_RINGS_STATE *RingState;
> + PVSCSI_DMA_DESC RingStateDmaDesc;
> +
> + PVSCSI_RING_REQ_DESC *RingReqs;
> + PVSCSI_DMA_DESC RingReqsDmaDesc;
> +
> + PVSCSI_RING_CMP_DESC *RingCmps;
> + PVSCSI_DMA_DESC RingCmpsDmaDesc;
> +} PVSCSI_RING_DESC;
> +
> #define PVSCSI_SIG SIGNATURE_32 ('P', 'S', 'C', 'S')
>
> typedef struct {
> UINT32 Signature;
> EFI_PCI_IO_PROTOCOL *PciIo;
> UINT64 OriginalPciAttributes;
> + PVSCSI_RING_DESC RingDesc;
> UINT8 MaxTarget;
> UINT8 MaxLun;
> EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56163): https://edk2.groups.io/g/devel/message/56163
Mute This Topic: https://groups.io/mt/72001281/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.