From: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
To: Jian J Wang <jian.j.wang@intel.com>, edk2-devel@lists.01.org
Cc: Liming Gao <liming.gao@intel.com>,
Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [PATCH v2 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access
Date: Thu, 7 Dec 2017 15:00:24 +0800 [thread overview]
Message-ID: <288d4778-5c5b-6804-74b9-e86eb1d2a33d@Intel.com> (raw)
In-Reply-To: <20171207054049.18140-2-jian.j.wang@intel.com>
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
next prev parent reply other threads:[~2017-12-07 6:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-07 5:40 [PATCH v2 0/3] Remove dependency on PcdNullPointerDetectionPropertyMask Jian J Wang
2017-12-07 5:40 ` [PATCH v2 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access Jian J Wang
2017-12-07 7:00 ` Ni, Ruiyu [this message]
2017-12-07 8:03 ` Wang, Jian J
2017-12-07 5:40 ` [PATCH v2 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use macro to enable/disable page 0 Jian J Wang
2017-12-07 7:01 ` Ni, Ruiyu
2017-12-07 5:40 ` [PATCH v2 3/3] IntelFrameworkModulePkg/KeyboardDxe: " Jian J Wang
2017-12-07 7:01 ` Ni, Ruiyu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=288d4778-5c5b-6804-74b9-e86eb1d2a33d@Intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox