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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 19 Dec 2013, Ian Campbell wrote:
> 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>

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


>  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