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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]


On 9/26/25 9:14 AM, Jan Beulich wrote:
> On 26.09.2025 08:57, Penny, Zheng wrote:
>>> -----Original Message-----
>>> From: Jan Beulich<jbeulich@suse.com>
>>> Sent: Friday, September 26, 2025 2:53 PM
>>>
>>> On 26.09.2025 06:41, Penny, Zheng wrote:
>>>>> -----Original Message-----
>>>>> From: Jan Beulich<jbeulich@suse.com>
>>>>> Sent: Thursday, September 25, 2025 10:29 PM
>>>>>
>>>>> On 25.09.2025 11:41, Penny, Zheng wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Jan Beulich<jbeulich@suse.com>
>>>>>>> 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.

I agree, the patch series is still quite far from being ready to merge.
So let’s consider it for the next release.

As mentioned in the earlier (related) patch series, reverting the tail of the
sysctl work is still, in my opinion, the best option.

~ Oleksii

[-- Attachment #2: Type: text/html, Size: 6680 bytes --]