[edk2] [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access

Jian J Wang posted 3 patches 7 years ago
There is a newer version of this series
[edk2] [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access
Posted by Jian J Wang 7 years ago
Due to the introduction of NULL pointer detection feature, page 0 will be
disabled if the feature is enabled, which will cause legacy code failed to
update legacy data in page 0. This macro is introduced to make sure the
page 0 is enabled before those code and restore the original status of it
afterwards.

Another reason to introduce this macro is to eliminate the dependency on
the PcdNullPointerDetectionPropertyMask. Because this is a new PCD, it
could cause some backward compatibility issue for some old packages.

This macro will simply check if the page 0 is disabled or not. If it's
disabled, it will enable it before code updating page 0 and disable it
afterwards. Otherwise, this macro will do nothing to page 0.

The usage of the macro will be look like (similar to DEBUG_CODE macro):

    ACCESS_PAGE0_CODE(
      {
          <code accessing page 0>
      }
    );

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 IntelFrameworkPkg/Include/Protocol/LegacyBios.h | 34 +++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
index 641f101bce..f77c92ba21 100644
--- a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
+++ b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
@@ -1518,6 +1518,40 @@ struct _EFI_LEGACY_BIOS_PROTOCOL {
   EFI_LEGACY_BIOS_BOOT_UNCONVENTIONAL_DEVICE  BootUnconventionalDevice;
 };
 
+//
+// Legacy BIOS needs to access memory in page 0 (0-4095), which is disabled if
+// NULL pointer detection feature is enabled. Following macro can be used to
+// enable/disable page 0 before/after accessing it.
+//
+#define ACCESS_PAGE0_CODE(statements)                           \
+  do {                                                          \
+    EFI_STATUS                            Status_;              \
+    EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc_;                \
+                                                                \
+    Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_);        \
+    if (!EFI_ERROR (Status_)) {                                 \
+      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
+        Status_ = gDS->SetMemorySpaceAttributes (               \
+                        0,                                      \
+                        EFI_PAGES_TO_SIZE(1),                   \
+                        Desc_.Attributes &= ~EFI_MEMORY_RP      \
+                        );                                      \
+        ASSERT_EFI_ERROR (Status_);                             \
+      }                                                         \
+                                                                \
+      statements;                                               \
+                                                                \
+      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
+        Status_ = gDS->SetMemorySpaceAttributes (               \
+                        0,                                      \
+                        EFI_PAGES_TO_SIZE(1),                   \
+                        Desc_.Attributes                        \
+                        );                                      \
+        ASSERT_EFI_ERROR (Status_);                             \
+      }                                                         \
+    }                                                           \
+  } while (FALSE)
+
 extern EFI_GUID gEfiLegacyBiosProtocolGuid;
 
 #endif
-- 
2.15.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access
Posted by Ni, Ruiyu 7 years ago
On 12/6/2017 3:31 PM, Jian J Wang wrote:
> Due to the introduction of NULL pointer detection feature, page 0 will be
> disabled if the feature is enabled, which will cause legacy code failed to
> update legacy data in page 0. This macro is introduced to make sure the
> page 0 is enabled before those code and restore the original status of it
> afterwards.
> 
> Another reason to introduce this macro is to eliminate the dependency on
> the PcdNullPointerDetectionPropertyMask. Because this is a new PCD, it
> could cause some backward compatibility issue for some old packages.
> 
> This macro will simply check if the page 0 is disabled or not. If it's
> disabled, it will enable it before code updating page 0 and disable it
> afterwards. Otherwise, this macro will do nothing to page 0.
> 
> The usage of the macro will be look like (similar to DEBUG_CODE macro):
> 
>      ACCESS_PAGE0_CODE(
>        {
>            <code accessing page 0>
>        }
>      );
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>   IntelFrameworkPkg/Include/Protocol/LegacyBios.h | 34 +++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> index 641f101bce..f77c92ba21 100644
> --- a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> +++ b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> @@ -1518,6 +1518,40 @@ struct _EFI_LEGACY_BIOS_PROTOCOL {
>     EFI_LEGACY_BIOS_BOOT_UNCONVENTIONAL_DEVICE  BootUnconventionalDevice;
>   };
>   
> +//
> +// Legacy BIOS needs to access memory in page 0 (0-4095), which is disabled if
> +// NULL pointer detection feature is enabled. Following macro can be used to
> +// enable/disable page 0 before/after accessing it.
> +//
> +#define ACCESS_PAGE0_CODE(statements)                           \
> +  do {                                                          \
> +    EFI_STATUS                            Status_;              \
> +    EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc_;                \
> +                                                                \
> +    Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_);        \
> +    if (!EFI_ERROR (Status_)) {                                 \
> +      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
> +        Status_ = gDS->SetMemorySpaceAttributes (               \
> +                        0,                                      \
> +                        EFI_PAGES_TO_SIZE(1),                   \
> +                        Desc_.Attributes &= ~EFI_MEMORY_RP      \

&= is used here so Desc_.Attributes is updated to have RP cleared.
Then the below if will be always FALSE.

> +                        );                                      \
> +        ASSERT_EFI_ERROR (Status_);                             \
> +      }                                                         \
> +                                                                \
> +      statements;                                               \
> +                                                                \
> +      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
> +        Status_ = gDS->SetMemorySpaceAttributes (               \
> +                        0,                                      \
> +                        EFI_PAGES_TO_SIZE(1),                   \
> +                        Desc_.Attributes                        \
> +                        );                                      \
> +        ASSERT_EFI_ERROR (Status_);                             \
> +      }                                                         \
> +    }                                                           \
> +  } while (FALSE)
> +
>   extern EFI_GUID gEfiLegacyBiosProtocolGuid;
>   
>   #endif
> 


