On Tue, 1 Apr 2025, Jason Andryuk wrote:
> On 2025-04-01 08:16, Jan Beulich wrote:
> > On 31.03.2025 23:43, Jason Andryuk wrote:
>
> > > --- a/xen/arch/arm/dom0less-build.c
> > > +++ b/xen/arch/arm/dom0less-build.c
> > > @@ -865,6 +865,10 @@ static void __init initialize_domU_xenstore(void)
> > > rc = alloc_xenstore_evtchn(d);
> > > if ( rc < 0 )
> > > panic("%pd: Failed to allocate xenstore_evtchn\n", d);
> > > +
> > > + if ( gfn != ~0ULL )
> >
> > Is this an odd open-coding of INVALID_GFN? And even if not - why ULL when
> > "gfn" is unsigned long? The way you have it the condition will always be
> > false on Arm32, if I'm not mistaken.
>
> The gfn is pulled out of the HVM_PARAMS, which is a uint64_t. It is set like:
>
> d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
>
> But pulled out by:
>
> unsigned long gfn = d->arch.hvm.params[HVM_PARAM_STORE_PFN];
>
> So your comment highlights that unsigned long is incorrect for ARM32.
>
> While I realize fixed types are discouraged, I'd prefer to use uint64_t for
> the replacement. That is the type of HVM_PARAMS, and uint64_t is used on the
> init-dom0less side as well. Using unsigned long long to get a 64bit value on
> ARM32 seems less clear to me.
The types that correspond to hypercall struct field types should match
the hypercall struct field types.
I think gfn should be uint64_t to match the definition of params.
Similarly among the arguments of gnttab_seed_entry, flags should be
uint16_t and I think frame should be uint32_t. This last one I am
confused why you defined it as uint64_t, maybe I am missing something.
