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::444; helo=mail-pf1-x444.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf1-x444.google.com (mail-pf1-x444.google.com [IPv6:2607:f8b0:4864:20::444]) (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 E70EF210E0FA0 for ; Wed, 8 Aug 2018 02:02:28 -0700 (PDT) Received: by mail-pf1-x444.google.com with SMTP id j26-v6so808276pfi.10 for ; Wed, 08 Aug 2018 02:02:28 -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-transfer-encoding; bh=X/iY6IN6aIyGJHsple4qAmj712BNYsDAJ6cNcLvdKB0=; b=JDwB/+7Wfb74crZKQ+1kUmJPnIyAr2zGZIIDPHmgfOELraFMNddRinolV/xS87lfyl LU6I23ZMa+wxh6CUJTDqI1dDRM5UHulu7GBlxbHJaKMKpD3QgSBkAV8OBTBzqbM7Cs+O Xvwl7/kddxC7gIfZlgw15FgSaYTTLf4KFIfZk= 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-transfer-encoding; bh=X/iY6IN6aIyGJHsple4qAmj712BNYsDAJ6cNcLvdKB0=; b=QwtiL/qct4/8pP+88LmxAulYm5EF+wdSii05jvvKn3nk0jDELULa+c9jHmPZynSf2R 7i6PKv7CxruZDHFDqUhIjraZogH4yywFfNds824gaIcyI6CHjxyxI9N37K/wP+E22136 WYEZtWohQXiY8ZnND0Sw0vEfW2t7dgoBpjUd2tOSytgKh6dDiAYiRa3AVXz88MtAeJuE Fp5fkFstq0R4Qj7oAw9djGcx0hnueipg7VPGxOJJwvU08dPdWur0clV7OfTSZi4QQa47 IKM51fXilPLpTtsH5GFvykYO4FHlWaKuKtRWbQ7W22S9F8M2DXlT066p5hj7bpPo+bd4 /H5g== X-Gm-Message-State: AOUpUlHWGdY6FeXP7Zjpl9SIqcssoc+jV7mN0gRjR1k56MxKJYxcHeNj pI4i8jt0QbQwFqWTC8ksYcWBF+RqF2k= X-Google-Smtp-Source: AA+uWPxQjgEVDK4CXmE/lL5uqnjX3JeysusgYlK5fCkoARAq7BFwPTPld1KPD7J6ucn3JF0/zcTLyQ== X-Received: by 2002:a63:f18:: with SMTP id e24-v6mr1731679pgl.320.1533718948573; Wed, 08 Aug 2018 02:02:28 -0700 (PDT) Received: from [10.199.0.182] ([64.64.108.224]) by smtp.gmail.com with ESMTPSA id x4-v6sm4505812pfm.119.2018.08.08.02.02.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 08 Aug 2018 02:02:27 -0700 (PDT) To: Leif Lindholm Cc: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, graeme.gregory@linaro.org, ard.biesheuvel@linaro.org, guoheyi@huawei.com, wanghuiqiang@huawei.com, huangming23@huawei.com, zhangjinsong2@huawei.com, huangdaode@hisilicon.com, john.garry@huawei.com, xinliang.liu@linaro.org, Heyi Guo References: <20180724070922.63362-1-ming.huang@linaro.org> <20180724070922.63362-8-ming.huang@linaro.org> <20180802173647.paepialtuopbe6y5@bivouac.eciton.net> From: Ming Message-ID: <22f0c26b-0666-82b5-2b1c-f71af63526a4@linaro.org> Date: Wed, 8 Aug 2018 17:02:15 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180802173647.paepialtuopbe6y5@bivouac.eciton.net> Subject: Re: [PATCH edk2-platforms v1 07/38] Silicon/Hisilicon/D06: Wait for all disk ready X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Aug 2018 09:02:29 -0000 Content-Type: text/plain; charset=gbk Content-Transfer-Encoding: 8bit ÔÚ 8/3/2018 1:36 AM, Leif Lindholm дµÀ: > On Tue, Jul 24, 2018 at 03:08:51PM +0800, Ming Huang wrote: >> This patch is relative to D06 SasDxe driver. The SasDxe set a >> variable to notice this libray. Here Wait for all disk ready >> for 30S at most. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ming Huang >> Signed-off-by: Heyi Guo >> --- >> Silicon/Hisilicon/HisiPkg.dec | 1 + >> Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c | 43 ++++++++++++++++++++ >> Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 + >> 3 files changed, 46 insertions(+) >> >> diff --git a/Silicon/Hisilicon/HisiPkg.dec b/Silicon/Hisilicon/HisiPkg.dec >> index 35bea970ec..b56a6a6af7 100644 >> --- a/Silicon/Hisilicon/HisiPkg.dec >> +++ b/Silicon/Hisilicon/HisiPkg.dec >> @@ -45,6 +45,7 @@ >> >> gHisiEfiMemoryMapGuid = {0xf8870015, 0x6994, 0x4b98, {0x95, 0xa2, 0xbd, 0x56, 0xda, 0x91, 0xc0, 0x7f}} >> gVersionInfoHobGuid = {0xe13a14c, 0x859c, 0x4f22, {0x82, 0xbd, 0x18, 0xe, 0xe1, 0x42, 0x12, 0xbf}} >> + gHisiOemVariableGuid = {0xac62b9a5, 0x9939, 0x41d3, {0xff, 0x5c, 0xc5, 0x80, 0x32, 0x7d, 0x9b, 0x29}} >> gOemBootVariableGuid = {0xb7784577, 0x5aaf, 0x4557, {0xa1, 0x99, 0xd4, 0xa4, 0x2f, 0x45, 0x06, 0xf8}} > > What is the difference between gHisiOemVariableGuid and gOemBootVariableGuid? > Just a guid using to a variable. Do you suggest to use the old one? >> gEfiHisiSocControllerGuid = {0xee369cc3, 0xa743, 0x5382, {0x75, 0x64, 0x53, 0xe4, 0x31, 0x19, 0x38, 0x35}} >> >> diff --git a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c >> index 7dd5ba615c..f7536bfea3 100644 >> --- a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c >> +++ b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c >> @@ -20,6 +20,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -554,6 +555,47 @@ PlatformBootManagerBeforeConsole ( >> PlatformRegisterOptionsAndKeys (); >> } >> >> +STATIC >> +VOID >> +WaitForDiskReady ( >> + ) >> +{ >> + EFI_STATUS Status; >> + UINT32 Index; >> + UINTN DataSize; >> + UINT32 DiskInfo; >> + UINT8 IsFinished; >> + >> + Status = EFI_NOT_FOUND; >> + DataSize = sizeof (UINT32); >> + // Wait for 30 seconds at most. >> + for (Index=0; Index<30; Index++) { > > Spaces around '=' and '<'. > >> + Status = gRT->GetVariable ( >> + L"SASDiskInfo", >> + &gHisiOemVariableGuid, >> + NULL, >> + &DataSize, >> + &DiskInfo >> + ); > > Wait... > Are we synchronizing against the storage controller driver using an > environment variable and looping over it for 30 seconds? > D06: For using hard disk backboard, some disk need 15 seconds to ready. Actually, wait less 15 seconds here(minus the time from end of SAS driver to here). For using expander, wait less 6 seconds here(minus the time from end of SAS driver to here). D03/D05: no waiting here. Maybe I should change 30 to 15, it will never loop over for 30 seconds. > That can't go in. > Please look into implementing an event in the SAS driver which you can > wait for here. > I don't really understand what differece between implementing with variable and implementing with event. >> + if (EFI_ERROR(Status)) { >> + DEBUG ((DEBUG_ERROR, "Get DiskInfo:%r\n", Status)); >> + break; >> + } >> + >> + IsFinished = (UINT8)(DiskInfo >> 24); >> + if (IsFinished) { >> + break; >> + } >> + DEBUG ((DEBUG_ERROR, "%a", Index == 0 ? "Wait for disk." : ".")); >> + MicroSecondDelay(1000*1000); // 1S > > Spaces around '*'. > The code already says to sleep a million microseconds, comment superfluous. > >> + } >> + >> + if (!EFI_ERROR(Status)) { >> + DEBUG ((DEBUG_ERROR, "DiskInfo:%x\n", DiskInfo)); >> + EfiBootManagerConnectAll (); > > Why not call WaitForDiskReady() before EfiBootManagerConnectAll () in > PlatformBootManagerAfterConsole ()? > The Sas controller of D06 is a pci device, SAS driver (Start functio) run after pci enumeration. WaitForDiskReady should be called after SAS driver and before creating boot options. Ming > / > Leif > >> + } >> +} >> + >> /** >> Do the platform specific action after the console is ready >> Possible things that can be done in PlatformBootManagerAfterConsole: >> @@ -583,6 +625,7 @@ PlatformBootManagerAfterConsole ( >> // Connect the rest of the devices. >> // >> EfiBootManagerConnectAll (); >> + WaitForDiskReady (); >> >> // >> // Enumerate all possible boot options. >> diff --git a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> index 7a53befc44..a093f13fb0 100644 >> --- a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> +++ b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> @@ -49,6 +49,7 @@ >> MemoryAllocationLib >> PcdLib >> PrintLib >> + TimerLib >> UefiBootManagerLib >> UefiBootServicesTableLib >> UefiLib >> @@ -67,6 +68,7 @@ >> [Guids] >> gEfiEndOfDxeEventGroupGuid >> gEfiTtyTermGuid >> + gHisiOemVariableGuid >> >> [Protocols] >> gEfiGenericMemTestProtocolGuid >> -- >> 2.17.0 >>