Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Revert don't connect all devices on an ordinary boot

["Pete Batard" <pete@akeo.ie>]

On 2020.06.08 18:10, Ard Biesheuvel wrote:
> On 6/8/20 7:01 PM, Pete Batard wrote:
>> On 2020.06.08 17:43, Ard Biesheuvel wrote:
>>> On 6/8/20 5:51 PM, Pete Batard wrote:
>>>> This reverts commit c8000ecccc83b728baf04ced2fedb870bc3bc1b3 on account
>>>> of USB keyboard devices not being detected until after the timeout to
>>>> enter the Setup menu or run the UEFI Shell has expired.
>>>>
>>>> The core of the issue is that, when EfiBootManagerConnectAll() is not
>>>> invoked, PCI bus scanning only starts after the BDS wait period has
>>>> elapsed, which effectively takes away the ability to use an USB device
>>>> to enact Boot Device Selection or interact with the Setup menu.
>>>>
>>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>>
>>> Could we move the call to
>>>
>>> EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
>>>
>>> to the start of PlatformBootManagerBeforeConsole() instead?
>>
>> Moving EfiEventGroupSignal(&gEfiEndOfDxeEventGroupGuid) early into 
>> PlatformBootManagerBeforeConsole() doesn't seem to have any effect, as 
>> far as I could test.
>>
>> We can however move the EfiBootManagerConnectAll() there if that's 
>> what you are asking.
>>
> 
> No, the whole idea was to get rid of EfiBootManagerConnectAll().
> 
> You are saying the PCI bus scanning starts after the BDS wait period, 
> right? This is strange, I wouldn't expect that. That is why I suggested 
> moving the EfiEventGroupSignal() call earlier.
> 
> Looking at the ArmPkg version, it appears we need a sequence like
> 
> EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
> 
> EfiBootManagerDispatchDeferredImages ();
> 
> FilterAndProcess (&gEfiPciRootBridgeIoProtocolGuid, NULL, Connect);
> 
> and that last call is missing from the RPi4 implementation.
> 
> After those calls, all the PCI I/O handles should be enumerated.

Yup, that seems to work.

I need to test a bit more and clean up, then I'll send a patch to 
supersede this one.

Regards,

/Pete

> 
> 
>>
>>>
>>>
>>>
>>>
>>>> ---
>>>>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 
>>>> 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git 
>>>> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c 
>>>> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>>>> index 253614a646c1..2bd625ad7e7c 100644
>>>> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>>>> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>>>> @@ -655,6 +655,11 @@ PlatformBootManagerAfterConsole (
>>>>       Print (BOOT_PROMPT);
>>>>     }
>>>> +  //
>>>> +  // Connect the rest of the devices.
>>>> +  //
>>>> +  EfiBootManagerConnectAll ();
>>>> +
>>>>     Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, 
>>>> NULL, (VOID**)&EsrtManagement);
>>>>     if (!EFI_ERROR (Status)) {
>>>>       EsrtManagement->SyncEsrtFmp ();
>>>>
>>>
>>
>