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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3859CC77B7E for ; Sun, 28 May 2023 11:05:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229483AbjE1LFG (ORCPT ); Sun, 28 May 2023 07:05:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34734 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229445AbjE1LFF (ORCPT ); Sun, 28 May 2023 07:05:05 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E7846A6 for ; Sun, 28 May 2023 04:05:03 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 73EA360B73 for ; Sun, 28 May 2023 11:05:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CA7BCC433D2; Sun, 28 May 2023 11:05:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685271902; bh=zn4SafuYiIteYV5WInbWC7srF7fv0j/nzgR9x3s99Po=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=syVRi+4YD9fUK2iAzaDOQJEZW/nWfnCNsoSfak2g49dcye9K0t+yglzYVDMHN2xk/ Af+PYmvztWva8ub0Y5AlMNoMjIG1dWhrLk7LjvE7YhhqhyjIg5Nym6oK2VqDctj3lL 89YEddXyzIJ6kdDWy9cuOiv+Ex+Qig32g8zxNj23kv5bbsJTj+Vvyjqw26G2ixua7c rJF1a48ouK+AUrZlkNJMiSFOxvEwaHbKBQCk5tmBwfXe47JamU+vDsMTtkUm318sCA o9hzz0FxCer0I7yzOCd1EL4cl2ipBIU6EDzfknbiqv5nd1+IXhFBKUTAWg863SuzYt jyDzPl8our5kg== 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 1q3ECq-000ruO-Da; Sun, 28 May 2023 12:05:00 +0100 Date: Sun, 28 May 2023 12:04:51 +0100 Message-ID: <87pm6kogx8.wl-maz@kernel.org> From: Marc Zyngier To: Jing Zhang Cc: KVM , KVMARM , ARMLinux , Oliver Upton , Will Deacon , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Fuad Tabba , Reiji Watanabe , Raghavendra Rao Ananta Subject: Re: [PATCH v10 5/5] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3 In-Reply-To: <20230522221835.957419-6-jingzhangos@google.com> References: <20230522221835.957419-1-jingzhangos@google.com> <20230522221835.957419-6-jingzhangos@google.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: jingzhangos@google.com, kvm@vger.kernel.org, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, oupton@google.com, will@kernel.org, pbonzini@redhat.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, tabba@google.com, reijiw@google.com, rananta@google.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 On Mon, 22 May 2023 23:18:35 +0100, Jing Zhang wrote: > > Refactor writings for ID_AA64PFR0_EL1.[CSV2|CSV3], > ID_AA64DFR0_EL1.PMUVer and ID_DFR0_ELF.PerfMon based on utilities > specific to ID register. > > Signed-off-by: Jing Zhang > --- > arch/arm64/include/asm/cpufeature.h | 1 + > arch/arm64/kernel/cpufeature.c | 2 +- > arch/arm64/kvm/sys_regs.c | 365 ++++++++++++++++++---------- > 3 files changed, 243 insertions(+), 125 deletions(-) Reading the result after applying this series, I feel like a stuck record. This final series still contains gems like this: static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, u64 val) { u8 csv2, csv3; /* * Allow AA64PFR0_EL1.CSV2 to be set from userspace as long as * it doesn't promise more than what is actually provided (the * guest could otherwise be covered in ectoplasmic residue). */ csv2 = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_EL1_CSV2_SHIFT); if (csv2 > 1 || (csv2 && arm64_get_spectre_v2_state() != SPECTRE_UNAFFECTED)) return -EINVAL; /* Same thing for CSV3 */ csv3 = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_EL1_CSV3_SHIFT); if (csv3 > 1 || (csv3 && arm64_get_meltdown_state() != SPECTRE_UNAFFECTED)) return -EINVAL; return set_id_reg(vcpu, rd, val); } Why do we have this? I've asked the question at least 3 times in the previous versions, and I still see the same code. If we have sane limits, the call to arm64_check_features() in set_id_reg() will catch the illegal write. So why do we have this at all? The whole point of the exercise was to unify the handling. But you're actually making it worse. So what's the catch? M. -- Without deviation from the norm, progress is not possible.