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 lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (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 83DE8C433EF for ; Sat, 9 Apr 2022 09:26:21 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.302134.515702 (Exim 4.92) (envelope-from ) id 1nd7M2-0002K3-25; Sat, 09 Apr 2022 09:26:02 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 302134.515702; Sat, 09 Apr 2022 09:26:02 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nd7M1-0002Jw-VR; Sat, 09 Apr 2022 09:26:01 +0000 Received: by outflank-mailman (input) for mailman id 302134; Sat, 09 Apr 2022 09:26:00 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nd7M0-0002Jq-Fe for xen-devel@lists.xenproject.org; Sat, 09 Apr 2022 09:26:00 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nd7M0-0001dI-3r; Sat, 09 Apr 2022 09:26:00 +0000 Received: from home.octic.net ([81.187.162.82] helo=[10.0.1.102]) by xenbits.xenproject.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nd7Lz-000475-Um; Sat, 09 Apr 2022 09:26:00 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:Subject: From:References:Cc:To:MIME-Version:Date:Message-ID; bh=Peckepcy7UonTRhBMBevPoPjsLWpmUfXjHGWe8OanzU=; b=tJfUAQIgWqY3Q6Q3WVZiPEeL4x EX5mATbxSdjzZ20WOXT7gRFURctP5TYF4Cp41rNhJLmPE3YceP2iQuv8J7Lpcvb+/mJjcxlN8cWt/ QkVvfXO/gtHF0gW815B8d9sZAEwUAO1V61iZV8x6P8QHN/RSf5ROlrln5CFmKMaXOanM=; Message-ID: <7c6a3af7-6052-aab8-829c-bc2c2a5db341@xen.org> Date: Sat, 9 Apr 2022 10:25:57 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 To: Penny Zheng , xen-devel@lists.xenproject.org Cc: nd@arm.com, Stefano Stabellini , Bertrand Marquis , Volodymyr Babchuk References: <20220311061123.1883189-1-Penny.Zheng@arm.com> <20220311061123.1883189-9-Penny.Zheng@arm.com> From: Julien Grall Subject: Re: [PATCH v1 08/13] xen/arm: destroy static shared memory when de-construct domain In-Reply-To: <20220311061123.1883189-9-Penny.Zheng@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Penny, On 11/03/2022 06:11, Penny Zheng wrote: > From: Penny Zheng > > This commit introduces a new helper destroy_domain_shm to destroy static > shared memory at domain de-construction. > > This patch only considers the scenario where the owner domain is the > default dom_shared, for user-defined owner domain, it will be covered in > the following patches. > > Since all domains are borrower domains, we could simply remove guest P2M > foreign mapping of statically shared memory region and drop the reference > added at guest_physmap_add_shm. > > Signed-off-by: Penny Zheng > --- > xen/arch/arm/domain.c | 48 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 1ff1df5d3f..f0bfd67fe5 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -993,6 +994,48 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list) > return ret; > } > > +#ifdef CONFIG_STATIC_SHM > +static int domain_destroy_shm(struct domain *d) > +{ > + int ret = 0; > + unsigned long i = 0UL, j; > + > + if ( d->arch.shm_mem == NULL ) > + return ret; You already return the value here. So... > + else ... the else is pointless. > + { > + for ( ; i < d->arch.shm_mem->nr_banks; i++ ) > + { > + unsigned long nr_gfns = PFN_DOWN(d->arch.shm_mem->bank[i].size); > + gfn_t gfn = gaddr_to_gfn(d->arch.shm_mem->bank[i].start); > + > + for ( j = 0; j < nr_gfns; j++ ) > + { > + mfn_t mfn; > + > + mfn = gfn_to_mfn(d, gfn_add(gfn, j)); A domain is allowed to modify its P2M. So there are no guarantee that the GFN will still point to the shared memory. This will allow the guest... > + if ( !mfn_valid(mfn) ) > + { > + dprintk(XENLOG_ERR, > + "Domain %pd page number %lx invalid.\n", > + d, gfn_x(gfn) + i); > + return -EINVAL; ... to actively prevent destruction. > + } > + > + ret = guest_physmap_remove_page(d, gfn_add(gfn, j), mfn, 0); > + if ( ret ) > + return ret; > + > + /* Drop the reference. */ > + put_page(mfn_to_page(mfn)); guest_physmap_remove_page() will already drop the reference taken for the foreign mapping. I couldn't find any other reference taken, what is the put_page() for? Also, as per above we don't know whether this is a page from the shared page. So we can't blindly call put_page(). However, I don't think we need any specific code here. We can rely on relinquish_p2m_mappings() to drop any reference. If there is an extra one for shared mappings, then we should update p2m_put_l3_page(). Cheers, -- Julien Grall