Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-19 Thread Ming Huang 



On 3/19/2019 1:56 PM, Wu, Hao A wrote:
>> -Original Message-
>> From: Ming Huang [mailto:ming.hu...@linaro.org]
>> Sent: Tuesday, March 19, 2019 12:14 PM
>> To: Wu, Hao A; Leif Lindholm
>> Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong,
>> Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org; Kinney,
>> Michael D; Gao, Liming; wanghuiqi...@huawei.com;
>> huangmin...@huawei.com; zhangjinso...@huawei.com;
>> huangda...@hisilicon.com; wai...@126.com; Wang, Jian J
>> Subject: Re: [MdeModulePkg/Library v1 1/1]
>> MdeModulePkg/UefiBootManangerLib: Fix exception issue
>>
>>
>>
>> On 3/19/2019 10:25 AM, Wu, Hao A wrote:
 -Original Message-
 From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
 Sent: Monday, March 18, 2019 8:43 PM
 To: Ming Huang
 Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; 
 Dong,
 Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org;
>> Kinney,
 Michael D; Gao, Liming; wanghuiqi...@huawei.com;
 huangmin...@huawei.com; zhangjinso...@huawei.com;
 huangda...@hisilicon.com; wai...@126.com; Wang, Jian J; Wu, Hao A;
>> Ni,
 Ray
 Subject: Re: [MdeModulePkg/Library v1 1/1]
 MdeModulePkg/UefiBootManangerLib: Fix exception issue

 +MdeModulePkg maintainers (you added MdePkg maintainers to cc)

 This looks like an improvement to me.

 Am I correct in guessing this behaviour refers to some specific corner
 case of a USB CDROM emulated from a BMC?

 On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
> The system environment: virtual-CDROM(USB interface) via BMC, insert
>> a
> iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
> to first boot option.
> With release version bios, disconnecting CDROM when boot to
> "1 seconds left, Press Esc or F2 to enter Setup"
> then system will get a exception.
>
> The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be
 uninstalled
> in this situation after print some transfer error. The status will be
> invalid parameter. This line will get a exception for BlockIo not point
>>>
>>> Do you mean 'EFI_INVALID_PARAMETER' is returned from:
>>>   Status = gBS->HandleProtocol (Handle, , (VOID
>> **) );
>>
>> Yes.
>>
>>>
>>> If so, my guess is that 'Handle' is NULL at this point. An improvement can
>>> be adding a previous check for 'Status' after the ASSERT at:
>>>
>>>   Status = gBS->LocateDevicePath (,
>> , );
>>>   ASSERT_EFI_ERROR (Status);
>>
>> As my debug output, this 'Status' is seccuss and Handle is not NULL, but
>> gBS->ConnectController return:Not Found
>>
>> Debug output:
>> [BmExpandMediaDevicePath]:[1056L] Handle=3E3F3D18 BlockIo=3B2757B6
>> Media=AFAF6C617470AFAF Status=Success
>> EhcExecTransfer: transfer failed with 40
>> EhcBulkTransfer: error - Device Error, transfer - 40
>> .
>> [UsbOnHubInterrupt]:[632L] SignalEvent (HubIf->HubNotify)
>> UsbBotExecCommand: UsbBotSendCommand (Device Error)
>> UsbBootExecCmd: Device Error to Exec 0x0 Cmd (Result = 1)
>> EhcExecTransfer: transfer failed with 40
>> ...
>> [USBMassDriverBindingStop]:[1010L] Uninstall USB block io, free:
>> 3E44F218(F0)
>> [BmExpandMediaDevicePath]:[1064L] Connect Not Found
>> [BmExpandMediaDevicePath]:[1076L] Handle=3E3F3D18 BlockIo=3B2757B6
>> Media=AFAF6C617470AFAF Status=Invalid Parameter
> 
> Thanks for the debug information, I got it now.
> 
> The call to the gBS->ConnectController() leads to protocols being
> uninstalled from 'Handle' and removing 'Handle' from the database. Then
> within the call to gBS->HandleProtocol(), CoreValidateHandle() returns
> EFI_INVALID_PARAMETER since the handle cannot be found.
> 
> I am good with this patch, please help to address Leif's previous comment
> to keep the ASSERT.

I will add ASSERT back in v2.

> 
> Also, I have filed a Bugzilla tracker for this:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1631
> 
> Could you help to add the reference to the above BZ in the commit log
> message? Thanks.

