Re: [PATCH v7 01/20] xen: introduce CONFIG_HAS_SHARED_INFO for archs without a shared page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.08.2026 19:03, Oleksii Kurochko wrote:
> On 8/13/26 4:54 PM, Jan Beulich wrote:
>> On 04.08.2026 17:47, Oleksii Kurochko wrote:
>>> @@ -1324,9 +1359,15 @@ int evtchn_reset(struct domain *d, bool resuming)
>>>           rc = -EAGAIN;
>>>       else if ( d->evtchn_fifo )
>>>       {
>>> -        /* Switching back to 2-level ABI. */
>>>           evtchn_fifo_destroy(d);
>>> -        evtchn_2l_init(d);
>>> +
>>> +        if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
>>> +            /* Switching back to 2-level ABI. */
>>> +            evtchn_2l_init(d);
>>> +        else if ( IS_ENABLED(CONFIG_EVTCHN_FIFO) )
>>> +            evtchn_fifo_init_ops(d);
>>> +        else
>>> +            evtchn_none_init(d);
>>
>> This being the same as ...
>>
>>> @@ -1625,7 +1666,13 @@ void evtchn_check_pollers(struct domain *d, unsigned 
>>> int port)
>>>   
>>>   int evtchn_init(struct domain *d, unsigned int max_port)
>>>   {
>>> -    evtchn_2l_init(d);
>>> +    if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
>>> +        evtchn_2l_init(d);
>>> +    else if ( IS_ENABLED(CONFIG_EVTCHN_FIFO) )
>>> +        evtchn_fifo_init_ops(d);
>>> +    else
>>> +        evtchn_none_init(d);
>>
>> ... this: Maybe have a small helper (evtchn_preinit()?), to reduce the
>> duplication? Would require comment updates then as well.
> 
> I think then it will be needed to fix a lot of comments. My suggestion 
> is the following:
> 
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 0911808fe861..809638ce4bfd 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -71,10 +71,23 @@ static void evtchn_none_init(struct domain *d)
>       d->evtchn_port_ops = &evtchn_port_ops_none;
>   }
>   #else
> -/* Declaration only; the calls below are DCE'd unless both configs are 
> off. */
> +/*
> + * Declaration only; the call in evtchn_preinit() is DCE'd unless both
> + * configs are off.
> + */
>   void evtchn_none_init(struct domain *d);
>   #endif /* !CONFIG_HAS_SHARED_INFO && !CONFIG_EVTCHN_FIFO */
> 
> +static void evtchn_preinit(struct domain *d)
> +{
> +    if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
> +        evtchn_2l_init(d);
> +    else if ( IS_ENABLED(CONFIG_EVTCHN_FIFO) )
> +        evtchn_fifo_init_ops(d);
> +    else
> +        evtchn_none_init(d);
> +}
> +
>   /*
>    * Lock an event channel exclusively. This is allowed only when the 
> channel is
>    * free or unbound either when taking or when releasing the lock, as any
> @@ -1359,15 +1372,9 @@ int evtchn_reset(struct domain *d, bool resuming)
>           rc = -EAGAIN;
>       else if ( d->evtchn_fifo )
>       {
> +        /* Switching back to the default ABI. */
>           evtchn_fifo_destroy(d);
> -
> -        if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
> -            /* Switching back to 2-level ABI. */
> -            evtchn_2l_init(d);
> -        else if ( IS_ENABLED(CONFIG_EVTCHN_FIFO) )
> -            evtchn_fifo_init_ops(d);
> -        else
> -            evtchn_none_init(d);
> +        evtchn_preinit(d);
>       }
> 
>       write_unlock(&d->event_lock);
> @@ -1666,12 +1673,7 @@ void evtchn_check_pollers(struct domain *d, 
> unsigned int port)
> 
>   int evtchn_init(struct domain *d, unsigned int max_port)
>   {
> -    if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
> -        evtchn_2l_init(d);
> -    else if ( IS_ENABLED(CONFIG_EVTCHN_FIFO) )
> -        evtchn_fifo_init_ops(d);
> -    else
> -        evtchn_none_init(d);
> +    evtchn_preinit(d);
> 
>       d->max_evtchn_port = min_t(unsigned int, max_port, INT_MAX);
> 
> diff --git a/xen/common/event_channel.h b/xen/common/event_channel.h
> index c8ee09807008..156514fefff6 100644
> --- a/xen/common/event_channel.h
> +++ b/xen/common/event_channel.h
> @@ -71,8 +71,8 @@ static inline void evtchn_fifo_destroy(struct domain *d)
>   #endif /* CONFIG_EVTCHN_FIFO */
> 
>   /*
> - * Declaration only when !CONFIG_EVTCHN_FIFO; the (dead) calls in
> - * evtchn_init() and evtchn_reset() are DCE'd in that case.
> + * Declaration only when !CONFIG_EVTCHN_FIFO; the (dead) call in
> + * evtchn_preinit() is DCE'd in that case.
>    */
>   void evtchn_fifo_init_ops(struct domain *d);
> 
> diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
> index 3b6e619c5278..f11c4c16efa3 100644
> --- a/xen/common/event_fifo.c
> +++ b/xen/common/event_fifo.c
> @@ -423,10 +423,9 @@ static const struct evtchn_port_ops 
> evtchn_port_ops_fifo =
>   };
> 
>   /*
> - * evtchn_fifo_init_ops()'s only call sites are in the
> - * IS_ENABLED(CONFIG_EVTCHN_FIFO) dead branches of evtchn_init() and
> - * evtchn_reset(), which are never reached on HAS_SHARED_INFO=y builds
> - * because of DCE.
> + * evtchn_fifo_init_ops()'s only call site is the
> + * IS_ENABLED(CONFIG_EVTCHN_FIFO) dead branch of evtchn_preinit(), which is
> + * never reached on HAS_SHARED_INFO=y builds because of DCE.
>    */
>   #ifndef CONFIG_HAS_SHARED_INFO
>   void evtchn_fifo_init_ops(struct domain *d)
> diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
> index 930190054cf0..595dedf0792c 100644
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -211,7 +211,7 @@ static bool evtchn_usable(const struct evtchn *evtchn)
> 
>   void evtchn_check_pollers(struct domain *d, unsigned int port);
> 
> -/* Close all event channels and reset to 2-level ABI. */
> +/* Close all event channels and reset to the default ABI. */
>   int evtchn_reset(struct domain *d, bool resuming);
> 
> Does it look good for you?

Yes, thanks.

Jan