diff options
author | Liran Alon <liran.alon@oracle.com> | 2020-04-01 01:56:37 +0300 |
---|---|---|
committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2020-04-01 14:12:09 +0000 |
commit | e210fc130e5c9738909dca432bbf8bf277ba6e37 (patch) | |
tree | 2986442b32988c567c17fb2f91fbebf71e50e8a7 /OvmfPkg | |
parent | 98936dc4f44b4ef47e7221d435de06a0813aa00a (diff) | |
download | edk2-e210fc130e5c9738909dca432bbf8bf277ba6e37.zip edk2-e210fc130e5c9738909dca432bbf8bf277ba6e37.tar.gz edk2-e210fc130e5c9738909dca432bbf8bf277ba6e37.tar.bz2 |
OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function
Previous to this change, PvScsiFreeRings() was not undoing all
operations that was done by PvScsiInitRings().
This is because PvScsiInitRings() was both preparing rings (Allocate
memory and map it for device DMA) and setup the rings against device by
issueing a device command. While PvScsiFreeRings() only unmaps the rings
and free their memory.
Driver do not have a functional error as it makes sure to reset device
before every call site to PvScsiFreeRings(). However, this is not
intuitive.
Therefore, prefer to refactor the setup of the ring against device to a
separate function than PvScsiInitRings().
Signed-off-by: Liran Alon <liran.alon@oracle.com>
Message-Id: <20200331225637.123318-1-liran.alon@oracle.com>
[lersek@redhat.com: rename FreeDMACommBuffer label to FreeDmaCommBuffer]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Diffstat (limited to 'OvmfPkg')
-rw-r--r-- | OvmfPkg/PvScsiDxe/PvScsi.c | 109 |
1 files changed, 58 insertions, 51 deletions
diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c index 1ca5039..843534e 100644 --- a/OvmfPkg/PvScsiDxe/PvScsi.c +++ b/OvmfPkg/PvScsiDxe/PvScsi.c @@ -991,13 +991,6 @@ PvScsiInitRings ( )
{
EFI_STATUS Status;
- union {
- PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
- UINT32 Uint32;
- } AlignedCmd;
- PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
-
- Cmd = &AlignedCmd.Cmd;
Status = PvScsiAllocateSharedPages (
Dev,
@@ -1032,46 +1025,8 @@ PvScsiInitRings ( }
ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
- ZeroMem (Cmd, sizeof (*Cmd));
- Cmd->ReqRingNumPages = 1;
- Cmd->CmpRingNumPages = 1;
- Cmd->RingsStatePPN = RShiftU64 (
- Dev->RingDesc.RingStateDmaDesc.DeviceAddress,
- EFI_PAGE_SHIFT
- );
- Cmd->ReqRingPPNs[0] = RShiftU64 (
- Dev->RingDesc.RingReqsDmaDesc.DeviceAddress,
- EFI_PAGE_SHIFT
- );
- Cmd->CmpRingPPNs[0] = RShiftU64 (
- Dev->RingDesc.RingCmpsDmaDesc.DeviceAddress,
- EFI_PAGE_SHIFT
- );
-
- STATIC_ASSERT (
- sizeof (*Cmd) % sizeof (UINT32) == 0,
- "Cmd must be multiple of 32-bit words"
- );
- Status = PvScsiWriteCmdDesc (
- Dev,
- PvScsiCmdSetupRings,
- (UINT32 *)Cmd,
- sizeof (*Cmd) / sizeof (UINT32)
- );
- if (EFI_ERROR (Status)) {
- goto FreeRingCmps;
- }
-
return EFI_SUCCESS;
-FreeRingCmps:
- PvScsiFreeSharedPages (
- Dev,
- 1,
- Dev->RingDesc.RingCmps,
- &Dev->RingDesc.RingCmpsDmaDesc
- );
-
FreeRingReqs:
PvScsiFreeSharedPages (
Dev,
@@ -1121,6 +1076,48 @@ PvScsiFreeRings ( STATIC
EFI_STATUS
+PvScsiSetupRings (
+ IN OUT PVSCSI_DEV *Dev
+ )
+{
+ union {
+ PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
+ UINT32 Uint32;
+ } AlignedCmd;
+ PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
+
+ Cmd = &AlignedCmd.Cmd;
+
+ ZeroMem (Cmd, sizeof (*Cmd));
+ Cmd->ReqRingNumPages = 1;
+ Cmd->CmpRingNumPages = 1;
+ Cmd->RingsStatePPN = RShiftU64 (
+ Dev->RingDesc.RingStateDmaDesc.DeviceAddress,
+ EFI_PAGE_SHIFT
+ );
+ Cmd->ReqRingPPNs[0] = RShiftU64 (
+ Dev->RingDesc.RingReqsDmaDesc.DeviceAddress,
+ EFI_PAGE_SHIFT
+ );
+ Cmd->CmpRingPPNs[0] = RShiftU64 (
+ Dev->RingDesc.RingCmpsDmaDesc.DeviceAddress,
+ EFI_PAGE_SHIFT
+ );
+
+ STATIC_ASSERT (
+ sizeof (*Cmd) % sizeof (UINT32) == 0,
+ "Cmd must be multiple of 32-bit words"
+ );
+ return PvScsiWriteCmdDesc (
+ Dev,
+ PvScsiCmdSetupRings,
+ (UINT32 *)Cmd,
+ sizeof (*Cmd) / sizeof (UINT32)
+ );
+}
+
+STATIC
+EFI_STATUS
PvScsiInit (
IN OUT PVSCSI_DEV *Dev
)
@@ -1172,6 +1169,14 @@ PvScsiInit ( }
//
+ // Setup rings against device
+ //
+ Status = PvScsiSetupRings (Dev);
+ if (EFI_ERROR (Status)) {
+ goto FreeDmaCommBuffer;
+ }
+
+ //
// Populate the exported interface's attributes
//
Dev->PassThru.Mode = &Dev->PassThruMode;
@@ -1202,13 +1207,15 @@ PvScsiInit ( return EFI_SUCCESS;
-FreeRings:
- //
- // Reset device to stop device usage of the rings.
- // This is required to safely free the rings.
- //
- PvScsiResetAdapter (Dev);
+FreeDmaCommBuffer:
+ PvScsiFreeSharedPages (
+ Dev,
+ EFI_SIZE_TO_PAGES (sizeof (*Dev->DmaBuf)),
+ Dev->DmaBuf,
+ &Dev->DmaBufDmaDesc
+ );
+FreeRings:
PvScsiFreeRings (Dev);
RestorePciAttributes:
|