From patchwork Wed Feb 28 15:25:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 130000 Delivered-To: patch@linaro.org Received: by 10.80.172.228 with SMTP id x91csp1665413edc; Wed, 28 Feb 2018 07:28:01 -0800 (PST) X-Google-Smtp-Source: AG47ELuJFcLYuwytvJeHWsn1qON9ZsvYzOQUc4FLeF4/Wf9T+sHTca6OmdfXSzz9sRKg2xFJRLSj X-Received: by 10.36.13.139 with SMTP id 133mr14404424itx.9.1519831681269; Wed, 28 Feb 2018 07:28:01 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519831681; cv=none; d=google.com; s=arc-20160816; b=uf7KrI+Dl3I0rlYjCa+G0cX3Iywvi3cLfrdtzB2uh+qCo6fPiuzLMN/obfpQ8Dt0cf Nc0J2tFoVHVKqxpxulC9u/4rt7a9wtQUZng+3LDIvyLP9ZTsqd8cdmW9Fbnv/gjLDbc+ 2A5kmIeaI6WH9nztpcKRhtdTGhbNm2fCZv3sYXGuH1taOggORra9nByKUxAbmFfVvkxH NCOtaXHRswHr5PkpAmahg4ibDaiOphvzwjatlziPTAEgXWRFTx8b4OnCsAcK1plfgzzc gpBAEK63YG3E99ogeipyb/Rrma0WOeaT5WOnkzUIL12hcXN2pR+lCia0RquCp+oP4IDF DxBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version :list-subscribe:list-help:list-post:list-unsubscribe:list-id :precedence:subject:cc:message-id:date:to:from :arc-authentication-results; bh=kb0xqVOlZA6XdfhuQ3MCy1MzwyGo0hMxPyd1BNk3buI=; b=zV2tQNNujySdujWfSm4p4jvEycYH+XYlqkxlAXRLBdD94I7M9m1tbQL0UDK9AlJheJ uA9Uq34EoJ6YNw4G9lC566lWDuENMkHZohjZQRl0GYKKqvk6FHAH3SGr91tgdQAf+LCD xT7nvFuk+PPf+6pwBgjsRmz7Om/9v8Kws+OEzEV3icRlFRrTwt2hO2CVrZq2FBy7O40P R9+FpZ0zE1Sj2IVi09A4brlzIQUHmc+tSZlTVFs2VtfaUFZZr8orG/Ff/8Q+0kmSuv8u BxQM8W0WQCHo4HZAc/SoucLcvpxn/g9HoHoRByFPPOOUbURVIshsmFiuBVBomcohaXSL oY/w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id a8si1055261ioc.185.2018.02.28.07.28.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Feb 2018 07:28:01 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1er3cI-0000sL-JL; Wed, 28 Feb 2018 15:26:02 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1er3cH-0000sB-Ed for xen-devel@lists.xen.org; Wed, 28 Feb 2018 15:26:01 +0000 X-Inumbo-ID: 8b41f8e6-1c9b-11e8-ba59-bc764e045a96 Received: from foss.arm.com (unknown [217.140.101.70]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTP id 8b41f8e6-1c9b-11e8-ba59-bc764e045a96; Wed, 28 Feb 2018 16:25:01 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4B2A15AD; Wed, 28 Feb 2018 07:25:58 -0800 (PST) Received: from e108454-lin.cambridge.arm.com (e108454-lin.cambridge.arm.com [10.1.206.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6DAD63F246; Wed, 28 Feb 2018 07:25:57 -0800 (PST) From: Julien Grall To: xen-devel@lists.xen.org Date: Wed, 28 Feb 2018 15:25:47 +0000 Message-Id: <20180228152547.12700-1-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 Cc: sstabellini@kernel.org, rcojocaru@bitdefender.com, proskurin@sec.in.tum.de, andre.przywara@linaro.org, Julien Grall , tamas@tklengyel.com Subject: [Xen-devel] [PATCH] xen/arm: p2m: Prevent deadlock when using memaccess X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Commit 7d623b358a4 "arm/mem_access: Add long-descriptor based gpt" assumed the read-write lock can be taken recursively. However, this assumption is wrong and will lead to deadlock when the lock is contended. To avoid the nested lock, rework the locking in get_page_from_gva and p2m_mem_access_check_and_get_page. The latter will now be called without the p2m lock. The new locking in p2m_mem_accces_check_and_get_page will not cover the translation of the VA to an IPA. This is fine because we can't promise that the stage-1 page-table have changed behind our back (they are under guest control). Modification in the stage-2 page-table can now happen, but I can't issue any potential issue here except with the break-before-make sequence used when updating page-table. gva_to_ipa may fail if the sequence is executed at the same on another CPU. In that case we would fallback in the software lookup path. Signed-off-by: Julien Grall Reviewed-by: Sergej Proskurin --- This patch should be backported to Xen 4.10. There are other potential optimization that I am working on. Although, I don't think they are backport material. --- xen/arch/arm/mem_access.c | 8 ++++++-- xen/arch/arm/p2m.c | 4 ++-- xen/include/asm-arm/p2m.h | 4 ---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c index 0f2cbb81d3..11c2b03b7b 100644 --- a/xen/arch/arm/mem_access.c +++ b/xen/arch/arm/mem_access.c @@ -126,7 +126,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, * is not mapped. */ if ( guest_walk_tables(v, gva, &ipa, &perms) < 0 ) - goto err; + return NULL; /* * Check permissions that are assumed by the caller. For instance in @@ -139,11 +139,13 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, * test for execute permissions this check can be left out. */ if ( (flag & GV2M_WRITE) && !(perms & GV2M_WRITE) ) - goto err; + return NULL; } gfn = gaddr_to_gfn(ipa); + p2m_read_lock(p2m); + /* * We do this first as this is faster in the default case when no * permission is set on the page. @@ -216,6 +218,8 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, page = NULL; err: + p2m_read_unlock(p2m); + return page; } diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 65e8b9c6ea..5de82aafe1 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1449,11 +1449,11 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, } err: + p2m_read_unlock(p2m); + if ( !page && p2m->mem_access_enabled ) page = p2m_mem_access_check_and_get_page(va, flags, v); - p2m_read_unlock(p2m); - return page; } diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index a0abc84ed8..45ef2cd58b 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -23,10 +23,6 @@ extern void memory_type_changed(struct domain *); struct p2m_domain { /* * Lock that protects updates to the p2m. - * - * Please note that we use this lock in a nested way by calling - * access_guest_memory_by_ipa in guest_walk_(sd|ld). This must be - * considered in the future implementation. */ rwlock_t lock;