On Fri, Nov 30, 2012 at 09:40:37PM +0000, Christoffer Dall wrote:

On Mon, Nov 19, 2012 at 10:07 AM, Will Deacon [off-list ref] wrote:

quoted

Why are PIPT caches affected by this? The virtual address is irrelevant.

The comment is slightly misleading, and I'll update it. Just so we're
clear, this is the culprit:

1. guest uses page X, containing instruction A
2. page X gets swapped out
3. host uses page X, containing instruction B
4. instruction B enters i-cache at page X's cache line
5. page X gets swapped out
6. guest swaps page X back in
7. guest executes instruction B from cache, should execute instruction A

Ok, that's clearer. Thanks for the explanation.

The point is that with PIPT we can flush only that page from the
icache using the host virtual address, as the MMU will do the
translation on the fly. In the VIPT we have to nuke the whole thing
(unless we .

Unless we what? Could we flush using the host VA + all virtual aliases
instead?

quoted

quoted

+        * (or another VM) may have used this page at the same virtual address
+        * as this guest, and we read incorrect data from the icache.  If
+        * we're using a PIPT cache, we can invalidate just that page, but if
+        * we are using a VIPT cache we need to invalidate the entire icache -
+        * damn shame - as written in the ARM ARM (DDI 0406C - Page B3-1384)
+        */
+       if (icache_is_pipt()) {
+               unsigned long hva = gfn_to_hva(kvm, gfn);
+               __cpuc_coherent_user_range(hva, hva + PAGE_SIZE);
+       } else if (!icache_is_vivt_asid_tagged()) {
+               /* any kind of VIPT cache */
+               __flush_icache_all();
+       }

so what if it *is* vivt_asid_tagged? Surely that necessitates nuking the
thing, unless it's VMID tagged as well (does that even exist?).

see page B3-1392 in the ARM ARM, if it's vivt_asid_tagged it is also
vmid tagged.

Great.

quoted

quoted

+               write_fault = false;
+       else
+               write_fault = true;
+
+       if (fault_status == FSC_PERM && !write_fault) {
+               kvm_err("Unexpected L2 read permission error\n");
+               return -EFAULT;
+       }
+
+       /* We need minimum second+third level pages */
+       ret = mmu_topup_memory_cache(memcache, 2, KVM_NR_MEM_OBJS);
+       if (ret)
+               return ret;
+
+       mmu_seq = vcpu->kvm->mmu_notifier_seq;
+       smp_rmb();

What's this barrier for and why isn't there a write barrier paired with
it?

The read barrier is to ensure that mmu_notifier_seq is read before we
call gfn_to_pfn_prot (which is essentially get_user_pages), so that we
don't get a page which is unmapped by an MMU notifier before we grab
the spinlock that we would never see. I also added a comment
explaining it in the patch below.

There is a write barrier paired with it, see virt/kvm/kvm_main.c,
specifically kvm_mmu_notifier_invalidate_page (the spin_unlock), and
kvm_mmu_notifier_invalidate_range_end.

Ok, so it sounds like a homebrew seqlock implementatiom. Any reason KVM
isn't just using those directly?

Will