From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Catterall Subject: Re: [PATCH v2 3/3] Convert map_domain_page() to use the new mfn_t type Date: Thu, 9 Jul 2015 11:29:39 +0100 Message-ID: <559E4D13.6050503@citrix.com> References: <1435838656-9219-1-git-send-email-Ben.Catterall@citrix.com> <1435838656-9219-3-git-send-email-Ben.Catterall@citrix.com> <559BC1CB020000780008D450@mail.emea.novell.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 1ZD95O-0005XX-9f for xen-devel@lists.xenproject.org; Thu, 09 Jul 2015 10:29:46 +0000 In-Reply-To: <559BC1CB020000780008D450@mail.emea.novell.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: Jan Beulich Cc: keir@xen.org, ian.campbell@citrix.com, andrew.cooper3@citrix.com, tim@xen.org, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 07/07/15 11:10, Jan Beulich wrote: >>>> On 02.07.15 at 14:04, wrote: >> Reworked the internals and declaration, applying (un)boxing >> where needed. Converted calls to map_domain_page() to >> provide mfn_t types, boxing where needed. >> >> Signed-off-by: Ben Catterall >> Reviewed-by: Andrew Cooper >> >> --- >> Changed since v1: >> * Created paddr_to_mfn() and mfn_to_paddr() for both x86 and ARM >> * Converted code to use the new paddr_to_mfn() rather than e.g. >> paddr>>PAGE_SHIFT > This was a bogus change - why can't you use paddr_to_pfn() and > pfn_to_paddr()? And if you needed new macros, they should be > named consistently, i.e. maddr_to_mfn() and mfn_to_maddr(). > And perhaps they should then produce/take mfn_t? > In [PATCH 3/3] Andrew said that I should use _mfn(paddr_to_mfn(ma)) rather than ma >> PAGE_SIZE so I made the change based on that. Can you clarify if I should proceed and use paddr_to_pfn() instead. Re. the rename: can you clarify what the difference between maddr (machine addr?) and paddr (physical addr?) are please? Thanks! >> @@ -194,7 +194,7 @@ static void update_pagetable_mac(vmac_ctx_t *ctx) >> { >> if ( page->count_info & PGC_page_table ) >> { >> - void *pg = map_domain_page(mfn); >> + void *pg = map_domain_page(_mfn(mfn)); >> vmac_update(pg, PAGE_SIZE, ctx); > Please take the opportunity and add the missing blank line in cases > like this. > > Jan >