On 03/24/15 20:15, Ard Biesheuvel wrote:
> This library may be used with the caches and MMU off, while the
> hypervisor has the caches enabled. So add explicit cache
> maintenance to deal with the potential incoherency.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> .../XenConsoleSerialPortLib.c | 29
> ++++++++++++++++++++++
> .../XenConsoleSerialPortLib.inf | 1 +
> 2 files changed, 30 insertions(+)
>
> diff --git
> a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> index 467cb27a30c4..17d5a5a38d30 100644
> --- a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> +++ b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> @@ -19,6 +19,7 @@
> #include <Library/BaseLib.h>
> #include <Library/SerialPortLib.h>
> #include <Library/XenHypercallLib.h>
> +#include <Library/CacheMaintenanceLib.h>
>
> #include <IndustryStandard/Xen/io/console.h>
> #include <IndustryStandard/Xen/hvm/params.h>
> @@ -80,6 +81,13 @@ SerialPortWrite (
> return 0;
> }
>
> + //
> + // We may be accessing DRAM directly, while the hypervisor has the caches
> + // enabled. This means we have to force a writeback to ensure that we have
> + // the latest data.
> + //
> + WriteBackDataCacheRange (mXenConsoleInterface,
> sizeof(*mXenConsoleInterface));
> +
> Consumer = mXenConsoleInterface->out_cons;
> Producer = mXenConsoleInterface->out_prod;
>
> @@ -93,6 +101,13 @@ SerialPortWrite (
>
> mXenConsoleInterface->out_prod = Producer;
>
> + //
> + // Ensure that the data cache is invalidated after we have written our
> data.
> + // This ensures that the hypervisor will have to fetch the value from DRAM,
> + // which may be where the data is if we are running with the MMU off.
> + //
> + InvalidateDataCacheRange (mXenConsoleInterface,
> sizeof(*mXenConsoleInterface));
> +
> if (Sent > 0) {
> XenHypercallEventChannelOp (EVTCHNOP_send, &mXenConsoleEventChain);
> }
> @@ -124,6 +139,13 @@ SerialPortRead (
> return 0;
> }
>
> + //
> + // We may be accessing DRAM directly, while the hypervisor has the caches
> + // enabled. This means we have to force a writeback to ensure that we have
> + // the latest data.
> + //
> + WriteBackDataCacheRange (mXenConsoleInterface,
> sizeof(*mXenConsoleInterface));
> +
> Consumer = mXenConsoleInterface->in_cons;
> Producer = mXenConsoleInterface->in_prod;
>
> @@ -137,6 +159,13 @@ SerialPortRead (
>
> mXenConsoleInterface->in_cons = Consumer;
>
> + //
> + // Ensure that the data cache is invalidated after we have written our
> data.
> + // This ensures that the hypervisor will have to fetch the value from DRAM,
> + // which may be where the data is if we are running with the MMU off.
> + //
> + InvalidateDataCacheRange (mXenConsoleInterface,
> sizeof(*mXenConsoleInterface));
> +
> XenHypercallEventChannelOp (EVTCHNOP_send, &mXenConsoleEventChain);
>
> return Received;
> diff --git
> a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> index 8a7411558fa3..2fd1c654be78 100644
> --- a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> +++ b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> @@ -29,6 +29,7 @@
> [LibraryClasses]
> BaseLib
> XenHypercallLib
> + CacheMaintenanceLib
>
> [Packages]
> MdePkg/MdePkg.dec
>
Looks good to me; but how about, in addition:
==================
diff --git a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
index 467cb27..a7bf015 100644
--- a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
+++ b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
@@ -31,8 +31,8 @@
// in general, it is actually fine for the Xen domU (guest) environment that
// this module is intended for, as UEFI always executes from DRAM in that case.
//
-STATIC evtchn_send_t mXenConsoleEventChain;
-STATIC struct xencons_interface *mXenConsoleInterface;
+STATIC evtchn_send_t mXenConsoleEventChain;
+STATIC struct xencons_interface volatile *mXenConsoleInterface;
RETURN_STATUS
EFIAPI
@@ -155,6 +155,9 @@ SerialPortPoll (
VOID
)
{
- return mXenConsoleInterface &&
- mXenConsoleInterface->in_cons != mXenConsoleInterface->in_prod;
+ if (!mXenConsoleInterface) {
+ return FALSE;
+ }
+ WriteBackDataCacheRange (mXenConsoleInterface,
sizeof(*mXenConsoleInterface));
+ return mXenConsoleInterface->in_cons != mXenConsoleInterface->in_prod;
}
==================
What do you think?
Thanks
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
