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.
