[PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[Marcin Wojtas]

Some modules (such as FaultTolerantWriteDxe) use the FlashNvStorage
PCDs (PcdFlashNvStorageFtw*). In case the flash contents are not
mapped in memory, the module loading order of the FVB driver
may become important.

To handle above, this patch allows to hook the dependency of
desired DXE_DRIVER type module in the .DSC file via
NvVarStoreFormattedLib NULL resolution.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
index fefc311..98a0049 100644
--- a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
+++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
@@ -39,8 +39,8 @@
 #
 # The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
 # EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
-# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
+# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER/DXE_DRIVER (EFI_FV_FILETYPE_DRIVER), or
 # DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
 #
-[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
+[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_DRIVER, Depex.common.DXE_SMM_DRIVER]
   gEdkiiNvVarStoreFormattedGuid
-- 
2.7.4

Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[Ard Biesheuvel]

On Thu, 25 Apr 2019 at 11:18, Marcin Wojtas <mw@semihalf.com> wrote:
>
> Some modules (such as FaultTolerantWriteDxe) use the FlashNvStorage
> PCDs (PcdFlashNvStorageFtw*). In case the flash contents are not
> mapped in memory, the module loading order of the FVB driver
> may become important.
>
> To handle above, this patch allows to hook the dependency of
> desired DXE_DRIVER type module in the .DSC file via
> NvVarStoreFormattedLib NULL resolution.
>
> Contributed-under: TianoCore Contribution Agreement 1.1

This line is no longer required, so you can drop it in the future.

Note that the licensing terms have changed accordingly: by
contributing patches under the new license terms, you are basically
granting the same IP rights you were granting before by adhering to
the TianoCore Contribution Agreement, so nothing has really changed.
However, it is your *own* responsibility to confirm that I am not
misrepresenting anything here, so please check the repository for the
license changes.

> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as b9d4847ec258..20029ca22baa

Thanks,
> ---
>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> index fefc311..98a0049 100644
> --- a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> +++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> @@ -39,8 +39,8 @@
>  #
>  # The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
>  # EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
> -# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
> +# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER/DXE_DRIVER (EFI_FV_FILETYPE_DRIVER), or
>  # DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
>  #
> -[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
> +[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_DRIVER, Depex.common.DXE_SMM_DRIVER]
>    gEdkiiNvVarStoreFormattedGuid
> --
> 2.7.4
>

Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[Laszlo Ersek]

On 04/25/19 13:04, Ard Biesheuvel wrote:
> On Thu, 25 Apr 2019 at 11:18, Marcin Wojtas <mw@semihalf.com> wrote:
>>
>> Some modules (such as FaultTolerantWriteDxe) use the FlashNvStorage
>> PCDs (PcdFlashNvStorageFtw*). In case the flash contents are not
>> mapped in memory, the module loading order of the FVB driver
>> may become important.
>>
>> To handle above, this patch allows to hook the dependency of
>> desired DXE_DRIVER type module in the .DSC file via
>> NvVarStoreFormattedLib NULL resolution.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> This line is no longer required, so you can drop it in the future.
> 
> Note that the licensing terms have changed accordingly: by
> contributing patches under the new license terms, you are basically
> granting the same IP rights you were granting before by adhering to
> the TianoCore Contribution Agreement, so nothing has really changed.
> However, it is your *own* responsibility to confirm that I am not
> misrepresenting anything here, so please check the repository for the
> license changes.
> 
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Pushed as b9d4847ec258..20029ca22baa

patch looks good to me as well, thanks

> 
> Thanks,
>> ---
>>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>> index fefc311..98a0049 100644
>> --- a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>> +++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>> @@ -39,8 +39,8 @@
>>  #
>>  # The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
>>  # EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
>> -# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
>> +# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER/DXE_DRIVER (EFI_FV_FILETYPE_DRIVER), or
>>  # DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
>>  #
>> -[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
>> +[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_DRIVER, Depex.common.DXE_SMM_DRIVER]
>>    gEdkiiNvVarStoreFormattedGuid
>> --
>> 2.7.4
>>

Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[Laszlo Ersek]

On 04/26/19 19:02, Laszlo Ersek wrote:
> On 04/25/19 13:04, Ard Biesheuvel wrote:
>> On Thu, 25 Apr 2019 at 11:18, Marcin Wojtas <mw@semihalf.com> wrote:
>>>
>>> Some modules (such as FaultTolerantWriteDxe) use the FlashNvStorage
>>> PCDs (PcdFlashNvStorageFtw*). In case the flash contents are not
>>> mapped in memory, the module loading order of the FVB driver
>>> may become important.
>>>
>>> To handle above, this patch allows to hook the dependency of
>>> desired DXE_DRIVER type module in the .DSC file via
>>> NvVarStoreFormattedLib NULL resolution.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>
>> This line is no longer required, so you can drop it in the future.
>>
>> Note that the licensing terms have changed accordingly: by
>> contributing patches under the new license terms, you are basically
>> granting the same IP rights you were granting before by adhering to
>> the TianoCore Contribution Agreement, so nothing has really changed.
>> However, it is your *own* responsibility to confirm that I am not
>> misrepresenting anything here, so please check the repository for the
>> license changes.
>>
>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Pushed as b9d4847ec258..20029ca22baa
> 
> patch looks good to me as well, thanks

Hmm, don't know how I missed it, but the INF file still has:

  LIBRARY_CLASS                  = NvVarStoreFormattedLib|PEIM DXE_RUNTIME_DRIVER DXE_SMM_DRIVER

I don't understand why BaseTools don't catch that, when the lib instance is hooked into a DXE_DRIVER.

... Perhaps because the "hooking" uses the "NULL class", and the above restriction list only applies to the NvVarStoreFormattedLib class (which is ultimately never used)?

I'm not sure; still for consistency's sake, I think we should add DXE_DRIVER to LIBRARY_CLASS too.

Thanks
Laszlo

> 
>>
>> Thanks,
>>> ---
>>>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>>> index fefc311..98a0049 100644
>>> --- a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>>> +++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>>> @@ -39,8 +39,8 @@
>>>  #
>>>  # The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
>>>  # EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
>>> -# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
>>> +# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER/DXE_DRIVER (EFI_FV_FILETYPE_DRIVER), or
>>>  # DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
>>>  #
>>> -[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
>>> +[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_DRIVER, Depex.common.DXE_SMM_DRIVER]
>>>    gEdkiiNvVarStoreFormattedGuid
>>> --
>>> 2.7.4
>>>
> 

Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib

[Marcin Wojtas]

Hi Laszlo,

sob., 27 kwi 2019 o 01:38 Laszlo Ersek <lersek@redhat.com> napisał(a):
>
> On 04/26/19 19:02, Laszlo Ersek wrote:
> > On 04/25/19 13:04, Ard Biesheuvel wrote:
> >> On Thu, 25 Apr 2019 at 11:18, Marcin Wojtas <mw@semihalf.com> wrote:
> >>>
> >>> Some modules (such as FaultTolerantWriteDxe) use the FlashNvStorage
> >>> PCDs (PcdFlashNvStorageFtw*). In case the flash contents are not
> >>> mapped in memory, the module loading order of the FVB driver
> >>> may become important.
> >>>
> >>> To handle above, this patch allows to hook the dependency of
> >>> desired DXE_DRIVER type module in the .DSC file via
> >>> NvVarStoreFormattedLib NULL resolution.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>
> >> This line is no longer required, so you can drop it in the future.
> >>
> >> Note that the licensing terms have changed accordingly: by
> >> contributing patches under the new license terms, you are basically
> >> granting the same IP rights you were granting before by adhering to
> >> the TianoCore Contribution Agreement, so nothing has really changed.
> >> However, it is your *own* responsibility to confirm that I am not
> >> misrepresenting anything here, so please check the repository for the
> >> license changes.
> >>
> >>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >>
> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> Pushed as b9d4847ec258..20029ca22baa
> >
> > patch looks good to me as well, thanks
>
> Hmm, don't know how I missed it, but the INF file still has:
>
>   LIBRARY_CLASS                  = NvVarStoreFormattedLib|PEIM DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
>
> I don't understand why BaseTools don't catch that, when the lib instance is hooked into a DXE_DRIVER.
>
> ... Perhaps because the "hooking" uses the "NULL class", and the above restriction list only applies to the NvVarStoreFormattedLib class (which is ultimately nev
>
> I'm not sure; still for consistency's sake, I think we should add DXE_DRIVER to LIBRARY_CLASS too.
>

I've just submitted a patch extending the LIBRARY_CLASS.

Best regards.
Marcin

> >>
> >> Thanks,
> >>> ---
> >>>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> >>> index fefc311..98a0049 100644
> >>> --- a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> >>> +++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> >>> @@ -39,8 +39,8 @@
> >>>  #
> >>>  # The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
> >>>  # EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
> >>> -# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
> >>> +# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER/DXE_DRIVER (EFI_FV_FILETYPE_DRIVER), or
> >>>  # DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
> >>>  #
> >>> -[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
> >>> +[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_DRIVER, Depex.common.DXE_SMM_DRIVER]
> >>>    gEdkiiNvVarStoreFormattedGuid
> >>> --
> >>> 2.7.4
> >>>
> >
>