From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E295C433DB for ; Mon, 8 Mar 2021 20:04:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 37FF065260 for ; Mon, 8 Mar 2021 20:04:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231408AbhCHUEP (ORCPT ); Mon, 8 Mar 2021 15:04:15 -0500 Received: from mail.kernel.org ([198.145.29.99]:55798 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231335AbhCHUDm (ORCPT ); Mon, 8 Mar 2021 15:03:42 -0500 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 45A0E64F91; Mon, 8 Mar 2021 20:03:42 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lJM6O-000Pct-4I; Mon, 08 Mar 2021 20:03:40 +0000 Date: Mon, 08 Mar 2021 20:03:39 +0000 Message-ID: <8735x5s99g.wl-maz@kernel.org> From: Marc Zyngier To: Alexandru Elisei Cc: Catalin Marinas , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, kernel-team@android.com, James Morse , Julien Thierry , Suzuki K Poulose , Will Deacon , Mark Rutland Subject: Re: [PATCH] KVM: arm64: Ensure I-cache isolation between vcpus of a same VM In-Reply-To: References: <20210303164505.68492-1-maz@kernel.org> <20210305190708.GL23855@arm.com> <877dmksgaw.wl-maz@kernel.org> <20210306141546.GB2932@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: alexandru.elisei@arm.com, catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, kernel-team@android.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, will@kernel.org, mark.rutland@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Alex, On Mon, 08 Mar 2021 16:53:09 +0000, Alexandru Elisei wrote: > > Hello, > > It's not clear to me why this patch is needed. If one VCPU in the VM is generating > code, is it not the software running in the VM responsible for keeping track of > the MMU state of the other VCPUs and making sure the new code is executed > correctly? Why should KVM get involved? > > I don't see how this is different than running on bare metal (no > hypervisor), and one CPU with the MMU on generates code that another > CPU with the MMU off must execute. The difference is that so far, we have always considered i-caches to be private to each CPU. With a hypervisor that allows migration of vcpus from one physical CPU to another, the i-cache isn't private anymore from the perspective of the vcpus. > > Some comments below. > > On 3/6/21 2:15 PM, Catalin Marinas wrote: > > On Sat, Mar 06, 2021 at 10:54:47AM +0000, Marc Zyngier wrote: > >> On Fri, 05 Mar 2021 19:07:09 +0000, > >> Catalin Marinas wrote: > >>> On Wed, Mar 03, 2021 at 04:45:05PM +0000, Marc Zyngier wrote: > >>>> It recently became apparent that the ARMv8 architecture has interesting > >>>> rules regarding attributes being used when fetching instructions > >>>> if the MMU is off at Stage-1. > >>>> > >>>> In this situation, the CPU is allowed to fetch from the PoC and > >>>> allocate into the I-cache (unless the memory is mapped with > >>>> the XN attribute at Stage-2). > >>> Digging through the ARM ARM is hard. Do we have this behaviour with FWB > >>> as well? > >> The ARM ARM doesn't seem to mention FWB at all when it comes to > >> instruction fetch, which is sort of expected as it only covers the > >> D-side. I *think* we could sidestep this when CTR_EL0.DIC is set > >> though, as the I-side would then snoop the D-side. > > Not sure this helps. CTR_EL0.DIC refers to the need for maintenance to > > PoU while the SCTLR_EL1.M == 0 causes the I-cache to fetch from PoC. I > > don't think I-cache snooping the D-cache would happen to the PoU when > > the S1 MMU is off. > > FEAT_FWB requires that CLIDR_EL1.{LoUIS, LoUU} = {0, 0} which means > that no dcache clean is required for instruction to data coherence > (page D13-3086). I interpret that as saying that with FEAT_FWB, > CTR_EL0.IDC is effectively 1, which means that dcache clean is not > required for instruction generation, and icache invalidation is > required only if CTR_EL0.DIC = 0 (according to B2-158). > > > My reading of D4.4.4 is that when SCTLR_EL1.M == 0 both I and D accesses > > are Normal Non-cacheable with a note in D4.4.6 that Non-cacheable > > accesses may be held in the I-cache. > > Nitpicking, but SCTLR_EL1.M == 0 and SCTLR_EL1.I == 1 means that > instruction fetches are to Normal Cacheable, Inner and Outer > Read-Allocate memory (ARM DDI 0487G.a, pages D5-2709 and indirectly > at D13-3586). I think that's the allocation in unified caches, and not necessarily the i-cache, given that it also mention things such as "Inner Write-Through", which makes no sense for the i-cache. > Like you've pointed out, as mentioned in D4.4.6, it is always > possible that instruction fetches are held in the instruction cache, > regardless of the state of the SCTLR_EL1.M bit. Exactly, and that's what breaks things. > > The FWB rules on combining S1 and S2 says that Normal Non-cacheable at > > S1 is "upgraded" to cacheable. This should happen irrespective of > > whether the S1 MMU is on or off and should apply to both I and D > > accesses (since it does not explicitly says). So I think we could skip > > this IC IALLU when FWB is present. > > > > The same logic should apply when the VMM copies the VM text. With FWB, > > we probably only need D-cache maintenance to PoU and only if > > CTR_EL0.IDC==0. I haven't checked what the code currently does. > > When FEAT_FWB, CTR_EL0.IDC is effectively 1 (see above), so we don't > need a dcache clean in this case. But that isn't what concerns me. FWB is exclusively documented in terms of d-cache, and doesn't describe how that affects the instruction fetch (which is why I'm reluctant to attribute any effect to it). Thanks, M. -- Without deviation from the norm, progress is not possible.