Re: [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 31/05/2022 09:54, Jan Beulich wrote:
> On 31.05.2022 05:12, Penny Zheng wrote:
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
> >   
> >                   mfn = _mfn(gpfn);
> >               }
> > +            else if ( is_domain_using_staticmem(d) )
> > +            {
> > +                /*
> > +                 * No easy way to guarantee the retrieved pages are contiguous,
> > +                 * so forbid non-zero-order requests here.
> > +                 */
> > +                if ( a->extent_order != 0 )
> > +                {
> > +                    gdprintk(XENLOG_WARNING,
> > +                             "Cannot allocate static order-%u pages for static 
> > %pd\n",
> > +                             a->extent_order, d);
> > +                    goto out;
> > +                }
> > +
> > +                mfn = acquire_reserved_page(d, a->memflags);
> > +                if ( mfn_eq(mfn, INVALID_MFN) )
> > +                {
> > +                    gdprintk(XENLOG_WARNING,
> > +                             "%pd: failed to retrieve a reserved page\n",
> > +                             d);
> > +                    goto out;
> > +                }
> > +            }
> I'm not convinced of having these gdprintk()s here. 
There are a number of time where I wished some error paths would contain 
debug printk(). Instead, I often end up to add them myself when I 
struggle to find the reason of a failure.
They are debug printk() and therefore there is no impact on the 
production build. So I would leave them around.
> The adjacent
> is_domain_direct_mapped() code is somewhat different - iirc only Dom0
> can be direct-mapped, and Dom0 having a problem can certainly be worth
> a log message.
There are plan to use direct-mapped domU.

Cheers,

--
Julien Grall