Sure, add it in v2.

Thanks

> 
> 
> Best Regards,
> Hao Wu
> 
>>
>> Thanks
>>
>>>
>>> And leave:
>>>
>>>   Status = gBS->HandleProtocol (Handle, , (VOID
>> **) );
>>>   ASSERT_EFI_ERROR (Status);
>>>
>>> unchanged.
>>
>>
>>
>>>
>>> Best Regards,
>>> Hao Wu
>>>
> to right address:
> AllocatePool (BlockIo->Media->BlockSize)
> So, here need to judge the status not using ASSERT_EFI_ERROR.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
 b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index d5957db610d9..c2f1c651b02f 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-19 Thread Leif Lindholm 
On Tue, Mar 19, 2019 at 12:01:51PM +0800, Ming Huang wrote:
> On 3/18/2019 8:42 PM, Leif Lindholm wrote:
> > +MdeModulePkg maintainers (you added MdePkg maintainers to cc)
> > 
> > This looks like an improvement to me.
> > 
> > Am I correct in guessing this behaviour refers to some specific corner
> > case of a USB CDROM emulated from a BMC?
> 
> Yes, I found this issue with a USB CDROM emulated from a BMC.
> I guess have the same symptom with physical USB CDROM.

Yes, you would just be less likely to disconnect it :)

Thanks for the confirmation.

Best Regards,

Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-19 Thread Wu, Hao A 
> -Original Message-
> From: Ming Huang [mailto:ming.hu...@linaro.org]
> Sent: Tuesday, March 19, 2019 12:14 PM
> To: Wu, Hao A; Leif Lindholm
> Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong,
> Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org; Kinney,
> Michael D; Gao, Liming; wanghuiqi...@huawei.com;
> huangmin...@huawei.com; zhangjinso...@huawei.com;
> huangda...@hisilicon.com; wai...@126.com; Wang, Jian J
> Subject: Re: [MdeModulePkg/Library v1 1/1]
> MdeModulePkg/UefiBootManangerLib: Fix exception issue
> 
> 
> 
> On 3/19/2019 10:25 AM, Wu, Hao A wrote:
> >> -Original Message-
> >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >> Sent: Monday, March 18, 2019 8:43 PM
> >> To: Ming Huang
> >> Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; 
> >> Dong,
> >> Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org;
> Kinney,
> >> Michael D; Gao, Liming; wanghuiqi...@huawei.com;
> >> huangmin...@huawei.com; zhangjinso...@huawei.com;
> >> huangda...@hisilicon.com; wai...@126.com; Wang, Jian J; Wu, Hao A;
> Ni,
> >> Ray
> >> Subject: Re: [MdeModulePkg/Library v1 1/1]
> >> MdeModulePkg/UefiBootManangerLib: Fix exception issue
> >>
> >> +MdeModulePkg maintainers (you added MdePkg maintainers to cc)
> >>
> >> This looks like an improvement to me.
> >>
> >> Am I correct in guessing this behaviour refers to some specific corner
> >> case of a USB CDROM emulated from a BMC?
> >>
> >> On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
> >>> The system environment: virtual-CDROM(USB interface) via BMC, insert
> a
> >>> iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
> >>> to first boot option.
> >>> With release version bios, disconnecting CDROM when boot to
> >>> "1 seconds left, Press Esc or F2 to enter Setup"
> >>> then system will get a exception.
> >>>
> >>> The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be
> >> uninstalled
> >>> in this situation after print some transfer error. The status will be
> >>> invalid parameter. This line will get a exception for BlockIo not point
> >
> > Do you mean 'EFI_INVALID_PARAMETER' is returned from:
> >   Status = gBS->HandleProtocol (Handle, , (VOID
> **) );
> 
> Yes.
> 
> >
> > If so, my guess is that 'Handle' is NULL at this point. An improvement can
> > be adding a previous check for 'Status' after the ASSERT at:
> >
> >   Status = gBS->LocateDevicePath (,
> , );
> >   ASSERT_EFI_ERROR (Status);
> 
> As my debug output, this 'Status' is seccuss and Handle is not NULL, but
> gBS->ConnectController return:Not Found
> 
> Debug output:
> [BmExpandMediaDevicePath]:[1056L] Handle=3E3F3D18 BlockIo=3B2757B6
> Media=AFAF6C617470AFAF Status=Success
> EhcExecTransfer: transfer failed with 40
> EhcBulkTransfer: error - Device Error, transfer - 40
> .
> [UsbOnHubInterrupt]:[632L] SignalEvent (HubIf->HubNotify)
> UsbBotExecCommand: UsbBotSendCommand (Device Error)
> UsbBootExecCmd: Device Error to Exec 0x0 Cmd (Result = 1)
> EhcExecTransfer: transfer failed with 40
> ...
> [USBMassDriverBindingStop]:[1010L] Uninstall USB block io, free:
> 3E44F218(F0)
> [BmExpandMediaDevicePath]:[1064L] Connect Not Found
> [BmExpandMediaDevicePath]:[1076L] Handle=3E3F3D18 BlockIo=3B2757B6
> Media=AFAF6C617470AFAF Status=Invalid Parameter

