From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::431; helo=mail-pf1-x431.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8765B211DB422 for ; Tue, 19 Mar 2019 05:26:15 -0700 (PDT) Received: by mail-pf1-x431.google.com with SMTP id a3so13559397pff.11 for ; Tue, 19 Mar 2019 05:26:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=QyOInJZeb8T4FCMZY3QDuYdbUlcHPcTUKQ4X05BC/4k=; b=muQqEbRpeY0RpgJs8VOd7rg73yay9O962ZCku4l4OkEReOo4o3kOjI5gxwfsrQiVwO 6mHSIruXWS9PhHZHu7DhRdT1/5UgO++DXAZXElCwSAmtgNELpfMsj4ape0lOD9nTPsgh kOK7JnWO4EkeGHYskf2jdssN4w+IonKK3jIZWgrewekxd7OhBoP/kkfZS8IZl/PxcAAk diAvV69Mb3QI0hkvrKk8awzBYphQ97IP16IzW1oNQxq5YZ5qooAr4MBgHMa0Lzx3H5Ul Hb5YKdWL/5sS3lxsO3GMfjjHVwPhq+EVj9uITMHt2AUw4izppougHrgrz8WdgriUxrlM FtGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=QyOInJZeb8T4FCMZY3QDuYdbUlcHPcTUKQ4X05BC/4k=; b=fsocKW3PZF7ECgfAYRYbsVe+oMsLKhX0hD2GZto1ZO6qY4BnRWU8PLYY1DyIkFHEFs Duv/z+F2BvlbqHnkHvdrxUPbo8r9xtnBPbTPXwbqY3nCumUqmOb1slGNxNpIcLF4ERjh w+ZlLquSFiHGOqI1o3M0E+Ur13Yd51qsLJUC/LMVRLG07Trx5UeEjNFNJZRQggjhaFML xz0DdmpPTgzfe1pVhHxFXwRHdimdyJHzeu8Yeqnq2moaM5pdwmxZY5kmhgYr9XndVHXE m+AxXukvKgpQQacjeVqQO5Axl0kfi/bMUu7YunQl2VojiXYU3t/5SVjK93rRxqO5z7oI yaTw== X-Gm-Message-State: APjAAAVhM7o/R8QxLYvNyHE962Grj0wP+slTjGFbwrDzqeoCphXZutpI qan5Oxzhd3stC/VIs6SLKQG8Mg== X-Google-Smtp-Source: APXvYqz/syMZVPdRp9PceLadgZN5mFEWC5ZLcMKETiyH95IXuUC4l67OkLDXXRN0y+vCQzcKlBE9AA== X-Received: by 2002:a65:5c49:: with SMTP id v9mr1741753pgr.150.1552998375318; Tue, 19 Mar 2019 05:26:15 -0700 (PDT) Received: from [10.95.0.66] ([64.64.108.214]) by smtp.gmail.com with ESMTPSA id u11sm16022543pfh.23.2019.03.19.05.26.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Mar 2019 05:26:14 -0700 (PDT) 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" References: <20190225091052.25461-1-ming.huang@linaro.org> <20190225091052.25461-2-ming.huang@linaro.org> <20190318124232.67pblu6tbdi47g2w@bivouac.eciton.net> <0732ee09-0955-965d-1ff1-167cbc3a1348@linaro.org> From: Ming Huang Message-ID: Date: Tue, 19 Mar 2019 20:26:02 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [MdeModulePkg/Library v1 1/1] MdeModulePkg/UefiBootManangerLib: Fix exception issue X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Mar 2019 12:26:16 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 >>>>> --- >>>>> 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 >>>>>