* [PATCH v2 0/1] Refactor DiscoverScsiDevice()
@ 2023-01-31 6:03 Yuan Yu
2023-01-31 6:03 ` [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: " Yuan Yu
0 siblings, 1 reply; 5+ messages in thread
From: Yuan Yu @ 2023-01-31 6:03 UTC (permalink / raw)
To: devel
Refactor DiscoverScsiDevice() so that it returns EFI_STATUS instead of
BOOLEAN. This will enable its callers to distinguish a "not found"
situation from real problems like memory allocation failures.
The changes can be seen at:
https://github.com/yyu/edk2/tree/scsi_bus_fix_v2
Yuan Yu (1):
MdeModulePkg: ScsiBusDxe: Refactor DiscoverScsiDevice()
MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.h | 10 ++++---
MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c | 30 ++++++++++----------
2 files changed, 21 insertions(+), 19 deletions(-)
--
2.39.1.456.gfc5497dd1b-goog
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor DiscoverScsiDevice()
2023-01-31 6:03 [PATCH v2 0/1] Refactor DiscoverScsiDevice() Yuan Yu
@ 2023-01-31 6:03 ` Yuan Yu
2023-02-01 6:48 ` Wu, Hao A
0 siblings, 1 reply; 5+ messages in thread
From: Yuan Yu @ 2023-01-31 6:03 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Liming Gao, Hao A Wu, Ray Ni,
Sivaparvathi chellaiah
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+tianocore@kernel.org>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sivaparvathi chellaiah <sivaparvathic@ami.com>
Signed-off-by: Yuan Yu <yuanyu@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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor DiscoverScsiDevice()
2023-01-31 6:03 ` [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: " Yuan Yu
@ 2023-02-01 6:48 ` Wu, Hao A
2023-02-01 18:16 ` Yuan Yu
0 siblings, 1 reply; 5+ messages in thread
From: Wu, Hao A @ 2023-02-01 6:48 UTC (permalink / raw)
To: Yuan Yu, devel@edk2.groups.io
Cc: Ard Biesheuvel, Gao, Liming, Ni, Ray, C, sivaparvathi
Reviewed-by: Hao A Wu <hao.a.wu@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 <yuanyu@google.com>
> Sent: Tuesday, January 31, 2023 2:04 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; C, sivaparvathi <sivaparvathic@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+tianocore@kernel.org>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sivaparvathi chellaiah <sivaparvathic@ami.com>
>
> Signed-off-by: Yuan Yu <yuanyu@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor DiscoverScsiDevice()
2023-02-01 6:48 ` Wu, Hao A
@ 2023-02-01 18:16 ` Yuan Yu
2023-02-02 2:01 ` Wu, Hao A
0 siblings, 1 reply; 5+ messages in thread
From: Yuan Yu @ 2023-02-01 18:16 UTC (permalink / raw)
To: Wu, Hao A
Cc: devel@edk2.groups.io, Ard Biesheuvel, Gao, Liming, Ni, Ray,
C, sivaparvathi
[-- Attachment #1: Type: text/plain, Size: 6802 bytes --]
Sounds good. Thank you!
Best,
Yuan
On Tue, Jan 31, 2023 at 10:48 PM Wu, Hao A <hao.a.wu@intel.com> wrote:
> Reviewed-by: Hao A Wu <hao.a.wu@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 <yuanyu@google.com>
> > Sent: Tuesday, January 31, 2023 2:04 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; C, sivaparvathi <sivaparvathic@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+tianocore@kernel.org>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Sivaparvathi chellaiah <sivaparvathic@ami.com>
> >
> > Signed-off-by: Yuan Yu <yuanyu@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
>
>
[-- Attachment #2: Type: text/html, Size: 9581 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: Refactor DiscoverScsiDevice()
2023-02-01 18:16 ` Yuan Yu
@ 2023-02-02 2:01 ` Wu, Hao A
0 siblings, 0 replies; 5+ messages in thread
From: Wu, Hao A @ 2023-02-02 2:01 UTC (permalink / raw)
To: Yuan Yu, devel@edk2.groups.io
Cc: Ard Biesheuvel, Gao, Liming, Ni, Ray, C, sivaparvathi
[-- Attachment #1: Type: text/plain, Size: 7606 bytes --]
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 <yuanyu@google.com>
Sent: Thursday, February 2, 2023 2:17 AM
To: Wu, Hao A <hao.a.wu@intel.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; C, sivaparvathi <sivaparvathic@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.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@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 <yuanyu@google.com<mailto:yuanyu@google.com>>
> Sent: Tuesday, January 31, 2023 2:04 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb%2Btianocore@kernel.org>>; Gao, Liming
> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ni, Ray
> <ray.ni@intel.com<mailto:ray.ni@intel.com>>; C, sivaparvathi <sivaparvathic@ami.com<mailto:sivaparvathic@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+tianocore@kernel.org<mailto:ardb%2Btianocore@kernel.org>>
> Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Sivaparvathi chellaiah <sivaparvathic@ami.com<mailto:sivaparvathic@ami.com>>
>
> Signed-off-by: Yuan Yu <yuanyu@google.com<mailto:yuanyu@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
[-- Attachment #2: Type: text/html, Size: 14157 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-02 2:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-31 6:03 [PATCH v2 0/1] Refactor DiscoverScsiDevice() Yuan Yu
2023-01-31 6:03 ` [PATCH v2 1/1] MdeModulePkg: ScsiBusDxe: " Yuan Yu
2023-02-01 6:48 ` Wu, Hao A
2023-02-01 18:16 ` Yuan Yu
2023-02-02 2:01 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox