[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 4/6] xen/arm: dom0less seed xenstore grant table entry
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |