| Message ID | 20180724070922.63362-8-ming.huang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Upload for D06 platform | expand |
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 <ming.huang@linaro.org> > Signed-off-by: Heyi Guo <heyi.guo@linaro.org> > --- > 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 <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? 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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
在 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 <ming.huang@linaro.org> >> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >> --- >> 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 <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. >> + 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 >>
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 <ming.huang@linaro.org> > >> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> > >> --- > >> 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? > >> 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. The other is using variables. > >> + 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. 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. / Leif
在 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 <ming.huang@linaro.org> >>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>> --- >>>> 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. >>>> 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. > The other is using variables. > >>>> + 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. > > 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. Ming > / > Leif >
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 <ming.huang@linaro.org> > >>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> > >>>> --- > >>>> 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. > >>>> 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. > >>> 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
在 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 <ming.huang@linaro.org> >>>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>>> --- >>>>>> 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 >
On Fri, Aug 10, 2018 at 09:44:29AM +0800, Ming wrote: > > 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? You can leave the driver out if you wish, but it cannot go into the 18.08 release with this design. This is why it is important to publish the code as soon as possible - especially for new platform ports. And to publish bits that are possible earlier, rather than wait and do everything in one go once the whole port is done. Best Regards, Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
在 8/14/2018 11:26 PM, Leif Lindholm 写道: > On Fri, Aug 10, 2018 at 09:44:29AM +0800, Ming wrote: >>> 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? > > You can leave the driver out if you wish, but it cannot go into the > 18.08 release with this design. With event, two problem should be solved: 1 How to share the event between SAS driver and PlatformBootManagerLib; 2 How to avoid waiting for D03/D05; For the two problems I plan to install a protocal in D06 SAS driver and locate the protocal in PlatformBootManagerLib. if fail to locate (D03/D05), it will not wait. struct _HISI_SAS_NOTIFY_PROTOCAL { EFI_EVENT WaitDiskEvent; }; How about this? > > This is why it is important to publish the code as soon as possible - > especially for new platform ports. And to publish bits that are > possible earlier, rather than wait and do everything in one go once > the whole port is done. > > Best Regards, > > Leif >
On Wed, Aug 15, 2018 at 12:01:25PM +0800, Ming wrote: > > > 在 8/14/2018 11:26 PM, Leif Lindholm 写道: > > On Fri, Aug 10, 2018 at 09:44:29AM +0800, Ming wrote: > >>> 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? > > > > You can leave the driver out if you wish, but it cannot go into the > > 18.08 release with this design. > > With event, two problem should be solved: > 1 How to share the event between SAS driver and PlatformBootManagerLib; > 2 How to avoid waiting for D03/D05; > > For the two problems I plan to install a protocal in D06 SAS driver and > locate the protocal in PlatformBootManagerLib. if fail to locate (D03/D05), > it will not wait. > struct _HISI_SAS_NOTIFY_PROTOCAL { > EFI_EVENT WaitDiskEvent; > }; > How about this? That works for me (If you rename it PROTOCAL-> PROTOCOL, and give it a typedef without the _). / Leif
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}} 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++) { + Status = gRT->GetVariable ( + L"SASDiskInfo", + &gHisiOemVariableGuid, + NULL, + &DataSize, + &DiskInfo + ); + 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 + } + + if (!EFI_ERROR(Status)) { + DEBUG ((DEBUG_ERROR, "DiskInfo:%x\n", DiskInfo)); + EfiBootManagerConnectAll (); + } +} + /** 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