On 8 January 2015 at 11:59, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> @@ -180,10 +177,32 @@ static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> *
> * VIVT caches are tagged using both the ASID and the VMID and doesn't
> * need any kind of flushing (DDI 0406C.b - Page B3-1392).
> + *
> + * We need to do this through a kernel mapping (using the
> + * user-space mapping has proved to be the wrong
> + * solution). For that, we need to kmap one page at a time,
> + * and iterate over the range.
> */
> - if (icache_is_pipt()) {
> - __cpuc_coherent_user_range(hva, hva + size);
> - } else if (!icache_is_vivt_asid_tagged()) {
> +
> + VM_BUG_ON(size & PAGE_MASK);
> +
> + while (size) {
> + void *va = kmap_atomic_pfn(pfn);
> +
> + if (!vcpu_has_cache_enabled(vcpu) || ipa_uncached)
> + kvm_flush_dcache_to_poc(va, PAGE_SIZE);
> +
> + if (icache_is_pipt())
> + __cpuc_coherent_user_range((unsigned long)va,
> + (unsigned long)va + PAGE_SIZE);
> +
> + size -= PAGE_SIZE;
> + pfn++;
> +
> + kunmap_atomic(va);
> + }
If (vcpu_has_cache_enabled(vcpu) && !ipa_uncached && !icache_is_pipt())
then we're going to run round this loop mapping and unmapping without
actually doing anything. Is it worth hoisting that check out of the
loop? (I think it's going to be the common case for a non-PIPT icache,
right?)
> + if (!icache_is_pipt() && !icache_is_vivt_asid_tagged()) {
> /* any kind of VIPT cache */
> __flush_icache_all();
> }
Can you remind me why it's OK not to flush the icache for an
ASID tagged VIVT icache? Making this page coherent might actually
be revealing a change in the instructions associated with the VA,
mightn't it?
thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
