From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web09.5455.1575647408562418880 for ; Fri, 06 Dec 2019 07:50:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=He1B80P3; spf=pass (domain: linaro.org, ip: 209.85.221.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f68.google.com with SMTP id a15so8251787wrf.9 for ; Fri, 06 Dec 2019 07:50:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Xrv0+go4/tJuYSUHvel1CKh285zaQwl2K3E/RFSRBe4=; b=He1B80P3S0B+oOwXscZqLNd9i1Ys1EK42ZPjpf5P11AHfeftWugXr8Ijlh3XYbe9hW AqA70nhGpIDvXHfnSRWAkzA66MBRG7oxgdt3qbO0tA6bw72amhSsCIzPtoAAO719ihWj wJlTWRuPdHF4njko3uoG5QVk8eNN+2H38SSoAU+7C/Bjl8oYLux2YFyLR11SRyKtEaIE wybmTx64sXKw/o1ZSpUjIC47MOfKEGLOOwTVfSxp2pqofqnbc5LIc800x+blqCnsMgcc Cf8Pm+ilKlabIcm4R0LNybCFXmuE0tWipnYzNIDpiuXB3h90emdbGLZmcLLqRhmOgd89 Gg8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Xrv0+go4/tJuYSUHvel1CKh285zaQwl2K3E/RFSRBe4=; b=Rley4EzkY5nCsI6545rGr6Ul7aBerHqqjGjQ4yCfB2OYbE0bO+ilcK2Qo2a+OWRtVt q31aqlx3GsNjhB7TKGxQ3H8mdVdmK4/ytAo+Sg1xF2K5l7xlyHROoeCa3lAfFftT6oUn W00mTV4j90ecwEn2CSfrmuH3tapIBSJXgyv7K5gYimKDwJCjwL3Y36MqJU4DcpavIFNW ZCagZIWtoXjG04ys8rFeI/VFDUo3gZ4TjOXH82V4hKCTqq9NJ6MUQm2rmm+ypGoQcLrH 6lkRchJU4xPy6ckrtalPVwDDq0D7SBilxuZuAnQpJLcitsqET+eh49IzQqLIhv4YjHGq 06+Q== X-Gm-Message-State: APjAAAUFc/+Ie2rpz+63sEIaFs3YD0ElnLFLdyoHMRhuqXYNUYo9lBEc CyubYUQ24R4kVwJdAdGMnkIVELEQgJxTJyP4Y69URw== X-Google-Smtp-Source: APXvYqw9Tx8Nmmzti7Pqfo1uhm3GUDoTacbuz/xLUIwDVgHC8QKo4/OxXFESxUH8a3MISOUGkHfh7ttP4ix90AHZP3Y= X-Received: by 2002:adf:cf0a:: with SMTP id o10mr15694793wrj.325.1575647406998; Fri, 06 Dec 2019 07:50:06 -0800 (PST) MIME-Version: 1.0 References: <20191206143128.19371-1-ard.biesheuvel@linaro.org> <20191206143128.19371-3-ard.biesheuvel@linaro.org> <8d49cca6-b1b9-cddc-0590-4e4a84643e4f@redhat.com> In-Reply-To: <8d49cca6-b1b9-cddc-0590-4e4a84643e4f@redhat.com> From: "Ard Biesheuvel" Date: Fri, 6 Dec 2019 15:50:03 +0000 Message-ID: Subject: Re: [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver#### To: Laszlo Ersek Cc: edk2-devel-groups-io , Leif Lindholm , Zhichao Gao , Ray Ni , Maurice Ma , Guo Dong , Benjamin You Content-Type: text/plain; charset="UTF-8" On Fri, 6 Dec 2019 at 15:44, Laszlo Ersek wrote: > > On 12/06/19 15:31, Ard Biesheuvel wrote: > > Currently, we dispatch drivers specified via Driver#### entries after > > calling PlatformBootManagerBeforeConsole(), which prevents us from > > loading drivers that provide consoles via Driver#### entries. > > I'd put this as: > > ... which prevents PlatformBootManagerBeforeConsole() from adding such > consoles to ConIn, ConOut and StdErr that are produced by drivers > loaded via Driver#### options. > > > This is particularly annoying on AArch64 systems, given that it prevents > > us from loading AArch64 drivers from the ESP for PCIe graphics cards > > that shipped with x86 drivers in the option ROM, which is the common > > case today. > > > > So let's reorder the handling of the Driver#### entries with the > > invocation of PlatformBootManagerBeforeConsole(), which results in > > Driver#### entries being dispatched in the same way as option ROM > > images. > > Ah, so you are saying the drivers will be added unconditionally to the > deferred list, with this patch (because, at the new ProcessLoadOptions() > call site, the platform cannot have signaled EndOfDxe yet). > > Then in PlatformBootManagerBeforeConsole(), the platform can signal > EndOfDxe, call EfiBootManagerDispatchDeferredImages(), connect GOP > drivers, see what new handles / device paths are created, and *then* > update ConIn / ConOut / ConErr. > > Can you point out where the deferral happens, inside > ProcessLoadOptions()? > > (If there is no deferral, then we can't / shouldn't do this, as drivers > from the ESP should not be loaded before EndOfDxe, I think.) > > Hmmm... it looks like: > > BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] > ProcessLoadOptions() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] > EfiBootManagerProcessLoadOption() [MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c] > CoreLoadImage() [MdeModulePkg/Core/Dxe/Image/Image.c] -- via gBS->LoadImage() > CoreLoadImageCommon() [MdeModulePkg/Core/Dxe/Image/Image.c] > Security2StubAuthenticate() [MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c] -- via gSecurity2->FileAuthentication() > Defer3rdPartyImageLoad() [MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c] > // checks "mEndOfDxe" > QueueImage() [MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c] > > OK. > Exactly. This flow is identical to how option ROMs are processed if they are discovered before EndOfDxe signalling completes (which is why the Juno platform was broken without the call to EfiBootManagerDispatchDeferredImages() in PlatformBootManagerBeforeConsole()) > I think this series makes sense. > > I defer to Ray, but from my POV, it should be OK. > > With the patch#2 commit message updated: > > series > Reviewed-by: Laszlo Ersek > > Thanks > Laszlo > > > > > Signed-off-by: Ard Biesheuvel > > --- > > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 15 ++++++++------- > > 1 file changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > index 1fb04dcbbcda..ad4c4c0406f6 100644 > > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > @@ -850,6 +850,14 @@ BdsEntry ( > > } > > } > > > > + // > > + // Execute Driver Options. Images will be loaded but dispatch will be > > + // deferred for 3rd party images until EndOfDxe is signalled. > > + // > > + LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypeDriver); > > + ProcessLoadOptions (LoadOptions, LoadOptionCount); > > + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > + > > // > > // Do the platform init, can be customized by OEM/IBV > > // Possible things that can be done in PlatformBootManagerBeforeConsole: > > @@ -868,13 +876,6 @@ BdsEntry ( > > // > > EfiBootManagerStartHotkeyService (&HotkeyTriggered); > > > > - // > > - // Execute Driver Options > > - // > > - LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypeDriver); > > - ProcessLoadOptions (LoadOptions, LoadOptionCount); > > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > - > > // > > // Connect consoles > > // > > >