Thanks for the debug information, I got it now.

The call to the gBS->ConnectController() leads to protocols being
uninstalled from 'Handle' and removing 'Handle' from the database. Then
within the call to gBS->HandleProtocol(), CoreValidateHandle() returns
EFI_INVALID_PARAMETER since the handle cannot be found.

I am good with this patch, please help to address Leif's previous comment
to keep the ASSERT.

Also, I have filed a Bugzilla tracker for this:
https://bugzilla.tianocore.org/show_bug.cgi?id=1631

Could you help to add the reference to the above BZ in the commit log
message? Thanks.


Best Regards,
Hao Wu

> 
> Thanks
> 
> >
> > And leave:
> >
> >   Status = gBS->HandleProtocol (Handle, , (VOID
> **) );
> >   ASSERT_EFI_ERROR (Status);
> >
> > unchanged.
> 
> 
> 
> >
> > Best Regards,
> > Hao Wu
> >
> >>> to right address:
> >>> AllocatePool (BlockIo->Media->BlockSize)
> >>> So, here need to judge the status not using ASSERT_EFI_ERROR.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Ming Huang 
> >>> ---
> >>>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >>> index d5957db610d9..c2f1c651b02f 100644
> >>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >>> @@ -1068,7 +1068,9 @@ BmExpandMediaDevicePath (
> >>>// Block IO read/write will success.
> >>>//
> >>>Status =

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-18 Thread Ming Huang 



On 3/19/2019 10:25 AM, Wu, Hao A wrote:
>> -Original Message-
>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>> Sent: Monday, March 18, 2019 8:43 PM
>> To: Ming Huang
>> Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong,
>> Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org; Kinney,
>> Michael D; Gao, Liming; wanghuiqi...@huawei.com;
>> huangmin...@huawei.com; zhangjinso...@huawei.com;
>> huangda...@hisilicon.com; wai...@126.com; Wang, Jian J; Wu, Hao A; Ni,
>> Ray
>> Subject: Re: [MdeModulePkg/Library v1 1/1]
>> MdeModulePkg/UefiBootManangerLib: Fix exception issue
>>
>> +MdeModulePkg maintainers (you added MdePkg maintainers to cc)
>>
>> This looks like an improvement to me.
>>
>> Am I correct in guessing this behaviour refers to some specific corner
>> case of a USB CDROM emulated from a BMC?
>>
>> On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
>>> The system environment: virtual-CDROM(USB interface) via BMC, insert a
>>> iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
>>> to first boot option.
>>> With release version bios, disconnecting CDROM when boot to
>>> "1 seconds left, Press Esc or F2 to enter Setup"
>>> then system will get a exception.
>>>
>>> The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be
>> uninstalled
>>> in this situation after print some transfer error. The status will be
>>> invalid parameter. This line will get a exception for BlockIo not point
> 
> Do you mean 'EFI_INVALID_PARAMETER' is returned from:
>   Status = gBS->HandleProtocol (Handle, , (VOID **) 
> );

Yes.

> 
> If so, my guess is that 'Handle' is NULL at this point. An improvement can
> be adding a previous check for 'Status' after the ASSERT at: 
> 
>   Status = gBS->LocateDevicePath (, , 
> );
>   ASSERT_EFI_ERROR (Status);

As my debug output, this 'Status' is seccuss and Handle is not NULL, but
gBS->ConnectController return:Not Found

Debug output:
[BmExpandMediaDevicePath]:[1056L] Handle=3E3F3D18 BlockIo=3B2757B6 
Media=AFAF6C617470AFAF Status=Success
EhcExecTransfer: transfer failed with 40
EhcBulkTransfer: error - Device Error, transfer - 40
.
[UsbOnHubInterrupt]:[632L] SignalEvent (HubIf->HubNotify)
UsbBotExecCommand: UsbBotSendCommand (Device Error)
UsbBootExecCmd: Device Error to Exec 0x0 Cmd (Result = 1)
EhcExecTransfer: transfer failed with 40
...
[USBMassDriverBindingStop]:[1010L] Uninstall USB block io, free: 3E44F218(F0)
[BmExpandMediaDevicePath]:[1064L] Connect Not Found
[BmExpandMediaDevicePath]:[1076L] Handle=3E3F3D18 BlockIo=3B2757B6 
Media=AFAF6C617470AFAF Status=Invalid Parameter

Thanks

> 
> And leave:
> 
>   Status = gBS->HandleProtocol (Handle, , (VOID **) 
> );
>   ASSERT_EFI_ERROR (Status);
> 
> unchanged.



> 
> Best Regards,
> Hao Wu
> 
>>> to right address:
>>> AllocatePool (BlockIo->Media->BlockSize)
>>> So, here need to judge the status not using ASSERT_EFI_ERROR.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ming Huang 
>>> ---
>>>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>>> index d5957db610d9..c2f1c651b02f 100644
>>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>>> @@ -1068,7 +1068,9 @@ BmExpandMediaDevicePath (
>>>// Block IO read/write will success.
>>>//
>>>Status = gBS->HandleProtocol (Handle, , (VOID
>> **) );
>>> -  ASSERT_EFI_ERROR (Status);
>>> +  if (EFI_ERROR (Status)) {
>>
>> It would still be worth including an ASSERT here, to let DEBUG builds
>> report on point of failure rather than several steps up the chain.
>>
>> /
>> Leif
>>
>>> +return NULL;
>>> +  }
>>>Buffer = AllocatePool (BlockIo->Media->BlockSize);
>>>if (Buffer != NULL) {
>>>  BlockIo->ReadBlocks (
>>> --
>>> 2.9.5
>>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-18 Thread Ming Huang 



On 3/18/2019 8:42 PM, Leif Lindholm wrote:
> +MdeModulePkg maintainers (you added MdePkg maintainers to cc)
> 
> This looks like an improvement to me.
> 
> Am I correct in guessing this behaviour refers to some specific corner
> case of a USB CDROM emulated from a BMC?

Yes, I found this issue with a USB CDROM emulated from a BMC.
I guess have the same symptom with physical USB CDROM.

Thanks

> 
> On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
>> The system environment: virtual-CDROM(USB interface) via BMC, insert a
>> iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
>> to first boot option.
>> With release version bios, disconnecting CDROM when boot to
>> "1 seconds left, Press Esc or F2 to enter Setup"
>> then system will get a exception.
>>
>> The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be uninstalled
>> in this situation after print some transfer error. The status will be
>> invalid parameter. This line will get a exception for BlockIo not point
>> to right address:
>> AllocatePool (BlockIo->Media->BlockSize)
>> So, here need to judge the status not using ASSERT_EFI_ERROR.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang 
>> ---
>>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
>> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> index d5957db610d9..c2f1c651b02f 100644
>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> @@ -1068,7 +1068,9 @@ BmExpandMediaDevicePath (
>>// Block IO read/write will success.
>>//
>>Status = gBS->HandleProtocol (Handle, , (VOID **) 
>> );
>> -  ASSERT_EFI_ERROR (Status);
>> +  if (EFI_ERROR (Status)) {
> 
> It would still be worth including an ASSERT here, to let DEBUG builds
> report on point of failure rather than several steps up the chain.
> 
> /
> Leif
> 
>> +return NULL;
>> +  }
>>Buffer = AllocatePool (BlockIo->Media->BlockSize);
>>if (Buffer != NULL) {
>>  BlockIo->ReadBlocks (
>> -- 
>> 2.9.5
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-18 Thread Wu, Hao A 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Monday, March 18, 2019 8:43 PM
> To: Ming Huang
> Cc: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong,
> Eric; Ni, Ray; dann.fraz...@canonical.com; ard.biesheu...@linaro.org; Kinney,
> Michael D; Gao, Liming; wanghuiqi...@huawei.com;
> huangmin...@huawei.com; zhangjinso...@huawei.com;
> huangda...@hisilicon.com; wai...@126.com; Wang, Jian J; Wu, Hao A; Ni,
> Ray
> Subject: Re: [MdeModulePkg/Library v1 1/1]
> MdeModulePkg/UefiBootManangerLib: Fix exception issue
> 
> +MdeModulePkg maintainers (you added MdePkg maintainers to cc)
> 
> This looks like an improvement to me.
> 
> Am I correct in guessing this behaviour refers to some specific corner
> case of a USB CDROM emulated from a BMC?
> 
> On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
> > The system environment: virtual-CDROM(USB interface) via BMC, insert a
> > iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
> > to first boot option.
> > With release version bios, disconnecting CDROM when boot to
> > "1 seconds left, Press Esc or F2 to enter Setup"
> > then system will get a exception.
> >
> > The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be
> uninstalled
> > in this situation after print some transfer error. The status will be
> > invalid parameter. This line will get a exception for BlockIo not point

Do you mean 'EFI_INVALID_PARAMETER' is returned from:
  Status = gBS->HandleProtocol (Handle, , (VOID **) 
);

If so, my guess is that 'Handle' is NULL at this point. An improvement can
be adding a previous check for 'Status' after the ASSERT at: 

  Status = gBS->LocateDevicePath (, , 
);
  ASSERT_EFI_ERROR (Status);

And leave:

  Status = gBS->HandleProtocol (Handle, , (VOID **) 
);
  ASSERT_EFI_ERROR (Status);

unchanged.

Best Regards,
Hao Wu

> > to right address:
> > AllocatePool (BlockIo->Media->BlockSize)
> > So, here need to judge the status not using ASSERT_EFI_ERROR.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ming Huang 
> > ---
> >  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > index d5957db610d9..c2f1c651b02f 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > @@ -1068,7 +1068,9 @@ BmExpandMediaDevicePath (
> >// Block IO read/write will success.
> >//
> >Status = gBS->HandleProtocol (Handle, , (VOID
> **) );
> > -  ASSERT_EFI_ERROR (Status);
> > +  if (EFI_ERROR (Status)) {
> 
> It would still be worth including an ASSERT here, to let DEBUG builds
> report on point of failure rather than several steps up the chain.
> 
> /
> Leif
> 
> > +return NULL;
> > +  }
> >Buffer = AllocatePool (BlockIo->Media->BlockSize);
> >if (Buffer != NULL) {
> >  BlockIo->ReadBlocks (
> > --
> > 2.9.5
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue

2019-03-18 Thread Leif Lindholm 
+MdeModulePkg maintainers (you added MdePkg maintainers to cc)

This looks like an improvement to me.

Am I correct in guessing this behaviour refers to some specific corner
case of a USB CDROM emulated from a BMC?

On Mon, Feb 25, 2019 at 05:10:52PM +0800, Ming Huang wrote:
> The system environment: virtual-CDROM(USB interface) via BMC, insert a
> iso file to CDROM, like ubuntu-18.04.1-server-arm64.iso, change CDROM
> to first boot option.
> With release version bios, disconnecting CDROM when boot to
> "1 seconds left, Press Esc or F2 to enter Setup"
> then system will get a exception.
> 
> The root cause is the EFI_BLOCK_IO_PROTOCOL for UsbMass will be uninstalled
> in this situation after print some transfer error. The status will be
> invalid parameter. This line will get a exception for BlockIo not point
> to right address:
> AllocatePool (BlockIo->Media->BlockSize)
> So, here need to judge the status not using ASSERT_EFI_ERROR.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index d5957db610d9..c2f1c651b02f 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1068,7 +1068,9 @@ BmExpandMediaDevicePath (
>// Block IO read/write will success.
>//
>Status = gBS->HandleProtocol (Handle, , (VOID **) 
> );
> -  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {

It would still be worth including an ASSERT here, to let DEBUG builds
report on point of failure rather than several steps up the chain.

/
Leif

> +return NULL;
> +  }
>Buffer = AllocatePool (BlockIo->Media->BlockSize);
>if (Buffer != NULL) {
>  BlockIo->ReadBlocks (
> -- 
> 2.9.5
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel