Re: [PATCH v1 15/19] xen/sysctl: wrap around XEN_SYSCTL_physinfo

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.03.2025 05:06, Penny Zheng wrote:
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -15,6 +15,7 @@
>  #include <asm/arm64/sve.h>
>  #include <public/sysctl.h>
>  
> +#ifdef CONFIG_SYSCTL
>  void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>  {
>      pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
> @@ -22,6 +23,7 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>      pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>  }
> +#endif

This is being removed again in patch 18. Can we try to avoid such back and 
forth,
if at all possible?

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -581,6 +581,8 @@ out:
>      return ret;
>  }
>  
> +#ifdef CONFIG_SYSCTL
> +static unsigned long avail_domheap_pages(void);

Instead of this, ...

> @@ -2796,12 +2799,14 @@ unsigned long avail_domheap_pages_region(
>      return avail_heap_pages(zone_lo, zone_hi, node);
>  }
>  
> +#ifdef CONFIG_SYSCTL
>  static unsigned long avail_domheap_pages(void)
>  {
>      return avail_heap_pages(MEMZONE_XEN + 1,
>                              NR_ZONES - 1,
>                              -1);
>  }
> +#endif

... can this be moved up please? Then also reducing the number of #ifdef-s?
Arguably avail_domheap_pages() could be inlined into its sole caller. That
might then be a legitimate replacement for patch 14, without breaking the
build.

> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -131,7 +131,12 @@ int populate_pt_range(unsigned long virt, unsigned long 
> nr_mfns);
>  unsigned long __must_check domain_adjust_tot_pages(struct domain *d,
>      long pages);
>  int domain_set_outstanding_pages(struct domain *d, unsigned long pages);
> +#ifdef CONFIG_SYSCTL
>  void get_outstanding_claims(uint64_t *free_pages, uint64_t 
> *outstanding_pages);
> +#else
> +static inline void get_outstanding_claims(uint64_t *free_pages,
> +                                          uint64_t *outstanding_pages) {}

This isn't a valid stub: Both arguments point to outputs. As it stands the
function now requires the caller to clear respective fields/variables up
front.

Jan