From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 6/8] xen/arm: Retrieve p2m type in get_page_from_gfn Date: Mon, 09 Dec 2013 02:36:00 +0000 Message-ID: <52A52C90.3070205@linaro.org> References: <1386258131-755-1-git-send-email-julien.grall@linaro.org> <1386258131-755-7-git-send-email-julien.grall@linaro.org> <1386260639.20047.109.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VpqhY-00058B-89 for xen-devel@lists.xenproject.org; Mon, 09 Dec 2013 02:36:04 +0000 Received: by mail-wg0-f42.google.com with SMTP id a1so2837127wgh.1 for ; Sun, 08 Dec 2013 18:36:02 -0800 (PST) In-Reply-To: <1386260639.20047.109.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/05/2013 04:23 PM, Ian Campbell wrote: > On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote: >> Signed-off-by: Julien Grall >> --- >> xen/include/asm-arm/p2m.h | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h >> index 3de69c4..54d1dab 100644 >> --- a/xen/include/asm-arm/p2m.h >> +++ b/xen/include/asm-arm/p2m.h >> @@ -105,9 +105,8 @@ static inline struct page_info *get_page_from_gfn( >> struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) >> { >> struct page_info *page; >> - unsigned long mfn = gmfn_to_mfn(d, gfn); >> - >> - ASSERT(t == NULL); >> + paddr_t maddr = p2m_get_entry(d, pfn_to_paddr(gfn), t); >> + unsigned long mfn = maddr >> PAGE_SHIFT; > > Do we need to eg. convert p2m_invalid into returning NULL instead of > whatever maddr contains in that case? In that case maddr will contain INVALID_PADDR, which will turn in INVALID_MFN with the shift. So we don't need to check the p2m type. > > In the case of p2m_mmio_direct we need to be careful because there is no > struct page. Ah.. here it is in the context: >> if (!mfn_valid(mfn)) >> return NULL; > > Although I wonder if we should convert mmio_direct into NULL even if the > MMIO region happens to have a struct page? (e.g. due to holes in the > frametable) Actually, I think it's fine. get_page will fail because the page won't belong to the domain. So the function will return NULL. -- Julien Grall