Re: [PATCH v2] x86+libxl: correct p2m (shadow) memory pool size calculation

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Apr 25, 2022 at 03:19:46PM +0200, Jan Beulich wrote:
> On 25.04.2022 14:59, Roger Pau Monné wrote:
> > On Mon, Apr 25, 2022 at 10:49:34AM +0200, Jan Beulich wrote:
> >>  char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
> >> --- a/xen/arch/x86/dom0_build.c
> >> +++ b/xen/arch/x86/dom0_build.c
> >> @@ -317,9 +317,12 @@ unsigned long __init dom0_paging_pages(c
> >>      /* Copied from: libxl_get_required_shadow_memory() */
> > 
> > Could you also update the comment, maybe better would be:
> > 
> > /* Keep in sync with libxl__get_required_paging_memory(). */
> 
> Oh, of course.
> 
> >>      unsigned long memkb = nr_pages * (PAGE_SIZE / 1024);
> >>  
> >> -    memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
> >> +    memkb = 4 * (256 * d->max_vcpus +
> >> +                 (paging_mode_enabled(d) +
> >> +                  (opt_dom0_shadow || opt_pv_l1tf_hwdom)) *
> > 
> > opt_pv_l1tf_hwdom is only relevant for PV guests, so maybe it would be
> > best to use:
> > 
> > paging_mode_enabled(d) ? 1 + opt_dom0_shadow
> >                        : 0 + (opt_dom0_shadow || opt_pv_l1tf_hwdom)
> > 
> > Or something similar.
> 
> Originally I was thinking that people simply shouldn't use the option
> when Dom0 isn't PV. But meanwhile I've figured that late-hwdom may be
> PV even if domain 0 is PVH. So yes.
> 
> >  Maybe placing this inside the sum will make the
> > expression too complex, so we could use a separate is_shadow boolean
> > to signal whether the domain will use shadow pagetables?
> 
> I think
> 
>     memkb = 4 * (256 * d->max_vcpus +
>                  (is_pv_domain(d) ? opt_dom0_shadow || opt_pv_l1tf_hwdom
>                                   : 1 + opt_dom0_shadow) *
>                  (memkb / 1024));
> 
> is still okay-ish. Note that I've switched to is_pv_domain() to be
> independent of the point in time when shadow mode would be enabled
> for a PV Dom0.

Thanks, LGTM.