Re: [XEN PATCH 1/7] xen: introduce XENFEAT_xenstore_late_init

[Julien Grall <julien@xen.org>]

Hi,

On 08/01/2022 00:49, Stefano Stabellini wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> Introduce a new feature flag to signal that xenstore will not be
> immediately available at boot time. Instead, xenstore will become
> available later, and a notification of xenstore readiness will be
> signalled to the guest using the xenstore event channel.

Hmmm... On the thread [1], you semmed to imply that new Linux version (I 
am assuming master) are ready to be used in dom0less with the node xen. 
So I am bit confused why this is necessary?

> 
> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: Wei Liu <wl@xen.org>
> ---
>   xen/arch/arm/include/asm/domain.h | 2 ++
>   xen/common/kernel.c               | 2 ++
>   xen/include/public/features.h     | 6 ++++++
>   3 files changed, 10 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index 9b3647587a..e5ae57cd09 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -89,6 +89,8 @@ struct arch_domain
>   #ifdef CONFIG_TEE
>       void *tee;
>   #endif
> +    /* Is this guest a dom0less domain? */
> +    bool is_dom0less;
>   }  __cacheline_aligned;
>   
>   struct arch_vcpu
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index e119e5401f..c00ea67e5f 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -550,6 +550,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>               if ( is_hardware_domain(d) )
>                   fi.submap |= 1U << XENFEAT_dom0;
>   #ifdef CONFIG_ARM
> +            if ( d->arch.is_dom0less )
> +                fi.submap |= (1U << XENFEAT_xenstore_late_init);
>               fi.submap |= (1U << XENFEAT_ARM_SMCCC_supported);
>   #endif
>   #ifdef CONFIG_X86
> diff --git a/xen/include/public/features.h b/xen/include/public/features.h
> index 9ee2f760ef..18f32b1a98 100644
> --- a/xen/include/public/features.h
> +++ b/xen/include/public/features.h
> @@ -128,6 +128,12 @@
>   #define XENFEAT_not_direct_mapped         16
>   #define XENFEAT_direct_mapped             17
>   
> +/*
> + * The xenstore interface should be initialized only after receiving a
> + * xenstore event channel notification.
> + */
> +#define XENFEAT_xenstore_late_init 18

You are assuming that there will be no event until Xenstored has 
discovered the domain. If I am not mistaken, this works because when you 
allocate an unbound port, we will not raise the event.

But I am not sure this is a guarantee for the event channel ABI. For 
instance, when using bind interdomain an event will be raised on the 
local port.

Looking at the Xenstore interface, there are a field connection. Could 
we use it (maybe a flag) to tell when the connection was fully initiated?

> +
>   #define XENFEAT_NR_SUBMAPS 1
>   
>   #endif /* __XEN_PUBLIC_FEATURES_H__ */

Cheers,

[1] <alpine.DEB.2.22.394.2112131729100.3376@ubuntu-linux-20-04-desktop>

-- 
Julien Grall