From: Jan Beulich <jbeulich@suse.com>
To: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Cc: Romain Caritey <Romain.Caritey@microchip.com>,
Baptiste Le Duc <baptiste.le-duc@vates.tech>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v4] xen: introduce CONFIG_HAS_SHARED_INFO for archs without a shared page
Date: Mon, 29 Jun 2026 16:26:51 +0200 [thread overview]
Message-ID: <5d344cec-bb97-4d3b-87ff-e7175772fd45@suse.com> (raw)
In-Reply-To: <775c88457e5ec7fc7889002c6f9829669f9bce97.1782388193.git.oleksii.kurochko@gmail.com>
On 25.06.2026 18:02, Oleksii Kurochko wrote:
> On architectures that run guests in dom0less mode without the PV ABI
> (currently RISC-V), no shared_info page is allocated and d->shared_info
> remains NULL throughout the domain lifetime. Several places in common
> code access d->shared_info through the shared_info() macro or directly,
> causing UBSAN null-pointer errors on such architectures.
>
> Rather than adding runtime NULL guards that are logically unreachable
> on x86 and Arm (where shared_info is always allocated), introduce a new
> Kconfig symbol CONFIG_HAS_SHARED_INFO selected by x86 and Arm.
>
> On !HAS_SHARED_INFO the shared_info() macro expands to a dereference
> of a pointer returned by shared_info_absent(), which is declared but
> intentionally never defined.
This looks to need updating.
> Any use of shared_info() that is not
> dead-code-eliminated will therefore cause a link-time failure, making
> missed guards impossible to overlook.
>
> The 2L event-channel ops call shared_info() and must not be compiled on
> architectures without a shared_info page, so event_2l.o is gated on
> CONFIG_HAS_SHARED_INFO. On such architectures evtchn_init() installs
> the FIFO ops as a placeholder instead; evtchn_fifo_word_from_port() is
> guarded against uninitialised d->evtchn_fifo so the FIFO ops are safe
> before evtchn_fifo_init_control() is called by the guest.
>
> With CONFIG_HAS_SHARED_INFO=n all vCPUs fall back to the global
> dummy_vcpu_info, so writes through vcpu_info() could leak data between
> vCPUs. Reviewing the write paths in common code: the write in
> map_guest_area() stores the constant ~0 so nothing serious would happen
> if it were leaked; the event_2l.c paths are unreachable because the
> preceding shared_info() call would trap first;
Why "trap"? You can't build an image that way, can you?
> @@ -1624,7 +1626,11 @@ 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);
For this to build when !HAS_SHARED_INFO, all you need is a declaration of
the function. The compiler will DCE the call. Hence ...
> --- a/xen/common/event_channel.h
> +++ b/xen/common/event_channel.h
> @@ -44,7 +44,11 @@ static inline void evtchn_port_print_state(struct domain *d,
>
> /* 2-level */
>
> +#ifdef CONFIG_HAS_SHARED_INFO
> void evtchn_2l_init(struct domain *d);
> +#else
> +static inline void evtchn_2l_init(struct domain *d) {}
> +#endif
>
> /* FIFO */
>
... this hunk should be unnecessary?
> @@ -55,6 +59,7 @@ struct evtchn_expand_array;
> int evtchn_fifo_init_control(struct evtchn_init_control *init_control);
> int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array);
> void evtchn_fifo_destroy(struct domain *d);
> +void evtchn_fifo_init_ops(struct domain *d);
> #else
> static inline int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
> {
> @@ -68,6 +73,7 @@ static inline void evtchn_fifo_destroy(struct domain *d)
> {
> return;
> }
> +static inline void evtchn_fifo_init_ops(struct domain *d) {}
> #endif /* CONFIG_EVTCHN_FIFO */
Unlike these two. Which raise a different question though: What will be the
behavior when EVTCHN_FIFO=n and HAS_SHARED_INFO=n? Taking
evtchn_alloc_unbound() as example, afaict evtchn_port_init() will stumble
over a NULL pointer. Looks like for that (and only that) case we still need
your earlier dummy fallback.
> @@ -420,6 +423,11 @@ static const struct evtchn_port_ops evtchn_port_ops_fifo =
> .print_state = evtchn_fifo_print_state,
> };
>
> +void evtchn_fifo_init_ops(struct domain *d)
> +{
> + d->evtchn_port_ops = &evtchn_port_ops_fifo;
> +}
Isn't this unreachable code when HAS_SHARED_INFO=y, violating Misra rule 2.1?
Jan
next prev parent reply other threads:[~2026-06-29 14:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-25 16:02 [PATCH v4] xen: introduce CONFIG_HAS_SHARED_INFO for archs without a shared page Oleksii Kurochko
2026-06-29 14:26 ` Jan Beulich [this message]
2026-06-30 15:02 ` Oleksii Kurochko
2026-07-01 15:30 ` Oleksii Kurochko
2026-07-01 15:25 ` Oleksii Kurochko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d344cec-bb97-4d3b-87ff-e7175772fd45@suse.com \
--to=jbeulich@suse.com \
--cc=Romain.Caritey@microchip.com \
--cc=baptiste.le-duc@vates.tech \
--cc=oleksii.kurochko@gmail.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.