Re: [PATCH] x86/pv: Inline domain_set_alloc_bitsize() into it's single caller

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

Hi Andrew,

On 09.12.25 20:07, Andrew Cooper wrote:
> Prior to commit 02e78311cdc6 ("x86/domctl: Make XEN_DOMCTL_set_address_size
> singleshot") (Xen 4.9, 2016), it was possible for domains to switch to being
> compat, and back again.  Since then however, becoming compat is a singleton
> action that can't be undone.
>
>  From the context it's clear to see the is_pv_32bit_domain() check is
> redundant, and from the singleton nature being the only place setting
> physaddr_bitsize, there's no need to check it for being 0.
>
> No functional change.
>
> Co-developed-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> Split out of series to simplify things.
>
> bloat-o-meter reports:
>
>    add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
>    Function                                     old     new   delta
>    switch_compat                                447     472     +25
>    domain_set_alloc_bitsize                      96       -     -96
>
> which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
Thank you for doing this.
Not sure if it's needed, any way.
Reviewed-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>

> ---
>   xen/arch/x86/include/asm/mm.h |  1 -
>   xen/arch/x86/pv/domain.c      |  6 +++++-
>   xen/arch/x86/x86_64/mm.c      | 13 -------------
>   3 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
> index 17ca6666a34e..9438f5ea0119 100644
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -619,7 +619,6 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
>   
>   extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
>   
> -void domain_set_alloc_bitsize(struct domain *d);
>   unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits);
>   #define domain_clamp_alloc_bitsize(d, bits) domain_clamp_alloc_bitsize(d, 
> bits)
>   
> diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
> index 9c4785c187dd..11db6a6d8396 100644
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -254,7 +254,11 @@ int switch_compat(struct domain *d)
>               goto undo_and_fail;
>       }
>   
> -    domain_set_alloc_bitsize(d);
> +    if ( MACH2PHYS_COMPAT_NR_ENTRIES(d) < max_page )
> +        d->arch.physaddr_bitsize =
> +            /* 2^n entries can be contained in guest's p2m mapping space */
> +            fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1 + PAGE_SHIFT;
> +
>       recalculate_cpuid_policy(d);
>   
>       d->arch.x87_fip_width = 4;
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index d4e6a9c0a2e0..42fd4fe4e9b5 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1119,19 +1119,6 @@ int handle_memadd_fault(unsigned long addr, struct 
> cpu_user_regs *regs)
>       return ret;
>   }
>   
> -void domain_set_alloc_bitsize(struct domain *d)
> -{
> -    if ( !is_pv_32bit_domain(d) ||
> -         (MACH2PHYS_COMPAT_NR_ENTRIES(d) >= max_page) ||
> -         d->arch.physaddr_bitsize > 0 )
> -        return;
> -    d->arch.physaddr_bitsize =
> -        /* 2^n entries can be contained in guest's p2m mapping space */
> -        fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1
> -        /* 2^n pages -> 2^(n+PAGE_SHIFT) bits */
> -        + PAGE_SHIFT;
> -}
> -
>   unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits)
>   {
>       if ( (d == NULL) || (d->arch.physaddr_bitsize == 0) )
--
Best regards,
-grygorii