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 1B2F7C77B7E for ; Sat, 27 May 2023 13:41:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=IDakjD6RgYg+4tJ+JoWfA04WNW+l1TmsYl5V+1mAhfU=; b=nKgHopuU9ge14l epExyNJu0cyxdqtzxWhFdhOJ6xr7u0ab8kl8xKC9sl+TOh/pMabqsz86ON2OTgGvz8t8auP6PXTvh 2EzgQZg7EiM8ITTHcP9zKbrbDjivQqeqZVZnCDgVMoa1E3AA4UUnQCSg1UpvObbClEVwxlubBggg/ U0FVG4m6WSQ/pRqCs1Nai9Q7lME3jrBqe2Mbswqw1VLpjW2F7EOumR3xdMN7Dv9LWOA7HSpCtHYZr ygXVUQEF5rgsibVKR0HrEPh/W7WKg56nHHlGw8FnshlskPSWliOmTO72Tn4bevkS3zG+xXFN7AabI jgPh/y1YSfo9AylWtCMQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q2uAo-005pB5-0z; Sat, 27 May 2023 13:41:34 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q2uAl-005pAc-1p for linux-arm-kernel@lists.infradead.org; Sat, 27 May 2023 13:41:33 +0000 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 2669B60F83; Sat, 27 May 2023 13:41:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7C49DC433D2; Sat, 27 May 2023 13:41:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685194890; bh=mwhTDDByu0K/0clH3pT3wZsy+vlC2IbWjHkxpXdmxuo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=XOSi2fqQ6YSex80VsJRvmY/q2b1gUXvXf+I2icP9Htfb032C9LhoUk6/hMueZa0lv Z7C8oLI8S5VVMlaYyruM2xSb7VWnWpTIZcob8CBgQzJ5JeICbuczgA8RkxEZuITGID 9HpMfj/rb7mBvSjV1rA7NSa7B6o3yCKk/sXIZdeKcC3HXamibev+nz9UfvrEzqTYac ZbJYce3+Yazy+fFvNTE8ic9ZvYBp9HGZdeGTfj1TCFoW8DsViu7PYbFgMRdI5soumL ijd/M01GVe/znG9As1GGtjBy+cS49RVDrRddcIKxZHGVIlG7PR+isQULi6dRnhRIKe Y2XMxhvtHLk7A== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1q2uAi-000lBR-3c; Sat, 27 May 2023 14:41:28 +0100 Date: Sat, 27 May 2023 14:41:27 +0100 Message-ID: <86ilcddh88.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: Jing Zhang , 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 4/5] KVM: arm64: Reuse fields of sys_reg_desc for idreg In-Reply-To: References: <20230522221835.957419-1-jingzhangos@google.com> <20230522221835.957419-5-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 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, 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 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230527_064131_681691_4B66F1A4 X-CRM114-Status: GOOD ( 36.06 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 26 May 2023 22:37:04 +0100, Oliver Upton wrote: > > Hi Jing, > > On Mon, May 22, 2023 at 10:18:34PM +0000, Jing Zhang wrote: > > Since reset() and val are not used for idreg in sys_reg_desc, they would > > be used with other purposes for idregs. > > The callback reset() would be used to return KVM sanitised id register > > values. The u64 val would be used as mask for writable fields in idregs. > > Only bits with 1 in val are writable from userspace. > > The tense of the changelog is wrong (should be in an imperative mood). > Maybe something like: > > sys_reg_desc::{reset, val} are presently unused for ID register > descriptors. Repurpose these fields to support user-configurable ID > registers. > > Use the ::reset() function pointer to return the sanitised value of a > given ID register, optionally with KVM-specific feature sanitisation. > Additionally, keep a mask of writable register fields in ::val. > > > Signed-off-by: Jing Zhang > > --- > > arch/arm64/kvm/sys_regs.c | 101 +++++++++++++++++++++++++++----------- > > arch/arm64/kvm/sys_regs.h | 15 ++++-- > > 2 files changed, 82 insertions(+), 34 deletions(-) > > > > [...] > > > +/* > > + * Since reset() callback and field val are not used for idregs, they will be > > + * used for specific purposes for idregs. > > + * The reset() would return KVM sanitised register value. The value would be the > > + * same as the host kernel sanitised value if there is no KVM sanitisation. > > + * The val would be used as a mask indicating writable fields for the idreg. > > + * Only bits with 1 are writable from userspace. This mask might not be > > + * necessary in the future whenever all ID registers are enabled as writable > > + * from userspace. > > + */ > > + > > /* sys_reg_desc initialiser for known cpufeature ID registers */ > > #define ID_SANITISED(name) { \ > > SYS_DESC(SYS_##name), \ > > @@ -1751,6 +1788,8 @@ static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu, > > .get_user = get_id_reg, \ > > .set_user = set_id_reg, \ > > .visibility = id_visibility, \ > > + .reset = general_read_kvm_sanitised_reg,\ > > + .val = 0, \ > > I generally think unions are more trouble than they're worth, but it > might make sense to throw the fields with dual meaning into one, like > > struct sys_reg_desc { > > [...] > union { > struct { > void (*reset)(struct kvm_vcpu *, const struct sys_reg_desc *); > u64 val; > }; > struct { > u64 (*read_sanitised)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *); > u64 mask; > }; > }; > } > > You could then avoid repainting the world to handle ->reset() returning > a value and usage of the fields in an id register context become a bit > more self-documenting. And you get to play with fire while you do it! > > Let's see if the other side of the pond agrees with my bikeshedding... I don't think this works just as suggested. It completely breaks all the existing macros that use implicit (in order) initialisers. Not hard to fix, but pretty invasive (I count 656 warnings in my current tree). I agree it is much cleaner though. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel