[PATCH v2 2/8] arm: KVM: Add optimized PIPT icache flushing

[Mark Rutland]

On Fri, Oct 20, 2017 at 05:53:39PM +0100, Marc Zyngier wrote:
> Hi Mark,
> 
> On 20/10/17 17:27, Mark Rutland wrote:
> > Hi Marc,
> > 
> > On Fri, Oct 20, 2017 at 04:48:58PM +0100, Marc Zyngier wrote:
> >> @@ -181,18 +185,40 @@ static inline void __invalidate_icache_guest_page(struct kvm_vcpu *vcpu,
> >>  		return;
> >>  	}
> >>  
> >> -	/* PIPT cache. As for the d-side, use a temporary kernel mapping. */
> >> +	/*
> >> +	 * CTR IminLine contains Log2 of the number of words in the
> >> +	 * cache line, so we can get the number of words as
> >> +	 * 2 << (IminLine - 1).  To get the number of bytes, we
> >> +	 * multiply by 4 (the number of bytes in a 32-bit word), and
> >> +	 * get 4 << (IminLine).
> >> +	 */
> >> +	iclsz = 4 << (read_cpuid(CPUID_CACHETYPE) & 0xf);
> >> +
> >>  	while (size) {
> >>  		void *va = kmap_atomic_pfn(pfn);
> >> +		void *end = va + PAGE_SIZE;
> >> +		void *addr = va;
> >>  
> >> -		__cpuc_coherent_user_range((unsigned long)va,
> >> -					   (unsigned long)va + PAGE_SIZE);
> >> +		do {
> >> +			write_sysreg(addr, ICIMVAU);
> >> +			addr += iclsz;
> >> +		} while (addr < end);
> >> +
> >> +		dsb(ishst);
> > 
> > I believe this needs to be ISH rather than ISHST.
> > 
> > Per, ARM DDI 0487B.b, page G3-4701, "G3.4 AArch32 cache and branch
> > predictor support":
> > 
> >     A DSB or DMB instruction intended to ensure the completion of cache
> >     maintenance instructions or branch predictor instructions must have
> >     an access type of both loads and stores.
> 
> Right. This actually comes from 6abdd491698a ("ARM: mm: use
> inner-shareable barriers for TLB and user cache operations"), and the
> ARMv7 ARM doesn't mention any of this.

Ah; so it doesn't. :/

> My take is that we want to be consistent. Given that KVM/ARM on 32bit is
> basically ARMv7 only, I'd rather keep the ST version of the barrier
> here, and change it everywhere if/when someone decides to support a
> 32bit kernel on ARMv8 (yes, we already do as a guest, but it doesn't
> seem to really matter so far).

Keeping things consistent makes sense to me.

Another one for the backlog...

Thanks,
Mark.