Re: [PATCH 1/8] x86/pdx: simplify calculation of domain struct allocation boundary

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.06.2025 19:16, Roger Pau Monne wrote:
> @@ -498,14 +474,15 @@ struct domain *alloc_domain_struct(void)
>       * On systems with CONFIG_BIGMEM there's no packing, and so there's no
>       * such restriction.
>       */
> -#if defined(CONFIG_BIGMEM) || !defined(CONFIG_PDX_COMPRESSION)
> -    const unsigned int bits = IS_ENABLED(CONFIG_BIGMEM) ? 0 :
> -                                                          32 + PAGE_SHIFT;
> +#if defined(CONFIG_BIGMEM)
> +    const unsigned int bits = 0;
>  #else
> -    static unsigned int __read_mostly bits;
> +    static unsigned int __ro_after_init bits;
>  
>      if ( unlikely(!bits) )
> -         bits = _domain_struct_bits();
> +         bits = flsl(pfn_to_paddr(pdx_to_pfn(
> +             1UL << (sizeof(((struct page_info *)NULL)->v.inuse._domain) * 
> 8))))
> +             - 1;

While Andrew did point you at sizeof_field(), we can have this even less verbose
by utilizing that frame_table is of the right type and (almost) globally in 
scope.

Further, why use pfn_to_paddr()?

         bits = flsl(pdx_to_pfn(1UL << 
                                (sizeof(frame_table->v.inuse._domain) * 8))) +
                PAGE_SHIFT - 1;

However, it further feels like this was off by one; we had similar issues over
time in several places. There potentially being a gap between one less than
the PDX used here and that very PDX, don't we need to calculate based on the
"one less" value here? Hmm, there being a gap means no allocation would
succeed for the precise value of "bits" (in the mask-compression scheme), so
functionally all would be fine. Yet just to avoid setting a bad precedent I
think we'd still be better off using

         bits = flsl(pdx_to_pfn((1UL << 
                                 (sizeof(frame_table->v.inuse._domain) * 8)) -
                                1)) + PAGE_SHIFT;

If one would log the value of bits, the result would then also be less
confusing in (at least) the mask-compression scheme.

Jan