setting PcdPlatformBootTimeOut in PlatformBootManagerLib

setting PcdPlatformBootTimeOut in PlatformBootManagerLib

[Laszlo Ersek]

Hi Ray,

OvmfPkg and ArmVirtPkg platforms set PcdPlatformBootTimeOut in their
PlatformBootManagerLib instances, in BeforeConsole(). This generally
works, except for one aspect: in BdsEntry(), the Timeout UEFI variable
is set from the PCD before BeforeConsole() is called.

The Timeout variable is in this case purely informative, but I'd still
like it to reflect the PCD's value. Given that I'd like to keep the
subject PcdSet16S() call in PlatformBootManagerLib, I see two options:

(1) Modify BdsEntry(): re-check the value of the PCD a bit before
BdsWait() -- when the PCD is really needed --, and if it has changed
relative to the first seen value, store the new PCD value to the UEFI
variable *again*.

This would keep platforms happy that expect Timeout to already carry the
PCDs value upon entering BeforeConsole(), and it would allow platforms
to re-set the PCD (and Timeout too) in either BeforeConsole() or
AfterConsole().

(2) Alternatively, add a CONSTRUCTOR function to the
PlatformBootManagerLib instances for setting the PCD before BdsEntry()
is invoked at all. I'm a bit concerned about this, as we'd have to
ensure that *all* the lib instances that participate in this PCD setting
(including the lib instances that let us calculate the value) have
CONSTRUCTOR functions (otherwise the topological sorting of CONSTRUCTOR
calls is not complete).

Thanks,
Laszlo

Re: setting PcdPlatformBootTimeOut in PlatformBootManagerLib

[Laszlo Ersek]

On 03/02/20 00:13, Laszlo Ersek wrote:
> Hi Ray,
> 
> OvmfPkg and ArmVirtPkg platforms set PcdPlatformBootTimeOut in their
> PlatformBootManagerLib instances, in BeforeConsole(). This generally
> works, except for one aspect: in BdsEntry(), the Timeout UEFI variable
> is set from the PCD before BeforeConsole() is called.
> 
> The Timeout variable is in this case purely informative, but I'd still
> like it to reflect the PCD's value. Given that I'd like to keep the
> subject PcdSet16S() call in PlatformBootManagerLib, I see two options:
> 
> (1) Modify BdsEntry(): re-check the value of the PCD a bit before
> BdsWait() -- when the PCD is really needed --, and if it has changed
> relative to the first seen value, store the new PCD value to the UEFI
> variable *again*.
> 
> This would keep platforms happy that expect Timeout to already carry the
> PCDs value upon entering BeforeConsole(), and it would allow platforms
> to re-set the PCD (and Timeout too) in either BeforeConsole() or
> AfterConsole().
> 
> (2) Alternatively, add a CONSTRUCTOR function to the
> PlatformBootManagerLib instances for setting the PCD before BdsEntry()
> is invoked at all. I'm a bit concerned about this, as we'd have to
> ensure that *all* the lib instances that participate in this PCD setting
> (including the lib instances that let us calculate the value) have
> CONSTRUCTOR functions (otherwise the topological sorting of CONSTRUCTOR
> calls is not complete).

(3) We could also just rewrite Timeout ourselves, in
PlatformBootManagerLib, where we set the PCD.

Thanks
Laszlo

Re: setting PcdPlatformBootTimeOut in PlatformBootManagerLib

[Ard Biesheuvel]

On Mon, 2 Mar 2020 at 10:40, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/02/20 00:13, Laszlo Ersek wrote:
> > Hi Ray,
> >
> > OvmfPkg and ArmVirtPkg platforms set PcdPlatformBootTimeOut in their
> > PlatformBootManagerLib instances, in BeforeConsole(). This generally
> > works, except for one aspect: in BdsEntry(), the Timeout UEFI variable
> > is set from the PCD before BeforeConsole() is called.
> >
> > The Timeout variable is in this case purely informative, but I'd still
> > like it to reflect the PCD's value. Given that I'd like to keep the
> > subject PcdSet16S() call in PlatformBootManagerLib, I see two options:
> >
> > (1) Modify BdsEntry(): re-check the value of the PCD a bit before
> > BdsWait() -- when the PCD is really needed --, and if it has changed
> > relative to the first seen value, store the new PCD value to the UEFI
> > variable *again*.
> >
> > This would keep platforms happy that expect Timeout to already carry the
> > PCDs value upon entering BeforeConsole(), and it would allow platforms
> > to re-set the PCD (and Timeout too) in either BeforeConsole() or
> > AfterConsole().
> >
> > (2) Alternatively, add a CONSTRUCTOR function to the
> > PlatformBootManagerLib instances for setting the PCD before BdsEntry()
> > is invoked at all. I'm a bit concerned about this, as we'd have to
> > ensure that *all* the lib instances that participate in this PCD setting
> > (including the lib instances that let us calculate the value) have
> > CONSTRUCTOR functions (otherwise the topological sorting of CONSTRUCTOR
> > calls is not complete).
>
> (3) We could also just rewrite Timeout ourselves, in
> PlatformBootManagerLib, where we set the PCD.
>

