On 18.11.2025 07:45, Penny, Zheng wrote: > [Public] > >> -----Original Message----- >> From: Jan Beulich <[email protected]> >> Sent: Thursday, October 30, 2025 9:35 PM >> To: Penny, Zheng <[email protected]> >> Cc: Huang, Ray <[email protected]>; [email protected]; Andrew >> Cooper <[email protected]>; Roger Pau Monné <[email protected]>; >> [email protected] >> Subject: Re: [PATCH v3 27/28] xen/domctl: make HVM_PARAM_IDENT_PT >> conditional upon CONFIG_MGMT_HYPERCALLS >> >> On 13.10.2025 12:15, Penny Zheng wrote: >>> Helper domctl_lock_{acquire,release} is domctl_lock, which >>> HVM_PARAM_IDENT_PT uses to ensure synchronization and hence being a >> toolstack-only operation. >>> So we shall make HVM_PARAM_IDENT_PT conditional upon >>> CONFIG_MGMT_HYPERCALLS, returning -EOPNOTSUPP when >> MGMT_HYPERCALLS=n. >>> >>> Suggested-by: Jan Beulich <[email protected]> >> >> I fear this isn't quite what I suggested. The param get/set are XSM_TARGET, >> i.e. >> can be used by DM as well. The particular one here shouldn't be used by a >> DM, but >> that's a different question. Similarly in principle the PVH Dom0 building >> code should >> be able to use this path; it doesn't right now in favor of some open- coding. >> >> What iirc I did suggest was that the serialization isn't needed when no >> domctl can >> be used to otherwise alter (relevant) guest state. > > Ah, true, serialization isn't needed when MGMT_HYPERCALLS=n, as no domctl-op > could alter the guest state at the same time. > Then maybe adding IS_ENABLED() checking is enough:
Yes, that or indeed introducing stubs. Which one is the lesser evil I'm having a hard time determining. Hence I'd suggest that you go with the below, unless someone else chimes in. Jan > ``` > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 5a50721bd0..4e1b3ee5f4 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4324,7 +4324,7 @@ static int hvm_set_param(struct domain *d, uint32_t > index, uint64_t value) > * the domctl_lock. > */ > rc = -ERESTART; > - if ( !domctl_lock_acquire() ) > + if ( IS_ENABLED(CONFIG_MGMT_HYPERCALLS) && !domctl_lock_acquire() ) > break; > > rc = 0; > @@ -4334,7 +4334,8 @@ static int hvm_set_param(struct domain *d, uint32_t > index, uint64_t value) > paging_update_cr3(v, false); > domain_unpause(d); > > - domctl_lock_release(); > + if ( IS_ENABLED(CONFIG_MGMT_HYPERCALLS) ) > + domctl_lock_release(); > break; > case HVM_PARAM_DM_DOMAIN: > /* The only value this should ever be set to is DOMID_SELF */ > ``` > >> >> Jan