-- 
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access
Posted by Wang, Jian J 7 years ago
Good catch. I think it's a typo not on purpose. Many thanks!

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, December 06, 2017 5:31 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro
> to guarantee page 0 access
> 
> On 12/6/2017 3:31 PM, Jian J Wang wrote:
> > Due to the introduction of NULL pointer detection feature, page 0 will be
> > disabled if the feature is enabled, which will cause legacy code failed to
> > update legacy data in page 0. This macro is introduced to make sure the
> > page 0 is enabled before those code and restore the original status of it
> > afterwards.
> >
> > Another reason to introduce this macro is to eliminate the dependency on
> > the PcdNullPointerDetectionPropertyMask. Because this is a new PCD, it
> > could cause some backward compatibility issue for some old packages.
> >
> > This macro will simply check if the page 0 is disabled or not. If it's
> > disabled, it will enable it before code updating page 0 and disable it
> > afterwards. Otherwise, this macro will do nothing to page 0.
> >
> > The usage of the macro will be look like (similar to DEBUG_CODE macro):
> >
> >      ACCESS_PAGE0_CODE(
> >        {
> >            <code accessing page 0>
> >        }
> >      );
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >   IntelFrameworkPkg/Include/Protocol/LegacyBios.h | 34
> +++++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> >
> > diff --git a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> > index 641f101bce..f77c92ba21 100644
> > --- a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> > +++ b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
> > @@ -1518,6 +1518,40 @@ struct _EFI_LEGACY_BIOS_PROTOCOL {
> >     EFI_LEGACY_BIOS_BOOT_UNCONVENTIONAL_DEVICE
> BootUnconventionalDevice;
> >   };
> >
> > +//
> > +// Legacy BIOS needs to access memory in page 0 (0-4095), which is disabled
> if
> > +// NULL pointer detection feature is enabled. Following macro can be used to
> > +// enable/disable page 0 before/after accessing it.
> > +//
> > +#define ACCESS_PAGE0_CODE(statements)                           \
> > +  do {                                                          \
> > +    EFI_STATUS                            Status_;              \
> > +    EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc_;                \
> > +                                                                \
> > +    Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_);        \
> > +    if (!EFI_ERROR (Status_)) {                                 \
> > +      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
> > +        Status_ = gDS->SetMemorySpaceAttributes (               \
> > +                        0,                                      \
> > +                        EFI_PAGES_TO_SIZE(1),                   \
> > +                        Desc_.Attributes &= ~EFI_MEMORY_RP      \
> 
> &= is used here so Desc_.Attributes is updated to have RP cleared.
> Then the below if will be always FALSE.
> 
> > +                        );                                      \
> > +        ASSERT_EFI_ERROR (Status_);                             \
> > +      }                                                         \
> > +                                                                \
> > +      statements;                                               \
> > +                                                                \
> > +      if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {            \
> > +        Status_ = gDS->SetMemorySpaceAttributes (               \
> > +                        0,                                      \
> > +                        EFI_PAGES_TO_SIZE(1),                   \
> > +                        Desc_.Attributes                        \
> > +                        );                                      \
> > +        ASSERT_EFI_ERROR (Status_);                             \
> > +      }                                                         \
> > +    }                                                           \
> > +  } while (FALSE)
> > +
> >   extern EFI_GUID gEfiLegacyBiosProtocolGuid;
> >
> >   #endif
> >
> 
> 
> --
> Thanks,
> Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel