public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Bret Barkelew" <bret.barkelew@microsoft.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"afish@apple.com" <afish@apple.com>,
	Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>,
	"Chang, Abner (HPS SW/FW Technologist)" <abner.chang@hpe.com>,
	Atish Patra <atishp@atishpatra.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Atish Patra <atish.patra@wdc.com>,
	Alexander Graf <agraf@csgraf.de>, Anup Patel <anup.patel@wdc.com>,
	"leif@nuviainc.com" <leif@nuviainc.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [EXTERNAL] Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?
Date: Thu, 7 May 2020 16:45:51 +0000	[thread overview]
Message-ID: <CY4PR21MB07432F6F0A597C94C28AFDACEFA50@CY4PR21MB0743.namprd21.prod.outlook.com> (raw)
In-Reply-To: <9B467C1E-7EAC-49D9-B646-D91E60FD7539@apple.com>


[-- Attachment #1.1: Type: text/plain, Size: 6520 bytes --]

I know I’ve also seen tests that randomize the driver dispatch order to try to catch these “implementation-specific” edge cases. Perhaps we could instrument something similar with a weekly OVMF CI test?

- Bret

From: Andrew Fish via groups.io<mailto:afish=apple.com@groups.io>
Sent: Thursday, May 7, 2020 9:43 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel<mailto:ard.biesheuvel@arm.com>
Cc: Daniel Schaefer<mailto:daniel.schaefer@hpe.com>; Chang, Abner (HPS SW/FW Technologist)<mailto:abner.chang@hpe.com>; Atish Patra<mailto:atishp@atishpatra.org>; Heinrich Schuchardt<mailto:xypron.glpk@gmx.de>; Atish Patra<mailto:atish.patra@wdc.com>; Alexander Graf<mailto:agraf@csgraf.de>; Anup Patel<mailto:anup.patel@wdc.com>; leif@nuviainc.com<mailto:leif@nuviainc.com>; Jordan Justen<mailto:jordan.l.justen@intel.com>; Laszlo Ersek<mailto:lersek@redhat.com>
Subject: [EXTERNAL] Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?




On May 7, 2020, at 6:53 AM, Ard Biesheuvel <Ard.Biesheuvel@arm.com<mailto:Ard.Biesheuvel@arm.com>> wrote:

(+ Laszlo)

On 5/7/20 3:43 PM, Daniel Schaefer wrote:

On 5/7/20 3:24 PM, Ard Biesheuvel wrote:

On 5/7/20 3:18 PM, Daniel Schaefer via groups.io<https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgroups.io%2F&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C55cf729cb7f2493df70508d7f2a5b761%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637244665879150410&sdata=3m1H%2BjDxdT258g641hJMpIXoSR3C7EmkGo3fpkZhVgw%3D&reserved=0> wrote:

Hi Ard and others,

TLDR; We have APRIORI definitions from other places in EDK2 but there's no explanation as to why they are there.

I'm taking this to the EDK2 list, since it doesn't concern U-Boot.
I kept some other people related to UEFI, maybe you're interested ;)

On 2/25/20 10:07 AM, Ard Biesheuvel wrote:
 > What I did notice is the use of APRIORI PEI and APRIORI DXE sections
 > in your platform descriptions. I recommend you try to avoid that, as
 > it is a maintenance burden going forward: instead, please use dummy
 > protocols and NULL library class resolutions if you need to make
 > generic components depend on platform specific protocols. Also, please
 > document this - the APRIORI section does not explain *why* you have to
 > circumvent the ordinary dependency tree based module dispatch.

I'm taking a look at this right now.
You're absolutely right - we should reduce or document APRIORIs.

However, Abner told me that he had only copied most of the FDF from other
places in EDK2.This is what we currently have:

APRIORI PEI {
   INF MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
   INF MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
   INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
}
APRIORI DXE {
   INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
   INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
   INF Platform/SiFive/U5SeriesPkg/Universal/Dxe/RamFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
}

I can remove all of APRIORI PEI and it boots properly. Of the DXEs I can only
remove FvbServicesRuntimeDxe, otherwise some DXEs are dispatched in the wrong
order and boot fails.

This means some modules have an undeclared dependency on one of the remaining modules. Can you elaborate on how the boot fails in this case?
The error is
  ASSERT [FvbServicesRuntimeDxe] /edk2/MdePkg/Library/DxePcdLib/DxePcdLib.c(72): !EFI_ERROR (Status)
In this line, DxePcdLib tries to consume gPcdProtocolGuid. Therefor if I add
the following to FvbServicesRuntimeDxe.inf:
[Depex]
  gEfiPcdProtocolGuid
[Protocols]
  gPcdProtocolGuid                              ## SOMETIMES_CONSUMES
  gEfiPcdProtocolGuid                           ## CONSUMES
  gGetPcdInfoProtocolGuid                       ## SOMETIMES_CONSUMES
  gEfiGetPcdInfoProtocolGuid                    ## SOMETIMES_CONSUMES
I can boot without error.
Looking at MdePkg/Library/DxePcdLib/DxePcdLib.inf I see that the library has
exactly the same Depex and Protocols specified. Do DXEs have re-specify them?
If yes, of what use is it to declare them for the library? Documentation only?

No this is unexpected. If the PcdLib dependency of FvbServicesRuntimeDxe.inf resolves to MdePkg/Library/DxePcdLib/DxePcdLib.inf, it should inherit the depex and the protocol dependencies.


Could be the dreaded my INF is wrong but it compiles issue. What happens is dependencies in the INF are missing but get resolved by the library instances or even worse libraries pulled in by the libraries. So it looks to me like it is a missing Depex in the driver. Not all instances of the library, that are valid for the module, imply the dependency.

/Volumes/Case/edk2(master)>git grep PcdLib -- *.inf | grep LIBRARY_CLASS
MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf:21:  LIBRARY_CLASS                  = PcdLib
MdePkg/Library/DxePcdLib/DxePcdLib.inf:35:  LIBRARY_CLASS                  = PcdLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVER
MdePkg/Library/PeiPcdLib/PeiPcdLib.inf:32:  LIBRARY_CLASS                  = PcdLib|PEIM PEI_CORE SEC


The easiest way to debug these kind of problems is to generate a report file by passing `-y REPORTFILE` to build. For my projects I usually always build a report file and dump it into the Build results directory. This will show what libraries got linked in, and what the actual dependency expression resolved to.



Should I/we try to remove the APRIORI entries from OVMF in a similar way?

Let's get to the bottom of this first. Laszlo may remember why exactly those entries are there in the first place, and I suspect it is a different issue.


It is good to get the history. In general the APRIORI files are used to force a dispatch order for debugging, like getting status codes or serial output quicker.

>From an architectural point of view the dispatch order is only defined as "if your Depex is TRUE you can be dispatched". So they APRORI file lets you define the undefined behavior. The implementation happens to walk the FV in order  and will dispatch and drivers with a Depex of TRUE and keep iterating until all the Depex'es are TRUE or all the drivers dispatch. In the "good old days" we would sort the PEIMs in dispatch order so that everything would dispatch on the 1st pass to reduce the number of slow FLASH reads required to boot.

Thanks,

Andrew Fish






[-- Attachment #1.2: Type: text/html, Size: 14683 bytes --]

[-- Attachment #2: 3C121AAE0A3549E984277926BF20E545.png --]
[-- Type: image/png, Size: 140 bytes --]

  reply	other threads:[~2020-05-07 16:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200224221949.28826-1-atish.patra@wdc.com>
     [not found] ` <dcab6806-fa1c-8c7e-b0dc-2d96c017872d@gmx.de>
     [not found]   ` <CAKv+Gu_-1BrjiKKw6qa9a6vjXHrf=iYa1oaKCRr5HZf0HM+mZA@mail.gmail.com>
     [not found]     ` <CAOnJCULfew0aZ3FiDQAZZTPjzE5bvdN5of5AP_r6_1W+CQDh=A@mail.gmail.com>
     [not found]       ` <TU4PR8401MB0429018FEDB13B1057A452E4FFED0@TU4PR8401MB0429.NAMPRD84.PROD.OUTLOOK.COM>
     [not found]         ` <CAKv+Gu9vSVMg3L++7xM2LYV7XPoMtY-E1HXZa290srk0CfBqig@mail.gmail.com>
     [not found]           ` <TU4PR8401MB04290CF16E0037DD90FDAE15FFED0@TU4PR8401MB0429.NAMPRD84.PROD.OUTLOOK.COM>
     [not found]             ` <CAKv+Gu9kuiOag+BV5--QbhkEFCD8q8FJFzQ=uYV=oEMfrAo0Zg@mail.gmail.com>
2020-05-07 13:18               ` APRIORI in RISC-V or Where did OVMF APRIORIs come from? Daniel Schaefer
2020-05-07 13:24                 ` [edk2-devel] " Ard Biesheuvel
2020-05-07 13:43                   ` Daniel Schaefer
2020-05-07 13:53                     ` Ard Biesheuvel
2020-05-07 16:42                       ` Andrew Fish
2020-05-07 16:45                         ` Bret Barkelew [this message]
2020-05-07 16:54                           ` [EXTERNAL] " Andrew Fish
2020-05-07 17:00                             ` Michael D Kinney
2020-05-08 11:05                           ` [EXTERNAL] " Laszlo Ersek
2020-05-08  9:48                       ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CY4PR21MB07432F6F0A597C94C28AFDACEFA50@CY4PR21MB0743.namprd21.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox