From patchwork Tue Jun 21 19:36:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Heimes X-Patchwork-Id: 1646197 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.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=D8bBIQIZ; dkim-atps=neutral Authentication-Results: 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=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LSGxR2nSkz9sGF for ; Wed, 22 Jun 2022 05:37:15 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1o3jgT-0007Tj-17; Tue, 21 Jun 2022 19:37:09 +0000 Received: from smtp-relay-canonical-0.internal ([10.131.114.83] helo=smtp-relay-canonical-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 1o3jgN-0007Nm-AG for kernel-team@lists.ubuntu.com; Tue, 21 Jun 2022 19:37:03 +0000 Received: from T570.fritz.box (p54abbbf9.dip0.t-ipconnect.de [84.171.187.249]) (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-canonical-0.canonical.com (Postfix) with ESMTPSA id CE358427DF for ; Tue, 21 Jun 2022 19:37:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1655840222; bh=TgWY/fIBHI/tlrkVo+xnwmXsKtZUrI21CgYCGahZTfs=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=D8bBIQIZXlhsUyRKsBrh5J8dGN92wyXxE6hBaMFB5noq/LrNcPHDrgLnOeIzZeWH1 1iXlGzEwcph90TIh0idhFGbmh90dxFt4gkeGEzutItqhC7nzbq2i7IDn1ekOSAUE2r j9SRrlWYkhqTZijANxiB8zsgfpL4m5Nm0JVnif1YBIqXgdp+04zY00buzBoGXedNIE hB/J50xFTTmS2jtNzrKCHGfbONufAJn+XmGtsgvLxdRUXrXxcThXDT2ivoHzEj7MYD PglT+01xGGwTbN4kE2uOBrx2GzBBNNS9swqsAnAaa2tfQZhoSYWeZlM7kDFvj9ySkP xVNLIYqkSznAA== From: frank.heimes@canonical.com To: kernel-team@lists.ubuntu.com Subject: [SRU][F][J][PATCH 3/3] KVM: s390: pv: avoid stalls when making pages secure Date: Tue, 21 Jun 2022 21:36:59 +0200 Message-Id: <20220621193659.700787-4-frank.heimes@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220621193659.700787-1-frank.heimes@canonical.com> References: <20220621193659.700787-1-frank.heimes@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/1979296 Improve make_secure_pte to avoid stalls when the system is heavily overcommitted. This was especially problematic in kvm_s390_pv_unpack, because of the loop over all pages that needed unpacking. Due to the locks being held, it was not possible to simply replace uv_call with uv_call_sched. A more complex approach was needed, in which uv_call is replaced with __uv_call, which does not loop. When the UVC needs to be executed again, -EAGAIN is returned, and the caller (or its caller) will try again. When -EAGAIN is returned, the path is the same as when the page is in writeback (and the writeback check is also performed, which is harmless). Fixes: 214d9bbcd3a672 ("s390/mm: provide memory management functions for protected KVM guests") Signed-off-by: Claudio Imbrenda Reviewed-by: Janosch Frank Reviewed-by: Christian Borntraeger Link: https://lore.kernel.org/r/20210920132502.36111-5-imbrenda@linux.ibm.com Signed-off-by: Christian Borntraeger (cherry picked from commit f0a1a0615a6ff6d38af2c65a522698fb4bb85df6) Signed-off-by: Frank Heimes --- arch/s390/kernel/uv.c | 29 +++++++++++++++++++++++------ arch/s390/kvm/intercept.c | 5 +++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c index f9e2b4d024bf..094d5a1d1380 100644 --- a/arch/s390/kernel/uv.c +++ b/arch/s390/kernel/uv.c @@ -162,7 +162,7 @@ static int make_secure_pte(pte_t *ptep, unsigned long addr, { pte_t entry = READ_ONCE(*ptep); struct page *page; - int expected, rc = 0; + int expected, cc = 0; if (!pte_present(entry)) return -ENXIO; @@ -178,12 +178,25 @@ static int make_secure_pte(pte_t *ptep, unsigned long addr, if (!page_ref_freeze(page, expected)) return -EBUSY; set_bit(PG_arch_1, &page->flags); - rc = uv_call(0, (u64)uvcb); + /* + * If the UVC does not succeed or fail immediately, we don't want to + * loop for long, or we might get stall notifications. + * On the other hand, this is a complex scenario and we are holding a lot of + * locks, so we can't easily sleep and reschedule. We try only once, + * and if the UVC returned busy or partial completion, we return + * -EAGAIN and we let the callers deal with it. + */ + cc = __uv_call(0, (u64)uvcb); page_ref_unfreeze(page, expected); - /* Return -ENXIO if the page was not mapped, -EINVAL otherwise */ - if (rc) - rc = uvcb->rc == 0x10a ? -ENXIO : -EINVAL; - return rc; + /* + * Return -ENXIO if the page was not mapped, -EINVAL for other errors. + * If busy or partially completed, return -EAGAIN. + */ + if (cc == UVC_CC_OK) + return 0; + else if (cc == UVC_CC_BUSY || cc == UVC_CC_PARTIAL) + return -EAGAIN; + return uvcb->rc == 0x10a ? -ENXIO : -EINVAL; } /* @@ -236,6 +249,10 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb) up_read(&gmap->mm->mmap_sem); if (rc == -EAGAIN) { + /* + * If we are here because the UVC returned busy or partial + * completion, this is just a useless check, but it is safe. + */ wait_on_page_writeback(page); } else if (rc == -EBUSY) { /* diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index 57f1b121d27c..5c10f96a50b1 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -517,6 +517,11 @@ static int handle_pv_uvc(struct kvm_vcpu *vcpu) */ if (rc == -EINVAL) return 0; + /* + * If we got -EAGAIN here, we simply return it. It will eventually + * get propagated all the way to userspace, which should then try + * again. + */ return rc; }