public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: Ming Huang <ming.huang@linaro.org>,
	Leif Lindholm <leif.lindholm@linaro.org>
Cc: "linaro-uefi@lists.linaro.org" <linaro-uefi@lists.linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Zeng, Star" <star.zeng@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
	"dann.frazier@canonical.com" <dann.frazier@canonical.com>,
	"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"wanghuiqiang@huawei.com" <wanghuiqiang@huawei.com>,
	"huangming23@huawei.com" <huangming23@huawei.com>,
	"zhangjinsong2@huawei.com" <zhangjinsong2@huawei.com>,
	"huangdaode@hisilicon.com" <huangdaode@hisilicon.com>,
	"waip23@126.com" <waip23@126.com>,
	"Wang, Jian J" <jian.j.wang@intel.com>
Subject: Re: [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue
Date: Tue, 19 Mar 2019 05:56:16 +0000	[thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C8AC058@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <0732ee09-0955-965d-1ff1-167cbc3a1348@linaro.org>

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

  reply	other threads:[~2019-03-19  5:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-25  9:10 [MdeModulePkg/Library v1 0/1] Fix exception issue while UsbMass block io uninstalled Ming Huang
2019-02-25  9:10 ` [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue Ming Huang
2019-03-18 12:42   ` Leif Lindholm
2019-03-19  2:25     ` Wu, Hao A
2019-03-19  4:13       ` Ming Huang
2019-03-19  5:56         ` Wu, Hao A [this message]
2019-03-19 12:26           ` Ming Huang
2019-03-19  4:01     ` Ming Huang
2019-03-19 10:00       ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C8AC058@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox