Re: [edk2-devel] [PATCH v1 1/1] ArmPkg: Dispatch deferred images after EndOfDxe

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Fri, 6 Dec 2019 at 10:33, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 12/06/19 11:02, Ard Biesheuvel wrote:
> > On Fri, 6 Dec 2019 at 00:05, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 12/06/19 00:54, Laszlo Ersek wrote:
> >>> On 12/05/19 21:25, Ard Biesheuvel wrote:
> >>>> On Wed, 1 May 2019 at 15:02, Sami Mujawar <sami.mujawar@arm.com>
> >>>> wrote:
> >>
> >>>> I'm still curious why this difference exists,
> >>>
> >>> What difference do you mean?
> >>>
> >>> (I can't see the original patch posting in my list folder, so I could
> >>> be missing parts of the discussion thus far.)
> >>
> >> Haha, I missed that Sami's email, which you just replied to, is dated "1
> >> May 2019" -- that's the reason I couldn't find the original posting in
> >> my edk2-devel folder. That message has already been moved into one of my
> >> archive folders :)
> >>
> >> But, now I do see it, and I also see that your first question in
> >> response was spot-on:
> >>
> >>   https://edk2.groups.io/g/devel/message/39901
> >>
> >> (alternative link:
> >>
> >>   http://mid.mail-archive.com/CAKv+Gu92gPzGvGZ3M9B52q1TOAvnBjgxpvykbAZPevMULkcF6w@mail.gmail.com
> >> )
> >>
> >> Your question there had a small typo -- I think you meant, "It might be
> >> that the PCI hierarchy is enumerated *after* EndOfDxe on Juno, and
> >> *before* on the other platforms."
> >>
> >> And yes, that would explain the difference between Juno and {SynQuacer,
> >> Overdrive} very well -- i.e. why Juno was broken and the other platforms
> >> were OK. (Assuming in all cases, the 3rd party dispatch occurred after
> >> EndOfDxe, where it is supposed to.)
> >>
> >
> > Hi Laszlo,
> >
> > Thanks for chiming in.
> >
> > My question did not have a typo: if Juno enumerates PCI before
> > EndOfDxe, any option ROMs it encounters will not be dispatched and put
> > on the deferred list, which never gets processed. If it enumerates
> > after EndOfDxe, they just get dispatched immediately.
>
> Ugh, indeed!
>
> > That would
> > explain this behavior, *except* for the fact that these platforms use
> > the same PCI host bridge and bus drivers, the only difference is the
> > PciHostBridgeLib used.
> >
> > The difference is probably explained by the explicit
> > gBS->ConnectController () connecting the PCI root bridge that takes
> > place in ArmJunoDxe in an EndOfDxe handler, which seems to be there so
> > we can set the MAC address on the Marvell Yukon.
>
> Aha! That does seem to explain why this patch makes a difference.
> Because, signaling EndOfDxe leads *internally* to the production of some
> PciIo instances, and the deferral of the oprom images found on those. So
> there *are* some deferred images to dispatch, in platform BDS, right
> after signaling EndOfDxe (even before blanket-enumerating all (other)
> root bridges).
>
> So commit 0f9395d7c5cc looks justified, only its message may not tell
> the whole story. (I didn't realize oproms could be found and deferred
> *inside* signaling EndofDxe, and was looking at the blanket-enumeration
> that *followed* the proposed location of
> EfiBootManagerDispatchDeferredImages().)
>
> > We should probably
> > move that to a protocol registration notification callback on the
> > PciIo protocol, and remove the ConnectController altogether.
>
> I think this makes sense. It sounds like a platform tweak, so I agree
> that a PciIo notification callback is acceptable (a UEFI driver
> following the UEFI driver model doesn't seem to make any sense just for
> this one-shot platform tweak).
>
> > Concerning your analysis regarding the order of connecting the PCI
> > root bridge and signalling EndOfDxe: I agree the OvmfPkg order is
> > better, since it permits builtin drivers (which are 'trusted') to
> > attach to devices in the PCIe hierarchy before EndOfDxe is signalled,
> > which may be useful.
>
> Thanks -- but, ultimately, I do think my analysis, in the form I meant
> it, was flawed. I missed that, if the enumeration occurs after EndOfDxe,
> then the PCI oprom images will simply be dispatched at once!
>
> So, I have to change my mind now: I realize & accept that there is no
> immediate need to update the order of operations around signaling
> EndOfDxe, in the ArmVirtPkg and ArmPks PlatformBootManagerLib instances.
>

Agreed. UEFI drivers shouldn't generalled depend on being able to
connect to the devices before EndOfDxe, and if there is ever a valid
reason for doing so, we can revisit this.

> In OvmfPkg, the order is stricter / more intricate , due to a chain of
> dependencies around S3 (FACS, boot script opcodes, SMRAM lockbox,
> locking down SMRAM etc).
>
> Thank you for the enlightenment! :)

Likewise :-)