From patchwork Thu Aug 15 04:52:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 19164 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ve0-f199.google.com (mail-ve0-f199.google.com [209.85.128.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 94677246B8 for ; Thu, 15 Aug 2013 04:52:58 +0000 (UTC) Received: by mail-ve0-f199.google.com with SMTP id m1sf392076ves.2 for ; Wed, 14 Aug 2013 21:52:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:delivered-to:to:subject:mime-version:date:from :cc:organization:in-reply-to:references:message-id:user-agent :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe:content-type; bh=QnnHywFntV7c/cDKk2iKuZoJTh7qaJUfZt7PKalvBds=; b=c3AtIoNmzQKKQv5RZ74roOpJR3C5DTy+yKZsQv+EMws28IAbGyEJ5ZUMwGyzzRLO5T SeT8NV2BiExcOa8odhnrWskTNgOhnQE98PC/6dHD7z6Ioj09RgMKW+Djl5mUzrasuUC8 MrQlu9eOR8AqhAOdBz0Fzvg6dIB3Jxrv+vTTuMi09ta4oYRbh5LCml2sLv35+FR3ic1f qdigmN3vo/0O0ubQLnBdaH/gGXt9gZQCajGrKfnjWm5Wts/BIyfRjeHftIVhRIHSY5ar mXIvy56R8T1uohmF81qDmzjIfVYCcJnadGu8RWgCJ7ojh30nyGjJX7Tl2VyXzhJ78nBb BxOg== X-Received: by 10.58.67.7 with SMTP id j7mr3559124vet.37.1376542378146; Wed, 14 Aug 2013 21:52:58 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.2.3 with SMTP id 3ls165501qeq.83.gmail; Wed, 14 Aug 2013 21:52:58 -0700 (PDT) X-Received: by 10.58.118.70 with SMTP id kk6mr12701631veb.1.1376542377986; Wed, 14 Aug 2013 21:52:57 -0700 (PDT) Received: from mail-vb0-f43.google.com (mail-vb0-f43.google.com [209.85.212.43]) by mx.google.com with ESMTPS id l8si12100862vez.124.2013.08.14.21.52.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 Aug 2013 21:52:57 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.212.43 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.43; Received: by mail-vb0-f43.google.com with SMTP id h11so246121vbh.16 for ; Wed, 14 Aug 2013 21:52:57 -0700 (PDT) X-Gm-Message-State: ALoCoQkaMT41GlbDR0hq3OsPNJr1T7yiCiR7PgHny/SzVFKD2W+JF/pqviAmlfEEMEfT59DAu3/A X-Received: by 10.52.34.74 with SMTP id x10mr10729854vdi.13.1376542377858; Wed, 14 Aug 2013 21:52:57 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp251907vcz; Wed, 14 Aug 2013 21:52:57 -0700 (PDT) X-Received: by 10.14.219.198 with SMTP id m46mr19498753eep.41.1376542376668; Wed, 14 Aug 2013 21:52:56 -0700 (PDT) Received: from inca-roads.misterjones.org (inca-roads.misterjones.org. [213.251.177.50]) by mx.google.com with ESMTPS id l42si39631664eef.268.2013.08.14.21.52.56 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 14 Aug 2013 21:52:56 -0700 (PDT) Received-SPF: softfail (google.com: domain of transitioning marc.zyngier@arm.com does not designate 213.251.177.50 as permitted sender) client-ip=213.251.177.50; Received: from www-data by cheepnis.misterjones.org with local (Exim 4.80) (envelope-from ) id 1V9pYM-0001Ym-AV; Thu, 15 Aug 2013 06:52:54 +0200 To: Anup Patel Subject: Re: [PATCH] ARM64: KVM: Fix =?UTF-8?Q?coherent=5Ficache=5Fguest?= =?UTF-8?Q?=5Fpage=28=29=20for=20host=20with=20external=20L=33-cache=2E?= X-PHP-Originating-Script: 0:main.inc MIME-Version: 1.0 Date: Thu, 15 Aug 2013 05:52:53 +0100 From: Marc Zyngier Cc: , , Catalin Marinas , Will Deacon , , linux-arm-kernel Organization: ARM Ltd In-Reply-To: References: <1376480832-18705-1-git-send-email-pranavkumar@linaro.org> Message-ID: X-Sender: marc.zyngier@arm.com User-Agent: Roundcube Webmail/0.7.2 X-SA-Exim-Connect-IP: X-SA-Exim-Rcpt-To: anup@brainfault.org, linaro-kernel@lists.linaro.org, patches@linaro.org, catalin.marinas@arm.com, will.deacon@arm.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: marc.zyngier@arm.com X-SA-Exim-Scanned: No (on cheepnis.misterjones.org); SAEximRunCond expanded to false X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: marc.zyngier@arm.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.43 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi Anup, On 2013-08-14 15:22, Anup Patel wrote: > On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier > wrote: >> Hi Pranav, >> >> On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote: >>> Systems with large external L3-cache (few MBs), might have dirty >>> content belonging to the guest page in L3-cache. To tackle this, >>> we need to flush such dirty content from d-cache so that guest >>> will see correct contents of guest page when guest MMU is disabled. >>> >>> The patch fixes coherent_icache_guest_page() for external L3-cache. >>> >>> Signed-off-by: Pranavkumar Sawargaonkar >>> Signed-off-by: Anup Patel >>> --- >>> arch/arm64/include/asm/kvm_mmu.h | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/arch/arm64/include/asm/kvm_mmu.h >>> b/arch/arm64/include/asm/kvm_mmu.h >>> index efe609c..5129038 100644 >>> --- a/arch/arm64/include/asm/kvm_mmu.h >>> +++ b/arch/arm64/include/asm/kvm_mmu.h >>> @@ -123,6 +123,8 @@ static inline void >>> coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) >>> if (!icache_is_aliasing()) { /* PIPT */ >>> unsigned long hva = gfn_to_hva(kvm, gfn); >>> flush_icache_range(hva, hva + PAGE_SIZE); >>> + /* Flush d-cache for systems with external caches. */ >>> + __flush_dcache_area((void *) hva, PAGE_SIZE); >>> } else if (!icache_is_aivivt()) { /* non ASID-tagged >>> VIVT */ >>> /* any kind of VIPT cache */ >>> __flush_icache_all(); >> >> [adding Will to the discussion as we talked about this in the past] >> >> That's definitely an issue, but I'm not sure the fix is to hit the >> data >> cache on each page mapping. It looks overkill. >> >> Wouldn't it be enough to let userspace do the cache cleaning? >> kvmtools >> knows which bits of the guest memory have been touched, and can do a >> "DC >> DVAC" on this region. > > It seems a bit unnatural to have cache cleaning is user-space. I am > sure > other architectures don't have such cache cleaning in user-space for > KVM. > >> >> The alternative is do it in the kernel before running any vcpu - but >> that's not very nice either (have to clean the whole of the guest >> memory, which makes a full dcache clean more appealing). > > Actually, cleaning full d-cache by set/way upon first run of VCPU was > our second option but current approach seemed very simple hence > we went for this. > > If more people vote for full d-cache clean upon first run of VCPU > then > we should revise this patch. Can you please give the attached patch a spin on your HW? I've boot-tested it on a model, but of course I can't really verify that it fixes your issue. As far as I can see, it should fix it without any additional flushing. Please let me know how it goes. Thanks, M. diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index a5f28e2..a71a9bf 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -60,10 +60,12 @@ /* * The bits we set in HCR: * RW: 64bit by default, can be overriden for 32bit VMs + * TVM: Trap writes to VM registers (until MMU is on) * TAC: Trap ACTLR * TSC: Trap SMC * TSW: Trap cache operations by set/way * TWI: Trap WFI + * DC: Default Cacheable (until MMU is on) * TIDCP: Trap L2CTLR/L2ECTLR * BSU_IS: Upgrade barriers to the inner shareable domain * FB: Force broadcast of all maintainance operations @@ -74,7 +76,7 @@ */ #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \ HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \ - HCR_SWIO | HCR_TIDCP | HCR_RW) + HCR_DC | HCR_TVM | HCR_SWIO | HCR_TIDCP | HCR_RW) #define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF) /* Hyp System Control Register (SCTLR_EL2) bits */ diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 9492360..6d81d87 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -121,6 +121,42 @@ done: } /* + * Generic accessor for VM registers. Only called as long as HCR_TVM + * is set. + */ +static bool access_vm_reg(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + BUG_ON(!p->is_write); + + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); + return true; +} + +/* + * SCTLR_EL1 accessor. Only called as long as HCR_TVM is set. If the + * guest enables the MMU, we stop trapping the VM sys_regs and leave + * it in complete control of the caches. + */ +static bool access_sctlr_el1(struct kvm_vcpu *vcpu, + const struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + unsigned long val; + + BUG_ON(!p->is_write); + + val = *vcpu_reg(vcpu, p->Rt); + vcpu_sys_reg(vcpu, r->reg) = val; + + if (val & (1 << 0)) /* MMU Enabled? */ + vcpu->arch.hcr_el2 &= ~(HCR_DC | HCR_TVM); + + return true; +} + +/* * We could trap ID_DFR0 and tell the guest we don't support performance * monitoring. Unfortunately the patch to make the kernel check ID_DFR0 was * NAKed, so it will read the PMCR anyway. @@ -185,32 +221,32 @@ static const struct sys_reg_desc sys_reg_descs[] = { NULL, reset_mpidr, MPIDR_EL1 }, /* SCTLR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0001), CRm(0b0000), Op2(0b000), - NULL, reset_val, SCTLR_EL1, 0x00C50078 }, + access_sctlr_el1, reset_val, SCTLR_EL1, 0x00C50078 }, /* CPACR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0001), CRm(0b0000), Op2(0b010), NULL, reset_val, CPACR_EL1, 0 }, /* TTBR0_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b000), - NULL, reset_unknown, TTBR0_EL1 }, + access_vm_reg, reset_unknown, TTBR0_EL1 }, /* TTBR1_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b001), - NULL, reset_unknown, TTBR1_EL1 }, + access_vm_reg, reset_unknown, TTBR1_EL1 }, /* TCR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b010), - NULL, reset_val, TCR_EL1, 0 }, + access_vm_reg, reset_val, TCR_EL1, 0 }, /* AFSR0_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0001), Op2(0b000), - NULL, reset_unknown, AFSR0_EL1 }, + access_vm_reg, reset_unknown, AFSR0_EL1 }, /* AFSR1_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0001), Op2(0b001), - NULL, reset_unknown, AFSR1_EL1 }, + access_vm_reg, reset_unknown, AFSR1_EL1 }, /* ESR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0010), Op2(0b000), - NULL, reset_unknown, ESR_EL1 }, + access_vm_reg, reset_unknown, ESR_EL1 }, /* FAR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b0110), CRm(0b0000), Op2(0b000), - NULL, reset_unknown, FAR_EL1 }, + access_vm_reg, reset_unknown, FAR_EL1 }, /* PMINTENSET_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1001), CRm(0b1110), Op2(0b001), @@ -221,17 +257,17 @@ static const struct sys_reg_desc sys_reg_descs[] = { /* MAIR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1010), CRm(0b0010), Op2(0b000), - NULL, reset_unknown, MAIR_EL1 }, + access_vm_reg, reset_unknown, MAIR_EL1 }, /* AMAIR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1010), CRm(0b0011), Op2(0b000), - NULL, reset_amair_el1, AMAIR_EL1 }, + access_vm_reg, reset_amair_el1, AMAIR_EL1 }, /* VBAR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b0000), Op2(0b000), NULL, reset_val, VBAR_EL1, 0 }, /* CONTEXTIDR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1101), CRm(0b0000), Op2(0b001), - NULL, reset_val, CONTEXTIDR_EL1, 0 }, + access_vm_reg, reset_val, CONTEXTIDR_EL1, 0 }, /* TPIDR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1101), CRm(0b0000), Op2(0b100), NULL, reset_unknown, TPIDR_EL1 },