Re: [Xen-devel] [PATCH RFC] xen: arm: use uncached foreign mappings when building guests

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-12-18 at 18:41 +0000, David Vrabel wrote:
> On 18/12/2013 17:28, Ian Campbell wrote:
> > When building an ARM guest we need to take care of cache maintenance
> > because the guest starts with MMU and cache disabled, which means we
> > need to make sure that the initial images (kernel, initrd, dtb) which we
> > write to guest memory are not in the cache.
> > 
> > We thought we had solved this with "tools: libxc: flush data cache after
> > loading images into guest memory" (a0035ecc0d82) however it turns out
> > that there are a couple of issues with this approach:
> > 
> > Firstly we need to do a cache flush from userspace, on arm64 this is
> > possible by directly using the instructions from userspace, but on arm32
> > this is not possible and so we need to use a system call. Unfortunately
> > the system call provided by Linux for this purpose does not flush far
> > enough down the cache hierarchy. Extending the system call would not be
> > an insurmountable barrier, were it not for the second issue:
> > 
> > Secondly, and more importantly, Catalin Marinas points out (via Marc
> > Zyngier) that there is a race between the cache flush and the point
> > where we tear down the mappings, where the processor might speculatively
> > pull some data into the cache (cache flushes are by Virtual Address, so
> > this race is unavoidable).
> > 
> > If this happens then guest kernels which modify some code/data before
> > enabling MMUs + caches may see stale data in the cache.
> 
> Would this same problem with occur with save/restore of a guest that has
> caching disabled?

We basically only support guests running without caches at start of day.
We expect and require them to come up and enable caches and then keep
them enabled. Fortunately this is normal practice...

> If so, this would require using uncached foreign
> mappings for save/restore which I would think would make performance suck.

Indeed.

> Does the same problem occur if the guest has MMU and caching enabled but
> has uncached mappings?

Basically the only reason I can think of to have such a mapping would be
related to DMA for passthrough devices, and we wouldn't support
migration of a guest with a passthrough device.

If this does turn out to be a bridge we need to cross then I think a
strategy of ignoring uncached mappings during the live phase and
sweeping them up during the final stop and copy would work.

> Is there not some sort of cache flush you can do /after/ tearing down
> the foreign mappings?  Flush all by ASID or similar?

Sadly not. dcache flush options are by MVA (which has the described race
against unlocking) and flush by set/way which is not broadcast to all
processors and is therefore pretty useless on an SMP system (you would
need stop_machine or something similar to use it effectively). It's also
not terribly virt friendly, since it blows the cache for everyone...

> Would this also require that granted pages must only have cachable
> mappings in the granting domain?  If so, this should be documented as
> part of the ABI.

As of yesterday in arch-arm.h we have:
 * All hypercall arguments passed via a pointer to guest memory must
 * reside in memory which is mapped as Normal Inner-cacheable. Any
 * Inner cache allocation strategy (Write-Back, Write-Through etc) is
 * acceptable. There is no restriction on the Outer-cacheability.

I could have sworn I mentioned I/O in there when I wrote it. I must have
accidentally dropped it during my initial (unposted) drafts. Oops

8<------------------------

From 8637d439e529b90b2e50ea148b935ba81a0d412d Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Thu, 19 Dec 2013 10:08:39 +0000
Subject: [PATCH] xen: arm: further clarify the requirement for cached
 mappings

We need to include all shared memory, including grant table mappings etc
in this statement.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 xen/include/public/arch-arm.h |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index ef6217d..2e7fb3e 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -59,10 +59,17 @@
  * used regardless of guest type. Structures which are passed as
  * hypercall arguments are always little endian.
  *
- * All hypercall arguments passed via a pointer to guest memory must
- * reside in memory which is mapped as Normal Inner-cacheable. Any
- * Inner cache allocation strategy (Write-Back, Write-Through etc) is
- * acceptable. There is no restriction on the Outer-cacheability.
+ * All memory which is shared with other entities in the system
+ * (including the hypervisor and other guests) must reside in memory
+ * which is mapped as Normal Inner-cacheable. This applies to:
+ *  - hypercall arguments passed via a pointer to guest memory.
+ *  - memory shared via the grant table mechanism (including PV I/O
+ *    rings etc).
+ *  - memory shared with the hypervisor (struct shared_info, struct
+ *    vcpu_info, the grant table, etc).
+ *
+ * Any Inner cache allocation strategy (Write-Back, Write-Through etc)
+ * is acceptable. There is no restriction on the Outer-cacheability.
  */
 
 /*
-- 
1.7.10.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel