在 8/8/2018 8:53 PM, Leif Lindholm 写道:
> On Wed, Aug 08, 2018 at 07:44:36PM +0800, Ming wrote:
>>
>>
>> 在 8/8/2018 5:59 PM, Leif Lindholm 写道:
>>> On Wed, Aug 08, 2018 at 05:02:15PM +0800, Ming wrote:
>>>> 在 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 <[email protected]>
>>>>>> Signed-off-by: Heyi Guo <[email protected]>
>>>>>> ---
>>>>>> 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?
>>>
>>> Well, I am unsure about the intended scope for either. So I don't
>>> know. How is HisiOem different from Oem? Can you explain the structure?
>>
>> HisiOem and Oem are the same scope, just need a guid for SetVariable.
>
> The only purpose for the GUID is so that you can have variables of the
> same name in different modules without risking clashes. Sharing the
> same variable GUID across multiple modules is perfectly fine (and
> often preferable) when those modules are closely related.
>
> But as I think this was only used for the communication with the SAS
> controller driver, I think maybe this one can just go.
OK.
>
>>>>>> 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 <Library/BmcConfigBootLib.h>
>>>>>> #include <Library/DevicePathLib.h>
>>>>>> #include <Library/PcdLib.h>
>>>>>> +#include <Library/TimerLib.h>
>>>>>> #include <Library/UefiBootManagerLib.h>
>>>>>> #include <Library/UefiLib.h>
>>>>>> #include <Protocol/DevicePath.h>
>>>>>> @@ -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.
>>>
>>> One is using the mechanism explicitly designed for this sort of
>>> thing. (And hence making the code a lot more clear and avoiding
>>> unintended side effects.)
>>> It also mean you would wait exactly as long as you needed, and not
>>> need to worry about how long.
>>
>> In this case, it don't want to wait exactly time, the time depand on
>> SAS driver and which backboard is used. After optimizing SAS driver,
>> wait time is reduced. Here,30 just for avoiding dead loop.
>
> Which is exactly what an event is for.
> At the point where the SAS controller driver is currently updating the
> variable, it can instead call SignalEvent ().
>
> The WaitForDiskReady () function can be changed to wait/check for that event.
>
As current solution(using variables) have run stable, I prefer don't change
SAS driver and this wait function. Is it OK?
>>>>> 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.
>>>
>>> OK, but EfiBootManagerConnectAll () is a bit of a sledge hammer, and
>>> will affect boot times.
>>>
>>> The SAS controller is onboard (so always present), right?
>>> If so, you could connect it manually.
>>
>> Yes, it's always present.
>> I intend to optimize SAS driver in future. It's not enouth time to do
>> this before 18.08.
>> Thanks.
>
> OK.
>
> /
> Leif
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
