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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan