From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web12.332.1591636226040904161
 for <devel@edk2.groups.io>;
 Mon, 08 Jun 2020 10:10:26 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 99B171FB;
	Mon,  8 Jun 2020 10:10:23 -0700 (PDT)
Received: from [192.168.1.69] (unknown [10.37.8.184])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A39ED3F73D;
	Mon,  8 Jun 2020 10:10:22 -0700 (PDT)
Subject: Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Revert don't connect all devices on an ordinary boot
To: Pete Batard <pete@akeo.ie>, devel@edk2.groups.io
Cc: leif@nuviainc.com
References: <20200608155159.8748-1-pete@akeo.ie>
 <5009d194-b701-b602-ba3a-dd32891173bd@arm.com>
 <246570db-3b98-a893-fec0-85d6b13d7641@akeo.ie>
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
Message-ID: <0c1125b9-146a-2b1c-e8b2-bade49ff7620@arm.com>
Date: Mon, 8 Jun 2020 19:10:20 +0200
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <246570db-3b98-a893-fec0-85d6b13d7641@akeo.ie>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

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 accou=
nt
>>> 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 devic=
e
>>> 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?
>=20
> Moving EfiEventGroupSignal(&gEfiEndOfDxeEventGroupGuid) early into=20
> PlatformBootManagerBeforeConsole() doesn't seem to have any effect, as=20
> far as I could test.
>=20
> We can however move the EfiBootManagerConnectAll() there if that's what=
=20
> you are asking.
>=20

No, the whole idea was to get rid of EfiBootManagerConnectAll().

You are saying the PCI bus scanning starts after the BDS wait period,=20
right? This is strange, I wouldn't expect that. That is why I suggested=20
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.


>=20
>>
>>
>>
>>
>>> ---
>>> =C2=A0 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm=
.c |=20
>>> 5 +++++
>>> =C2=A0 1 file changed, 5 insertions(+)
>>>
>>> diff --git=20
>>> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c=20
>>> 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 (
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Print (BOOT_PROMPT);
>>> =C2=A0=C2=A0=C2=A0 }
>>> +=C2=A0 //
>>> +=C2=A0 // Connect the rest of the devices.
>>> +=C2=A0 //
>>> +=C2=A0 EfiBootManagerConnectAll ();
>>> +
>>> =C2=A0=C2=A0=C2=A0 Status =3D gBS->LocateProtocol (&gEsrtManagementPr=
otocolGuid, NULL,=20
>>> (VOID**)&EsrtManagement);
>>> =C2=A0=C2=A0=C2=A0 if (!EFI_ERROR (Status)) {
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EsrtManagement->SyncEsrtFmp ();
>>>
>>
>=20