Re: [Xen-devel] [PATCH for-next 7/9] xen: Convert is_xen_heap_mfn to use typesafe MFN

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 3/14/19 10:14 AM, Andrew Cooper wrote:
> On 14/03/2019 10:12, Julien Grall wrote:
> > Hi Jan,
> >
> > On 3/14/19 7:52 AM, Jan Beulich wrote:
> > > > > > On 13.03.19 at 18:24, <julien.grall@xxxxxxx> wrote:
> > > > On 13/03/2019 15:04, Jan Beulich wrote:
> > > > > > > > On 18.02.19 at 12:35, <julien.grall@xxxxxxx> wrote:
> > > > > > --- a/xen/common/page_alloc.c
> > > > > > +++ b/xen/common/page_alloc.c
> > > > > > @@ -2121,9 +2121,9 @@ void init_xenheap_pages(paddr_t ps, paddr_t pe)
> > > > > >          * Yuk! Ensure there is a one-page buffer between Xen and
> > > > > > Dom zones, to
> > > > > >          * prevent merging of power-of-two blocks across the zone
> > > > > > boundary.
> > > > > >          */
> > > > > > -    if ( ps && !is_xen_heap_mfn(paddr_to_pfn(ps)-1) )
> > > > > > +    if ( ps && !is_xen_heap_mfn(_mfn(paddr_to_pfn(ps)-1)) )
> > > > > >             ps += PAGE_SIZE;
> > > > > > -    if ( !is_xen_heap_mfn(paddr_to_pfn(pe)) )
> > > > > > +    if ( !is_xen_heap_mfn(maddr_to_mfn(pe)) )
> > > > > Why maddr_to_mfn() here but still paddr_to_pfn() above? Oh,
> > > > > we don't have any mfn_sub(), I see.
> > > > Yes we don't have mfn_sub() (or even gfn_sub()). I only found a
> > > > couple of places
> > > > where such helpers might be useful. I can introduce the 2 helpers if
> > > > you think
> > > > it is worth it.
> > > Well, I guess in the end I'm fine either way. It simply struck me
> > > as odd at the first glance that you use maddr_to_mfn() in one
> > > case but not the other.
> > I wanted to avoid adding mfn_x(...) on the line:
> >
> > _mfn(mfn_x(maddr_to_mfn(ps)) - 1)
> >
> > I will look to introduce mfn_sub()/gfn_sub().
> You do know that mfn_add(mfn, -1) will DTRT?
It didn't occur to me until you said it. I will use that then.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel