On 23/02/2024 15:12, Zhou, Jianfeng wrote:
While it may well cause the compiler to generate less optimised code, there is
absolutely no way that this volatile declaration on a local stack variable can
possibly change the outcome of the code.
There can never be any meaningful side-effects from reading or writing a stack
variable.
I would suggest dropping the volatile on LocalPte4K, since its *only* possible
impact is to confuse a future reader of the code.
The change is for preventing compiler from optimizing.
As a temporary variable, LocalPte4K may be replaced by function parameter
Pte4K.
No, it can't. If Pte4K is marked as a volatile pointer, then the
compiler is not allowed to unilaterally decide to treat it as a
non-volatile pointer.
In this case, code like "LocalPte4K.Bits.Present = Attribute->Bits.Present" may
lead to unexpected result, as it is not atomic. Assembly code look like:
mov eax, [r8]
and dword [rcx], 0xfffffffe // this instruction clear the present bit and
may leads to unexpected result.
and eax, 0x1
or [rcx], eax
Please test with Pte4K marked as volatile and LocalPte4K marked as
non-volatile. If you can still generate assembly code that writes to
*Pte4K more than once, then that would be a serious compiler bug.
As a separate note, I would also suggest removing the unnecessary second
read through Pte4K, since once Pte4K is marked as volatile the compiler
will generate an extra read from that address:
--- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
@@ -30,7 +30,7 @@ PageTableLibSetPte4K (
LocalPte4K.Uint64 = Pte4K->Uint64;
if (Mask->Bits.PageTableBaseAddressLow ||
Mask->Bits.PageTableBaseAddressHigh) {
- LocalPte4K.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
(Attribute) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40);
+ LocalPte4K.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
(Attribute) + Offset) | (LocalPte4K.Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40);
}
if (Mask->Bits.Present) {
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115888): https://edk2.groups.io/g/devel/message/115888
Mute This Topic: https://groups.io/mt/104524857/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
