Re: [PATCH v2 10/35] xen/domain: add get_initial_domain_id()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Dec 05, 2024 at 08:41:40PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Move get_initial_domain_id() to a public API and enable for all architectures.
> That is pre-requisite change for console focus switch logic cleanup.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
>  xen/arch/x86/include/asm/pv/shim.h |  4 ++--
>  xen/arch/x86/pv/shim.c             |  4 ++--
>  xen/common/domain.c                | 10 ++++++++++
>  xen/include/xen/domain.h           |  2 ++
>  4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/pv/shim.h 
> b/xen/arch/x86/include/asm/pv/shim.h
> index 
> 6153e27005986881ad87e9db0b555b30edc59fc0..1515ad1b0680aa11ab91a152a1944fc1bb477a79
>  100644
> --- a/xen/arch/x86/include/asm/pv/shim.h
> +++ b/xen/arch/x86/include/asm/pv/shim.h
> @@ -31,7 +31,7 @@ long cf_check pv_shim_cpu_up(void *data);
>  long cf_check pv_shim_cpu_down(void *data);
>  void pv_shim_online_memory(unsigned int nr, unsigned int order);
>  void pv_shim_offline_memory(unsigned int nr, unsigned int order);
> -domid_t get_initial_domain_id(void);
> +domid_t pv_shim_initial_domain_id(void);
>  uint64_t pv_shim_mem(uint64_t avail);
>  void pv_shim_fixup_e820(void);
>  const struct platform_bad_page *pv_shim_reserved_pages(unsigned int *size);
> @@ -76,7 +76,7 @@ static inline void pv_shim_offline_memory(unsigned int nr, 
> unsigned int order)
>  {
>      ASSERT_UNREACHABLE();
>  }
> -static inline domid_t get_initial_domain_id(void)
> +static inline domid_t pv_shim_initial_domain_id(void)
>  {
>      return 0;
>  }
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index 
> 81e4a0516d18b359561f471f1d96e38977661ca7..17cb30620290c76cf42251f70cfa4199c0e165d1
>  100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -328,7 +328,7 @@ int pv_shim_shutdown(uint8_t reason)
>      }
>  
>      /* Update domain id. */
> -    d->domain_id = get_initial_domain_id();
> +    d->domain_id = pv_shim_initial_domain_id();

Can't you leave this instance using get_initial_domain_id(), it should
DTRT when running in pv-shim mode.

>  
>      /* Clean the iomem range. */
>      BUG_ON(iomem_deny_access(d, 0, ~0UL));
> @@ -1016,7 +1016,7 @@ void pv_shim_offline_memory(unsigned int nr, unsigned 
> int order)
>      }
>  }
>  
> -domid_t get_initial_domain_id(void)
> +domid_t pv_shim_initial_domain_id(void)
>  {
>      uint32_t eax, ebx, ecx, edx;
>  
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 
> 92263a4fbdc57159b4a32d9d4ee038f9f37804ed..2f67aa06ed50e69c27cedc8d7f6eb0b469fe81cd
>  100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -45,6 +45,7 @@
>  
>  #ifdef CONFIG_X86
>  #include <asm/guest.h>
> +#include <asm/pv/shim.h>
>  #endif
>  
>  /* Linux config option: propageted to domain0 */
> @@ -2229,6 +2230,15 @@ int continue_hypercall_on_cpu(
>      return 0;
>  }
>  
> +domid_t get_initial_domain_id(void)
> +{
> +#ifdef CONFIG_X86
> +    return pv_shim_initial_domain_id();
> +#else
> +    return 0;
> +#endif
> +}

Maybe there are further changes that make this a not suitable option,
but won't it be better to maybe do something like:

#ifndef HAS_ARCH_INITIAL_DOMID
static inline domid_t get_initial_domain_id(void) { return 0; }
#else
domid_t get_initial_domain_id(void);
#endif

In a generic header, and then in an x86 header you just

#define HAS_ARCH_INITIAL_DOMID

The ifdefary in get_initial_domain_id() if other arches need different
implementations seems undesirable.

Thanks, Roger.