On Mon, Oct 16, 2017 at 02:35:47PM -0700, Roy Franz (Cavium) wrote:
> On Mon, Oct 9, 2017 at 8:20 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> > As we're about to introduce opportunistic invalidation of the icache,
> > let's split dcache and icache flushing.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > ---
> > arch/arm/include/asm/kvm_mmu.h | 60 ++++++++++++++++++++++++++++------------
> > arch/arm64/include/asm/kvm_mmu.h | 13 +++++++--
> > virt/kvm/arm/mmu.c | 20 ++++++++++----
> > 3 files changed, 67 insertions(+), 26 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> > index fa6f2174276b..f553aa62d0c3 100644
> > --- a/arch/arm/include/asm/kvm_mmu.h
> > +++ b/arch/arm/include/asm/kvm_mmu.h
> > @@ -126,21 +126,12 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
> > return (vcpu_cp15(vcpu, c1_SCTLR) & 0b101) == 0b101;
> > }
> >
> > -static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
> > - kvm_pfn_t pfn,
> > - unsigned long size)
> > +static inline void __coherent_dcache_guest_page(struct kvm_vcpu *vcpu,
> > + kvm_pfn_t pfn,
> > + unsigned long size)
> > {
> > /*
> > - * If we are going to insert an instruction page and the icache is
> > - * either VIPT or PIPT, there is a potential problem where the host
> > - * (or another VM) may have used the same page 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.b - Page B3-1393).
> > - *
> > - * 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).
> > + * Clean the dcache to the Point of Coherency.
> > *
> > * We need to do this through a kernel mapping (using the
> > * user-space mapping has proved to be the wrong
> > @@ -155,19 +146,52 @@ static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
> >
> > 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 (!icache_is_pipt() && !icache_is_vivt_asid_tagged()) {
> > +static inline void __coherent_icache_guest_page(struct kvm_vcpu *vcpu,
> > + kvm_pfn_t pfn,
> > + unsigned long size)
> > +{
> > + /*
> > + * If we are going to insert an instruction page and the icache is
> > + * either VIPT or PIPT, there is a potential problem where the host
> > + * (or another VM) may have used the same page 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.b - Page B3-1393).
> > + *
> > + * 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).
> > + */
> > +
> > + VM_BUG_ON(size & ~PAGE_MASK);
> > +
> > + if (icache_is_vivt_asid_tagged())
> > + return;
> > +
> > + if (!icache_is_pipt()) {
> > /* any kind of VIPT cache */
> > __flush_icache_all();
> > + return;
> > + }
> How does cache_is_vivt() fit into these checks? From my digging it looks like
> that is ARMv5 and earlier only, so am I right in thinking those don't support
> virtualization? It looks like this code properly handles all the cache types
> described in the ARM ARM that you referenced, and that the 'extra' cache
> types in Linux are for older spec chips.
>
>
That's certainly my understanding. From the ARMv7 ARM the only types of
instruction caches we should worry about are:
- PIPT instruction caches
- Virtually-indexed, physically-tagged (VIPT) instruction caches
- ASID and VMID tagged Virtually-indexed, virtually-tagged (VIVT)
instruction caches.
And I think that's covered here.
Thanks,
-Christoffer