'Rewrite' as in access the global variable? Or access the HII guts of
the 'timeout' entry in the menus?

Re: setting PcdPlatformBootTimeOut in PlatformBootManagerLib

[Laszlo Ersek]

On 03/02/20 10:43, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 10:40, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 03/02/20 00:13, Laszlo Ersek wrote:
>>> Hi Ray,
>>>
>>> OvmfPkg and ArmVirtPkg platforms set PcdPlatformBootTimeOut in their
>>> PlatformBootManagerLib instances, in BeforeConsole(). This generally
>>> works, except for one aspect: in BdsEntry(), the Timeout UEFI variable
>>> is set from the PCD before BeforeConsole() is called.
>>>
>>> The Timeout variable is in this case purely informative, but I'd still
>>> like it to reflect the PCD's value. Given that I'd like to keep the
>>> subject PcdSet16S() call in PlatformBootManagerLib, I see two options:
>>>
>>> (1) Modify BdsEntry(): re-check the value of the PCD a bit before
>>> BdsWait() -- when the PCD is really needed --, and if it has changed
>>> relative to the first seen value, store the new PCD value to the UEFI
>>> variable *again*.
>>>
>>> This would keep platforms happy that expect Timeout to already carry the
>>> PCDs value upon entering BeforeConsole(), and it would allow platforms
>>> to re-set the PCD (and Timeout too) in either BeforeConsole() or
>>> AfterConsole().
>>>
>>> (2) Alternatively, add a CONSTRUCTOR function to the
>>> PlatformBootManagerLib instances for setting the PCD before BdsEntry()
>>> is invoked at all. I'm a bit concerned about this, as we'd have to
>>> ensure that *all* the lib instances that participate in this PCD setting
>>> (including the lib instances that let us calculate the value) have
>>> CONSTRUCTOR functions (otherwise the topological sorting of CONSTRUCTOR
>>> calls is not complete).
>>
>> (3) We could also just rewrite Timeout ourselves, in
>> PlatformBootManagerLib, where we set the PCD.
>>
> 
> 'Rewrite' as in access the global variable? Or access the HII guts of
> the 'timeout' entry in the menus?

Just call gRT->SetVariable().

Thanks
Laszlo

Re: [edk2-devel] setting PcdPlatformBootTimeOut in PlatformBootManagerLib

[Ard Biesheuvel]

On Mon, 2 Mar 2020 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/02/20 10:43, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 10:40, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 03/02/20 00:13, Laszlo Ersek wrote:
> >>> Hi Ray,
> >>>
> >>> OvmfPkg and ArmVirtPkg platforms set PcdPlatformBootTimeOut in their
> >>> PlatformBootManagerLib instances, in BeforeConsole(). This generally
> >>> works, except for one aspect: in BdsEntry(), the Timeout UEFI variable
> >>> is set from the PCD before BeforeConsole() is called.
> >>>
> >>> The Timeout variable is in this case purely informative, but I'd still
> >>> like it to reflect the PCD's value. Given that I'd like to keep the
> >>> subject PcdSet16S() call in PlatformBootManagerLib, I see two options:
> >>>
> >>> (1) Modify BdsEntry(): re-check the value of the PCD a bit before
> >>> BdsWait() -- when the PCD is really needed --, and if it has changed
> >>> relative to the first seen value, store the new PCD value to the UEFI
> >>> variable *again*.
> >>>
> >>> This would keep platforms happy that expect Timeout to already carry the
> >>> PCDs value upon entering BeforeConsole(), and it would allow platforms
> >>> to re-set the PCD (and Timeout too) in either BeforeConsole() or
> >>> AfterConsole().
> >>>
> >>> (2) Alternatively, add a CONSTRUCTOR function to the
> >>> PlatformBootManagerLib instances for setting the PCD before BdsEntry()
> >>> is invoked at all. I'm a bit concerned about this, as we'd have to
> >>> ensure that *all* the lib instances that participate in this PCD setting
> >>> (including the lib instances that let us calculate the value) have
> >>> CONSTRUCTOR functions (otherwise the topological sorting of CONSTRUCTOR
> >>> calls is not complete).
> >>
> >> (3) We could also just rewrite Timeout ourselves, in
> >> PlatformBootManagerLib, where we set the PCD.
> >>
> >
> > 'Rewrite' as in access the global variable? Or access the HII guts of
> > the 'timeout' entry in the menus?
>
> Just call gRT->SetVariable().
>

Ah, ok - the *EFI* variable. I missed that detail :-)