Re: [PATCH] ARM: cache coherence problem in guestcopy.c

[Jaeyong Yoo <jaeyong.yoo@samsung.com>]

> On Tue, 2013-06-18 at 13:18 +0100, Ian Campbell wrote:
> > On Tue, 2013-06-18 at 12:05 +0000, Jaeyong Yoo wrote:
> > > > We were also speculating that we probably want some DMBs in
> > > > context_switch_{from,to} as well as at return_to_guest.
> > > 
> > > Actually, I just learned ReOrder Buffer, and it looks like so.
> 
> Does this patch help with the issue you are seeing?

I tried the combinations and it does not work. I think my problem maybe stem 
from a different reason. Since this problem happens while
we try to migrate domU, something really weird may happen.

Actually, one of my colleage told me that this problem I'm having has been 
magically disappeared while he tried with copying more vcpu registers and 
lots of printks places to places. At this moment, I'm not sure that this is the 
common problem in xen or the problem due to poor migration, but I'm more 
believing that maybe it is due to the poor migration. Since I'm keep investigating
tihs issue, I will tell you if anything turns out.

> I think in theory it should *BUT* (and it's a big But)...
> 
> ... it doesn't work for me on an issue I've been seeing booting dom0 on
> the Foundation model. However doing the dmb in map_domain_page() *twice*

BTW, did you put barriers in map_domain_page? 
since the patch below looks like in unmap_domain_page.

> does work for me. In fact doing a dmb+nop works too. This is
> inexplicable to me. It may be a model bug or it may (more likely) be
> indicative of some other underlying issue. I also tried dsb() instead of
> dmb() and that doesn't make a difference.
> 
> Anyway, I'm interested about its behaviour on real hardware.
> 
> These are doing full system barriers. I think in reality only a "dmb
> nsh" should be needed for this use case, since we only care about the
> local processor. I didn't want to go there when the supposedly more
> obvious case wasn't doing as I expected!
> 
> Ian.
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index beae61e..ef67953 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -218,6 +218,13 @@ void *map_domain_page(unsigned long mfn)
> 
>      local_irq_save(flags);
> 
> +    /*
> +     * Ensure that any previous writes which occurred via another
> +     * mapping, specifically the guest's own mapping have been
> +     * completed such that they are visible via this mapping.
> +     */
> +    dmb();
> +
>      /* The map is laid out as an open-addressed hash table where each
>       * entry is a 2MB superpage pte.  We use the available bits of each
>       * PTE as a reference count; when the refcount is zero the slot can
> @@ -286,6 +297,13 @@ void unmap_domain_page(const void *va)
> 
>      map[slot].pt.avail--;
> 
> +    /*
> +     * Ensure that any writes which occurred via this mapping have been
> +     * completed such that they are visible via other virtual
> +     * mappings, specifically the guest's own mapping.
> +     */
> +    dmb();
> +
>      local_irq_restore(flags);
> }
> 
> 
>