public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib
@ 2019-04-25  9:18 Marcin Wojtas
  2019-04-25 11:04 ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Marcin Wojtas @ 2019-04-25  9:18 UTC (permalink / raw)
  To: devel
  Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, kostap, Jici.Gao,
	feng.tian, michael.d.kinney, liming.gao, lersek

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib
  2019-04-25  9:18 [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib Marcin Wojtas
@ 2019-04-25 11:04 ` Ard Biesheuvel
  2019-04-26 17:02   ` Laszlo Ersek
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2019-04-25 11:04 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś,
	Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Tian, Feng,
	Kinney, Michael D, Gao, Liming, Laszlo Ersek

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib
  2019-04-25 11:04 ` Ard Biesheuvel
@ 2019-04-26 17:02   ` Laszlo Ersek
  2019-04-26 23:38     ` Laszlo Ersek
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2019-04-26 17:02 UTC (permalink / raw)
  To: Ard Biesheuvel, Marcin Wojtas
  Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś,
	Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Tian, Feng,
	Kinney, Michael D, Gao, Liming

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib
  2019-04-26 17:02   ` Laszlo Ersek
@ 2019-04-26 23:38     ` Laszlo Ersek
  2019-04-27  9:17       ` Marcin Wojtas
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2019-04-26 23:38 UTC (permalink / raw)
  To: Ard Biesheuvel, Marcin Wojtas
  Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś,
	Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Tian, Feng,
	Kinney, Michael D, Gao, Liming

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib
  2019-04-26 23:38     ` Laszlo Ersek
@ 2019-04-27  9:17       ` Marcin Wojtas
  0 siblings, 0 replies; 5+ messages in thread
From: Marcin Wojtas @ 2019-04-27  9:17 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Ard Biesheuvel, edk2-devel-groups-io, Leif Lindholm,
	Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
	Jici Gao, Tian, Feng, Kinney, Michael D, Gao, Liming

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-27  9:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-25  9:18 [PATCH] EmbeddedPkg: Allow DXE_DRIVER to depend on NvVarStoreFormattedLib Marcin Wojtas
2019-04-25 11:04 ` Ard Biesheuvel
2019-04-26 17:02   ` Laszlo Ersek
2019-04-26 23:38     ` Laszlo Ersek
2019-04-27  9:17       ` Marcin Wojtas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox