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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91668): https://edk2.groups.io/g/devel/message/91668
Mute This Topic: https://groups.io/mt/92482724/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
