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 12945EB64D9 for ; Mon, 10 Jul 2023 18:16:54 +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=uGgcFYSK0WjAoe9bz661303rde2aoxZ0aZcvV/upjRw=; b=rYv4WRNsUK6uJb FKHVyycHYVWyufAmlcMpiyz/RPD1uqkFI4Y32lt1sqY7wSwk7S3uBScxW+JXSwR1EFIzgidPfcEWV cBBNfC6dVrTvk9DD8mPhobp1GC/tOJcumKgTY8+QSlvBq74cDmyTvhCI0zYPmlHRkZCnNL+BQ5cV5 zA3ePOlduP4sCO+MNMBSAhMN8c0bR3z3wfI8noKJ5Wnfei2ryGCcIMF0NTO+NytquIFvT3iOfA+NE sK/DMRbYy6jB+CbHOc02LGumuJEaO8wUQBnl6zLH2itSdlX9nepa0ew8+zoxn8U560MeNuT1G0gcZ aUspACjDKmd2TpjvkbQA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qIvQt-00CRHb-11; Mon, 10 Jul 2023 18:16:23 +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 1qIvQq-00CRH2-0Y for linux-arm-kernel@lists.infradead.org; Mon, 10 Jul 2023 18:16:21 +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 7ADD461174; Mon, 10 Jul 2023 18:16:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E272EC433C8; Mon, 10 Jul 2023 18:16:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689012978; bh=h9Hgdj96ugYML2YQUfLRwM5WPrmk57h0zt+mgCFCJ2E=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=gG08h1gk6TDmBZXTyOKNh1b6H+Ijwe5WUSN6o+5Mp8T0atKb0VDTqPBtqPLZOnQOA HahHdBFJNQDt4LK7Qbi3kbOy6IHVc9DbxBbl/cntYqPpfhCzdwn++tWxffZjpPd0en KQtKz819kn27Es3YLpTCQ46URFYUQT7qA6yg/00+Lxvw1QrhfnPlFdoO1jrLF3VvNb OVb9a+Mz4HyybpOBFE1dau4ghQO7W+up1GgJ2avoVrfiTeWk1dlzs0Ai+1/pEmtMQz WetHhimsj8D+sEbMVjZyjhhnqe9uaoHy5CZqXpGhnAkH8L+7Yi8JCjTfQuSWlQHpRN QKOuoXJ3hmPMQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1qIvQm-00BsxL-FC; Mon, 10 Jul 2023 19:16:16 +0100 Date: Mon, 10 Jul 2023 19:16:16 +0100 Message-ID: <86mt03ws27.wl-maz@kernel.org> From: Marc Zyngier To: Sean Christopherson Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, James Morse , Suzuki K Poulose , Oliver Upton , Zenghui Yu , isaku.yamahata@intel.com, pbonzini@redhat.com, Kristina Martsenko , stable@vger.kernek.org Subject: Re: [PATCH] KVM: arm64: Disable preemption in kvm_arch_hardware_enable() In-Reply-To: References: <20230703163548.1498943-1-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 (aarch64-unknown-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: seanjc@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, yuzenghui@huawei.com, isaku.yamahata@intel.com, pbonzini@redhat.com, kristina.martsenko@arm.com, stable@vger.kernek.org 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-20230710_111620_314158_20846D32 X-CRM114-Status: GOOD ( 33.10 ) 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 Mon, 10 Jul 2023 19:04:08 +0100, Sean Christopherson wrote: > > On Mon, Jul 03, 2023, Marc Zyngier wrote: > > Since 0bf50497f03b ("KVM: Drop kvm_count_lock and instead protect > > kvm_usage_count with kvm_lock"), hotplugging back a CPU whilst > > a guest is running results in a number of ugly splats as most > > of this code expects to run with preemption disabled, which isn't > > the case anymore. > > > > While the context is preemptable, it isn't migratable, which should > > be enough. But we have plenty of preemptible() checks all over > > the place, and our per-CPU accessors also disable preemption. > > > > Since this affects released versions, let's do the easy fix first, > > disabling preemption in kvm_arch_hardware_enable(). We can always > > revisit this with a more invasive fix in the future. > > > > Fixes: 0bf50497f03b ("KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock") > > Reported-by: Kristina Martsenko > > Tested-by: Kristina Martsenko > > Signed-off-by: Marc Zyngier > > Link: https://lore.kernel.org/r/aeab7562-2d39-e78e-93b1-4711f8cc3fa5@arm.com > > Cc: stable@vger.kernek.org # v6.3, v6.4 > > --- > > arch/arm64/kvm/arm.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > index aaeae1145359..a28c4ffe4932 100644 > > --- a/arch/arm64/kvm/arm.c > > +++ b/arch/arm64/kvm/arm.c > > @@ -1894,8 +1894,17 @@ static void _kvm_arch_hardware_enable(void *discard) > > > > int kvm_arch_hardware_enable(void) > > { > > - int was_enabled = __this_cpu_read(kvm_arm_hardware_enabled); > > + int was_enabled; > > > > + /* > > + * Most calls to this function are made with migration > > + * disabled, but not with preemption disabled. The former is > > + * enough to ensure correctness, but most of the helpers > > + * expect the later and will throw a tantrum otherwise. > > + */ > > + preempt_disable(); > > + > > + was_enabled = __this_cpu_read(kvm_arm_hardware_enabled); > > IMO, this_cpu_has_cap() is at fault. E.g. why not do this? > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 7d7128c65161..b862477de2ce 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -3193,7 +3193,9 @@ static void __init setup_boot_cpu_capabilities(void) > > bool this_cpu_has_cap(unsigned int n) > { > - if (!WARN_ON(preemptible()) && n < ARM64_NCAPS) { > + __this_cpu_preempt_check("has_cap"); > + > + if (n < ARM64_NCAPS) { > const struct arm64_cpu_capabilities *cap = cpu_hwcaps_ptrs[n]; > > if (cap) > Because this check is not on at all times (it relies on DEBUG_PREEMPT), and we really want it to be there. 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