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 3D043C001DE for ; Fri, 28 Jul 2023 18:34:05 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=r1czqRrfGAIUr/dNdbCPJQHN1Y0++8V/QGARiHq9q1g=; b=ggOXx8gWVy1+Sl 7kWRQPyW3BMMC+Xv9rktfSEAaS8IrDWNCbv+y2Ck5eVWMb2g1oLeEmi8OCiao8WEbPXwuBzmUiIJe TiryuHGJvnXHhHYX6U7gcHTyeE8wLyCbZOJlvASGEuKGZkWKA5wYarLN4JSvbN4gnvGunRqWrEd4g I7rEn765jDaa52hyTSGCxoFsipY6CCdtGMtnrEHYqYF3kE6EooLOXfWNmgXYyhSFPwiXlWTytujV6 KRygQmM1spJUhJPjE7rX+NugVFXSuYHyz9FPVsXMDIK8VU6QgTyHvzXKVTHkgX1GCf/WZVWzst47n yQA7l6koGkSosbBhtc6Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qPSHY-004YDE-1j; Fri, 28 Jul 2023 18:33:44 +0000 Received: from out-89.mta0.migadu.com ([2001:41d0:1004:224b::59]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qPSHV-004YCZ-0k for linux-arm-kernel@lists.infradead.org; Fri, 28 Jul 2023 18:33:43 +0000 Date: Fri, 28 Jul 2023 18:33:31 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1690569216; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Jrx5aHEojE/xaYVofIxjbxwFqRcq5TCTvqXzSOvaD+w=; b=MKWugsJjBcoOzco0vnyQq9bssNxNpUL2c+be5QVp7ccwiOnu3J2jqvCdjHgQ0HdEGPjCsk Ie+VNEHwbgg6Z09yY/mGpjlE29T3JgRUv84p3+JzaIrvya43IdA3jYQx4RpexRAmnm0coN vo6pV5SZF74el/A+QGSYb68n3UNqAiQ= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Marc Zyngier 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 Message-ID: References: <20230728082952.959212-1-maz@kernel.org> <20230728082952.959212-15-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230728082952.959212-15-maz@kernel.org> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230728_113341_942182_62FD4FF1 X-CRM114-Status: GOOD ( 24.35 ) 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, 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)); > +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. > +} > + > +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. > + } > + > + 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? -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel