[edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.

Grzegorz Bernacki posted 1 patch 2 years, 8 months ago
Failed in applying to current master (apply log)
Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24 +++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
[edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Grzegorz Bernacki 2 years, 8 months ago
This patch adds checks if Boot Discovery Policy has been
changed. Only in that case EfiBootManagerRefreshAllBootOption()
should be called.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24 +++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
index c8305ce4f5..378ba0ebf4 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
@@ -617,6 +617,7 @@ BootDiscoveryPolicyHandler (
 {
   EFI_STATUS                       Status;
   UINT32                           DiscoveryPolicy;
+  UINT32                           DiscoveryPolicyOld;
   UINTN                            Size;
   EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
   EFI_GUID                         *Class;
@@ -678,7 +679,28 @@ BootDiscoveryPolicyHandler (
     return Status;
   }
 
-  EfiBootManagerRefreshAllBootOption();
+  //
+  // Refresh Boot Options if Boot Discovery Policy has been changed
+  //
+  Size = sizeof (DiscoveryPolicyOld);
+  Status = gRT->GetVariable (
+                  BOOT_DISCOVERY_POLICY_OLD_VAR,
+                  &gBootDiscoveryPolicyMgrFormsetGuid,
+                  NULL,
+                  &Size,
+                  &DiscoveryPolicyOld
+                  );
+  if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
+    EfiBootManagerRefreshAllBootOption();
+
+    Status = gRT->SetVariable (
+                    BOOT_DISCOVERY_POLICY_OLD_VAR,
+                    &gBootDiscoveryPolicyMgrFormsetGuid,
+                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                    sizeof (DiscoveryPolicyOld),
+                    &DiscoveryPolicy
+                    );
+  }
 
   return EFI_SUCCESS;
 }
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79472): https://edk2.groups.io/g/devel/message/79472
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Samer El-Haj-Mahmoud 2 years, 8 months ago
Thanks for the patch!

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Grzegorz Bernacki <gjb@semihalf.com>
> Sent: Wednesday, August 18, 2021 3:38 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; mw@semihalf.com; upstream@semihalf.com;
> pete@akeo.ie; Grzegorz Bernacki <gjb@semihalf.com>
> Subject: [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot
> Discovery Policy change.
>
> This patch adds checks if Boot Discovery Policy has been
> changed. Only in that case EfiBootManagerRefreshAllBootOption()
> should be called.
>
> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> ---
>  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24
> +++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index c8305ce4f5..378ba0ebf4 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -617,6 +617,7 @@ BootDiscoveryPolicyHandler (
>  {
>    EFI_STATUS                       Status;
>    UINT32                           DiscoveryPolicy;
> +  UINT32                           DiscoveryPolicyOld;
>    UINTN                            Size;
>    EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
>    EFI_GUID                         *Class;
> @@ -678,7 +679,28 @@ BootDiscoveryPolicyHandler (
>      return Status;
>    }
>
> -  EfiBootManagerRefreshAllBootOption();
> +  //
> +  // Refresh Boot Options if Boot Discovery Policy has been changed
> +  //
> +  Size = sizeof (DiscoveryPolicyOld);
> +  Status = gRT->GetVariable (
> +                  BOOT_DISCOVERY_POLICY_OLD_VAR,
> +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> +                  NULL,
> +                  &Size,
> +                  &DiscoveryPolicyOld
> +                  );
> +  if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy))
> {
> +    EfiBootManagerRefreshAllBootOption();
> +
> +    Status = gRT->SetVariable (
> +                    BOOT_DISCOVERY_POLICY_OLD_VAR,
> +                    &gBootDiscoveryPolicyMgrFormsetGuid,
> +                    EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS,
> +                    sizeof (DiscoveryPolicyOld),
> +                    &DiscoveryPolicy
> +                    );
> +  }
>
>    return EFI_SUCCESS;
>  }
> --
> 2.25.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79509): https://edk2.groups.io/g/devel/message/79509
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Mario Bălănică 2 years, 8 months ago
Tested-by: Mario Bălănică <mariobalanica02@gmail.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79687): https://edk2.groups.io/g/devel/message/79687
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Ard Biesheuvel 2 years, 7 months ago
On Wed, 18 Aug 2021 at 09:38, Grzegorz Bernacki <gjb@semihalf.com> wrote:
>
> This patch adds checks if Boot Discovery Policy has been
> changed. Only in that case EfiBootManagerRefreshAllBootOption()
> should be called.
>
> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>

