Re: [PATCH v2 18/26] xen/domctl: wrap xsm_getdomaininfo() with CONFIG_MGMT_HYPERCALLS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.09.2025 08:57, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Friday, September 26, 2025 2:53 PM
>>
>> On 26.09.2025 06:41, Penny, Zheng wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>> Sent: Thursday, September 25, 2025 10:29 PM
>>>>
>>>> On 25.09.2025 11:41, Penny, Zheng wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>> Sent: Thursday, September 11, 2025 9:30 PM
>>>>>>
>>>>>> On 10.09.2025 09:38, Penny Zheng wrote:
>>>>>>> --- a/xen/include/xsm/xsm.h
>>>>>>> +++ b/xen/include/xsm/xsm.h
>>>>>>> @@ -55,8 +55,8 @@ struct xsm_ops {
>>>>>>>      void (*security_domaininfo)(struct domain *d,
>>>>>>>                                  struct xen_domctl_getdomaininfo *info);
>>>>>>>      int (*domain_create)(struct domain *d, uint32_t ssidref);
>>>>>>> -    int (*getdomaininfo)(struct domain *d);
>>>>>>>  #ifdef CONFIG_MGMT_HYPERCALLS
>>>>>>> +    int (*getdomaininfo)(struct domain *d);
>>>>>>>      int (*domctl_scheduler_op)(struct domain *d, int op);
>>>>>>>      int (*sysctl_scheduler_op)(int op);
>>>>>>>      int (*set_target)(struct domain *d, struct domain *e); @@
>>>>>>> -234,7
>>>>>>> +234,11 @@ static inline int xsm_domain_create(
>>>>>>>
>>>>>>>  static inline int xsm_getdomaininfo(xsm_default_t def, struct
>>>>>>> domain
>>>>>>> *d)  {
>>>>>>> +#ifdef CONFIG_MGMT_HYPERCALLS
>>>>>>>      return alternative_call(xsm_ops.getdomaininfo, d);
>>>>>>> +#else
>>>>>>> +    return -EOPNOTSUPP;
>>>>>>> +#endif
>>>>>>>  }
>>>>>>
>>>>>> This is in use by a Xenstore sysctl and a Xenstore domctl. The
>>>>>> sysctl is hence already broken with the earlier series. Now the
>>>>>> domctl is also being screwed up. I don't think MGMT_HYPERCALLS
>>>>>> really ought to extend to any operations available to other than the core
>> toolstack.
>>>>>> That's the Xenstore ones here, but also the ones used by qemu
>>>>>> (whether run in
>>>> Dom0 or a stubdom).
>>>>>
>>>>> Maybe not only limited to the core toolstack. In
>>>>> dom0less/hyperlaunched
>>>> scenarios, hypercalls are strictly limited. QEMU is also limited to
>>>> pvh machine type and with very restricted functionality(, only acting
>>>> as a few virtio-pci devices backend). @Andryuk, Jason @Stabellini,
>>>> Stefano Am I understanding correctly and thoroughly about our scenario 
>>>> here for
>> upstream?
>>>>> Tracking the codes, if Xenstore is created as a stub domain, it
>>>>> requires
>>>> getdomaininfo-domctl to acquire related info.  Sorry, I haven't found
>>>> how it was called in QEMU...
>>>>
>>>> It's not "it"; it's different ones. First and foremost I was thinking
>>>> of
>>>>  * XEN_DOMCTL_ioport_mapping
>>>>  * XEN_DOMCTL_memory_mapping
>>>>  * XEN_DOMCTL_bind_pt_irq
>>>>  * XEN_DOMCTL_unbind_pt_irq
>>>> but there may be others (albeit per the dummy xsm_domctl() this is
>>>> the full set). As a general criteria, anything using XSM_DM_PRIV
>>>> checking can in principle be called by qemu.
>>>>
>>>
>>> Understood.
>>> I assume that they are all for device passthrough. We are not accepting 
>>> device
>> passthrough via core toolstack in dom0less/hyperlaunch-ed scenarios. Jason 
>> has
>> developed device passthrough through device tree to only accept "static
>> configured" passthrough in dom0less/hyperlaunch-ed scenario, while it is 
>> still
>> internal , it may be the only accept way to do device passthrough in
>> dom0less/hyperlaunch-ed scenario.
>>
>> Right, but no matter what your goals, the upstream contributions need to be 
>> self-
>> consistent. I.e. not (risk to) break other functionality. (Really the four 
>> domctl-s
>> mentioned above might better have been put elsewhere, e.g. as dm-ops. Moving
>> them may be an option here.)
> 
> Understood.
> I'll move them all to the dm-ops

Before you do so, please consider the consequences, though (I said "may" for a
reason). Also please allow others to chime in. (In this context I notice that
several REST maintainers weren't even Cc-ed here, and hence may not have seen
the earlier discussion.)

One thing seems pretty clear to me: This work likely isn't going to be suitable
for 4.21 anymore. Hence we're back to considering alternatives to address the
still pending build issue. (My take on it remains: Revert the tail of the
sysctl work.) Adding Oleksii to Cc as well.

Jan