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=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 29E5CC433B4 for ; Sat, 10 Apr 2021 09:54:51 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 8F65261182 for ; Sat, 10 Apr 2021 09:54:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8F65261182 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 212864B722; Sat, 10 Apr 2021 05:54:50 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OQXBG8jNj6K8; Sat, 10 Apr 2021 05:54:48 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id E86D44B724; Sat, 10 Apr 2021 05:54:48 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4FD664B722 for ; Sat, 10 Apr 2021 05:54:47 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v0G0ZpMkPAnN for ; Sat, 10 Apr 2021 05:54:46 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 1256E4B1F6 for ; Sat, 10 Apr 2021 05:54:46 -0400 (EDT) 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 29F87610A2; Sat, 10 Apr 2021 09:54:44 +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 1lVAKA-006gnw-7W; Sat, 10 Apr 2021 10:54:42 +0100 Date: Sat, 10 Apr 2021 10:54:41 +0100 Message-ID: <875z0ufori.wl-maz@kernel.org> From: Marc Zyngier To: Shaokun Zhang Subject: Re: [RFC] KVM: arm64: Support FEAT_CCIDX In-Reply-To: <1618042597-59294-1-git-send-email-zhangshaokun@hisilicon.com> References: <1618042597-59294-1-git-send-email-zhangshaokun@hisilicon.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") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: zhangshaokun@hisilicon.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Sat, 10 Apr 2021 09:16:37 +0100, Shaokun Zhang wrote: > > CCSIDR_EL1 can be implemented as 64-bit format inferred by CCIDX field > in ID_AA64MMFR2_EL1. The bits of Numsets and Associativity are different > from the 32-bit format. Let's support this feature. > > Cc: Marc Zyngier > Cc: James Morse > Cc: Alexandru Elisei > Cc: Suzuki K Poulose > Signed-off-by: Shaokun Zhang > --- > arch/arm64/kvm/sys_regs.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 52fdb9a015a4..0dc822cef20b 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -18,6 +18,7 @@ > > #include > #include > +#include If you are going to add this (why?), at least add it in alphabetic order. > #include > #include > #include > @@ -95,9 +96,9 @@ static u32 cache_levels; > #define CSSELR_MAX 14 > > /* Which cache CCSIDR represents depends on CSSELR value. */ > -static u32 get_ccsidr(u32 csselr) > +static u64 get_ccsidr(u32 csselr) > { > - u32 ccsidr; > + u64 ccsidr; > > /* Make sure noone else changes CSSELR during this! */ > local_irq_disable(); > @@ -1275,12 +1276,16 @@ static bool access_csselr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > - u32 csselr; > + u32 csselr, ccidx; > + u64 mmfr2; > > if (p->is_write) > return write_to_read_only(vcpu, p, r); > > csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1); > + mmfr2 = read_sysreg_s(SYS_ID_AA64MMFR2_EL1); > + ccidx = cpuid_feature_extract_unsigned_field(mmfr2, > + ID_AA64MMFR2_CCIDX_SHIFT); What happens on an asymmetric system where only some of the CPUs have FEAT_CCIDX? > p->regval = get_ccsidr(csselr); > > /* > @@ -1295,8 +1300,13 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > * geometry (which is not permitted by the architecture), they would > * only do so for virtually indexed caches.] > */ > - if (!(csselr & 1)) // data or unified cache > - p->regval &= ~GENMASK(27, 3); > + if (!(csselr & 1)) { // data or unified cache > + if (ccidx) > + p->regval &= ~(GENMASK(23, 3) | GENMASK(55, 32)); > + else > + p->regval &= ~GENMASK(27, 3); > + } > + > return true; > } > > @@ -2521,7 +2531,7 @@ static bool is_valid_cache(u32 val) > static int demux_c15_get(u64 id, void __user *uaddr) > { > u32 val; > - u32 __user *uval = uaddr; > + u64 __user *uval = uaddr; What? Has the user API changed while I wasn't looking? Getting CCSIDR can only return a 32bit quantity on AArch32. In fact, CCSIDR is *always* 32bit, CCIDX or not. I have no idea what you are trying to do here, but at best you are now corrupting 32bit of userspace. > > /* Fail if we have unknown bits set. */ > if (id & ~(KVM_REG_ARCH_MASK|KVM_REG_SIZE_MASK|KVM_REG_ARM_COPROC_MASK > @@ -2545,8 +2555,9 @@ static int demux_c15_get(u64 id, void __user *uaddr) > > static int demux_c15_set(u64 id, void __user *uaddr) > { > - u32 val, newval; > - u32 __user *uval = uaddr; > + u32 val; > + u64 newval; > + u64 __user *uval = uaddr; Same brokenness. > > /* Fail if we have unknown bits set. */ > if (id & ~(KVM_REG_ARCH_MASK|KVM_REG_SIZE_MASK|KVM_REG_ARM_COPROC_MASK What about CCSIDR2_EL1, which is mandatory when FEAT_CCSIDX is present? How about the AArch32 handling of that register? I don't think you have though this one through. Another question is: why should we care at all? Why don't we drop all that and only implement a virtual cache topology? A VM shouldn't care at all about this, and we are already lying about the topology anyway. We could even get the VMM to set whatever stupid topology it wants for the sake of it (and to restore previously created VMs). M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm