Re: [PATCH 1/1] OvmfPkg/XenPvBlkDxe: Fix memory barrier macro


Andrew Cooper <Andrew.Cooper3@...>
 

On 19/07/2022 14:52, Anthony Perard wrote:
diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
index 350b7bd309c0..67ee1899e9a8 100644
--- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
+++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.h
@@ -11,8 +11,9 @@
#define __EFI_XEN_PV_BLK_DXE_H__

#include <Uefi.h>
+#include "FullMemoryFence.h"

-#define xen_mb() MemoryFence()
+#define xen_mb() FullMemoryFence()
#define xen_rmb() MemoryFence()
#define xen_wmb() MemoryFence()
Ok, so the old MemoryFence() is definitely bogus here.

However, it doesn't need to be an mfence instruction.  All that is
needed is smp_mb(), which these days is

asm volatile ( "lock addl $0, -4(%%rsp)" ::: "memory" )

because that has the required read/write ordering properties without the
extra serialising property that mfence has.

Furthermore, ...


diff --git a/OvmfPkg/XenPvBlkDxe/X86GccFullMemoryFence.c b/OvmfPkg/XenPvBlkDxe/X86GccFullMemoryFence.c
new file mode 100644
index 000000000000..92d107def470
--- /dev/null
+++ b/OvmfPkg/XenPvBlkDxe/X86GccFullMemoryFence.c
@@ -0,0 +1,20 @@
+/** @file
+ Copyright (C) 2022, Citrix Ltd.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "FullMemoryFence.h"
+
+//
+// Like MemoryFence() but prevent stores from been reorded with loads by
+// the CPU on X64.
+//
+VOID
+EFIAPI
+FullMemoryFence (
+ VOID
+ )
+{
+ __asm__ __volatile__ ("mfence":::"memory");
+}
... stuff like this needs to come from a single core location, and not
opencoded for each driver.

~Andrew

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