From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::22e; helo=mail-wm0-x22e.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) (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 CEF19210D515D for ; Thu, 2 Aug 2018 10:36:52 -0700 (PDT) Received: by mail-wm0-x22e.google.com with SMTP id c14-v6so3439829wmb.4 for ; Thu, 02 Aug 2018 10:36:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=5lYtI3Z2DYROFsDH8XpZ3XfA0zb4dWkNfKeLC3jh0hU=; b=Qfup/DNC0scxvoLSzVFuzhf3hZHz5ITKppOxsQh7/dHmaslE2/Fv/ghdGWTVQZqjd4 lh991+NebKFFCHqthOgWpz3BP2jRi87Lksg38FbSBALdwwNlXuqh8/JyDa/6kda6silZ Du7VVJlX733ExzvO31dSgX9G3iXeWZJLR5UOQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=5lYtI3Z2DYROFsDH8XpZ3XfA0zb4dWkNfKeLC3jh0hU=; b=cNQ3MH13WB6DDmF3CnTTVtMBshaI31ZP70+yGzOtRzV6b5EExKJLqO3Nfn/DsnGEyN lG9WHOdZYhIbnQ1nWsaYAtdVScHbbslnt5N08v4ugDOG0I3lfy/EoqHC/rU/uOBcLh2S jWhQ7+Afn6c9NvrfkljIaRnPuuv8p+JjlrUh/OsPj3Fv3DKCq5XG0oGY103Bo9syrFyP wKMbGUSEJQ5Q0bcNAd+0lAPcDAWI07+LDNxjiYJi2LeA+LuA/fk8TIjQlZkgq2h2lFc9 8soCJLBDV4T9TC5RHyhBgTrzNiXIG31C7BEwtXjojOmqhiDOq5Kbf8ccwcUZdgZTzFaZ rKgg== X-Gm-Message-State: AOUpUlFzCBfNn9km4wMKIS1t0J+MuiHM9Uxz4DtyBJ7c/P/exh+a3RsY qf/e7l5QgPaFVvN/CgmFnJD+IjU87rA= X-Google-Smtp-Source: AAOMgpez2aflB8qjBtO4AldPandTKOiZUhay5CZvhmJVI6fDUkWRY5yqVk9Cr5Q87EJX03kwd6/5xA== X-Received: by 2002:a1c:d750:: with SMTP id o77-v6mr2612597wmg.67.1533231409931; Thu, 02 Aug 2018 10:36:49 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id n14-v6sm2410007wmc.14.2018.08.02.10.36.48 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 02 Aug 2018 10:36:48 -0700 (PDT) Date: Thu, 2 Aug 2018 18:36:47 +0100 From: Leif Lindholm To: Ming Huang 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 Message-ID: <20180802173647.paepialtuopbe6y5@bivouac.eciton.net> References: <20180724070922.63362-1-ming.huang@linaro.org> <20180724070922.63362-8-ming.huang@linaro.org> MIME-Version: 1.0 In-Reply-To: <20180724070922.63362-8-ming.huang@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) 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: Thu, 02 Aug 2018 17:36:53 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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? > 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? That can't go in. Please look into implementing an event in the SAS driver which you can wait for here. > + 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 ()? / 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 >