Another exception issue while UsbMass block io uninstalled when boot to grub: The system environment: virtual-CDROM(USB interface) via BMC, insert a iso file to virtual-CDROM, like ubuntu-18.04.1-server-arm64.iso, change virtaul-CDROM to first boot option.
Disconnecting virtual-CDROM when boot to grub menu "Install Ubuntu Server" then select "Install Ubuntu Server", system will also get exception.
The root cause is the EFI_BLOCK_IO_PROTOCOL for USBMass will be uninstalled in this situation after print some transfer error(see blow), but grub will still use the block io which had initialized by grub_efidisk_init() in efidisk.c. When run m->io_align in grub_efidisk_open (): if (m->io_align & (m->io_align - 1)) grub will get exception for the EFI_BLOCK_IO_PROTOCOL had uninstalled and the memory had set to 0xAF by PcdDebugClearMemoryValue.
This exception look like the matching problem grub and uefi. Is it need to do something in uefi side or grub side?
The open source grub grub_efidisk_open function chunk: m = d->block_io->media; /* FIXME: Probably it is better to store the block size in the disk, and total sectors should be replaced with total blocks. */ grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x, io align = %x\n", m, (unsigned long long) m->last_block, m->block_size, m->io_align);
/* Ensure required buffer alignment is a power of two (or is zero). */ if (m->io_align & (m->io_align - 1))
USB transfer error log: UsbBootExecCmd: Device Error to Exec 0x0 Cmd (Result = 1) EhcExecTransfer: transfer failed with 40 EhcBulkTransfer: error - Device Error, transfer - 40 UsbBotExecCommand: UsbBotSendCommand (Device Error) UsbBootRequestSense: (Device Error) CmdResult=0x1 UsbBootDetectMedia: UsbBootIsUnitReady (Device Error) -----------------------------------------------------------
Ming Huang (1): MdeModulePkg/UefiBootManangerLib: Fix exception issue
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
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 ming.huang@linaro.org --- 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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo); - ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + return NULL; + } Buffer = AllocatePool (BlockIo->Media->BlockSize); if (Buffer != NULL) { BlockIo->ReadBlocks (
+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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo);
- 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
-----Original Message----- From: Leif Lindholm [mailto:leif.lindholm@linaro.org] Sent: Monday, March 18, 2019 8:43 PM To: Ming Huang Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org; Kinney, Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo);
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 (&gEfiBlockIoProtocolGuid, &TempDevicePath, &Handle); ASSERT_EFI_ERROR (Status);
And leave:
Status = gBS->HandleProtocol (Handle, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo); 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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
- 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
On 3/19/2019 10:25 AM, Wu, Hao A wrote:
-----Original Message----- From: Leif Lindholm [mailto:leif.lindholm@linaro.org] Sent: Monday, March 18, 2019 8:43 PM To: Ming Huang Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org; Kinney, Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo);
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 (&gEfiBlockIoProtocolGuid, &TempDevicePath, &Handle); 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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo); 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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
- 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
-----Original Message----- From: Ming Huang [mailto:ming.huang@linaro.org] Sent: Tuesday, March 19, 2019 12:14 PM To: Wu, Hao A; Leif Lindholm Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org; Kinney, Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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.lindholm@linaro.org] Sent: Monday, March 18, 2019 8:43 PM To: Ming Huang Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org;
Kinney,
Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
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 (&gEfiBlockIoProtocolGuid,
&TempDevicePath, &Handle);
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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid,
(VOID
**) &BlockIo);
- 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
On 3/19/2019 1:56 PM, Wu, Hao A wrote:
-----Original Message----- From: Ming Huang [mailto:ming.huang@linaro.org] Sent: Tuesday, March 19, 2019 12:14 PM To: Wu, Hao A; Leif Lindholm Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org; Kinney, Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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.lindholm@linaro.org] Sent: Monday, March 18, 2019 8:43 PM To: Ming Huang Cc: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star; Dong, Eric; Ni, Ray; dann.frazier@canonical.com; ard.biesheuvel@linaro.org;
Kinney,
Michael D; Gao, Liming; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; huangdaode@hisilicon.com; waip23@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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
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 (&gEfiBlockIoProtocolGuid,
&TempDevicePath, &Handle);
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, &gEfiBlockIoProtocolGuid, (VOID
**) &BlockIo);
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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid,
(VOID
**) &BlockIo);
- 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
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 ming.huang@linaro.org
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, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo);
- 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
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
-
Leif Lindholm -
Ming Huang -
Wu, Hao A