[edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

Liran Alon posted 1 patch 4 years ago
Failed in applying to current master (apply log)
There is a newer version of this series
OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++++++++++++++++++------------------
1 file changed, 76 insertions(+), 74 deletions(-)
[edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function
Posted by Liran Alon 4 years ago
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>
---
 OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++++++++++++++++++------------------
 1 file changed, 76 insertions(+), 74 deletions(-)

diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index 1ca50390c0e5..5b7fdcbda10b 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,6 +1025,69 @@ PvScsiInitRings (
   }
   ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
 
+  return EFI_SUCCESS;
+
+FreeRingReqs:
+  PvScsiFreeSharedPages (
+    Dev,
+    1,
+    Dev->RingDesc.RingReqs,
+    &Dev->RingDesc.RingReqsDmaDesc
+    );
+
+FreeRingState:
+  PvScsiFreeSharedPages (
+    Dev,
+    1,
+    Dev->RingDesc.RingState,
+    &Dev->RingDesc.RingStateDmaDesc
+    );
+
+  return Status;
+}
+
+STATIC
+VOID
+PvScsiFreeRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  PvScsiFreeSharedPages (
+    Dev,
+    1,
+    Dev->RingDesc.RingCmps,
+    &Dev->RingDesc.RingCmpsDmaDesc
+    );
+
+  PvScsiFreeSharedPages (
+    Dev,
+    1,
+    Dev->RingDesc.RingReqs,
+    &Dev->RingDesc.RingReqsDmaDesc
+    );
+
+  PvScsiFreeSharedPages (
+    Dev,
+    1,
+    Dev->RingDesc.RingState,
+    &Dev->RingDesc.RingStateDmaDesc
+    );
+}
+
+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;
@@ -1052,71 +1108,12 @@ PvScsiInitRings (
     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,
-    1,
-    Dev->RingDesc.RingReqs,
-    &Dev->RingDesc.RingReqsDmaDesc
-    );
-
-FreeRingState:
-  PvScsiFreeSharedPages (
-    Dev,
-    1,
-    Dev->RingDesc.RingState,
-    &Dev->RingDesc.RingStateDmaDesc
-    );
-
-  return Status;
-}
-
-STATIC
-VOID
-PvScsiFreeRings (
-  IN OUT PVSCSI_DEV *Dev
-  )
-{
-  PvScsiFreeSharedPages (
-    Dev,
-    1,
-    Dev->RingDesc.RingCmps,
-    &Dev->RingDesc.RingCmpsDmaDesc
-    );
-
-  PvScsiFreeSharedPages (
-    Dev,
-    1,
-    Dev->RingDesc.RingReqs,
-    &Dev->RingDesc.RingReqsDmaDesc
-    );
-
-  PvScsiFreeSharedPages (
-    Dev,
-    1,
-    Dev->RingDesc.RingState,
-    &Dev->RingDesc.RingStateDmaDesc
-    );
+  return PvScsiWriteCmdDesc (
+           Dev,
+           PvScsiCmdSetupRings,
+           (UINT32 *)Cmd,
+           sizeof (*Cmd) / sizeof (UINT32)
+           );
 }
 
 STATIC
