From: Grygorii Strashko <grygorii_strashko@epam.com>

The d->arch.physaddr_bitsize field is used only by PV32 code so as the
domain_clamp_alloc_bitsize() function.

Hence move domain_clamp_alloc_bitsize() function into PV32 code and convert
it to macro.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
changes in v4:
 - rebase
 - split out moving d->arch.physaddr_bitsize in separate patch
 - fix comments for domain_clamp_alloc_bitsize() macro

 xen/arch/x86/include/asm/mm.h | 12 ++++++++++--
 xen/arch/x86/x86_64/mm.c      |  7 -------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 9438f5ea0119..89e8940c3316 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -619,8 +619,16 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
 
 extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
 
-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) ({                                 \
+    struct domain *_d = (d);                                                   \
+                                                                               \
+    ((_d &&                                                                    \
+      _d->arch.physaddr_bitsize)                                               \
+         ? min_t(unsigned int, _d->arch.physaddr_bitsize, bits)                \
+         : bits);                                                              \
+})
+#endif
 
 unsigned long domain_get_maximum_gpfn(struct domain *d);
 
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 42fd4fe4e9b5..8eadab7933d0 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1119,13 +1119,6 @@ unmap:
     return ret;
 }
 
-unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits)
-{
-    if ( (d == NULL) || (d->arch.physaddr_bitsize == 0) )
-        return bits;
-    return min(d->arch.physaddr_bitsize, bits);
-}
-
 static int transfer_pages_to_heap(struct mem_hotadd_info *info)
 {
     unsigned long i;
-- 
2.34.1

On 17.12.2025 00:13, Grygorii Strashko wrote:
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -619,8 +619,16 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
>  
>  extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
>  
> -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) ({                                 \
> +    struct domain *_d = (d);                                                   \

This imo wants to be pointer-to-const. Question is whether then I'm upsetting you
again, Andrew?

> +    ((_d &&                                                                    \
> +      _d->arch.physaddr_bitsize)                                               \
> +         ? min_t(unsigned int, _d->arch.physaddr_bitsize, bits)                \
> +         : bits);                                                              \

This imo wants to look more like

    ((_d && _d->arch.physaddr_bitsize)                      \
     ? min_t(unsigned int, _d->arch.physaddr_bitsize, bits) \
     : (bits));                                             \

The parenthesization of the latter use of "bits" is a must.

With the adjustments (happy to carry out while committing, so long as there's
agreement):
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Albeit, as indicated before, I'm not quite happy with the use of min_t(). Maybe
another macro-local variable _bits should be introduced?

Jan

On 17.12.25 09:56, Jan Beulich wrote:
> On 17.12.2025 00:13, Grygorii Strashko wrote:
>> --- a/xen/arch/x86/include/asm/mm.h
>> +++ b/xen/arch/x86/include/asm/mm.h
>> @@ -619,8 +619,16 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
>>   
>>   extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
>>   
>> -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) ({                                 \
>> +    struct domain *_d = (d);                                                   \
> 
> This imo wants to be pointer-to-const. Question is whether then I'm upsetting you
> again, Andrew?
> 
>> +    ((_d &&                                                                    \
>> +      _d->arch.physaddr_bitsize)                                               \
>> +         ? min_t(unsigned int, _d->arch.physaddr_bitsize, bits)                \
>> +         : bits);                                                              \
> 
> This imo wants to look more like
> 
>      ((_d && _d->arch.physaddr_bitsize)                      \
>       ? min_t(unsigned int, _d->arch.physaddr_bitsize, bits) \
>       : (bits));                                             \
> 
> The parenthesization of the latter use of "bits" is a must.
> 

I'm ok with above adjustments, thank you.

> With the adjustments (happy to carry out while committing, so long as there's
> agreement):
> Reviewed-by: Jan Beulich <jbeulich@suse.com>



> Albeit, as indicated before, I'm not quite happy with the use of min_t(). Maybe
> another macro-local variable _bits should be introduced?


-- 
Best regards,
-grygorii