Hi,
On Mon, Oct 03, 2016 at 07:09:59PM +0200, Jan Kiszka wrote:
> I could use a good explanation (for a commit log) why I need this
>
> diff --git a/hypervisor/arch/arm64/include/asm/paging.h
> b/hypervisor/arch/arm64/include/asm/paging.h
> index 2d17acd..76804e0 100644
> --- a/hypervisor/arch/arm64/include/asm/paging.h
> +++ b/hypervisor/arch/arm64/include/asm/paging.h
> @@ -258,8 +258,11 @@ static inline void arm_paging_vcpu_flush_tlbs(void)
> /* Only executed on hypervisor paging struct changes */
> static inline void arch_paging_flush_page_tlbs(unsigned long page_addr)
> {
> - asm volatile("tlbi vae2, %0\n"
> - : : "r" (page_addr >> PAGE_SHIFT));
> + asm volatile(
> + "dsb ishst\n\t"
> + "tlbi vae2, %0\n\t"
> + "dsb ish\n\t"
> + : : "r" (page_addr >> PAGE_SHIFT));
> }
Apologies if the below isn't all that relevant; I haven't dug through
the jailhouse code, and you may already have the necessary barriers and
maintenance elsewhere.
Taking this in a few steps (assuming an SMP system for the use of ISH):
(a) In general, to ensure that any write is visible any page table
walker (AKA PTW) that fills a TLB in the system, a DSB ISHST is
required. That ensures that it's left any store buffer, and is
visible to coherent observers including the PTWs.
(b) If there was previously a valid entry for that VA in the translation
tables, and there has been no TLB maintenance affecting that VA
since the translation table was modified, TLB maintenance is
necessary for that VA. This maintenance requires completion with a
DSB ISH.
If you never had any valid entries for this VA, TLB maintenance is
necessary (and neither is the related DSB ISH).
(c) After completion of a DSB for either (a) or (a+b), an ISB (or other
context synchronising event) is necessary to synchronise context on
the local PE.
This ISB appears to be missing above.
> /* Used to clean the PAGE_MAP_COHERENT page table changes */
>
> on the Hi6220 to use the hypervisor mappings reliably. Without it, the
> access to the first device mapping (GICD) fails. It's not needed on the
> AMD Opteron, though.
In any case, (a) and (c) above should be necessary.
> And we don't have such barriers on 32-bit ARM.
I believe that you needed them. The ARM ARM makes this fairly clear in
the appendix on TLB maintenance requirements:
* In the latest ARMv7-A manual (ARM DDI 0406C.c), see D7.5.3 "TLB
maintenance operations and barriers".
* In the latest ARMv8-A manual, ARM DDI 0487A.j, see K10.5.3, "TLB
maintenance instructions and barriers".
Thanks,
Mark.
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