@@ -1157,6 +1154,10 @@ PvScsiInit (
   if (EFI_ERROR (Status)) {
     goto RestorePciAttributes;
   }
+  Status = PvScsiSetupRings (Dev);
+  if (EFI_ERROR (Status)) {
+    goto FreeRings;
+  }
 
   //
   // Allocate DMA communication buffer
@@ -1168,7 +1169,7 @@ PvScsiInit (
              &Dev->DmaBufDmaDesc
              );
   if (EFI_ERROR (Status)) {
-    goto FreeRings;
+    goto UnsetupRings;
   }
 
   //
@@ -1202,13 +1203,14 @@ PvScsiInit (
 
   return EFI_SUCCESS;
 
-FreeRings:
+UnsetupRings:
   //
   // Reset device to stop device usage of the rings.
   // This is required to safely free the rings.
   //
   PvScsiResetAdapter (Dev);
 
+FreeRings:
   PvScsiFreeRings (Dev);
 
 RestorePciAttributes:
-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56749): https://edk2.groups.io/g/devel/message/56749
Mute This Topic: https://groups.io/mt/72674474/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function
Posted by Laszlo Ersek 4 years ago
Hi Liran,

On 03/31/20 13:47, Liran Alon wrote:
> 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>
> ---
>  OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++++++++++++++++++------------------
>  1 file changed, 76 insertions(+), 74 deletions(-)

Thanks for the patch.

I didn't expect this approach -- hoisting the rings exposure from
SetupRings to Init --, but it turns out that I like it more than my own
previous suggestion -- pushing ResetAdapter into FreeRings :)

May I just trouble you with one further request?

> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index 1ca50390c0e5..5b7fdcbda10b 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,6 +1025,69 @@ PvScsiInitRings (
>    }
>    ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
>  
> +  return EFI_SUCCESS;
> +
> +FreeRingReqs:
> +  PvScsiFreeSharedPages (
> +    Dev,
> +    1,
> +    Dev->RingDesc.RingReqs,
> +    &Dev->RingDesc.RingReqsDmaDesc
> +    );
> +
> +FreeRingState:
> +  PvScsiFreeSharedPages (
> +    Dev,
> +    1,
> +    Dev->RingDesc.RingState,
> +    &Dev->RingDesc.RingStateDmaDesc
> +    );
> +
> +  return Status;
> +}
> +
> +STATIC
> +VOID
> +PvScsiFreeRings (
> +  IN OUT PVSCSI_DEV *Dev
> +  )
> +{
> +  PvScsiFreeSharedPages (
> +    Dev,
> +    1,
> +    Dev->RingDesc.RingCmps,
> +    &Dev->RingDesc.RingCmpsDmaDesc
> +    );
> +
> +  PvScsiFreeSharedPages (
> +    Dev,
> +    1,
> +    Dev->RingDesc.RingReqs,
> +    &Dev->RingDesc.RingReqsDmaDesc
> +    );
> +
> +  PvScsiFreeSharedPages (
> +    Dev,
> +    1,
> +    Dev->RingDesc.RingState,
> +    &Dev->RingDesc.RingStateDmaDesc
> +    );
> +}
> +
> +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;
> @@ -1052,71 +1108,12 @@ PvScsiInitRings (
>      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,
> -    1,
> -    Dev->RingDesc.RingReqs,
> -    &Dev->RingDesc.RingReqsDmaDesc
> -    );
> -
> -FreeRingState:
> -  PvScsiFreeSharedPages (
> -    Dev,
> -    1,
> -    Dev->RingDesc.RingState,
> -    &Dev->RingDesc.RingStateDmaDesc
> -    );
> -
> -  return Status;
> -}
> -
> -STATIC
> -VOID
> -PvScsiFreeRings (
> -  IN OUT PVSCSI_DEV *Dev
> -  )
> -{
> -  PvScsiFreeSharedPages (
> -    Dev,
> -    1,
> -    Dev->RingDesc.RingCmps,
> -    &Dev->RingDesc.RingCmpsDmaDesc
> -    );
> -
> -  PvScsiFreeSharedPages (
> -    Dev,
> -    1,
> -    Dev->RingDesc.RingReqs,
> -    &Dev->RingDesc.RingReqsDmaDesc
> -    );
> -
> -  PvScsiFreeSharedPages (
> -    Dev,
> -    1,
> -    Dev->RingDesc.RingState,
> -    &Dev->RingDesc.RingStateDmaDesc
> -    );
> +  return PvScsiWriteCmdDesc (
> +           Dev,
> +           PvScsiCmdSetupRings,
> +           (UINT32 *)Cmd,
> +           sizeof (*Cmd) / sizeof (UINT32)
> +           );
>  }
>  
>  STATIC
> @@ -1157,6 +1154,10 @@ PvScsiInit (
>    if (EFI_ERROR (Status)) {
>      goto RestorePciAttributes;
>    }
> +  Status = PvScsiSetupRings (Dev);
> +  if (EFI_ERROR (Status)) {
> +    goto FreeRings;
> +  }

If you could move the PvScsiSetupRings() call *below* the DMA buffer
allocation (and rework the error path accordingly -- basically replacing
the "UnsetupRings" action with a "FreeDmaBuffer" action), then this
patch would be *perfect*.

Because then, the PvScsiUninit() order of actions would be a 100% mirror
for PvScsiInit()'s; namely with PvScsiResetAdapter() at the top of
PvScsiUninit() perfectly matching PvScsiSetupRings() at the end of
PvScsiInit()!

I don't want to annoy you too much, so if you really want to stick with
this version, I'll take it as well.

Thanks!
Laszlo

>  
>    //
>    // Allocate DMA communication buffer
> @@ -1168,7 +1169,7 @@ PvScsiInit (
>               &Dev->DmaBufDmaDesc
>               );
>    if (EFI_ERROR (Status)) {
> -    goto FreeRings;
> +    goto UnsetupRings;
>    }
>  
>    //
> @@ -1202,13 +1203,14 @@ PvScsiInit (
>  
>    return EFI_SUCCESS;
>  
> -FreeRings:
> +UnsetupRings:
>    //
>    // Reset device to stop device usage of the rings.
>    // This is required to safely free the rings.
>    //
>    PvScsiResetAdapter (Dev);
>  
> +FreeRings:
>    PvScsiFreeRings (Dev);
>  
>  RestorePciAttributes:
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56783): https://edk2.groups.io/g/devel/message/56783
Mute This Topic: https://groups.io/mt/72674474/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function
Posted by Liran Alon 4 years ago
On 01/04/2020 1:19, Laszlo Ersek wrote:
> Hi Liran,
>
> On 03/31/20 13:47, Liran Alon wrote:
>> 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>
>> ---
>>   OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++++++++++++++++++------------------
>>   1 file changed, 76 insertions(+), 74 deletions(-)
> Thanks for the patch.
>
> I didn't expect this approach -- hoisting the rings exposure from
> SetupRings to Init --, but it turns out that I like it more than my own
> previous suggestion -- pushing ResetAdapter into FreeRings :)
Yeah I thought about it later and liked it to. Hoped you will think the 
same ;)
>
> May I just trouble you with one further request?
>
>> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
>> index 1ca50390c0e5..5b7fdcbda10b 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,6 +1025,69 @@ PvScsiInitRings (
>>     }
>>     ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
>>   
>> +  return EFI_SUCCESS;
>> +
>> +FreeRingReqs:
>> +  PvScsiFreeSharedPages (
>> +    Dev,
>> +    1,
>> +    Dev->RingDesc.RingReqs,
>> +    &Dev->RingDesc.RingReqsDmaDesc
>> +    );
>> +
>> +FreeRingState:
>> +  PvScsiFreeSharedPages (
>> +    Dev,
>> +    1,
>> +    Dev->RingDesc.RingState,
>> +    &Dev->RingDesc.RingStateDmaDesc
>> +    );
>> +
>> +  return Status;
>> +}
>> +
>> +STATIC
>> +VOID
>> +PvScsiFreeRings (
>> +  IN OUT PVSCSI_DEV *Dev
>> +  )
>> +{
>> +  PvScsiFreeSharedPages (
>> +    Dev,
>> +    1,
>> +    Dev->RingDesc.RingCmps,
>> +    &Dev->RingDesc.RingCmpsDmaDesc
>> +    );
>> +
>> +  PvScsiFreeSharedPages (
>> +    Dev,
>> +    1,
>> +    Dev->RingDesc.RingReqs,
>> +    &Dev->RingDesc.RingReqsDmaDesc
>> +    );
>> +
>> +  PvScsiFreeSharedPages (
>> +    Dev,
>> +    1,
>> +    Dev->RingDesc.RingState,
>> +    &Dev->RingDesc.RingStateDmaDesc
>> +    );
>> +}
>> +
>> +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;
>> @@ -1052,71 +1108,12 @@ PvScsiInitRings (
>>       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,
>> -    1,
>> -    Dev->RingDesc.RingReqs,
>> -    &Dev->RingDesc.RingReqsDmaDesc
>> -    );
>> -
>> -FreeRingState:
>> -  PvScsiFreeSharedPages (
>> -    Dev,
>> -    1,
>> -    Dev->RingDesc.RingState,
>> -    &Dev->RingDesc.RingStateDmaDesc
>> -    );
>> -
>> -  return Status;
>> -}
>> -
>> -STATIC
>> -VOID
>> -PvScsiFreeRings (
>> -  IN OUT PVSCSI_DEV *Dev
>> -  )
>> -{
>> -  PvScsiFreeSharedPages (
>> -    Dev,
>> -    1,
>> -    Dev->RingDesc.RingCmps,
>> -    &Dev->RingDesc.RingCmpsDmaDesc
>> -    );
>> -
>> -  PvScsiFreeSharedPages (
>> -    Dev,
>> -    1,
>> -    Dev->RingDesc.RingReqs,
>> -    &Dev->RingDesc.RingReqsDmaDesc
>> -    );
>> -
>> -  PvScsiFreeSharedPages (
>> -    Dev,
>> -    1,
>> -    Dev->RingDesc.RingState,
>> -    &Dev->RingDesc.RingStateDmaDesc
>> -    );
>> +  return PvScsiWriteCmdDesc (
>> +           Dev,
>> +           PvScsiCmdSetupRings,
>> +           (UINT32 *)Cmd,
>> +           sizeof (*Cmd) / sizeof (UINT32)
>> +           );
>>   }
>>   
>>   STATIC
>> @@ -1157,6 +1154,10 @@ PvScsiInit (
>>     if (EFI_ERROR (Status)) {
>>       goto RestorePciAttributes;
>>     }
>> +  Status = PvScsiSetupRings (Dev);
>> +  if (EFI_ERROR (Status)) {
>> +    goto FreeRings;
>> +  }
> If you could move the PvScsiSetupRings() call *below* the DMA buffer
> allocation (and rework the error path accordingly -- basically replacing
> the "UnsetupRings" action with a "FreeDmaBuffer" action), then this
> patch would be *perfect*.
>
> Because then, the PvScsiUninit() order of actions would be a 100% mirror
> for PvScsiInit()'s; namely with PvScsiResetAdapter() at the top of
> PvScsiUninit() perfectly matching PvScsiSetupRings() at the end of
> PvScsiInit()!
>
> I don't want to annoy you too much, so if you really want to stick with
> this version, I'll take it as well.

Good suggestion. Let me submit a v2 of this patch.

-Liran

>
> Thanks!
> Laszlo
>
>>   
>>     //
>>     // Allocate DMA communication buffer
>> @@ -1168,7 +1169,7 @@ PvScsiInit (
>>                &Dev->DmaBufDmaDesc
>>                );
>>     if (EFI_ERROR (Status)) {
>> -    goto FreeRings;
>> +    goto UnsetupRings;
>>     }
>>   
>>     //
>> @@ -1202,13 +1203,14 @@ PvScsiInit (
>>   
>>     return EFI_SUCCESS;
>>   
>> -FreeRings:
>> +UnsetupRings:
>>     //
>>     // Reset device to stop device usage of the rings.
>>     // This is required to safely free the rings.
>>     //
>>     PvScsiResetAdapter (Dev);
>>   
>> +FreeRings:
>>     PvScsiFreeRings (Dev);
>>   
>>   RestorePciAttributes:
>>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56784): https://edk2.groups.io/g/devel/message/56784
Mute This Topic: https://groups.io/mt/72674474/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-