* [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