On 2025-04-01 19:50, Stefano Stabellini wrote:
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.
With Jan's suggestion, I am dropping flags.
Yes, frame should be uin32_t since it is filling in a grant table v1
entry, and that is limited to 32bit frame numbers. If the frame number
is >32bits, then the grant can't be seeded. I guess I'll put a check in
the caller to ensure that. Looking at the supported guest memory
maximums, 32bit gfns should be enough to cover all the limits. > 16TiB
would be needed to exceed a 32bit frame number.
Regards,
Jason
