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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6BD83C30658 for ; Sat, 29 Jun 2024 08:56:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Qg0M2wu82+ut1h1zyoGcn9GG9ubq7vsJyZOUDz6D5I8=; b=eJhUY3TTzpXFfonOOvDq1IaFRB MprAw7pA5GjTtmbD6oW/dgG0pXfZwbl0nQWhHzMz00SlZfo5xzzRdGUt2LW8n1f5PGPCQvk7lWgrY 1ZuLlRRan9h6tTQ4RLbHFYdh8jT2FuPqPejX8cxUXUlnadYiUfbye/iN2XWhLPVI9r8PUfDhvoIFa FJ/2twqurzba3nUfAd7ZRYyzlPcNG8ZLKHrmIEGGZZNjCW4rxaKB+geOvP5gkguxGMX16LPHn5gHX Xi9D5Dr7gKeTm+dbpIXRrsuMvEdWjKgnuBggq0MItN7bRYONBbgLSr9eYlid5ODBd3phN/e+xy2nj A7Z9zBhg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sNTsb-0000000G9Hz-2B7d; Sat, 29 Jun 2024 08:56:21 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sNTsR-0000000G9G2-1q2c for linux-arm-kernel@lists.infradead.org; Sat, 29 Jun 2024 08:56:13 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 7589B60203; Sat, 29 Jun 2024 08:56:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1A94CC4AF0B; Sat, 29 Jun 2024 08:56:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1719651369; bh=Usk4hC7B6imkCxrHaQXk94Nplnxwr+jME/OfM7kVFc4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=iegBsroSG2xpT5nJosVosuNactH50axeeo1QH67qfyq0eTxMpVc7MKuvqBtW/SK9F 26AifYxbFC/BIF6iTXvTces1j31xC/DXwCm2rCqdPfP/mimlQyK8yD11NQEeIjGj6w V96dc5UAsBVRWTjsl3uVYSPUpyDWknAx72TV/VjYTsKfSs5cCyTDF4SoMNKITZEMBX LiDJCJhY6900sH6al1XZGqWYTHOR7ZXf6/1kRqnCFMa/SzMrfxxzaI2GHzjktD2OAz w0ksU9risLol2i5W6yTe0iH4xqHQyNojn2r4TD5U6jUicFExJGwwoZMiSUztJkmxHY arPftWRUhKchQ== Received: from sofa.misterjones.org ([185.219.108.64] 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.95) (envelope-from ) id 1sNTsM-008MU8-PI; Sat, 29 Jun 2024 09:56:06 +0100 Date: Sat, 29 Jun 2024 09:55:54 +0100 Message-ID: <87ikxsi0v9.wl-maz@kernel.org> From: Marc Zyngier To: Shaoqin Huang Cc: Oliver Upton , kvmarm@lists.linux.dev, James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/2] KVM: arm64: Allow userspace to change ID_AA64PFR1_EL1 In-Reply-To: <20240628060454.1936886-2-shahuang@redhat.com> References: <20240628060454.1936886-1-shahuang@redhat.com> <20240628060454.1936886-2-shahuang@redhat.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/28.2 (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: 185.219.108.64 X-SA-Exim-Rcpt-To: shahuang@redhat.com, oliver.upton@linux.dev, kvmarm@lists.linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240629_015611_631541_F9BEE0F9 X-CRM114-Status: GOOD ( 30.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 28 Jun 2024 07:04:51 +0100, Shaoqin Huang wrote: > > Allow userspace to change the guest-visible value of the register with > some severe limitation: > > - No changes to features not virtualized by KVM (MPAM_frac, RAS_frac, > SME, RNDP_trap). > > - No changes to features (CSV2_frac, NMI, MTE_frac, GCS, THE, MTEX, > DF2, PFAR) which haven't been added into the ftr_id_aa64pfr1[]. > Because the struct arm64_ftr_bits definition for each feature in the > ftr_id_aa64pfr1[] is used by arm64_check_features. If they're not > existing in the ftr_id_aa64pfr1[], the for loop won't check the if > the new_val is safe for those features. > --- > arch/arm64/kvm/sys_regs.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) This is getting very tiring: - this isn't a valid patch, as it doesn't carry a proper SoB. You did it last time, I pointed it out, and you ignored me. - this is *wrong*. The moment the kernel publishes any of the fields you have decided to ignore, the restoring of a VM on a new kernel will fail if the host and the VM have different values. > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 22b45a15d068..159cded22357 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2306,7 +2306,9 @@ static const struct sys_reg_desc sys_reg_descs[] = { > ID_AA64PFR0_EL1_GIC | > ID_AA64PFR0_EL1_AdvSIMD | > ID_AA64PFR0_EL1_FP), }, > - ID_SANITISED(ID_AA64PFR1_EL1), > + ID_WRITABLE(ID_AA64PFR1_EL1, ID_AA64PFR1_EL1_MTE | Why? If the VM has been created with MTE support, this must be obeyed. > + ID_AA64PFR1_EL1_SSBS | > + ID_AA64PFR1_EL1_BT), > ID_UNALLOCATED(4,2), > ID_UNALLOCATED(4,3), > ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0), So let me be very blunt: - you *must* handle *all* the fields described in that register. There are 15 valid fields there, and I want to see all 15 fields being explicitly dealt with. - fields that can be changed without ill effect must be enabled for write, irrespective of what the cpufeature code does (CSV2_frac, for example). - fields that have a fixed value because KVM doesn't handle the corresponding feature must be explicitly disabled in the register accessor (MPAM, RNDR, MTEX, THE...). Just like we do for SME. - fields that correspond to a feature that is controlled by an internal flag (MTE) must not be writable. Just like we do for PAuth in ID_AA64ISAR1_EL1. - you *must* handle *all* the fields described in that register. Until I see all of the above, I will not take this patch. If you don't want to do this, that's absolutely fine by me. Just don't post another patch. But if you do, this is the deal. M. -- Without deviation from the norm, progress is not possible.