APRIORI in RISC-V or Where did OVMF APRIORIs come from?

APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Daniel Schaefer]

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.
If we compare our APRIORIs with OVMF (OvmfPkg/OvmfPkgX64.fdf) we can see 
that
it contains the same DXEs from MdeModulePkg in APRIORI, as well as the OVMF
version of FvbServicesRuntimeDxe:

APRIORI PEI {
   INF  MdeModulePkg/Universal/PCD/Pei/Pcd.inf
}
APRIORI DXE {
   INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
   INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
   INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
!if $(SMM_REQUIRE) == FALSE
   INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
!endif
}

I conclude, that we cannot give an explanation for why we added them. 
I'm not
sure whether to remove as many as possible now or to keep a similar list as
OvmfPkg. If we keep them, people can just add DXEs like they do in OVMF and
they will behave the same. If we remove them, we might expose some 
dependency
issues in existing DXEs. That would be an advantage, so I would vote for 
this
option.

By the way, currently our most recent code is here:
https://github.com/changab/edk2-platforms/tree/riscv-smode-lib/Platform/SiFive/U5SeriesPkg
https://github.com/changab/edk2-platforms/tree/riscv-smode-lib/Silicon/RISC-V/ProcessorPkg
https://github.com/changab/edk2-platforms/tree/riscv-smode-lib/Platform/RISC-V/PlatformPkg

Thanks,
Daniel

Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Ard Biesheuvel]

On 5/7/20 3:18 PM, Daniel Schaefer via groups.io 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?

Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Daniel Schaefer]

On 5/7/20 3:24 PM, Ard Biesheuvel wrote:
> On 5/7/20 3:18 PM, Daniel Schaefer via groups.io 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?

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

Thanks,
Daniel

Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Ard Biesheuvel]

(+ 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 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.

> 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.

Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Andrew Fish]

[-- Attachment #1: Type: text/plain, Size: 5480 bytes --]



> On May 7, 2020, at 6:53 AM, Ard Biesheuvel <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 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 #2: Type: text/html, Size: 15217 bytes --]

Re: [EXTERNAL] Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Bret Barkelew]

[-- 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 --]

Re: [EXTERNAL] [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Andrew Fish]

