Merged via:
PR - https://github.com/tianocore/edk2/pull/3979
Commit - 
https://github.com/tianocore/edk2/commit/d375273c899bfa279c2732c509caf455af285006

Best Regards,
Hao Wu

From: Yuan Yu <yua...@google.com>
Sent: Thursday, February 2, 2023 2:17 AM
To: Wu, Hao A <hao.a...@intel.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianoc...@kernel.org>; Gao, 
Liming <gaolim...@byosoft.com.cn>; Ni, Ray <ray...@intel.com>; C, sivaparvathi 
<sivaparvat...@ami.com>
Subject: Re: [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor 
DiscoverScsiDevice()

Sounds good. Thank you!

Best,
Yuan

On Tue, Jan 31, 2023 at 10:48 PM Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>> wrote:
Reviewed-by: Hao A Wu <hao.a...@intel.com<mailto:hao.a...@intel.com>>
Will update the function comment for ScsiScanCreateDevice() in ScsiBus.c to 
match the change in ScsiBus.h during merge.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Yuan Yu <yua...@google.com<mailto:yua...@google.com>>
> Sent: Tuesday, January 31, 2023 2:04 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Ard Biesheuvel 
> <ardb+tianoc...@kernel.org<mailto:ardb%2btianoc...@kernel.org>>; Gao, Liming
> <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>; Wu, Hao A 
> <hao.a...@intel.com<mailto:hao.a...@intel.com>>; Ni, Ray
> <ray...@intel.com<mailto:ray...@intel.com>>; C, sivaparvathi 
> <sivaparvat...@ami.com<mailto:sivaparvat...@ami.com>>
> Subject: [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor
> DiscoverScsiDevice()
>
> Currently DiscoverScsiDevice() returns a boolean which cannot
> distinguish a "not found" situation from a real problem like
> memory allocation failures.
>
> This patch changes the return value to an EFI_STATUS so that when
> memory allocation fails, it will return EFI_OUT_OF_RESOURCES.
>
> Without this change, any FALSE returned by DiscoverScsiDevice()
> will result in EFI_OUT_OF_RESOURCES being returned by
> ScsiScanCreateDevice(), which will cause a while loop in
> SCSIBusDriverBindingStart() to abort before other possible Puns in
> the SCSI channel are scanned, which means good devices may not have
> a chance to be discovered.  If this good device is the boot device,
> boot will fail.
>
> Cc: Ard Biesheuvel 
> <ardb+tianoc...@kernel.org<mailto:ardb%2btianoc...@kernel.org>>
> Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
> Cc: Hao A Wu <hao.a...@intel.com<mailto:hao.a...@intel.com>>
> Cc: Ray Ni <ray...@intel.com<mailto:ray...@intel.com>>
> Cc: Sivaparvathi chellaiah 
> <sivaparvat...@ami.com<mailto:sivaparvat...@ami.com>>
>
> Signed-off-by: Yuan Yu <yua...@google.com<mailto:yua...@google.com>>
> ---
>  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h | 10 ++++---
>  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c | 30 ++++++++++----------
>  2 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h
> b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h
> index 68c5c02a9161..35a8a46ca7a2 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h
> @@ -455,7 +455,8 @@ ScsiExecuteSCSICommand (
>
>    @retval EFI_SUCCESS           Successfully to discover the device and 
> attach
>                                  ScsiIoProtocol to it.
> -  @retval EFI_OUT_OF_RESOURCES  Fail to discover the device.
> +  @retval EFI_NOT_FOUND         Fail to discover the device.
> +  @retval EFI_OUT_OF_RESOURCES  Fail to allocate memory resources.
>
>  **/
>  EFI_STATUS
> @@ -473,11 +474,12 @@ ScsiScanCreateDevice (
>
>    @param  ScsiIoDevice    The pointer of SCSI_IO_DEV
>
> -  @retval  TRUE   Find SCSI Device and verify it.
> -  @retval  FALSE  Unable to find SCSI Device.
> +  @retval EFI_SUCCESS           Find SCSI Device and verify it.
> +  @retval EFI_NOT_FOUND         Unable to find SCSI Device.
> +  @retval EFI_OUT_OF_RESOURCES  Fail to allocate memory resources.
>
>  **/
> -BOOLEAN
> +EFI_STATUS
>  DiscoverScsiDevice (
>    IN  OUT  SCSI_IO_DEV  *ScsiIoDevice
>    );
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
> b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
> index fbe14c772496..4414a65eb1e6 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
> @@ -1210,8 +1210,8 @@ ScsiScanCreateDevice (
>      ScsiBusDev->DevicePath
>      );
>
> -  if (!DiscoverScsiDevice (ScsiIoDevice)) {
> -    Status = EFI_OUT_OF_RESOURCES;
> +  Status = DiscoverScsiDevice (ScsiIoDevice);
> +  if (EFI_ERROR (Status)) {
>      goto ErrorExit;
>    }
>
> @@ -1276,11 +1276,12 @@ ErrorExit:
>
>    @param  ScsiIoDevice    The pointer of SCSI_IO_DEV
>
> -  @retval  TRUE   Find SCSI Device and verify it.
> -  @retval  FALSE  Unable to find SCSI Device.
> +  @retval EFI_SUCCESS           Find SCSI Device and verify it.
> +  @retval EFI_NOT_FOUND         Unable to find SCSI Device.
> +  @retval EFI_OUT_OF_RESOURCES  Fail to allocate memory resources.
>
>  **/
> -BOOLEAN
> +EFI_STATUS
>  DiscoverScsiDevice (
>    IN OUT  SCSI_IO_DEV  *ScsiIoDevice
>    )
> @@ -1294,7 +1295,6 @@ DiscoverScsiDevice (
>    EFI_SCSI_SENSE_DATA    *SenseData;
>    UINT8                  MaxRetry;
>    UINT8                  Index;
> -  BOOLEAN                ScsiDeviceFound;
>
>    HostAdapterStatus = 0;
>    TargetStatus      = 0;
> @@ -1302,7 +1302,7 @@ DiscoverScsiDevice (
>
>    InquiryData = AllocateAlignedBuffer (ScsiIoDevice, sizeof
> (EFI_SCSI_INQUIRY_DATA));
>    if (InquiryData == NULL) {
> -    ScsiDeviceFound = FALSE;
> +    Status = EFI_OUT_OF_RESOURCES;
>      goto Done;
>    }
>
> @@ -1311,7 +1311,7 @@ DiscoverScsiDevice (
>                  sizeof (EFI_SCSI_SENSE_DATA)
>                  );
>    if (SenseData == NULL) {
> -    ScsiDeviceFound = FALSE;
> +    Status = EFI_OUT_OF_RESOURCES;
>      goto Done;
>    }
>
> @@ -1342,7 +1342,7 @@ DiscoverScsiDevice (
>            (SenseData->Error_Code == 0x70) &&
>            (SenseData->Sense_Key == EFI_SCSI_SK_ILLEGAL_REQUEST))
>        {
> -        ScsiDeviceFound = FALSE;
> +        Status = EFI_NOT_FOUND;
>          goto Done;
>        }
>
> @@ -1353,13 +1353,13 @@ DiscoverScsiDevice (
>          (Status == EFI_INVALID_PARAMETER) ||
>          (Status == EFI_UNSUPPORTED))
>      {
> -      ScsiDeviceFound = FALSE;
> +      Status = EFI_NOT_FOUND;
>        goto Done;
>      }
>    }
>
>    if (Index == MaxRetry) {
> -    ScsiDeviceFound = FALSE;
> +    Status = EFI_NOT_FOUND;
>      goto Done;
>    }
>
> @@ -1367,14 +1367,14 @@ DiscoverScsiDevice (
>    // Retrieved inquiry data successfully
>    //
>    if (InquiryData->Peripheral_Qualifier != 0) {
> -    ScsiDeviceFound = FALSE;
> +    Status = EFI_NOT_FOUND;
>      goto Done;
>    }
>
>    if ((InquiryData->Peripheral_Type >= EFI_SCSI_TYPE_RESERVED_LOW) &&
>        (InquiryData->Peripheral_Type <= EFI_SCSI_TYPE_RESERVED_HIGH))
>    {
> -    ScsiDeviceFound = FALSE;
> +    Status = EFI_NOT_FOUND;
>      goto Done;
>    }
>
> @@ -1392,13 +1392,13 @@ DiscoverScsiDevice (
>      ScsiIoDevice->ScsiVersion = (UINT8)(InquiryData->Version & 0x07);
>    }
>
> -  ScsiDeviceFound = TRUE;
> +  Status = EFI_SUCCESS;
>
>  Done:
>    FreeAlignedBuffer (SenseData, sizeof (EFI_SCSI_SENSE_DATA));
>    FreeAlignedBuffer (InquiryData, sizeof (EFI_SCSI_INQUIRY_DATA));
>
> -  return ScsiDeviceFound;
> +  return Status;
>  }
>
>  /**
> --
> 2.39.1.456.gfc5497dd1b-goog


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99425): https://edk2.groups.io/g/devel/message/99425
Mute This Topic: https://groups.io/mt/96647625/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to