To be honest, I'm not a fan of this bodge. Can you explain why we need
two separate EFI variables to keep track of this state?


> ---
>  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24 +++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index c8305ce4f5..378ba0ebf4 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -617,6 +617,7 @@ BootDiscoveryPolicyHandler (
>  {
>    EFI_STATUS                       Status;
>    UINT32                           DiscoveryPolicy;
> +  UINT32                           DiscoveryPolicyOld;
>    UINTN                            Size;
>    EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
>    EFI_GUID                         *Class;
> @@ -678,7 +679,28 @@ BootDiscoveryPolicyHandler (
>      return Status;
>    }
>
> -  EfiBootManagerRefreshAllBootOption();
> +  //
> +  // Refresh Boot Options if Boot Discovery Policy has been changed
> +  //
> +  Size = sizeof (DiscoveryPolicyOld);
> +  Status = gRT->GetVariable (
> +                  BOOT_DISCOVERY_POLICY_OLD_VAR,
> +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> +                  NULL,
> +                  &Size,
> +                  &DiscoveryPolicyOld
> +                  );
> +  if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
> +    EfiBootManagerRefreshAllBootOption();
> +
> +    Status = gRT->SetVariable (
> +                    BOOT_DISCOVERY_POLICY_OLD_VAR,
> +                    &gBootDiscoveryPolicyMgrFormsetGuid,
> +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> +                    sizeof (DiscoveryPolicyOld),
> +                    &DiscoveryPolicy
> +                    );
> +  }
>
>    return EFI_SUCCESS;
>  }
> --
> 2.25.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80010): https://edk2.groups.io/g/devel/message/80010
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Grzegorz Bernacki 2 years, 7 months ago
Hi Ard,

The problem is that for some reason EfiBootManagerRefreshAllBootOption
() overwrites BootOrder. For example, if USB bootable device is
removed from the top and added at the bottom of the list.
I will debug the problem further, however this patch is needed to
retain correct boot order unless I find the root cause of the problem.
thanks,
greg

wt., 31 sie 2021 o 14:11 Ard Biesheuvel <ardb@kernel.org> napisał(a):
>
> On Wed, 18 Aug 2021 at 09:38, Grzegorz Bernacki <gjb@semihalf.com> wrote:
> >
> > This patch adds checks if Boot Discovery Policy has been
> > changed. Only in that case EfiBootManagerRefreshAllBootOption()
> > should be called.
> >
> > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
>
> To be honest, I'm not a fan of this bodge. Can you explain why we need
> two separate EFI variables to keep track of this state?
>
>
> > ---
> >  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24 +++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > index c8305ce4f5..378ba0ebf4 100644
> > --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > @@ -617,6 +617,7 @@ BootDiscoveryPolicyHandler (
> >  {
> >    EFI_STATUS                       Status;
> >    UINT32                           DiscoveryPolicy;
> > +  UINT32                           DiscoveryPolicyOld;
> >    UINTN                            Size;
> >    EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
> >    EFI_GUID                         *Class;
> > @@ -678,7 +679,28 @@ BootDiscoveryPolicyHandler (
> >      return Status;
> >    }
> >
> > -  EfiBootManagerRefreshAllBootOption();
> > +  //
> > +  // Refresh Boot Options if Boot Discovery Policy has been changed
> > +  //
> > +  Size = sizeof (DiscoveryPolicyOld);
> > +  Status = gRT->GetVariable (
> > +                  BOOT_DISCOVERY_POLICY_OLD_VAR,
> > +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> > +                  NULL,
> > +                  &Size,
> > +                  &DiscoveryPolicyOld
> > +                  );
> > +  if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
> > +    EfiBootManagerRefreshAllBootOption();
> > +
> > +    Status = gRT->SetVariable (
> > +                    BOOT_DISCOVERY_POLICY_OLD_VAR,
> > +                    &gBootDiscoveryPolicyMgrFormsetGuid,
> > +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> > +                    sizeof (DiscoveryPolicyOld),
> > +                    &DiscoveryPolicy
> > +                    );
> > +  }
> >
> >    return EFI_SUCCESS;
> >  }
> > --
> > 2.25.1
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80064): https://edk2.groups.io/g/devel/message/80064
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms PATCH] Platform/RaspberryPi: Check for Boot Discovery Policy change.
Posted by Ard Biesheuvel 2 years, 7 months ago
On Wed, 1 Sept 2021 at 10:45, Grzegorz Bernacki <gjb@semihalf.com> wrote:
>
> Hi Ard,
>
> The problem is that for some reason EfiBootManagerRefreshAllBootOption
> () overwrites BootOrder. For example, if USB bootable device is
> removed from the top and added at the bottom of the list.
> I will debug the problem further, however this patch is needed to
> retain correct boot order unless I find the root cause of the problem.

Thanks for the explanation.

I would prefer to find the root cause first. Pasting this workaround
all over the place is really not the right way to deal with this.


>
> wt., 31 sie 2021 o 14:11 Ard Biesheuvel <ardb@kernel.org> napisał(a):
> >
> > On Wed, 18 Aug 2021 at 09:38, Grzegorz Bernacki <gjb@semihalf.com> wrote:
> > >
> > > This patch adds checks if Boot Discovery Policy has been
> > > changed. Only in that case EfiBootManagerRefreshAllBootOption()
> > > should be called.
> > >
> > > Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> >
> > To be honest, I'm not a fan of this bodge. Can you explain why we need
> > two separate EFI variables to keep track of this state?
> >
> >
> > > ---
> > >  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 24 +++++++++++++++++++-
> > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > > index c8305ce4f5..378ba0ebf4 100644
> > > --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> > > @@ -617,6 +617,7 @@ BootDiscoveryPolicyHandler (
> > >  {
> > >    EFI_STATUS                       Status;
> > >    UINT32                           DiscoveryPolicy;
> > > +  UINT32                           DiscoveryPolicyOld;
> > >    UINTN                            Size;
> > >    EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
> > >    EFI_GUID                         *Class;
> > > @@ -678,7 +679,28 @@ BootDiscoveryPolicyHandler (
> > >      return Status;
> > >    }
> > >
> > > -  EfiBootManagerRefreshAllBootOption();
> > > +  //
> > > +  // Refresh Boot Options if Boot Discovery Policy has been changed
> > > +  //
> > > +  Size = sizeof (DiscoveryPolicyOld);
> > > +  Status = gRT->GetVariable (
> > > +                  BOOT_DISCOVERY_POLICY_OLD_VAR,
> > > +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> > > +                  NULL,
> > > +                  &Size,
> > > +                  &DiscoveryPolicyOld
> > > +                  );
> > > +  if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
> > > +    EfiBootManagerRefreshAllBootOption();
> > > +
> > > +    Status = gRT->SetVariable (
> > > +                    BOOT_DISCOVERY_POLICY_OLD_VAR,
> > > +                    &gBootDiscoveryPolicyMgrFormsetGuid,
> > > +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> > > +                    sizeof (DiscoveryPolicyOld),
> > > +                    &DiscoveryPolicy
> > > +                    );
> > > +  }
> > >
> > >    return EFI_SUCCESS;
> > >  }
> > > --
> > > 2.25.1
> > >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80065): https://edk2.groups.io/g/devel/message/80065
Mute This Topic: https://groups.io/mt/84967553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-