[-- Attachment #1: Type: text/plain, Size: 7004 bytes --]

Bret,

How does that test work? Does it make a custom FDF file? 

Thanks,

Andrew Fish

> On May 7, 2020, at 9:45 AM, Bret Barkelew via groups.io <bret.barkelew=microsoft.com@groups.io> wrote:
> 
> 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
> 
> 
>
>
> 
> <3C121AAE0A3549E984277926BF20E545.png>


[-- Attachment #2: Type: text/html, Size: 20297 bytes --]

Re: [EXTERNAL] [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 7646 bytes --]

Reversing the order of files in each FV of an FDF is a very good test case.

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via groups.io
Sent: Thursday, May 7, 2020 9:55 AM
To: devel@edk2.groups.io; bret.barkelew@microsoft.com
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; 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; Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: Re: [EXTERNAL] [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

Bret,

How does that test work? Does it make a custom FDF file?

Thanks,

Andrew Fish


On May 7, 2020, at 9:45 AM, Bret Barkelew via groups.io<http://groups.io> <bret.barkelew=microsoft.com@groups.io<mailto:bret.barkelew=microsoft.com@groups.io>> wrote:

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





<3C121AAE0A3549E984277926BF20E545.png>



[-- Attachment #2: Type: text/html, Size: 60946 bytes --]

Re: [EXTERNAL] Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Laszlo Ersek]

On 05/07/20 18:45, Bret Barkelew wrote:
> 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?

That won't work for -- at least -- the SMM_REQUIRE build of OVMF.
Consider the following FDF snippet:

> !if $(SMM_REQUIRE) == TRUE
> [...]
> INF  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
>
> #
> # Variable driver stack (SMM)
> #
> INF  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
> INF  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
> INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
>
> !else

The pflash detection in FvbServicesSmm can only function if
FvbServicesSmm is dispatched after PiSmmCpuDxeSmm. QEMU restricts pflash
write accesses to such guest code that runs in SMM. And if
FvbServicesSmm is dispatched before PiSmmCpuDxeSmm, then FvbServicesSmm
does not run in SMM, and the pflash detection fails.

(

Note: the question is not whether FvbServicesSmm runs *from SMRAM*
(that's a given), but whether it runs *in SMM*.

Regardless of the dispatch order, SMRAM is open (not closed) at this
point, so FvbServicesSmm *can* run from SMRAM without the VCPU being in
SMM. But in that case, QEMU denies pflash writes, and so the pflash
detection fails.

)

In 2016, there was an attempt to add a new dependency to PiSmmCpuDxeSmm,
which ended up reversing the desired dispatch order between
PiSmmCpuDxeSmm and FvbServicesSmm. That triggered the above problem, and
we discussed it back then.

At the time, I did try enforcing the right dispatch order with adding a
DEPEX to FvbServicesSmm. But then the rest of the variable stack (which
builds upon the SMM FVB protocol produced by FvbServicesSmm) fell apart.

See my 2016 writeup here:

http://mid.mail-archive.com/cc211d5f-18f3-57c8-7b4f-d4f3433898f7@redhat.com

(
Alternative link:
https://lists.01.org/hyperkitty/list/edk2-devel@lists.01.org/message/WE3NGL24WTEB6E56MOJMVFBOIKPLKC3Q/
)

The PiSmmCpuDxeSmm DEPEX that upset the dispatch order for OVMF got
ultimately reverted in commit eadf70bdfbc1 ("UefiCpuPkg/PiSmmCpuDxeSmm:
Revert 7503cd70fb86", 2016-08-19).

(My understanding is that the same DEPEX ended up being injected into
PiSmmCpuDxeSmm in a particular platform tree (not in edk2), via that
platform's SmmCpuFeaturesLib instance -- PiSmmCpuDxeSmm consumes
SmmCpuFeaturesLib.)


Part of the complication is *likely* that VariableSmm only has TRUE for
Depex:

- VariableSmm consumes the SMM FTW protocol produced by
  FaultTolerantWriteSmm, but that protocol is not in the DEPEX, it's
  awaited with an SMM protocol notify.

- VariableSmm also consumes the SMM FVB protocol produced by
  FvbServicesSmm, but that protocol is not waited-for in *any* way
  (neither DEPEX nor notify). Only attempts are made to locate it.

  I *guess* (but haven't tracked down) that the SMM FVB availability is
  inferred from the SMM FTW availability (FaultTolerantWriteSmm does
  have an explicit DEPEX on SMM FVB).


The (working) dispatch order seen in the OVMF log file is:

> Loading SMM driver at 0x0007D094000 EntryPoint=0x0007D09D0B3 PiSmmCpuDxeSmm.efi
> Loading SMM driver at 0x0007D042000 EntryPoint=0x0007D047677 FvbServicesSmm.efi
> Loading SMM driver at 0x0007CF68000 EntryPoint=0x0007CFF882E VariableSmm.efi
> Loading SMM driver at 0x0007CE05000 EntryPoint=0x0007CE0923C SmmFaultTolerantWriteDxe.efi
> Loading driver at 0x0007BAA0000 EntryPoint=0x0007BAA47E2 VariableSmmRuntimeDxe.efi

Note a few things:

- FaultTolerantWriteSmm calls itself "SmmFaultTolerantWriteDxe" via its
  BASE_NAME (functionally harmless, but confusing).

- The dispatch order between PiSmmCpuDxeSmm and FvbServicesSmm is as
  desired.

- VariableSmm and FaultTolerantWriteSmm are *already* dispatched in
  reverse order relative to the FDF file. This -- while not wrong --
  does not seem necessary for satisfying any DEPEX, as far as I can see.

This is one of those "dark corners".

Thanks
Laszlo

Re: [edk2-devel] APRIORI in RISC-V or Where did OVMF APRIORIs come from?

[Laszlo Ersek]

On 05/07/20 15:53, Ard Biesheuvel wrote:
> On 5/7/20 3:43 PM, Daniel Schaefer wrote:

>> 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.
>

Some of the APRIORI entries are very old and not well documented (via
git-blame / git log). I've done some cursory tests now.

(1) "MdeModulePkg/Universal/PCD/Pei/Pcd.inf" in APRIORI PEI goes back to
commit 49ba9447c92d ("Add initial version of Open Virtual Machine
Firmware (OVMF) platform.", 2009-05-27).

I've tried removing it now (eliminating the whole APRIORI PEI file), and
there seem to be no ill effects.

Note that this module is built like this in the DSC file:

  MdeModulePkg/Universal/PCD/Pei/Pcd.inf  {
    <LibraryClasses>
      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
  }

That's because the PCD PEI depends on a number lib classes, and some of
the lib instances used to resolve those lib classes depend on PcdLib.
The "usual" PEI instance of PcdLib would depend on the PCD PPI, hence
the PCD PEI would depend on itself. The above lib instance override
breaks up the cycle.


(2) The same cannot be said about
"MdeModulePkg/Universal/PCD/Dxe/Pcd.inf" in APRIORI DXE. While the
addition of this entry goes back to the same historical commit
49ba9447c92d ("Add initial version of Open Virtual Machine Firmware
(OVMF) platform.", 2009-05-27); it does have a live dependency.

(2a) Namely,
"OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf"
depends on dynamic PCDs, and FvbServicesRuntimeDxe really does have to
be on the APRIORI DXE list. Refer to commit 182eb4562731 ("OvmfPkg: Add
QemuFlashFvbServicesRuntimeDxe to firmware image", 2013-11-12).

In short, FvbServicesRuntimeDxe and EmuVariableFvbRuntimeDxe are
alternatives (dependent on whether QEMU runs with or without pflash),
and dynamic PCDs are used to arbitrate between them.
FvbServicesRuntimeDxe performs the flash detection so it must come
first.

(2b) Now, if OVMF is built with SMM_REQUIRE, then both
FvbServicesRuntimeDxe and EmuVariableFvbRuntimeDxe fall away (pflash is
a hard requirement, and an *SMM* flash driver is used, namely
"OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf"). Per commit
46df0216b0ed ("OvmfPkg: pull in SMM-based variable driver stack",
2015-11-30), the FvbServicesRuntimeDxe driver is then removed from the
APRIORI DXE file as well. This suggests that the PCD DXE driver *too*
could be removed (with nothing depending on it in the APRIORI DXE list),
when SMM_REQUIRE is TRUE. I've tested conditionalizing the PCD DXE
driver like that, and it seems to work.


(3) The "MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf" driver
in the APRIORI DXE file is justified. It goes back to commit
5c3481b0b611 ("OvmfPkg: Use the new DevicePathLib for all platforms",
2013-08-19), and commit 863986b3c8e6.

(3a) This APRIORI DXE entry is needed because of the following
dependency chain:

  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
  ->
  MdePkg/Library/DxeHobLib/DxeHobLib.inf
  ->
  MdePkg/Library/UefiLib/UefiLib.inf
  ->
  MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf

Due to the PCD DXE driver being on the APRIORI DXE list (see point (2)
above), DevicePathDxe has to be as well.

This chain could be probably broken by modifying the DSC file, to link
the PCD DXE driver with the "thick" DevicePathLib instance:
"MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf".

(3b) Building upon (2b), we get tempted to take the PCD DXE driver and
the DevicePathDxe driver *together* off the APRIORI DXE list, if if
SMM_REQUIRE is defined.

Unfortunately, this doesn't work, as AmdSevDxe is also in the APRIORI
DXE file (see the next point), and it comes with the following
dependency chain:

  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
  ->
  MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
  ->
  MdePkg/Library/UefiLib/UefiLib.inf
  ->
  MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf


(4) AmdSevDxe belongs in the APRIORI DXE file. The explanation is given
in commit 24e4ad75546b ("OvmfPkg: Add AmdSevDxe driver", 2017-07-10):

    The driver is being added to the APRIORI DXE file so that, we clear the
    C-bit from MMIO regions before any driver accesses it.

This is not a perfect solution by far. But a better solution would have
required new infrastructure -- see
<https://bugzilla.tianocore.org/show_bug.cgi?id=623> --, and we could
never reach an agreement on that! (Just read through the ticket --
multiple approaches were proposed and some even prototyped (with patches
posted); nothing was accepted.)


Summarizing the status for APRIORI DXE, AmdSevDxe must be on the APRIORI
DXE list, and AmdSevDxe requires DevicePathDxe. "Dxe/Pcd.inf" could be
conditionalized on SMM_REQUIRE=FALSE.

Summarizing the status for APRIORI PEI, I think it could be eliminated
at this point.

Thanks,
Laszlo