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 69C45C0015E for ; Sat, 29 Jul 2023 09:19:47 +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=+Z3BCEC8HF/I3rKwkfCQYdtIuQlauUfloClZS24aFmU=; b=Z7LR53knEM329G LbSVpP6erizRMHkBgNUnmyjD5WNKj1mTTXhxe7nqvXWZ+koUG88gNSspfofZXE+UwajjwOjOUrvq3 MvklXv/6UF8IYuQ0USgjZA3M3INNxYB9OlVciqTKRPGVGTnqxY6aOuMfCwlp2ZPgj6nwcCnYL/3yi 1gmNjNa0R1m246XBg7B0I7zrfxjFRz7Bz2y0t3qxyY+3vAoI3s6jX2gfpy6kZdZE7dME2WRhS3OLG 9MoRV6XEgT8cghig7UL9pSu/yijEvawlaaNUsms5AvqQEloWNszSDrK0aECnPwVWpa/+2td+CCmel AgHAArZsoMKibiFHd5sg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qPg6e-006v6S-2v; Sat, 29 Jul 2023 09:19:24 +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 1qPg6b-006v5u-2A for linux-arm-kernel@lists.infradead.org; Sat, 29 Jul 2023 09:19:23 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D283560AF7; Sat, 29 Jul 2023 09:19:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3E6E5C433C7; Sat, 29 Jul 2023 09:19:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1690622360; bh=FX07c1aB+2Wb3InFYiIfVDpSZ+Ap+3iMqLoJ5Y9kWWE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=TWfouKzodPdEEIVYeXt2c8ZjPKeIAK+FPc474vAhrzvUOBDLEO5APKiAF8WCtCEer ZqBRuD+YSBWZ45rOuxGxQ7NiPDBaMSLZaqn6vpcCSqFVndVGT4oLiXZZkQRfZ8pbuy 3p1CShzwLoAabzOstDHx2rRnjE9lrySq5rJ3g0aiqzDtSEF+Tr5otoK4D5bHBrN853 WIkDhgVaX4K56hdxC0U8q5TL2xBWn0IvJ3L8lmTHSJq37yGxAjt6iHMUBDJh5n9/Nk k7jXz12W3OF3iSr6KeRWCPDEKCYaNwSmwL7aUqIa3RJF9sOZPZxIcnhItg3Mk+VCOp ghfdQsyaZn4IQ== 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 1qPg6X-000KMP-NK; Sat, 29 Jul 2023 10:19:17 +0100 Date: Sat, 29 Jul 2023 10:19:17 +0100 Message-ID: <87fs57qdm2.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Catalin Marinas , Eric Auger , Mark Brown , Mark Rutland , Will Deacon , Alexandru Elisei , Andre Przywara , Chase Conklin , Ganapatrao Kulkarni , Darren Hart , Miguel Luis , James Morse , Suzuki K Poulose , Zenghui Yu Subject: Re: [PATCH v2 14/26] KVM: arm64: nv: Add trap forwarding infrastructure In-Reply-To: References: <20230728082952.959212-1-maz@kernel.org> <20230728082952.959212-15-maz@kernel.org> 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") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, eric.auger@redhat.com, broonie@kernel.org, mark.rutland@arm.com, will@kernel.org, alexandru.elisei@arm.com, andre.przywara@arm.com, chase.conklin@arm.com, gankulkarni@os.amperecomputing.com, darren@os.amperecomputing.com, miguel.luis@oracle.com, james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.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-20230729_021921_821063_4FF0B1C1 X-CRM114-Status: GOOD ( 39.41 ) 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, 28 Jul 2023 19:33:31 +0100, Oliver Upton wrote: > > On Fri, Jul 28, 2023 at 09:29:40AM +0100, Marc Zyngier wrote: > > [...] > > > +/* > > + * Bit assignment for the trap controls. We use a 64bit word with the > > + * following layout for each trapped sysreg: > > + * > > + * [9:0] enum trap_group (10 bits) > > + * [13:10] enum fgt_group_id (4 bits) > > + * [19:14] bit number in the FGT register (6 bits) > > + * [20] trap polarity (1 bit) > > + * [62:21] Unused (42 bits) > > + * [63] RES0 - Must be zero, as lost on insertion in the xarray > > + */ > > +union trap_config { > > + u64 val; > > + struct { > > + unsigned long cgt:10; /* Coarse trap id */ > > + unsigned long fgt:4; /* Fing Grained Trap id */ > > + unsigned long bit:6; /* Bit number */ > > + unsigned long pol:1; /* Polarity */ > > + unsigned long unk:42; /* Unknown */ > > + unsigned long mbz:1; /* Must Be Zero */ > > + }; > > +}; > > Correct me if I'm wrong, but I don't think the compiler is going to > whine if any of these bitfields are initialized with a larger value than > can be represented... Do you think some BUILD_BUG_ON() is in order to > ensure that trap_group fits in ::cgt? > > BUILD_BUG_ON(__NR_TRAP_IDS__ >= BIT(10)); Indeed. This might also apply to ::fgt, and I want to add some sanity checks to verify that the whole union isn't larger than a 'void *', as we rely on that. > > > +struct encoding_to_trap_config { > > + const u32 encoding; > > + const u32 end; > > + const union trap_config tc; > > +}; > > + > > +#define SR_RANGE_TRAP(sr_start, sr_end, trap_id) \ > > + { \ > > + .encoding = sr_start, \ > > + .end = sr_end, \ > > + .tc = { \ > > + .cgt = trap_id, \ > > + }, \ > > + } > > + > > +#define SR_TRAP(sr, trap_id) SR_RANGE_TRAP(sr, sr, trap_id) > > + > > +/* > > + * Map encoding to trap bits for exception reported with EC=0x18. > > + * These must only be evaluated when running a nested hypervisor, but > > + * that the current context is not a hypervisor context. When the > > + * trapped access matches one of the trap controls, the exception is > > + * re-injected in the nested hypervisor. > > + */ > > +static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = { > > +}; > > + > > +static DEFINE_XARRAY(sr_forward_xa); > > + > > +static union trap_config get_trap_config(u32 sysreg) > > +{ > > + return (union trap_config) { > > + .val = xa_to_value(xa_load(&sr_forward_xa, sysreg)), > > + }; > > Should we be checking for NULL here? AFAICT, the use of sentinel values > in the trap_group enum would effectively guarantee each trap_config has > a nonzero value. if xa_load() returns NULL, xa_to_value() will still give us a 0, which is an indication of a sysreg that isn't present in the trap configuration. This can happen if we trap something that isn't yet supported in NV, which is quite common. This allows us to use features on the host without having to immediately write the same support for NV guests. But this is obviously a temporary situation. At some point, I'll become a total bastard and demand that people treat NV as a first class citizen. One day ;-). > > > +} > > + > > +void __init populate_nv_trap_config(void) > > +{ > > + for (int i = 0; i < ARRAY_SIZE(encoding_to_cgt); i++) { > > + const struct encoding_to_trap_config *cgt = &encoding_to_cgt[i]; > > + void *prev; > > + > > + prev = xa_store_range(&sr_forward_xa, cgt->encoding, cgt->end, > > + xa_mk_value(cgt->tc.val), GFP_KERNEL); > > + WARN_ON(prev); > > Returning the error here and failing the overall KVM initialization > seems to be the safest option. The WARN is still handy, though. Yeah, this has found a number of bugs initially. Happy to fail the initialisation after we've iterated over all the array (nothing is more annoying than fixing a bunch of errors iteratively). > > > + } > > + > > + kvm_info("nv: %ld coarse grained trap handlers\n", > > + ARRAY_SIZE(encoding_to_cgt)); > > + > > +} > > + > > +static enum trap_behaviour get_behaviour(struct kvm_vcpu *vcpu, > > + const struct trap_bits *tb) > > +{ > > + enum trap_behaviour b = BEHAVE_HANDLE_LOCALLY; > > + u64 val; > > + > > + val = __vcpu_sys_reg(vcpu, tb->index); > > + if ((val & tb->mask) == tb->value) > > + b |= tb->behaviour; > > + > > + return b; > > +} > > + > > +static enum trap_behaviour __do_compute_trap_behaviour(struct kvm_vcpu *vcpu, > > + const enum trap_group id, > > + enum trap_behaviour b) > > +{ > > + switch (id) { > > + const enum trap_group *cgids; > > + > > + case __RESERVED__ ... __MULTIPLE_CONTROL_BITS__ - 1: > > + if (likely(id != __RESERVED__)) > > + b |= get_behaviour(vcpu, &coarse_trap_bits[id]); > > + break; > > + case __MULTIPLE_CONTROL_BITS__ ... __COMPLEX_CONDITIONS__ - 1: > > + /* Yes, this is recursive. Don't do anything stupid. */ > > + cgids = coarse_control_combo[id - __MULTIPLE_CONTROL_BITS__]; > > + for (int i = 0; cgids[i] != __RESERVED__; i++) > > + b |= __do_compute_trap_behaviour(vcpu, cgids[i], b); > > Would it make sense to WARN here if one of the child trap ids was in the > recursive range? This might be needed at some point, but we can probably tighten it for now. Thanks, 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