Re: MemoryFence()


Laszlo Ersek
 

On 02/05/21 14:15, Ni, Ray wrote:
This is very interesting. I am trying to involve the discussion to
learn.
I did some experiment.
----a.c----
// Copy EDKII MemoryFence() and as Laszlo said it's just a compiler fence.
void CompilerFence () {
return;
}

void main () {
// I copied Paolo's code in below.
int *Address = &main;
int Value;

do {
// Force re-reading *Address on every iteration
CompilerFence ();
Value = *Address;
} while (Value == 0xFF);
// As you explained above.
__asm mfence;
}
----END of a.c----

When I use "Cl.exe /O2 /FAcs a.c" to compile it.
I got a.cod as below
-----a.cod----
; Listing generated by Microsoft (R) Optimizing Compiler Version 19.27.29112.0

TITLE E:\Work\edk2\a.c
.686P
.XMM
include listing.inc
.model flat

INCLUDELIB LIBCMT
INCLUDELIB OLDNAMES

PUBLIC _CompilerFence
PUBLIC _main
; Function compile flags: /Ogtpy
; File E:\Work\edk2\a.c
; COMDAT _main
_TEXT SEGMENT
_main PROC ; COMDAT

; 6 : int *Address = &main;

00000 b8 00 00 00 00 mov eax, OFFSET _main
00005 8b 00 mov eax, DWORD PTR [eax]
$LL4@main:

; 7 : int Value;
; 8 :
; 9 : do {
; 10 : // Force re-reading *Address on every iteration
; 11 : CompilerFence ();
; 12 : Value = *Address;
; 13 : } while (Value == 0xFF);

00007 3d ff 00 00 00 cmp eax, 255 ; 000000ffH
0000c 74 f9 je SHORT $LL4@main

; 14 : // As you explained above.
; 15 : __asm mfence;

0000e 0f ae f0 mfence
00011 c3 ret 0
_main ENDP
_TEXT ENDS
; Function compile flags: /Ogtpy
; File E:\Work\edk2\a.c
; COMDAT _CompilerFence
_TEXT SEGMENT
_CompilerFence PROC ; COMDAT

; 2 : return;
; 3 : }

00000 c2 00 00 ret 0
_CompilerFence ENDP
_TEXT ENDS
END
-----END of a.cod-----

Check the assembly:
00007 3d ff 00 00 00 cmp eax, 255 ; 000000ffH

The CompilerFence() call doesn't force to generate code to always read
the memory pointed by Address.
That's because your CompilerFence() function is not correct. You did:

void CompilerFence () {
return;
}
which is wrong. It enforces a C-language sequence point, but it does not
implement a compiler barrier.

And your code is wrong because the *original*, namely the MSFT
implementation of MemoryFence() in edk2, is bugged:

* MdePkg/Library/BaseLib/X86MemoryFence.c:

/**
Used to serialize load and store operations.

All loads and stores that proceed calls to this function are guaranteed to be
globally visible when this function returns.

**/
VOID
EFIAPI
MemoryFence (
VOID
)
{
return;
}
This code comes from historical commit e1f414b6a7d8 ("Import some basic
libraries instances for Mde Packages.", 2007-06-22).

* MdePkg/Library/BaseLib/BaseLib.inf:

[Sources.Ia32]
X86MemoryFence.c | MSFT

[Sources.X64]
X86MemoryFence.c | MSFT
Back to your email:

If I replace CompilerFence() with _ReadBarrier(), compiler can
generate assembly that always reads the memory pointed by Address.

It seems the EDKII MemoryFence() even cannot be used as a read
barrier.
Does it mean that the MSVC version of EDKII MemoryFence() is
implemented wrongly?
That's precisely the case.

This actually explains why MpInitLib and PiSmmCpuDxeSmm use "volatile",
rather than MemoryFence(). (This is one of those info bits that I've
been missing.) The reason is that, when built with Visual Studio,
MemoryFence() does absolutely nothing. So if MpInitLib and
PiSmmCpuDxeSmm used MemoryFence() rather than "volatile", things would
break immediately.

Thank you Ray for highlighting this, I couldn't have imagined.

The current status means that, considering all toolchains and all
architectures that edk2 supports, together, it's been *impossible* to
use MemoryFence() correctly.

And so my defensive thinking "whenever in doubt, use 'volatile' PLUS
MemoryFence()" apparently paid off, because when built with Visual
Studio, OVMF is left with 'volatile' *only*; in effect.

It also means that in Ankur's upcoming v7 series, relying *just* on
MemoryFence() will not suffice. We need 'volatile' (given the current
state of primitives); otherwise we'll have no barriers at all.

... I don't even know where to start changing the code. :(

Laszlo

Join rfc@edk2.groups.io to automatically receive all group messages.