* [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
@ 2017-10-17 1:46 Ruiyu Ni
2017-10-17 2:03 ` Shi, Steven
2017-10-17 7:56 ` Laszlo Ersek
0 siblings, 2 replies; 5+ messages in thread
From: Ruiyu Ni @ 2017-10-17 1:46 UTC (permalink / raw)
To: edk2-devel; +Cc: Steven Shi, Laszlo Ersek
ARRAY_SIZE(Mtrrs->Variables.Mtrr) was used in
MtrrDebugPrintAllMtrrsWorker() to parse the MTRR registers.
Instead, the actual variable MTRR count should be used.
Otherwise, the uninitialized random data in MtrrSetting may cause
MtrrLibSetMemoryType() hang.
Steven Shi found this bug in QEMU when using Q35 chip.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Steven Shi <steven.shi@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 2fd1d0153e..cb22558103 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -2776,6 +2776,7 @@ MtrrDebugPrintAllMtrrsWorker (
UINTN RangeCount;
UINT64 MtrrValidBitsMask;
UINT64 MtrrValidAddressMask;
+ UINT32 VariableMtrrCount;
MTRR_MEMORY_RANGE Ranges[
ARRAY_SIZE (mMtrrLibFixedMtrrTable) * sizeof (UINT64) + 2 * ARRAY_SIZE (Mtrrs->Variables.Mtrr) + 1
];
@@ -2785,6 +2786,8 @@ MtrrDebugPrintAllMtrrsWorker (
return;
}
+ VariableMtrrCount = GetVariableMtrrCountWorker ();
+
if (MtrrSetting != NULL) {
Mtrrs = MtrrSetting;
} else {
@@ -2802,7 +2805,7 @@ MtrrDebugPrintAllMtrrsWorker (
DEBUG((DEBUG_CACHE, "Fixed MTRR[%02d] : %016lx\n", Index, Mtrrs->Fixed.Mtrr[Index]));
}
- for (Index = 0; Index < ARRAY_SIZE (Mtrrs->Variables.Mtrr); Index++) {
+ for (Index = 0; Index < VariableMtrrCount; Index++) {
if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs->Variables.Mtrr[Index].Mask)->Bits.V == 0) {
//
// If mask is not valid, then do not display range
@@ -2829,11 +2832,11 @@ MtrrDebugPrintAllMtrrsWorker (
RangeCount = 1;
MtrrLibGetRawVariableRanges (
- &Mtrrs->Variables, ARRAY_SIZE (Mtrrs->Variables.Mtrr),
+ &Mtrrs->Variables, VariableMtrrCount,
MtrrValidBitsMask, MtrrValidAddressMask, RawVariableRanges
);
MtrrLibApplyVariableMtrrs (
- RawVariableRanges, ARRAY_SIZE (RawVariableRanges),
+ RawVariableRanges, VariableMtrrCount,
Ranges, ARRAY_SIZE (Ranges), &RangeCount
);
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
2017-10-17 1:46 [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang Ruiyu Ni
@ 2017-10-17 2:03 ` Shi, Steven
2017-10-17 2:05 ` Ni, Ruiyu
2017-10-17 7:56 ` Laszlo Ersek
1 sibling, 1 reply; 5+ messages in thread
From: Shi, Steven @ 2017-10-17 2:03 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Laszlo Ersek
Reviewed-by: Steven Shi <steven.shi@intel.com>
Steven Shi
Intel\SSG\STO\UEFI Firmware
Tel: +86 021-61166522
iNet: 821-6522
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, October 17, 2017 9:47 AM
> To: edk2-devel@lists.01.org
> Cc: Shi, Steven <steven.shi@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to
> avoid hang
>
> ARRAY_SIZE(Mtrrs->Variables.Mtrr) was used in
> MtrrDebugPrintAllMtrrsWorker() to parse the MTRR registers.
> Instead, the actual variable MTRR count should be used.
> Otherwise, the uninitialized random data in MtrrSetting may cause
> MtrrLibSetMemoryType() hang.
>
> Steven Shi found this bug in QEMU when using Q35 chip.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 2fd1d0153e..cb22558103 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -2776,6 +2776,7 @@ MtrrDebugPrintAllMtrrsWorker (
> UINTN RangeCount;
> UINT64 MtrrValidBitsMask;
> UINT64 MtrrValidAddressMask;
> + UINT32 VariableMtrrCount;
> MTRR_MEMORY_RANGE Ranges[
> ARRAY_SIZE (mMtrrLibFixedMtrrTable) * sizeof (UINT64) + 2 *
> ARRAY_SIZE (Mtrrs->Variables.Mtrr) + 1
> ];
> @@ -2785,6 +2786,8 @@ MtrrDebugPrintAllMtrrsWorker (
> return;
> }
>
> + VariableMtrrCount = GetVariableMtrrCountWorker ();
> +
> if (MtrrSetting != NULL) {
> Mtrrs = MtrrSetting;
> } else {
> @@ -2802,7 +2805,7 @@ MtrrDebugPrintAllMtrrsWorker (
> DEBUG((DEBUG_CACHE, "Fixed MTRR[%02d] : %016lx\n", Index, Mtrrs-
> >Fixed.Mtrr[Index]));
> }
>
> - for (Index = 0; Index < ARRAY_SIZE (Mtrrs->Variables.Mtrr); Index++) {
> + for (Index = 0; Index < VariableMtrrCount; Index++) {
> if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs-
> >Variables.Mtrr[Index].Mask)->Bits.V == 0) {
> //
> // If mask is not valid, then do not display range
> @@ -2829,11 +2832,11 @@ MtrrDebugPrintAllMtrrsWorker (
> RangeCount = 1;
>
> MtrrLibGetRawVariableRanges (
> - &Mtrrs->Variables, ARRAY_SIZE (Mtrrs->Variables.Mtrr),
> + &Mtrrs->Variables, VariableMtrrCount,
> MtrrValidBitsMask, MtrrValidAddressMask, RawVariableRanges
> );
> MtrrLibApplyVariableMtrrs (
> - RawVariableRanges, ARRAY_SIZE (RawVariableRanges),
> + RawVariableRanges, VariableMtrrCount,
> Ranges, ARRAY_SIZE (Ranges), &RangeCount
> );
>
> --
> 2.12.2.windows.2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
2017-10-17 2:03 ` Shi, Steven
@ 2017-10-17 2:05 ` Ni, Ruiyu
0 siblings, 0 replies; 5+ messages in thread
From: Ni, Ruiyu @ 2017-10-17 2:05 UTC (permalink / raw)
To: Shi, Steven, edk2-devel@lists.01.org; +Cc: Laszlo Ersek
Thanks Steven.
All,
I will check in this patch ASAP.
Because it's just a bug fix in a corner of implementation and it may cause all system hang.
-----Original Message-----
From: Shi, Steven
Sent: Tuesday, October 17, 2017 10:04 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
Reviewed-by: Steven Shi <steven.shi@intel.com>
Steven Shi
Intel\SSG\STO\UEFI Firmware
Tel: +86 021-61166522
iNet: 821-6522
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, October 17, 2017 9:47 AM
> To: edk2-devel@lists.01.org
> Cc: Shi, Steven <steven.shi@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker
> to avoid hang
>
> ARRAY_SIZE(Mtrrs->Variables.Mtrr) was used in
> MtrrDebugPrintAllMtrrsWorker() to parse the MTRR registers.
> Instead, the actual variable MTRR count should be used.
> Otherwise, the uninitialized random data in MtrrSetting may cause
> MtrrLibSetMemoryType() hang.
>
> Steven Shi found this bug in QEMU when using Q35 chip.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 2fd1d0153e..cb22558103 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -2776,6 +2776,7 @@ MtrrDebugPrintAllMtrrsWorker (
> UINTN RangeCount;
> UINT64 MtrrValidBitsMask;
> UINT64 MtrrValidAddressMask;
> + UINT32 VariableMtrrCount;
> MTRR_MEMORY_RANGE Ranges[
> ARRAY_SIZE (mMtrrLibFixedMtrrTable) * sizeof (UINT64) + 2 *
> ARRAY_SIZE (Mtrrs->Variables.Mtrr) + 1
> ];
> @@ -2785,6 +2786,8 @@ MtrrDebugPrintAllMtrrsWorker (
> return;
> }
>
> + VariableMtrrCount = GetVariableMtrrCountWorker ();
> +
> if (MtrrSetting != NULL) {
> Mtrrs = MtrrSetting;
> } else {
> @@ -2802,7 +2805,7 @@ MtrrDebugPrintAllMtrrsWorker (
> DEBUG((DEBUG_CACHE, "Fixed MTRR[%02d] : %016lx\n", Index, Mtrrs-
> >Fixed.Mtrr[Index]));
> }
>
> - for (Index = 0; Index < ARRAY_SIZE (Mtrrs->Variables.Mtrr); Index++) {
> + for (Index = 0; Index < VariableMtrrCount; Index++) {
> if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs-
> >Variables.Mtrr[Index].Mask)->Bits.V == 0) {
> //
> // If mask is not valid, then do not display range @@
> -2829,11 +2832,11 @@ MtrrDebugPrintAllMtrrsWorker (
> RangeCount = 1;
>
> MtrrLibGetRawVariableRanges (
> - &Mtrrs->Variables, ARRAY_SIZE (Mtrrs->Variables.Mtrr),
> + &Mtrrs->Variables, VariableMtrrCount,
> MtrrValidBitsMask, MtrrValidAddressMask, RawVariableRanges
> );
> MtrrLibApplyVariableMtrrs (
> - RawVariableRanges, ARRAY_SIZE (RawVariableRanges),
> + RawVariableRanges, VariableMtrrCount,
> Ranges, ARRAY_SIZE (Ranges), &RangeCount
> );
>
> --
> 2.12.2.windows.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
2017-10-17 1:46 [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang Ruiyu Ni
2017-10-17 2:03 ` Shi, Steven
@ 2017-10-17 7:56 ` Laszlo Ersek
2017-10-18 1:11 ` Ni, Ruiyu
1 sibling, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2017-10-17 7:56 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel
On 10/17/17 03:46, Ruiyu Ni wrote:
> ARRAY_SIZE(Mtrrs->Variables.Mtrr) was used in
> MtrrDebugPrintAllMtrrsWorker() to parse the MTRR registers.
> Instead, the actual variable MTRR count should be used.
> Otherwise, the uninitialized random data in MtrrSetting may cause
> MtrrLibSetMemoryType() hang.
>
> Steven Shi found this bug in QEMU when using Q35 chip.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 2fd1d0153e..cb22558103 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -2776,6 +2776,7 @@ MtrrDebugPrintAllMtrrsWorker (
> UINTN RangeCount;
> UINT64 MtrrValidBitsMask;
> UINT64 MtrrValidAddressMask;
> + UINT32 VariableMtrrCount;
> MTRR_MEMORY_RANGE Ranges[
> ARRAY_SIZE (mMtrrLibFixedMtrrTable) * sizeof (UINT64) + 2 * ARRAY_SIZE (Mtrrs->Variables.Mtrr) + 1
> ];
> @@ -2785,6 +2786,8 @@ MtrrDebugPrintAllMtrrsWorker (
> return;
> }
>
> + VariableMtrrCount = GetVariableMtrrCountWorker ();
> +
> if (MtrrSetting != NULL) {
> Mtrrs = MtrrSetting;
> } else {
> @@ -2802,7 +2805,7 @@ MtrrDebugPrintAllMtrrsWorker (
> DEBUG((DEBUG_CACHE, "Fixed MTRR[%02d] : %016lx\n", Index, Mtrrs->Fixed.Mtrr[Index]));
> }
>
> - for (Index = 0; Index < ARRAY_SIZE (Mtrrs->Variables.Mtrr); Index++) {
> + for (Index = 0; Index < VariableMtrrCount; Index++) {
> if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs->Variables.Mtrr[Index].Mask)->Bits.V == 0) {
> //
> // If mask is not valid, then do not display range
> @@ -2829,11 +2832,11 @@ MtrrDebugPrintAllMtrrsWorker (
> RangeCount = 1;
>
> MtrrLibGetRawVariableRanges (
> - &Mtrrs->Variables, ARRAY_SIZE (Mtrrs->Variables.Mtrr),
> + &Mtrrs->Variables, VariableMtrrCount,
> MtrrValidBitsMask, MtrrValidAddressMask, RawVariableRanges
> );
> MtrrLibApplyVariableMtrrs (
> - RawVariableRanges, ARRAY_SIZE (RawVariableRanges),
> + RawVariableRanges, VariableMtrrCount,
> Ranges, ARRAY_SIZE (Ranges), &RangeCount
> );
>
>
Assuming this patch is not committed yet:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
I have another (independent) comment:
This function is currently named "MtrrDebugPrintAllMtrrsWorker", and its
leading comment says "Worker function prints all MTRRs for debugging."
Because of this name and the documentation, I didn't understand
initially how the problem could cause a hang, given that none of the
printing loops would actually access anything out-of-bounds. Some of the
information printed would be garbage, but it should not cause a hang.
That's when I noticed that the function actually *applies* MTRR
settings, by calling MtrrLibApplyVariableMtrrs(). Even worse, the
MtrrLibApplyVariableMtrrs() and MtrrLibApplyFixedMtrrs() function calls
are wrapped by DEBUG_CODE(). This means that in a DEBUG/NOOPT build, the
function will apply MTRR settings, and in a RELEASE build, it won't.
I think this is wrong and should be fixed. A debug function (esp. one
that behaves differently in DEBUG/NOOPT vs. RELEASE) should have no side
effects.
The current situation is similar to:
ASSERT (FunctionWithSideEffects () == EXPECTED_RETURN_VALUE);
which we all know is wrong.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
2017-10-17 7:56 ` Laszlo Ersek
@ 2017-10-18 1:11 ` Ni, Ruiyu
0 siblings, 0 replies; 5+ messages in thread
From: Ni, Ruiyu @ 2017-10-18 1:11 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Laszlo,
I think the function name confused you.
MtrrLibApplyVariableMtrrs() is not to apply the MTRR setting to CPU/HW.
It's to apply the setting read from CPU/HW to the range array stored in memory.
It doesn't have side effect.
The basic idea of MtrrDebugPrintAllMtrrsWorker() is to read the setting from HW/CPU,
Convert that setting to range array. Then dump the range array.
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Tuesday, October 17, 2017 3:56 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang
On 10/17/17 03:46, Ruiyu Ni wrote:
> ARRAY_SIZE(Mtrrs->Variables.Mtrr) was used in
> MtrrDebugPrintAllMtrrsWorker() to parse the MTRR registers.
> Instead, the actual variable MTRR count should be used.
> Otherwise, the uninitialized random data in MtrrSetting may cause
> MtrrLibSetMemoryType() hang.
>
> Steven Shi found this bug in QEMU when using Q35 chip.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 2fd1d0153e..cb22558103 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -2776,6 +2776,7 @@ MtrrDebugPrintAllMtrrsWorker (
> UINTN RangeCount;
> UINT64 MtrrValidBitsMask;
> UINT64 MtrrValidAddressMask;
> + UINT32 VariableMtrrCount;
> MTRR_MEMORY_RANGE Ranges[
> ARRAY_SIZE (mMtrrLibFixedMtrrTable) * sizeof (UINT64) + 2 * ARRAY_SIZE (Mtrrs->Variables.Mtrr) + 1
> ];
> @@ -2785,6 +2786,8 @@ MtrrDebugPrintAllMtrrsWorker (
> return;
> }
>
> + VariableMtrrCount = GetVariableMtrrCountWorker ();
> +
> if (MtrrSetting != NULL) {
> Mtrrs = MtrrSetting;
> } else {
> @@ -2802,7 +2805,7 @@ MtrrDebugPrintAllMtrrsWorker (
> DEBUG((DEBUG_CACHE, "Fixed MTRR[%02d] : %016lx\n", Index, Mtrrs->Fixed.Mtrr[Index]));
> }
>
> - for (Index = 0; Index < ARRAY_SIZE (Mtrrs->Variables.Mtrr); Index++) {
> + for (Index = 0; Index < VariableMtrrCount; Index++) {
> if (((MSR_IA32_MTRR_PHYSMASK_REGISTER *)&Mtrrs->Variables.Mtrr[Index].Mask)->Bits.V == 0) {
> //
> // If mask is not valid, then do not display range @@
> -2829,11 +2832,11 @@ MtrrDebugPrintAllMtrrsWorker (
> RangeCount = 1;
>
> MtrrLibGetRawVariableRanges (
> - &Mtrrs->Variables, ARRAY_SIZE (Mtrrs->Variables.Mtrr),
> + &Mtrrs->Variables, VariableMtrrCount,
> MtrrValidBitsMask, MtrrValidAddressMask, RawVariableRanges
> );
> MtrrLibApplyVariableMtrrs (
> - RawVariableRanges, ARRAY_SIZE (RawVariableRanges),
> + RawVariableRanges, VariableMtrrCount,
> Ranges, ARRAY_SIZE (Ranges), &RangeCount
> );
>
>
Assuming this patch is not committed yet:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
I have another (independent) comment:
This function is currently named "MtrrDebugPrintAllMtrrsWorker", and its leading comment says "Worker function prints all MTRRs for debugging."
Because of this name and the documentation, I didn't understand initially how the problem could cause a hang, given that none of the printing loops would actually access anything out-of-bounds. Some of the information printed would be garbage, but it should not cause a hang.
That's when I noticed that the function actually *applies* MTRR settings, by calling MtrrLibApplyVariableMtrrs(). Even worse, the
MtrrLibApplyVariableMtrrs() and MtrrLibApplyFixedMtrrs() function calls are wrapped by DEBUG_CODE(). This means that in a DEBUG/NOOPT build, the function will apply MTRR settings, and in a RELEASE build, it won't.
I think this is wrong and should be fixed. A debug function (esp. one that behaves differently in DEBUG/NOOPT vs. RELEASE) should have no side effects.
The current situation is similar to:
ASSERT (FunctionWithSideEffects () == EXPECTED_RETURN_VALUE);
which we all know is wrong.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-18 1:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-17 1:46 [PATCH] UefiCpuPkg/MtrrLib: Fix MtrrDebugPrintAllMtrrsWorker to avoid hang Ruiyu Ni
2017-10-17 2:03 ` Shi, Steven
2017-10-17 2:05 ` Ni, Ruiyu
2017-10-17 7:56 ` Laszlo Ersek
2017-10-18 1:11 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox