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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

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