> v2:
> a. Fix a typo in expression in the macro ACCESS_PAGE0_CODE
> b. Fix GCC49 build error
ue 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>
Cc: Ruiyu Ni <ruiyu.ni@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..6a5f5464e7 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_; \
+ \
+ Desc_.Attributes = 0; \
+ Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_); \
+ ASSERT_EFI_ERROR (Status_); \
+ if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) { \
+ Status_ = gDS->SetMemorySpaceAttributes ( \
+ 0, \
+ EFI_PAGES_TO_SIZE(1), \
+ Desc_.Attributes & ~(UINT64)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
On 12/7/2017 1:40 PM, Jian J Wang wrote:
>> v2:
>> a. Fix a typo in expression in the macro ACCESS_PAGE0_CODE
>> b. Fix GCC49 build error
>
> ue 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>
> Cc: Ruiyu Ni <ruiyu.ni@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..6a5f5464e7 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_; \
> + \
> + Desc_.Attributes = 0; \
> + Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_); \
> + ASSERT_EFI_ERROR (Status_); \
> + if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) { \
> + Status_ = gDS->SetMemorySpaceAttributes ( \
> + 0, \
> + EFI_PAGES_TO_SIZE(1), \
> + Desc_.Attributes & ~(UINT64)EFI_MEMORY_RP \
> + ); \
> + ASSERT_EFI_ERROR (Status_); \
> + } \
> + \
> + statements; \
It's better to surrounded statements with {}.
So that when statements contains variable declaration, C compiler
doesn't complain.
> + \
> + 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
>
With the above suggested changes,
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Thanks for the comment. The {} will be added.
Sincerely,
Jian
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Thursday, December 07, 2017 3:00 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to
> guarantee page 0 access
>
> On 12/7/2017 1:40 PM, Jian J Wang wrote:
> >> v2:
> >> a. Fix a typo in expression in the macro ACCESS_PAGE0_CODE
> >> b. Fix GCC49 build error
> >
> > ue 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>
> > Cc: Ruiyu Ni <ruiyu.ni@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..6a5f5464e7 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_; \
> > + \
> > + Desc_.Attributes = 0; \
> > + Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_); \
> > + ASSERT_EFI_ERROR (Status_); \
> > + if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) { \
> > + Status_ = gDS->SetMemorySpaceAttributes ( \
> > + 0, \
> > + EFI_PAGES_TO_SIZE(1), \
> > + Desc_.Attributes & ~(UINT64)EFI_MEMORY_RP \
> > + ); \
> > + ASSERT_EFI_ERROR (Status_); \
> > + } \
> > + \
> > + statements; \
>
> It's better to surrounded statements with {}.
> So that when statements contains variable declaration, C compiler
> doesn't complain.
>
> > + \
> > + 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
> >
>
> With the above suggested changes,
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>
> --
> Thanks,
> Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2025 Red Hat, Inc.