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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 7/21/26 4:50 PM, Jan Beulich wrote:
> On 20.07.2026 17:59, Oleksii Kurochko wrote:
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -102,9 +102,14 @@ void getdomaininfo(struct domain *d, struct 
> > xen_domctl_getdomaininfo *info)
> >   #ifdef CONFIG_MEM_PAGING
> >       info->paged_pages       = atomic_read(&d->paged_pages);
> >   #endif
> > -    info->shared_info_frame =
> > -        gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info))));
> > -    BUG_ON(SHARED_M2P(info->shared_info_frame));
> > +    if ( IS_ENABLED(CONFIG_HAS_SHARED_INFO) )
> > +    {
> > +        info->shared_info_frame =
> > +            gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info))));
> > +        BUG_ON(SHARED_M2P(info->shared_info_frame));
> > +    }
> > +    else
> > +        info->shared_info_frame = gfn_x(INVALID_GFN);
> There's one issue left here: INVALID_GFN is a Xen internal concept. It could
> change value if we saw a need. Therefore you cannot use that value here, to
> supply it as hypercall output. It needs to be ~0, and imo the public header
> also wants amending to indicate the special meaning of this value.
I think I get your point. I will use ~0 here.

I don't see INVALID_GFN in public headers. If it exists then it isn't 
clear why it shouldn't be used here.
Don't we want to add something like:

/* Domain has no shared_info page? */
#define XEN_INVALID_SHARED_INFO_FRAME (~0ULL)
    uint64_aligned_t shared_info_frame; /* GMFN of shared_info struct */

in public/domctl.h.

> And there's one further aspect to consider: Do we really want to retain the
> shared_info struct domain field when !HAS_SHARED_INFO? 
I think we could consider that.

> Making it conditional
> would require some adjustment here, but might be tidier overall. In
> particular doing so would eliminate the risk of new uses of the field
> appearing, with people not noticing that they'd break RISC-V.
Would you be okay with the following change:

+#ifdef CONFIG_HAS_SHARED_INFO
+    info->shared_info_frame =
+        gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info))));
+    BUG_ON(SHARED_M2P(info->shared_info_frame));
+#else
+    info->shared_info_frame = ~0;
+#endif

~ Oleksii