From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752513AbcHZMVv (ORCPT ); Fri, 26 Aug 2016 08:21:51 -0400 Received: from foss.arm.com ([217.140.101.70]:38539 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751997AbcHZMVu (ORCPT ); Fri, 26 Aug 2016 08:21:50 -0400 Date: Fri, 26 Aug 2016 13:21:38 +0100 From: Marc Zyngier To: Punit Agrawal Cc: Will Deacon , , , Steven Rostedt , "Ingo Molnar" , , Subject: Re: [RFC PATCH 6/7] arm64: KVM: Handle trappable TLB instructions Message-ID: <20160826132138.04abb500@arm.com> In-Reply-To: <87h9a7rhvf.fsf@e105922-lin.cambridge.arm.com> References: <1471344312-26685-1-git-send-email-punit.agrawal@arm.com> <1471344312-26685-7-git-send-email-punit.agrawal@arm.com> <20160819151846.GE9893@arm.com> <87a8g2sb4o.fsf@e105922-lin.cambridge.arm.com> <87h9a7rhvf.fsf@e105922-lin.cambridge.arm.com> Organization: ARM Ltd X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.30; arm-unknown-linux-gnueabihf) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 26 Aug 2016 10:37:08 +0100 Punit Agrawal wrote: > Punit Agrawal writes: > > > Will Deacon writes: > > > >> Hi Punit, > >> > >> On Tue, Aug 16, 2016 at 11:45:11AM +0100, Punit Agrawal wrote: > >>> The ARMv8 architecture allows trapping of TLB maintenane instructions > >>> from EL0/EL1 to higher exception levels. On encountering a trappable TLB > >>> instruction in a guest, an exception is taken to EL2. > >>> > >>> Add functionality to handle emulating the TLB instructions. > >>> > >>> Signed-off-by: Punit Agrawal > >>> Cc: Christoffer Dall > >>> Cc: Marc Zyngier > >> > >> [...] > >> > >>> +void __hyp_text > >>> +__kvm_emulate_tlb_invalidate(struct kvm *kvm, u32 sys_op, u64 regval) > >>> +{ > >>> + kvm = kern_hyp_va(kvm); > >>> + > >>> + /* > >>> + * Switch to the guest before performing any TLB operations to > >>> + * target the appropriate VMID > >>> + */ > >>> + __switch_to_guest_regime(kvm); > >>> + > >>> + /* > >>> + * TLB maintenance operations broadcast to inner-shareable > >>> + * domain when HCR_FB is set (default for KVM). > >>> + */ > >>> + switch (sys_op) { > >>> + case TLBIALL: > >>> + case TLBIALLIS: > >>> + case ITLBIALL: > >>> + case DTLBIALL: > >>> + case TLBI_VMALLE1: > >>> + case TLBI_VMALLE1IS: > >>> + __tlbi(vmalle1is); > >>> + break; > >>> + case TLBIMVA: > >>> + case TLBIMVAIS: > >>> + case ITLBIMVA: > >>> + case DTLBIMVA: > >>> + case TLBI_VAE1: > >>> + case TLBI_VAE1IS: > >>> + __tlbi(vae1is, regval); > >> > >> I'm pretty nervous about this. Although you've switched in the guest stage-2 > >> page table before the TLB maintenance, we're still running on a host stage-1 > >> and it's not clear to me that the stage-1 context is completely ignored for > >> the purposes of a stage-1 TLBI executed at EL2. > >> > >> For example, if TCR_EL1.TBI0 is set in the guest but cleared in the host, > >> my reading of the architecture is that it will be treated as zero when > >> we perform this invalidation operation. I worry that we have similar > >> problems with the granule size, where bits become RES0 in the TLBI VA > >> ops. > > > > Some control bits seem to be explicitly called out to not affect TLB > > maintenance operations[0] but I hadn't considered the ones you highlight. > > > > [0] ARMv8 ARM DDI 0487A.j D4.7, Pg D4-1814 > > > >> > >> Finally, we should probably be masking out the RES0 bits in the TLBI > >> ops, just in case some future extension to the architecture defines them > >> in such a way where they have different meanings when executed at EL2 > >> or EL1. > > > > Although, the RES0 bits for TLBI VA ops are currently ignored, I agree > > that masking them out based on granule size protects against future > > incompatible changes. > > > >> > >> The easiest thing to do is just TLBI VMALLE1IS for all trapped operations, > >> but you might want to see how that performs. > > > > That sounds reasonable for correctness. But I suspect we'll have to do > > more to claw back some performance. Let me run a few tests and come back > > on this. > > Assuming I've correctly switched in TCR and replacing the various TLB > operations in this patch with TLBI VMALLE1IS, there is a drop in kernel > build times of ~5% (384s vs 363s). Note that if all you're doing is a VMALLE1IS, switching TCR_EL1 should not be necessary, as all that is required for this invalidation is the VMID. > For the next version, I'll use this as a starting point and try clawing > back the loss by using the appropriate TLB instructions albeit with > additional sanity checking based on context. Great, thanks! M. -- Jazz is not dead. It just smells funny.