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 2901BC27C4F for ; Thu, 13 Jun 2024 22:20:27 +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:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From: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=gvCVPrByglYHE/HphLfz/4s/chgo2zEH02FdJCIN0CE=; b=IxMX6tEKoksmDnhNV42I2npRBY ZV2e0tCAa72MijEHv6gOJvnrHVea0kghXJ7THlQ4wUih4JBHPZXgn3oqoR17pD+y+4pn6nf1//H2c aiuFshFD18+wrHLs/FwXEKy72A7AkeBIKuHHRAGQg+hY4YYLmm585/2FFGnymIT1oKRCPA62egaZL 4NTUnupmjtzVeaG3R3O3HfD8GCPcSrZuKow9JPIaKzMKkB2dgsDO/yZvCU33EevMPjlMGcpIvGqkB tpphXYhWcy0CFCh9KwmEEqqsPbNSLWyO8vFUE7CstuiIcRttxgjlddkbamulTbLUsavFSPlxlvJZP vycT6lYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sHsnm-00000000e8q-2dtB; Thu, 13 Jun 2024 22:20:14 +0000 Received: from out-173.mta1.migadu.com ([95.215.58.173]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sHsni-00000000e80-2BZ9 for linux-arm-kernel@lists.infradead.org; Thu, 13 Jun 2024 22:20:13 +0000 X-Envelope-To: sebott@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1718317203; 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=gvCVPrByglYHE/HphLfz/4s/chgo2zEH02FdJCIN0CE=; b=Hve6XeQuraY6xBWjfm3sMkSRrAEP0y8s/jcC1TS3Nk3c45SuqDIP/DtIq9iIsGsYY7ualL H40UQUxMkjXkNAhyjm6qv5IapVLxsUMva5/wzcIa2lQQUfD5XWLnX746+0FMwOihNz5RuC G1TaP73AgWKgPybrbQtboRjn2JtXWp0= X-Envelope-To: linux-arm-kernel@lists.infradead.org X-Envelope-To: kvmarm@lists.linux.dev X-Envelope-To: linux-kernel@vger.kernel.org X-Envelope-To: maz@kernel.org X-Envelope-To: james.morse@arm.com X-Envelope-To: suzuki.poulose@arm.com X-Envelope-To: catalin.marinas@arm.com X-Envelope-To: will@kernel.org X-Envelope-To: shahuang@redhat.com X-Envelope-To: eric.auger@redhat.com Date: Thu, 13 Jun 2024 22:19:56 +0000 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Sebastian Ott Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, Marc Zyngier , James Morse , Suzuki K Poulose , Catalin Marinas , Will Deacon , Shaoqin Huang , Eric Auger Subject: Re: [PATCH v4 3/6] KVM: arm64: add emulation for CTR_EL0 register Message-ID: References: <20240603130507.17597-1-sebott@redhat.com> <20240603130507.17597-4-sebott@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240603130507.17597-4-sebott@redhat.com> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240613_152011_231204_D96BB447 X-CRM114-Status: GOOD ( 16.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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Sebastian, On Mon, Jun 03, 2024 at 03:05:04PM +0200, Sebastian Ott wrote: [...] > +static int validate_cache_topology(struct kvm_vcpu *vcpu, u64 ctr_el0) > +{ > + const struct sys_reg_desc *clidr_el1; > + unsigned int i; > + int ret; > + > + clidr_el1 = get_sys_reg_desc(SYS_CLIDR_EL1); > + if (!clidr_el1) > + return -ENOENT; This doesn't actually matter if we agree on dropping the cross-checking, but if this lookup fails it is 100% a KVM bug. Returning ENOENT isn't exactly right here, since it gives userspace the impression that the sysreg index it tried to access does not exist. So in the future it'd be good to return EINVAL in places where the kernel did something stupid, probably with a warning for good measure. > +static int set_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > + u64 val) > +{ > + u64 ctr, writable_mask = rd->val; > + int ret = 0; > + > + mutex_lock(&vcpu->kvm->arch.config_lock); > + ctr = vcpu->kvm->arch.ctr_el0; > + if (val == ctr) > + goto out_unlock; > + > + ret = -EBUSY; > + if (kvm_vm_has_ran_once(vcpu->kvm)) > + goto out_unlock; > + > + ret = -EINVAL; > + if ((ctr & ~writable_mask) != (val & ~writable_mask)) > + goto out_unlock; > + > + if (((ctr & CTR_EL0_DIC_MASK) < (val & CTR_EL0_DIC_MASK)) || > + ((ctr & CTR_EL0_IDC_MASK) < (val & CTR_EL0_IDC_MASK)) || > + ((ctr & CTR_EL0_DminLine_MASK) < (val & CTR_EL0_DminLine_MASK)) || > + ((ctr & CTR_EL0_IminLine_MASK) < (val & CTR_EL0_IminLine_MASK))) { > + goto out_unlock; I'd prefer if we addressed the issue w/ arm64_check_features() by making CTR_EL0 behave like the other registers in the ID space instead of open-coding these sorts of checks. I believe that can be accomplished by using kvm_read_sanitised_id_reg() as the ::reset() function in the descriptor and initializing kvm->arch.ctr_el0 in kvm_reset_id_regs(). > + } > + ret = validate_cache_topology(vcpu, val); > + if (ret) > + goto out_unlock; My concerns about adding these sort of cross-checks remains. The sysreg code will become exponentially more messy with each cross-register check we add, given the complete lack of ordering on the UAPI. So long as KVM has independently tested the validity of the cache hierarchy and CTR_EL0 against the capabilities of hardware, we know that userspace cannot advertise more than what's supported in hardware. If CLIDR_EL1 doesn't line up with the value of CTR_EL0 exposed to the guest then it is a userspace bug. There simply is no amount of foolproofing that can be done in KVM to protect against a buggy VMM. -- Thanks, Oliver