Re: [PATCH v3 3/3] xen/x86: move d->arch.physaddr_bitsize field handling to pv32

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.11.2025 16:22, Grygorii Strashko wrote:
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -619,9 +619,12 @@ 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)
> +#ifdef CONFIG_PV32
> +#define domain_clamp_alloc_bitsize(d, bits)                                  
>   \
> +    (((d) && (d)->arch.pv.physaddr_bitsize)                                  
>   \
> +         ? min_t(uint32_t, (d)->arch.pv.physaddr_bitsize, (bits))            
>   \
> +         : (bits))

I'm not quite sure if converting to a macro was a good idea. But now that it's
done this way, so be it. Just that a couple of issues may want / need sorting:
- d is now evaluated up to 3 times,
- indentation is wrong,
- the use of uint32_t is against ./CODING_STYLE (I dislike the use of min_t()
  anyway, but what do you do when a macro was asked for; of course we could
  [easily] arrange for BITS_PER_LONG to be of type "unsigned int"),
- the use of "bits" in min_t() doesn't really need parentheses.

> --- 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 )

You mention the change in condition in the revlog (but not in the description),
and I'm having trouble to follow why ...

> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1119,26 +1119,6 @@ unmap:
>      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 )

... this 3rd part is going away.

Jan