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 0C127C54EBD for ; Sun, 8 Jan 2023 18:56:16 +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=ZdYXOoH8J5lLGsCp0JI7Q/VlF7J69D2H48/lx3A2324=; b=dWpItfH+aFt2jN qEx9MKo4L43H3rGGbWcjUrSp1Jr5URtN7+xMBBzPAx3G5tYH6L7tz8qC5pTT3vq/fp+xi62E9/mfx ExmQWeEe4demzbUuUApXz66IoZmThVWpCBzK0ajJiMujLI+FAdK9TKNcc83unHFW2SBulqjBpg3Ej tvJLbBXEQpeCgSzVBNWSP0awmU5ijJqGjgH4eieQ/xBC6zIE4IxPkGIMtDRLDNkvfu2VJ9DxS4XYZ tlZBIaro4X/XEnAe+yonFg3JtKrWgQb1VfBt9hbGN1ufAqfUyIOBM7st9oXBGvh/InD9GadheZPUl Iy+pPOKqLwIbwpRrb9+g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pEap5-00FMzr-7h; Sun, 08 Jan 2023 18:55:11 +0000 Received: from out2.migadu.com ([2001:41d0:2:aacc::]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pEap0-00FMxM-Mz for linux-arm-kernel@lists.infradead.org; Sun, 08 Jan 2023 18:55:09 +0000 Date: Sun, 8 Jan 2023 10:54:47 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1673204094; 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=4GStBHBtIG3HNcYtPXIN3DoEZyGgjZQviRxuFt95GoM=; b=X13ajs2XGrutReGcRqwlVGSU7/tqNFg1P9f23nK0lDUDSAiOeTw7qrF3ci84nm7YJOjcP4 Ov+VVCmftGYQCtYqKR6LCRELb+P4AeaslGWvkp82IcAwl6v6rbLOW9juS3umMnEVqWC5iI p9mHX2vw+0sqF4w7wSYx73h1aRCDelo= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Akihiko Odaki Cc: Mark Brown , Marc Zyngier , linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Mathieu Poirier , Suzuki K Poulose , Alexandru Elisei , James Morse , Will Deacon , Catalin Marinas , asahi@lists.linux.dev, Alyssa Rosenzweig , Sven Peter , Hector Martin Subject: Re: [PATCH v5 6/7] KVM: arm64: Mask FEAT_CCIDX Message-ID: References: <20221230095452.181764-1-akihiko.odaki@daynix.com> <20221230095452.181764-7-akihiko.odaki@daynix.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230108_105506_939857_D7BEA081 X-CRM114-Status: GOOD ( 26.66 ) 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 Akihiko, On Sat, Jan 07, 2023 at 06:53:28PM +0900, Akihiko Odaki wrote: > On 2023/01/06 7:22, Oliver Upton wrote: > > On Fri, Dec 30, 2022 at 06:54:51PM +0900, Akihiko Odaki wrote: > > > The CCSIDR access handler masks the associativity bits according to the > > > bit layout for processors without FEAT_CCIDX. KVM also assumes CCSIDR is > > > 32-bit where it will be 64-bit if FEAT_CCIDX is enabled. Mask FEAT_CCIDX > > > so that these assumptions hold. > > > > > > Suggested-by: Marc Zyngier > > > Signed-off-by: Akihiko Odaki > > > > FYI, I'm an idiot and replied to v4 of this patch... Forwarding comments > > below: > > > > > --- > > > arch/arm64/kvm/sys_regs.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index f4a7c5abcbca..aeabf1f3370b 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -1124,6 +1124,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r > > > ID_DFR0_PERFMON_SHIFT, > > > kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_4 : 0); > > > break; > > > + case SYS_ID_AA64MMFR2_EL1: > > > + val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK; > > > + break; > > > + case SYS_ID_MMFR4_EL1: > > > + val &= ~ARM64_FEATURE_MASK(ID_MMFR4_CCIDX); > > > + break; > > > > Not that it is necessarily worth addressing, but I wanted to point > > something out. > > > > This change breaks migration from older kernels on implementations w/ > > FEAT_CCIDX. There is most likely exactly 0 of those in the wild, but > > we need to be careful changing user-visible stuff like this. > > > > -- > > Thanks, > > Oliver > > I also don't think whether FEAT_CCIDX is visible matters for any guest > because the line size a guest would care is held in the same bits whether > FEAT_CCIDX is implemented. But if it concerns you I can write a bit more > code so that it won't mask CCIDX bit if it's set from the userspace. The particular issue I'm alluding to is the fact that KVM treats the ID registers as invariant. Userspace will save/restore the ID registers for the VM as part of a migration. Existing kernels advertize FEAT_CCIDX, whereas kernels with this patch will not. KVM will return an error (EINVAL) when ID_AA64MMFR2_EL1 is written by userspace. We've worked around this issue in the past by implementing set_user calls for ID registers that have changed to tolerate the 'old' value that KVM advertized. In any case, it may not be worth addressing given that there are no implementations in the wild with FEAT_CCIDX. But I wanted to bring it up on the list for the sake of posterity and also allow anyone to scream who might be adversely affected by the change. -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel