From patchwork Fri Feb 17 10:29:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roxana Nicolescu X-Patchwork-Id: 1744149 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=bCyIz8hM; dkim-atps=neutral Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PJ7NR2cR9z240c for ; Fri, 17 Feb 2023 21:29:43 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1pSxzm-0003cs-0C; Fri, 17 Feb 2023 10:29:38 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1pSxzj-0003cH-Nc for kernel-team@lists.ubuntu.com; Fri, 17 Feb 2023 10:29:35 +0000 Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 8A7983F583 for ; Fri, 17 Feb 2023 10:29:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1676629775; bh=FsmyH0bLYKEwY3T78uqNKL3+54SNkCdLiKy3GfAxxnQ=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=bCyIz8hM7SsTke9tvN8QUXxEu8C6cKo3HcccRGhMmpQSF8zgL5dC/VtFCoO2QdBOs mU9/KngoRGR6pDARJR8CpK78o+PfoDNpFxMDzuFkfg0wpSwSJ4quMC9ywy1OFZ9xyM aP8NWaRD0WaDa25iCkUFBMu6LIsQbCgoeew5PKzVdCLVqLfpJuKkQJK69dUl/ybFVb uM9lT2ltfg2INF1Mf1MAC57gZOFpzgMYybpDTj8NHJCo2x9ieYFqGCI1wG4MIGoRuP /KLigfxLerxqknPA4bUdXYdndlKh8fFfrcAAZ4lZps9/aJOKjvmyhY80wu/PJjCfZD Rxc+C3zt4vUlw== Received: by mail-ed1-f72.google.com with SMTP id g15-20020a056402428f00b004aab4319cedso730947edc.2 for ; Fri, 17 Feb 2023 02:29:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FsmyH0bLYKEwY3T78uqNKL3+54SNkCdLiKy3GfAxxnQ=; b=qD7qByZ9jnLqULrTBvNA6cj9ZYpDPXVEAvG6LmSu0FcNgGJXSo90LHSrVp2n3JRwBW jn27K9EIgP9AY4CUtbRHMnyzPOiL74azVzs084IbSMIWANX6pvY4qpDVCXoS4AmAVnx7 xrMm8RvuQFYPtVTOjgZGs7NSjmFDUjf094d+TMlk9yMvGU7bcyM/fFKatDf3nunLKScu Vn0sB6qQdvi6qrxsC0oBwcte0yGkgvNBE2p5+BtPsbZZEE/pVwkMSfhbIrEXeWgh88U4 R42d5JSeOi70yvlXZfYAPtqsO9aDAcxoY5XOHaafuivez4BHxCHhhi1pY1qvVg/MDeUA 50GQ== X-Gm-Message-State: AO0yUKVKsnTFeV66d5E20/BV+d+rLGaZwlWKIpXzKgpq59eWIKuiZmLp D1xEUmd3EpO8j8uwa08Eqzs+9wqtEf4Ab/uoBwMznXK2d2kdawUFZS/K6yVCJ3gXjvKGeaArjDV ZmMSuwzT71UawUl4avcSl4QFVaPETFTvSTcdN9Sh+a/mWCQknKKNi X-Received: by 2002:a17:906:fc0d:b0:887:2248:efd5 with SMTP id ov13-20020a170906fc0d00b008872248efd5mr440008ejb.77.1676629775231; Fri, 17 Feb 2023 02:29:35 -0800 (PST) X-Google-Smtp-Source: AK7set+3ZF7zakSheRs4onSvkK3FXlcsZj5YMpB0Wygs9SrRMwsdGlKvtSsMFCMcjKYDqWsASOIBMA== X-Received: by 2002:a17:906:fc0d:b0:887:2248:efd5 with SMTP id ov13-20020a170906fc0d00b008872248efd5mr439998ejb.77.1676629774896; Fri, 17 Feb 2023 02:29:34 -0800 (PST) Received: from localhost.localdomain ([2001:67c:1560:8007::aac:c490]) by smtp.gmail.com with ESMTPSA id n10-20020a170906840a00b0088452ca0666sm1957791ejx.196.2023.02.17.02.29.33 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Feb 2023 02:29:34 -0800 (PST) From: Roxana Nicolescu To: kernel-team@lists.ubuntu.com Subject: [SRU][F][PATCH v2 1/2] KVM: s390x: fix SCK locking Date: Fri, 17 Feb 2023 11:29:25 +0100 Message-Id: <20230217102926.84817-2-roxana.nicolescu@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230217102926.84817-1-roxana.nicolescu@canonical.com> References: <20230217102926.84817-1-roxana.nicolescu@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Claudio Imbrenda BugLink: https://bugs.launchpad.net/bugs/1999882 [ Upstream commit c0573ba5c5a2244dc02060b1f374d4593c1d20b7 ] When handling the SCK instruction, the kvm lock is taken, even though the vcpu lock is already being held. The normal locking order is kvm lock first and then vcpu lock. This is can (and in some circumstances does) lead to deadlocks. The function kvm_s390_set_tod_clock is called both by the SCK handler and by some IOCTLs to set the clock. The IOCTLs will not hold the vcpu lock, so they can safely take the kvm lock. The SCK handler holds the vcpu lock, but will also somehow need to acquire the kvm lock without relinquishing the vcpu lock. The solution is to factor out the code to set the clock, and provide two wrappers. One is called like the original function and does the locking, the other is called kvm_s390_try_set_tod_clock and uses trylock to try to acquire the kvm lock. This new wrapper is then used in the SCK handler. If locking fails, -EAGAIN is returned, which is eventually propagated to userspace, thus also freeing the vcpu lock and allowing for forward progress. This is not the most efficient or elegant way to solve this issue, but the SCK instruction is deprecated and its performance is not critical. The goal of this patch is just to provide a simple but correct way to fix the bug. Fixes: 6a3f95a6b04c ("KVM: s390: Intercept SCK instruction") Signed-off-by: Claudio Imbrenda Reviewed-by: Christian Borntraeger Reviewed-by: Janis Schoetterl-Glausch Link: https://lore.kernel.org/r/20220301143340.111129-1-imbrenda@linux.ibm.com Cc: stable@vger.kernel.org Signed-off-by: Christian Borntraeger Stable-dep-of: 6973091d1b50 ("KVM: s390: pv: don't allow userspace to set the clock under PV") Signed-off-by: Sasha Levin (backported from commit c0573ba5c5a2244dc02060b1f374d4593c1d20b7) [roxanan: no adjustments needed] Signed-off-by: Roxana Nicolescu --- arch/s390/kvm/kvm-s390.c | 19 ++++++++++++++++--- arch/s390/kvm/kvm-s390.h | 4 ++-- arch/s390/kvm/priv.c | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 1fb8b8a9fa0f..0f1b0dde0de3 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3940,14 +3940,12 @@ static int kvm_s390_handle_requests(struct kvm_vcpu *vcpu) return 0; } -void kvm_s390_set_tod_clock(struct kvm *kvm, - const struct kvm_s390_vm_tod_clock *gtod) +static void __kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod) { struct kvm_vcpu *vcpu; struct kvm_s390_tod_clock_ext htod; int i; - mutex_lock(&kvm->lock); preempt_disable(); get_tod_clock_ext((char *)&htod); @@ -3968,7 +3966,22 @@ void kvm_s390_set_tod_clock(struct kvm *kvm, kvm_s390_vcpu_unblock_all(kvm); preempt_enable(); +} + +void kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod) +{ + mutex_lock(&kvm->lock); + __kvm_s390_set_tod_clock(kvm, gtod); + mutex_unlock(&kvm->lock); +} + +int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod) +{ + if (!mutex_trylock(&kvm->lock)) + return 0; + __kvm_s390_set_tod_clock(kvm, gtod); mutex_unlock(&kvm->lock); + return 1; } /** diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h index 817d213693e0..c94c1e29eeca 100644 --- a/arch/s390/kvm/kvm-s390.h +++ b/arch/s390/kvm/kvm-s390.h @@ -335,8 +335,8 @@ int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu); int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu); /* implemented in kvm-s390.c */ -void kvm_s390_set_tod_clock(struct kvm *kvm, - const struct kvm_s390_vm_tod_clock *gtod); +void kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod); +int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod); long kvm_arch_fault_in_page(struct kvm_vcpu *vcpu, gpa_t gpa, int writable); int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr); int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, unsigned long addr); diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c index 840b383ba756..8f03992d25cc 100644 --- a/arch/s390/kvm/priv.c +++ b/arch/s390/kvm/priv.c @@ -103,7 +103,20 @@ static int handle_set_clock(struct kvm_vcpu *vcpu) return kvm_s390_inject_prog_cond(vcpu, rc); VCPU_EVENT(vcpu, 3, "SCK: setting guest TOD to 0x%llx", gtod.tod); - kvm_s390_set_tod_clock(vcpu->kvm, >od); + /* + * To set the TOD clock the kvm lock must be taken, but the vcpu lock + * is already held in handle_set_clock. The usual lock order is the + * opposite. As SCK is deprecated and should not be used in several + * cases, for example when the multiple epoch facility or TOD clock + * steering facility is installed (see Principles of Operation), a + * slow path can be used. If the lock can not be taken via try_lock, + * the instruction will be retried via -EAGAIN at a later point in + * time. + */ + if (!kvm_s390_try_set_tod_clock(vcpu->kvm, >od)) { + kvm_s390_retry_instr(vcpu); + return -EAGAIN; + } kvm_s390_set_psw_cc(vcpu, 0); return 0;