> On May 25, 2016, at 3:03 PM, Jordan Justen <[email protected]> wrote: > > On 2016-05-25 14:13:03, Laszlo Ersek wrote: >> On 05/25/16 21:53, Jordan Justen wrote: >>> On 2016-05-25 10:23:56, Laszlo Ersek wrote: >>>> On 05/25/16 19:00, Jordan Justen wrote: >>>>> On 2016-05-25 05:36:43, Laszlo Ersek wrote: >>>>>> OVMF's Platform BDS used to have a nice progress bar (with >>>>>> IntelFrameworkModulePkg BDS). We can restore it by copying the >>>>>> PlatformBootManagerWaitCallback() function verbatim from >>>>>> >>>>>> Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c >>>>>> >>>>>> It can be tested by passing the following option to QEMU (5 seconds): >>>>>> >>>>>> -boot menu=on,splash-time=5000 >>>>>> >>>>>> Cc: Jordan Justen <[email protected]> >>>>>> Cc: Ruiyu Ni <[email protected]> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>> Signed-off-by: Laszlo Ersek <[email protected]> >>>>>> --- >>>>>> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 17 >>>>>> +++++++++++++++++ >>>>>> 1 file changed, 17 insertions(+) >>>>>> >>>>>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c >>>>>> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c >>>>>> index 9eb9e390373d..dd8757f58ec3 100644 >>>>>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c >>>>>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c >>>>>> @@ -1425,5 +1425,22 @@ PlatformBootManagerWaitCallback ( >>>>>> UINT16 TimeoutRemain >>>>>> ) >>>>>> { >>>>>> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black; >>>>>> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL White; >>>>>> + UINT16 Timeout; >>>>>> + >>>>>> + Timeout = PcdGet16 (PcdPlatformBootTimeOut); >>>>>> + >>>>>> + Black.Blue = Black.Green = Black.Red = Black.Reserved = 0; >>>>>> + White.Blue = White.Green = White.Red = White.Reserved = 0xFF; >>>>> >>>>> I know this came from Nt32, but how about making these global vars? >>>>> >>>>> static EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black = { 0, 0, 0, 0 }; >>>>> static EFI_GRAPHICS_OUTPUT_BLT_PIXEL White = { 0xff, 0xff, 0xff, 0 }; >>>> >>>> I'm okay with giving them static storage duration. >>>> >>>> With that changed, >>>> - do you want me to keep them in function scope (with the same names), >>>> - or should I move them to file scope (and then, as Andrew suggests, >>>> rename them to, say, mPixelBlack and mPixelWhite)? >>>> >>> >>> The part that bugged me was setting .Reserved to 0xff. >>> >>> How about just making them initialized local variables? >>> >>> EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black = { 0, 0, 0, 0 }; >>> EFI_GRAPHICS_OUTPUT_BLT_PIXEL White = { 0xff, 0xff, 0xff, 0 }; >> >> That would conflict with the edk2 coding style (6.8 C Function Layout, >> "Initializing a variable as part of its declaration is illegal" -- >> although it clearly doesn't cover variables with static storage duration.) >> > > I do see that in the older style guide, but not the newer ones (2.0+). > >> I even suspect that the first initialization could cause some compilers >> to emit memset() intrinsics. >> > > Hmm, yeah that could be a problem. >
I've never seen the compiler emit a memcpy/memset for something < 64-bits (8 bytes). There is an overhead for making the call to memcpy/memset and you have to pass a pointer so even with optimizations disabled I think the compilers have a minimum threshold for the call. From what I've seen clang NOOPT is the best way to expose these kinds of issues. Most optimizers seem to decide to generate code vs. calling memcpy for a lot of smaller cases, like a GUID but clang at -O0 seems to default to memcpy/memset very quickly and defaults to the optimizer to remove it. Thanks, Andrew Fish >> How about: >> >> Black.Blue = Black.Green = Black.Red = 0; >> White.Blue = White.Green = White.Red = 0xFF; >> Black.Reserved = White.Reserved = 0; >> > > Sure, or a static local var. > > I hope that in the case of the above code that most compilers would > see that we are really just dealing with two uint32_t values. > > I guess there is the union wrapper of the struct, and combined with > the UEFI assumption of little-endian, you could force it. > > -Jordan > >> (I could submit a patch for Nt32Pkg too that fixed White.Reserved.) >> >> Thanks >> Laszlo > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
