* [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
@ 2019-01-21 15:16 Ruiyu Ni
2019-01-22 1:17 ` Chiu, Chasel
2019-02-19 5:33 ` Dong, Eric
0 siblings, 2 replies; 3+ messages in thread
From: Ruiyu Ni @ 2019-01-21 15:16 UTC (permalink / raw)
To: edk2-devel; +Cc: Ray Ni, Eric Dong, Chasel Chiu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481
Today's MtrrLib contains a bug, for example:
when the original cache setting is WB for [0xF_0000, 0xF_8000) and,
a new request to set [0xF_0000, 0xF_4000) to WP,
the cache setting for [0xF_4000, 0xF_8000) is reset to UC.
The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the
WorkingFixedSettings doesn't contain the actual MSR value stored in
hardware, but when writing the fixed MTRRs, the code logic assumes
WorkingFixedSettings contains the actual MSR value.
The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to
calculate the correct ClearMasks[] and OrMasks[], and use them
directly when writing the fixed MTRRs.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
---
UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +++++++++-------------------
1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 086f7ad8f0..2cf7d092e8 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -5,7 +5,7 @@
Most of services in this library instance are suggested to be invoked by BSP only,
except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to APs.
- Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges (
Set the below-1MB memory attribute to fixed MTRR buffer.
Modified flag array indicates which fixed MTRR is modified.
- @param [in, out] FixedSettings Fixed MTRR buffer.
- @param [out] Modified Flag array indicating which MTRR is modified.
+ @param [in, out] ClearMasks The bits to clear in the fixed MTRR MSR.
+ @param [in, out] OrMasks The bits to set in the fixed MTRR MSR.
@param [in] BaseAddress Base address.
@param [in] Length Length.
@param [in] Type Memory type.
@@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges (
**/
RETURN_STATUS
MtrrLibSetBelow1MBMemoryAttribute (
- IN OUT MTRR_FIXED_SETTINGS *FixedSettings,
- OUT BOOLEAN *Modified,
+ IN OUT UINT64 *ClearMasks,
+ IN OUT UINT64 *OrMasks,
IN PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
IN MTRR_MEMORY_CACHE_TYPE Type
@@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute (
UINT32 MsrIndex;
UINT64 ClearMask;
UINT64 OrMask;
- UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
- UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
- BOOLEAN LocalModified[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
ASSERT (BaseAddress < BASE_1MB);
- SetMem (LocalModified, sizeof (LocalModified), FALSE);
-
- //
- // (Value & ~0 | 0) still equals to (Value)
- //
- SetMem (ClearMasks, sizeof (ClearMasks), 0);
- SetMem (OrMasks, sizeof (OrMasks), 0);
-
MsrIndex = (UINT32)-1;
while ((BaseAddress < BASE_1MB) && (Length != 0)) {
Status = MtrrLibProgramFixedMtrr (Type, &BaseAddress, &Length, &MsrIndex, &ClearMask, &OrMask);
if (RETURN_ERROR (Status)) {
return Status;
}
- ClearMasks[MsrIndex] = ClearMask;
- OrMasks[MsrIndex] = OrMask;
- Modified[MsrIndex] = TRUE;
- LocalModified[MsrIndex] = TRUE;
- }
-
- for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable); MsrIndex++) {
- if (LocalModified[MsrIndex]) {
- FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] & ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex];
- }
+ ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask;
+ OrMasks[MsrIndex] = (OrMasks[MsrIndex] & ~ClearMask) | OrMask;
}
return RETURN_SUCCESS;
}
@@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings (
MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE (MtrrSetting->Variables.Mtrr)];
BOOLEAN VariableSettingModified[ARRAY_SIZE (MtrrSetting->Variables.Mtrr)];
- BOOLEAN FixedSettingsModified[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
- MTRR_FIXED_SETTINGS WorkingFixedSettings;
+ UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
+ UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
MTRR_CONTEXT MtrrContext;
BOOLEAN MtrrContextValid;
@@ -2226,10 +2207,6 @@ MtrrSetMemoryAttributesInMtrrSettings (
// TRUE indicating the accordingly Variable setting needs modificaiton in OriginalVariableMtrr.
//
SetMem (VariableSettingModified, ARRAY_SIZE (VariableSettingModified), FALSE);
- //
- // TRUE indicating the accordingly Fixed setting needs modification in WorkingFixedSettings.
- //
- SetMem (FixedSettingsModified, ARRAY_SIZE (FixedSettingsModified), FALSE);
//
// TRUE indicating the caller requests to set variable MTRRs.
@@ -2405,14 +2382,17 @@ MtrrSetMemoryAttributesInMtrrSettings (
//
// 3. Apply the below-1MB memory attribute settings.
//
- ZeroMem (WorkingFixedSettings.Mtrr, sizeof (WorkingFixedSettings.Mtrr));
+ // (Value & ~0 | 0) still equals to (Value)
+ //
+ ZeroMem (ClearMasks, sizeof (ClearMasks));
+ ZeroMem (OrMasks, sizeof (OrMasks));
for (Index = 0; Index < RangeCount; Index++) {
if (Ranges[Index].BaseAddress >= BASE_1MB) {
continue;
}
Status = MtrrLibSetBelow1MBMemoryAttribute (
- &WorkingFixedSettings, FixedSettingsModified,
+ ClearMasks, OrMasks,
Ranges[Index].BaseAddress, Ranges[Index].Length, Ranges[Index].Type
);
if (RETURN_ERROR (Status)) {
@@ -2424,19 +2404,16 @@ MtrrSetMemoryAttributesInMtrrSettings (
//
// 4. Write fixed MTRRs that have been modified
//
- for (Index = 0; Index < ARRAY_SIZE (FixedSettingsModified); Index++) {
- if (FixedSettingsModified[Index]) {
+ for (Index = 0; Index < ARRAY_SIZE (ClearMasks); Index++) {
+ if (ClearMasks[Index] != 0) {
if (MtrrSetting != NULL) {
- MtrrSetting->Fixed.Mtrr[Index] = WorkingFixedSettings.Mtrr[Index];
+ MtrrSetting->Fixed.Mtrr[Index] = (MtrrSetting->Fixed.Mtrr[Index] & ~ClearMasks[Index]) | OrMasks[Index];
} else {
if (!MtrrContextValid) {
MtrrLibPreMtrrChange (&MtrrContext);
MtrrContextValid = TRUE;
}
- AsmWriteMsr64 (
- mMtrrLibFixedMtrrTable[Index].Msr,
- WorkingFixedSettings.Mtrr[Index]
- );
+ AsmMsrAndThenOr64 (mMtrrLibFixedMtrrTable[Index].Msr, ~ClearMasks[Index], OrMasks[Index]);
}
}
}
--
2.20.1.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
2019-01-21 15:16 [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC Ruiyu Ni
@ 2019-01-22 1:17 ` Chiu, Chasel
2019-02-19 5:33 ` Dong, Eric
1 sibling, 0 replies; 3+ messages in thread
From: Chiu, Chasel @ 2019-01-22 1:17 UTC (permalink / raw)
To: Ni, Ray, edk2-devel@lists.01.org; +Cc: Dong, Eric
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Ni, Ray
> Sent: Monday, January 21, 2019 11:17 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>
> Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory
> <1MB to UC
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481
>
> Today's MtrrLib contains a bug, for example:
> when the original cache setting is WB for [0xF_0000, 0xF_8000) and, a new
> request to set [0xF_0000, 0xF_4000) to WP, the cache setting for [0xF_4000,
> 0xF_8000) is reset to UC.
>
> The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the
> WorkingFixedSettings doesn't contain the actual MSR value stored in hardware,
> but when writing the fixed MTRRs, the code logic assumes WorkingFixedSettings
> contains the actual MSR value.
>
> The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to calculate
> the correct ClearMasks[] and OrMasks[], and use them directly when writing the
> fixed MTRRs.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +++++++++-------------------
> 1 file changed, 18 insertions(+), 41 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 086f7ad8f0..2cf7d092e8 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -5,7 +5,7 @@
> Most of services in this library instance are suggested to be invoked by BSP
> only,
> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to APs.
>
> - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2008 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be found
> at @@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges (
> Set the below-1MB memory attribute to fixed MTRR buffer.
> Modified flag array indicates which fixed MTRR is modified.
>
> - @param [in, out] FixedSettings Fixed MTRR buffer.
> - @param [out] Modified Flag array indicating which MTRR is modified.
> + @param [in, out] ClearMasks The bits to clear in the fixed MTRR MSR.
> + @param [in, out] OrMasks The bits to set in the fixed MTRR MSR.
> @param [in] BaseAddress Base address.
> @param [in] Length Length.
> @param [in] Type Memory type.
> @@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges ( **/ RETURN_STATUS
> MtrrLibSetBelow1MBMemoryAttribute (
> - IN OUT MTRR_FIXED_SETTINGS *FixedSettings,
> - OUT BOOLEAN *Modified,
> + IN OUT UINT64 *ClearMasks,
> + IN OUT UINT64 *OrMasks,
> IN PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> IN MTRR_MEMORY_CACHE_TYPE Type
> @@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute (
> UINT32 MsrIndex;
> UINT64 ClearMask;
> UINT64 OrMask;
> - UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - BOOLEAN LocalModified[ARRAY_SIZE
> (mMtrrLibFixedMtrrTable)];
>
> ASSERT (BaseAddress < BASE_1MB);
>
> - SetMem (LocalModified, sizeof (LocalModified), FALSE);
> -
> - //
> - // (Value & ~0 | 0) still equals to (Value)
> - //
> - SetMem (ClearMasks, sizeof (ClearMasks), 0);
> - SetMem (OrMasks, sizeof (OrMasks), 0);
> -
> MsrIndex = (UINT32)-1;
> while ((BaseAddress < BASE_1MB) && (Length != 0)) {
> Status = MtrrLibProgramFixedMtrr (Type, &BaseAddress, &Length,
> &MsrIndex, &ClearMask, &OrMask);
> if (RETURN_ERROR (Status)) {
> return Status;
> }
> - ClearMasks[MsrIndex] = ClearMask;
> - OrMasks[MsrIndex] = OrMask;
> - Modified[MsrIndex] = TRUE;
> - LocalModified[MsrIndex] = TRUE;
> - }
> -
> - for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable);
> MsrIndex++) {
> - if (LocalModified[MsrIndex]) {
> - FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] &
> ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex];
> - }
> + ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask;
> + OrMasks[MsrIndex] = (OrMasks[MsrIndex] & ~ClearMask) | OrMask;
> }
> return RETURN_SUCCESS;
> }
> @@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings (
> MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE
> (MtrrSetting->Variables.Mtrr)];
> BOOLEAN VariableSettingModified[ARRAY_SIZE
> (MtrrSetting->Variables.Mtrr)];
>
> - BOOLEAN FixedSettingsModified[ARRAY_SIZE
> (mMtrrLibFixedMtrrTable)];
> - MTRR_FIXED_SETTINGS WorkingFixedSettings;
> + UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> + UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
>
> MTRR_CONTEXT MtrrContext;
> BOOLEAN MtrrContextValid;
> @@ -2226,10 +2207,6 @@ MtrrSetMemoryAttributesInMtrrSettings (
> // TRUE indicating the accordingly Variable setting needs modificaiton in
> OriginalVariableMtrr.
> //
> SetMem (VariableSettingModified, ARRAY_SIZE (VariableSettingModified),
> FALSE);
> - //
> - // TRUE indicating the accordingly Fixed setting needs modification in
> WorkingFixedSettings.
> - //
> - SetMem (FixedSettingsModified, ARRAY_SIZE (FixedSettingsModified),
> FALSE);
>
> //
> // TRUE indicating the caller requests to set variable MTRRs.
> @@ -2405,14 +2382,17 @@ MtrrSetMemoryAttributesInMtrrSettings (
> //
> // 3. Apply the below-1MB memory attribute settings.
> //
> - ZeroMem (WorkingFixedSettings.Mtrr, sizeof (WorkingFixedSettings.Mtrr));
> + // (Value & ~0 | 0) still equals to (Value) // ZeroMem (ClearMasks,
> + sizeof (ClearMasks)); ZeroMem (OrMasks, sizeof (OrMasks));
> for (Index = 0; Index < RangeCount; Index++) {
> if (Ranges[Index].BaseAddress >= BASE_1MB) {
> continue;
> }
>
> Status = MtrrLibSetBelow1MBMemoryAttribute (
> - &WorkingFixedSettings, FixedSettingsModified,
> + ClearMasks, OrMasks,
> Ranges[Index].BaseAddress, Ranges[Index].Length,
> Ranges[Index].Type
> );
> if (RETURN_ERROR (Status)) {
> @@ -2424,19 +2404,16 @@ MtrrSetMemoryAttributesInMtrrSettings (
> //
> // 4. Write fixed MTRRs that have been modified
> //
> - for (Index = 0; Index < ARRAY_SIZE (FixedSettingsModified); Index++) {
> - if (FixedSettingsModified[Index]) {
> + for (Index = 0; Index < ARRAY_SIZE (ClearMasks); Index++) {
> + if (ClearMasks[Index] != 0) {
> if (MtrrSetting != NULL) {
> - MtrrSetting->Fixed.Mtrr[Index] = WorkingFixedSettings.Mtrr[Index];
> + MtrrSetting->Fixed.Mtrr[Index] =
> + (MtrrSetting->Fixed.Mtrr[Index] & ~ClearMasks[Index]) |
> + OrMasks[Index];
> } else {
> if (!MtrrContextValid) {
> MtrrLibPreMtrrChange (&MtrrContext);
> MtrrContextValid = TRUE;
> }
> - AsmWriteMsr64 (
> - mMtrrLibFixedMtrrTable[Index].Msr,
> - WorkingFixedSettings.Mtrr[Index]
> - );
> + AsmMsrAndThenOr64 (mMtrrLibFixedMtrrTable[Index].Msr,
> + ~ClearMasks[Index], OrMasks[Index]);
> }
> }
> }
> --
> 2.20.1.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
2019-01-21 15:16 [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC Ruiyu Ni
2019-01-22 1:17 ` Chiu, Chasel
@ 2019-02-19 5:33 ` Dong, Eric
1 sibling, 0 replies; 3+ messages in thread
From: Dong, Eric @ 2019-02-19 5:33 UTC (permalink / raw)
To: Ni, Ray, edk2-devel@lists.01.org
Hi Ray,
Sorry for later response.
Reviewed-by: Eric Dong <eric.dong@intel.com>
Thanks,
Eric
> -----Original Message-----
> From: Ni, Ray
> Sent: Monday, January 21, 2019 11:17 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set
> memory <1MB to UC
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481
>
> Today's MtrrLib contains a bug, for example:
> when the original cache setting is WB for [0xF_0000, 0xF_8000) and, a new
> request to set [0xF_0000, 0xF_4000) to WP, the cache setting for [0xF_4000,
> 0xF_8000) is reset to UC.
>
> The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the
> WorkingFixedSettings doesn't contain the actual MSR value stored in
> hardware, but when writing the fixed MTRRs, the code logic assumes
> WorkingFixedSettings contains the actual MSR value.
>
> The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to
> calculate the correct ClearMasks[] and OrMasks[], and use them directly
> when writing the fixed MTRRs.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +++++++++-------------------
> 1 file changed, 18 insertions(+), 41 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 086f7ad8f0..2cf7d092e8 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -5,7 +5,7 @@
> Most of services in this library instance are suggested to be invoked by BSP
> only,
> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to
> APs.
>
> - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2008 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at @@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges (
> Set the below-1MB memory attribute to fixed MTRR buffer.
> Modified flag array indicates which fixed MTRR is modified.
>
> - @param [in, out] FixedSettings Fixed MTRR buffer.
> - @param [out] Modified Flag array indicating which MTRR is modified.
> + @param [in, out] ClearMasks The bits to clear in the fixed MTRR MSR.
> + @param [in, out] OrMasks The bits to set in the fixed MTRR MSR.
> @param [in] BaseAddress Base address.
> @param [in] Length Length.
> @param [in] Type Memory type.
> @@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges ( **/ RETURN_STATUS
> MtrrLibSetBelow1MBMemoryAttribute (
> - IN OUT MTRR_FIXED_SETTINGS *FixedSettings,
> - OUT BOOLEAN *Modified,
> + IN OUT UINT64 *ClearMasks,
> + IN OUT UINT64 *OrMasks,
> IN PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> IN MTRR_MEMORY_CACHE_TYPE Type
> @@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute (
> UINT32 MsrIndex;
> UINT64 ClearMask;
> UINT64 OrMask;
> - UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - BOOLEAN LocalModified[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
>
> ASSERT (BaseAddress < BASE_1MB);
>
> - SetMem (LocalModified, sizeof (LocalModified), FALSE);
> -
> - //
> - // (Value & ~0 | 0) still equals to (Value)
> - //
> - SetMem (ClearMasks, sizeof (ClearMasks), 0);
> - SetMem (OrMasks, sizeof (OrMasks), 0);
> -
> MsrIndex = (UINT32)-1;
> while ((BaseAddress < BASE_1MB) && (Length != 0)) {
> Status = MtrrLibProgramFixedMtrr (Type, &BaseAddress, &Length,
> &MsrIndex, &ClearMask, &OrMask);
> if (RETURN_ERROR (Status)) {
> return Status;
> }
> - ClearMasks[MsrIndex] = ClearMask;
> - OrMasks[MsrIndex] = OrMask;
> - Modified[MsrIndex] = TRUE;
> - LocalModified[MsrIndex] = TRUE;
> - }
> -
> - for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable);
> MsrIndex++) {
> - if (LocalModified[MsrIndex]) {
> - FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] &
> ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex];
> - }
> + ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask;
> + OrMasks[MsrIndex] = (OrMasks[MsrIndex] & ~ClearMask) | OrMask;
> }
> return RETURN_SUCCESS;
> }
> @@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings (
> MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE
> (MtrrSetting->Variables.Mtrr)];
> BOOLEAN VariableSettingModified[ARRAY_SIZE (MtrrSetting-
> >Variables.Mtrr)];
>
> - BOOLEAN FixedSettingsModified[ARRAY_SIZE
> (mMtrrLibFixedMtrrTable)];
> - MTRR_FIXED_SETTINGS WorkingFixedSettings;
> + UINT64 ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> + UINT64 OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
>
> MTRR_CONTEXT MtrrContext;
> BOOLEAN MtrrContextValid;
> @@ -2226,10 +2207,6 @@ MtrrSetMemoryAttributesInMtrrSettings (
> // TRUE indicating the accordingly Variable setting needs modificaiton in
> OriginalVariableMtrr.
> //
> SetMem (VariableSettingModified, ARRAY_SIZE (VariableSettingModified),
> FALSE);
> - //
> - // TRUE indicating the accordingly Fixed setting needs modification in
> WorkingFixedSettings.
> - //
> - SetMem (FixedSettingsModified, ARRAY_SIZE (FixedSettingsModified),
> FALSE);
>
> //
> // TRUE indicating the caller requests to set variable MTRRs.
> @@ -2405,14 +2382,17 @@ MtrrSetMemoryAttributesInMtrrSettings (
> //
> // 3. Apply the below-1MB memory attribute settings.
> //
> - ZeroMem (WorkingFixedSettings.Mtrr, sizeof
> (WorkingFixedSettings.Mtrr));
> + // (Value & ~0 | 0) still equals to (Value) // ZeroMem (ClearMasks,
> + sizeof (ClearMasks)); ZeroMem (OrMasks, sizeof (OrMasks));
> for (Index = 0; Index < RangeCount; Index++) {
> if (Ranges[Index].BaseAddress >= BASE_1MB) {
> continue;
> }
>
> Status = MtrrLibSetBelow1MBMemoryAttribute (
> - &WorkingFixedSettings, FixedSettingsModified,
> + ClearMasks, OrMasks,
> Ranges[Index].BaseAddress, Ranges[Index].Length,
> Ranges[Index].Type
> );
> if (RETURN_ERROR (Status)) {
> @@ -2424,19 +2404,16 @@ MtrrSetMemoryAttributesInMtrrSettings (
> //
> // 4. Write fixed MTRRs that have been modified
> //
> - for (Index = 0; Index < ARRAY_SIZE (FixedSettingsModified); Index++) {
> - if (FixedSettingsModified[Index]) {
> + for (Index = 0; Index < ARRAY_SIZE (ClearMasks); Index++) {
> + if (ClearMasks[Index] != 0) {
> if (MtrrSetting != NULL) {
> - MtrrSetting->Fixed.Mtrr[Index] = WorkingFixedSettings.Mtrr[Index];
> + MtrrSetting->Fixed.Mtrr[Index] =
> + (MtrrSetting->Fixed.Mtrr[Index] & ~ClearMasks[Index]) |
> + OrMasks[Index];
> } else {
> if (!MtrrContextValid) {
> MtrrLibPreMtrrChange (&MtrrContext);
> MtrrContextValid = TRUE;
> }
> - AsmWriteMsr64 (
> - mMtrrLibFixedMtrrTable[Index].Msr,
> - WorkingFixedSettings.Mtrr[Index]
> - );
> + AsmMsrAndThenOr64 (mMtrrLibFixedMtrrTable[Index].Msr,
> + ~ClearMasks[Index], OrMasks[Index]);
> }
> }
> }
> --
> 2.20.1.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-19 5:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-21 15:16 [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC Ruiyu Ni
2019-01-22 1:17 ` Chiu, Chasel
2019-02-19 5:33 ` Dong, Eric
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox