[edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Sunny Wang]

The RPi does not support storing UEFI NV variables at runtime. For now,
gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
proper error and would cause FWTS failures. Therefore, this patch is
to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.

For more information, please check the issues below:
   -https://github.com/pftf/RPi4/issues/6
   -https://github.com/pftf/RPi4/issues/93
   -https://github.com/pftf/RPi4/issues/163

I also tested this with the ACS 3.0 FWTS. All the failures
reported in issue 93 and 163 can be fixed by this patch.

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Pete Batard <pete@akeo.ie>
Cc: Leif Lindholm <leif@nuviainc.com>

Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
 .../Drivers/VarBlockServiceDxe/VarBlockService.c     | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
index 572309439a..16d4d4f178 100644
--- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
+++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
@@ -2,6 +2,7 @@
  *
  *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
+ *  Copyright (c) 2021, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -596,6 +597,7 @@ FvbProtocolWrite (
     EFI_DEVICE_ERROR      - The block device is not functioning correctly and
                             could not be written
     EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
+    EFI_UNSUPPORTED         This function is not supported at runtime
 
 --*/
 {
@@ -605,6 +607,16 @@ FvbProtocolWrite (
   EFI_STATUS Status;
   EFI_STATUS ReturnStatus;
 
+  //
+  // The current variables support relies on modifying RPI_EFI.FD on SD
+  // card, which works fine at boot time. However, at runtime, the SD 
+  // controller is exposed via ACPI and subsequently owned by the OS.
+  // Therefore, we need to direclty return EFI_UNSUPPORTED.
+  //
+  if (EfiAtRuntime ()) {
+   return EFI_UNSUPPORTED;
+  }
+
   //
   // Check for invalid conditions.
   //
-- 
2.31.0.windows.1

Re: [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 11:15, Sunny Wang <Sunny.Wang@arm.com> wrote:
>
> The RPi does not support storing UEFI NV variables at runtime. For now,
> gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
> proper error and would cause FWTS failures. Therefore, this patch is
> to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.
>
> For more information, please check the issues below:
>    -https://github.com/pftf/RPi4/issues/6
>    -https://github.com/pftf/RPi4/issues/93
>    -https://github.com/pftf/RPi4/issues/163
>
> I also tested this with the ACS 3.0 FWTS. All the failures
> reported in issue 93 and 163 can be fixed by this patch.
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Leif Lindholm <leif@nuviainc.com>
>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>

This looks ok to me, but we should also expose this fact via the
EFI_RT_PROPERTIES_TABLE, so that the OS can anticipate this result.

> ---
>  .../Drivers/VarBlockServiceDxe/VarBlockService.c     | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> index 572309439a..16d4d4f178 100644
> --- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> +++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> @@ -2,6 +2,7 @@
>   *
>   *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>   *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -596,6 +597,7 @@ FvbProtocolWrite (
>      EFI_DEVICE_ERROR      - The block device is not functioning correctly and
>                              could not be written
>      EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
> +    EFI_UNSUPPORTED         This function is not supported at runtime
>
>  --*/
>  {
> @@ -605,6 +607,16 @@ FvbProtocolWrite (
>    EFI_STATUS Status;
>    EFI_STATUS ReturnStatus;
>
> +  //
> +  // The current variables support relies on modifying RPI_EFI.FD on SD
> +  // card, which works fine at boot time. However, at runtime, the SD
> +  // controller is exposed via ACPI and subsequently owned by the OS.
> +  // Therefore, we need to direclty return EFI_UNSUPPORTED.
> +  //
> +  if (EfiAtRuntime ()) {
> +   return EFI_UNSUPPORTED;
> +  }
> +
>    //
>    // Check for invalid conditions.
>    //
> --
> 2.31.0.windows.1
>

Re: [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Sunny Wang]

Please ignore this patch. Making gRT->SetVariable at runtime return EFI_UNSUPPORTED would cause some OSes' installation failure/error.
I thought the latest OS may support UNSUPPORTED case, but it turned out still an error/failure. I checked both Ubuntu 21.04 and OpenSUSE 15.3 Leap, and both of them could not be smoothly installed.
I will take a further look into this and figure out a better solution.

Hi Ard,
Yeah, I'm also working on EFI_RT_PROPERTIES_TABLE, but ran into some problems. I'm still debugging it. Hope using EFI_RT_PROPERTIES_TABLE won't cause OS installation failure.

Best Regards,
Sunny Wang

-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Friday, July 23, 2021 11:10 PM
To: Sunny Wang <Sunny.Wang@arm.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Pete Batard <pete@akeo.ie>; Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

On Fri, 23 Jul 2021 at 11:15, Sunny Wang <Sunny.Wang@arm.com> wrote:
>
> The RPi does not support storing UEFI NV variables at runtime. For now,
> gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
> proper error and would cause FWTS failures. Therefore, this patch is
> to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.
>
> For more information, please check the issues below:
>    -https://github.com/pftf/RPi4/issues/6
>    -https://github.com/pftf/RPi4/issues/93
>    -https://github.com/pftf/RPi4/issues/163
>
> I also tested this with the ACS 3.0 FWTS. All the failures
> reported in issue 93 and 163 can be fixed by this patch.
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Leif Lindholm <leif@nuviainc.com>
>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>

This looks ok to me, but we should also expose this fact via the
EFI_RT_PROPERTIES_TABLE, so that the OS can anticipate this result.

> ---
>  .../Drivers/VarBlockServiceDxe/VarBlockService.c     | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> index 572309439a..16d4d4f178 100644
> --- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> +++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> @@ -2,6 +2,7 @@
>   *
>   *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>   *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -596,6 +597,7 @@ FvbProtocolWrite (
>      EFI_DEVICE_ERROR      - The block device is not functioning correctly and
>                              could not be written
>      EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
> +    EFI_UNSUPPORTED         This function is not supported at runtime
>
>  --*/
>  {
> @@ -605,6 +607,16 @@ FvbProtocolWrite (
>    EFI_STATUS Status;
>    EFI_STATUS ReturnStatus;
>
> +  //
> +  // The current variables support relies on modifying RPI_EFI.FD on SD
> +  // card, which works fine at boot time. However, at runtime, the SD
> +  // controller is exposed via ACPI and subsequently owned by the OS.
> +  // Therefore, we need to direclty return EFI_UNSUPPORTED.
> +  //
> +  if (EfiAtRuntime ()) {
> +   return EFI_UNSUPPORTED;
> +  }
> +
>    //
>    // Check for invalid conditions.
>    //
> --
> 2.31.0.windows.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.

Re: [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Ard Biesheuvel]

On Mon, 26 Jul 2021 at 09:18, Sunny Wang <Sunny.Wang@arm.com> wrote:
>
> Please ignore this patch. Making gRT->SetVariable at runtime return EFI_UNSUPPORTED would cause some OSes' installation failure/error.
> I thought the latest OS may support UNSUPPORTED case, but it turned out still an error/failure. I checked both Ubuntu 21.04 and OpenSUSE 15.3 Leap, and both of them could not be smoothly installed.
> I will take a further look into this and figure out a better solution.
>

ok

> Hi Ard,
> Yeah, I'm also working on EFI_RT_PROPERTIES_TABLE, but ran into some problems. I'm still debugging it. Hope using EFI_RT_PROPERTIES_TABLE won't cause OS installation failure.
>

ok


> Best Regards,
> Sunny Wang
>
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Friday, July 23, 2021 11:10 PM
> To: Sunny Wang <Sunny.Wang@arm.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Pete Batard <pete@akeo.ie>; Leif Lindholm <leif@nuviainc.com>
> Subject: Re: [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime
>
> On Fri, 23 Jul 2021 at 11:15, Sunny Wang <Sunny.Wang@arm.com> wrote:
> >
> > The RPi does not support storing UEFI NV variables at runtime. For now,
> > gRT->SetVariable at runtime returns EFI_OUT_OF_RESOURCES which is not a
> > proper error and would cause FWTS failures. Therefore, this patch is
> > to make gRT->SetVariable at runtime return EFI_UNSUPPORTED.
> >
> > For more information, please check the issues below:
> >    -https://github.com/pftf/RPi4/issues/6
> >    -https://github.com/pftf/RPi4/issues/93
> >    -https://github.com/pftf/RPi4/issues/163
> >
> > I also tested this with the ACS 3.0 FWTS. All the failures
> > reported in issue 93 and 163 can be fixed by this patch.
> >
> > Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> >
> > Signed-off-by: Sunny Wang <sunny.wang@arm.com>
>
> This looks ok to me, but we should also expose this fact via the
> EFI_RT_PROPERTIES_TABLE, so that the OS can anticipate this result.
>
> > ---
> >  .../Drivers/VarBlockServiceDxe/VarBlockService.c     | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> > index 572309439a..16d4d4f178 100644
> > --- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> > +++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
> > @@ -2,6 +2,7 @@
> >   *
> >   *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> >   *  Copyright (c) 2006-2014, Intel Corporation. All rights reserved.
> > + *  Copyright (c) 2021, ARM Limited. All rights reserved.
> >   *
> >   *  SPDX-License-Identifier: BSD-2-Clause-Patent
> >   *
> > @@ -596,6 +597,7 @@ FvbProtocolWrite (
> >      EFI_DEVICE_ERROR      - The block device is not functioning correctly and
> >                              could not be written
> >      EFI_INVALID_PARAMETER - NumBytes or Buffer are NULL
> > +    EFI_UNSUPPORTED         This function is not supported at runtime
> >
> >  --*/
> >  {
> > @@ -605,6 +607,16 @@ FvbProtocolWrite (
> >    EFI_STATUS Status;
> >    EFI_STATUS ReturnStatus;
> >
> > +  //
> > +  // The current variables support relies on modifying RPI_EFI.FD on SD
> > +  // card, which works fine at boot time. However, at runtime, the SD
> > +  // controller is exposed via ACPI and subsequently owned by the OS.
> > +  // Therefore, we need to direclty return EFI_UNSUPPORTED.
> > +  //
> > +  if (EfiAtRuntime ()) {
> > +   return EFI_UNSUPPORTED;
> > +  }
> > +
> >    //
> >    // Check for invalid conditions.
> >    //
> > --
> > 2.31.0.windows.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.

Re: [edk2-devel] [edk2-platform PATCH v1 1/1] Platform/RaspberryPi: Make SetVariable return EFI_UNSUPPORTED at runtime

[Tinh Nguyen]

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

Hi everyone,

I found my answer, Reclaim() function calls Fault Tolerant Write protocol, it is a boot-time service and not available on runtime

Maybe it is a dummy question, why do we support Fault Tolerant Write protocol in runtime?

Thanks a lot
Tinh Nguyen

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