Re: [Xen-devel] [PATCH v2 3/3] Convert map_domain_page() to use the new mfn_t type

[Ben Catterall <Ben.Catterall@xxxxxxxxxx>]

On 07/07/15 11:10, Jan Beulich wrote:
> > > > On 02.07.15 at 14:04, <Ben.Catterall@xxxxxxxxxx> 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 <Ben.Catterall@xxxxxxxxxx>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >
> > ---
> > 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel