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 F2466C433F5 for ; Fri, 4 Mar 2022 14:58:42 +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=iLKeQk9sH4FFo1K2SLKI0BRlSWoJfwnBydy7b+NLU7c=; b=dCPq3EmTZ/004A 1//p6y1wVP1YKTqiPrgTZlckRHNAnWCG7QMBoOFZhkPdRhRZbNSXIDNK3EYyJUPDqDomUGGB+4OXn 4BaP/F5V1qcnP8xP5fCq88pjKny67n9tkD+ZCU5L13vLGTZ8k07h9198MJzSNQugrUfkKA1tm/jAG pdm7xM6vAKhAzfqJFVGpo0IDOemIeNZfVIxccmNxCoIHYjjHVmnkPTtBvnpxlZtSsjBmp2Gd04Uwn E93wMqYyM3iEmP/TBEyMDGnWNgXG/S1O5Ql0ymcByFYOTG8cCLJKeOei7xjA0qwWjKBrB3+4ka/JX t+KWMNPXeGw2ihxlJkhQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQ9N8-00Act8-JU; Fri, 04 Mar 2022 14:57:34 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQ9N5-00AcsB-9B for linux-arm-kernel@lists.infradead.org; Fri, 04 Mar 2022 14:57:32 +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 27E2D619AC; Fri, 4 Mar 2022 14:57:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 880F8C340F1; Fri, 4 Mar 2022 14:57:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1646405849; bh=7Ldz7YSA4Ps9qM53w7/XL0hJH0liwzBbGhFCg76DoUM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=djD7WUAwKjFQ6Rd3Naa94CttQM5U8Ei0ivjUrivNahMlokBDPm4ZS5cTtWsSfqMEL vhHaC4HD3x+imsKjYMiA/vjdsc3cDmLQN6RHb89IJlDpgwOaXcu/VnE2SkGr2VHX4G y/I3XHCxMhtXMhJ6/jhZGfLaD4pziH4scVpRjoxdo7PvLOKvoAEd/WyyB8YOOMb7zr JFp2eytiKlhJvzWp24xQjZ9dTAGvmP9NRA1dHgV5qbhiVtEaY5JtRD40rpJ3hhWuGK ktEmJIzGN53ox3qPi1Uu3yMKMj/9aHlN+AdgDHTemYksdYn6FNkpzAEGDMao5G3d4G gQDPlr/gWmG8g== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nQ9N1-00CFQC-6Z; Fri, 04 Mar 2022 14:57:27 +0000 Date: Fri, 04 Mar 2022 14:57:26 +0000 Message-ID: <87mti522ax.wl-maz@kernel.org> From: Marc Zyngier To: Reiji Watanabe Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Linux ARM , James Morse , Alexandru Elisei , Suzuki K Poulose , Paolo Bonzini , Will Deacon , Andrew Jones , Peng Liang , Peter Shier , Ricardo Koller , Oliver Upton , Jing Zhang , Raghavendra Rao Anata Subject: Re: [PATCH v3 2/3] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs In-Reply-To: <75e90ab4-141f-21a8-1559-f792b84d60fa@google.com> References: <20220303035408.3708241-1-reijiw@google.com> <20220303035408.3708241-2-reijiw@google.com> <87tucf10fg.wl-maz@kernel.org> <75e90ab4-141f-21a8-1559-f792b84d60fa@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/27.1 (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: reijiw@google.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, pbonzini@redhat.com, will@kernel.org, drjones@redhat.com, liangpeng10@huawei.com, pshier@google.com, ricarkol@google.com, oupton@google.com, jingzhangos@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-20220304_065731_425869_FAC46678 X-CRM114-Status: GOOD ( 28.36 ) 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, 04 Mar 2022 08:00:20 +0000, Reiji Watanabe wrote: > > > > +{ > > > + bool is32bit; > > > + bool allowed = true; > > > + struct kvm *kvm = vcpu->kvm; > > > + > > > + is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT); > > > + > > > + mutex_lock(&kvm->lock); > > > + > > > + if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) { > > > + allowed = (is32bit == > > > + test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags)); > > > + } else { > > > + if (is32bit) > > > + set_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags); > > > > nit: probably best written as: > > > > __assign_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags, is32bit); > > > > > + > > > + set_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags); > > > > Since this is only ever set whilst holding the lock, you can user the > > __set_bit() version. > > Thank you for the proposal. But since other CPUs could attempt > to set other bits without holding the lock, I don't think we > can use the non-atomic version here. Ah, good point. Keep the atomic accesses then. > > > > > > + } > > > + > > > + mutex_unlock(&kvm->lock); > > > + > > > + return allowed ? 0 : -EINVAL; > > > +} > > > + > > > static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu, > > > const struct kvm_vcpu_init *init) > > > { > > > @@ -1140,6 +1177,10 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu, > > > > > > /* Now we know what it is, we can reset it. */ > > > ret = kvm_reset_vcpu(vcpu); > > > + > > > + if (!ret) > > > + ret = kvm_register_width_check_or_init(vcpu); > > > > Why is that called *after* resetting the vcpu, which itself relies on > > KVM_ARM_VCPU_EL1_32BIT, which we agreed to get rid of as much as > > possible? > > That's because I didn't want to set EL1_32BIT/REG_WIDTH_CONFIGURED > for the guest based on the vCPU for which KVM_ARM_VCPU_INIT would fail. > The flags can be set in the kvm_reset_vcpu() and cleared in > case of failure. But then that temporary value could lead > KVM_ARM_VCPU_INIT for other vCPUs to fail, which I don't think > is nice to do. But it also means that userspace is trying to create incompatible vcpus concurrently. Why should we care? We shouldn't even consider resetting the flags on failure, as userspace has already indicated its intention to create a 32 or 64bit VM. 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