diff mbox series

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

Message ID 20241205-vuart-ns8250-v1-10-e9aa923127eb@ford.com (mailing list archive)
State New
Headers show
Series Introduce NS8250 UART emulator | expand

Commit Message

Denis Mukhin via B4 Relay Dec. 6, 2024, 4:41 a.m. UTC 
From: Denis Mukhin <dmukhin@ford.com>

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@ford.com>
---
 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(-)

Comments

Jan Beulich Dec. 10, 2024, 1:50 p.m. UTC | #1 
On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote:
> 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.

Yet then how does this fit with dom0less, let alone hyperlaunch,
where multiple domains may be created right when Xen starts?

Plus, if you make this generic, shouldn't Arm also be adjusted to
use this function (if nothing else then to avoid things going out
of sync later on)?

> @@ -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
> +}

Imo this either wants to use CONFIG_PV_SHIM instead, eliminating the
need for the pv_shim_initial_domain_id() stub.

Jan
Roger Pau Monné Dec. 11, 2024, 4:50 p.m. UTC | #2 
On Thu, Dec 05, 2024 at 08:41:40PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> 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@ford.com>
> ---
>  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.
diff mbox series

Patch

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();
 
     /* 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
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 3de56352911347a54cce310f0211bcc213d8a08d..601ef431cf621af44c867400499b73b845eb137a 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -171,4 +171,6 @@  extern bool vmtrace_available;
 
 extern bool vpmu_is_available;
 
+domid_t get_initial_domain_id(void);
+
 #endif /* __XEN_DOMAIN_H__ */