Re: [PATCH 07/15] xen/overlay: Enable device tree overlay assignment to running domains

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.04.2024 05:36, Henry Wang wrote:
> Hi Jan, Julien, Stefano,
> 
> On 4/24/2024 2:05 PM, Jan Beulich wrote:
>> On 24.04.2024 05:34, Henry Wang wrote:
>>> --- a/xen/include/public/sysctl.h
>>> +++ b/xen/include/public/sysctl.h
>>> @@ -1197,7 +1197,9 @@ struct xen_sysctl_dt_overlay {
>>>   #define XEN_SYSCTL_DT_OVERLAY_ADD                   1
>>>   #define XEN_SYSCTL_DT_OVERLAY_REMOVE                2
>>>       uint8_t overlay_op;                     /* IN: Add or remove. */
>>> -    uint8_t pad[3];                         /* IN: Must be zero. */
>>> +    bool domain_mapping;                    /* IN: True of False. */
>>> +    uint8_t pad[2];                         /* IN: Must be zero. */
>>> +    uint32_t domain_id;
>>>   };
>> If you merely re-purposed padding fields, all would be fine without
>> bumping the interface version. Yet you don't, albeit for an unclear
>> reason: Why uint32_t rather than domid_t? And on top of that - why a
>> separate boolean when you could use e.g. DOMID_INVALID to indicate
>> "no domain mapping"?
> 
> I think both of your suggestion make great sense. I will follow the 
> suggestion in v2.
> 
>> That said - anything taking a domain ID is certainly suspicious in a
>> sysctl. Judging from the description you really mean this to be a
>> domctl. Anything else will require extra justification.
> 
> I also think a domctl is better. I had a look at the history of the 
> already merged series, it looks like in the first version of merged part 
> 1 [1], the hypercall was implemented as the domctl in the beginning but 
> later in v2 changed to sysctl. I think this makes sense as the scope of 
> that time is just to make Xen aware of the device tree node via Xen 
> device tree.
> 
> However this is now a problem for the current part where the scope (and 
> the end goal) is extended to assign the added device to Linux Dom0/DomU 
> via device tree overlays. I am not sure which way is better, should we 
> repurposing the sysctl to domctl or maybe add another domctl (I am 
> worrying about the duplication because basically we need the same sysctl 
> functionality but now with a domid in it)? What do you think?

I'm taking it that what is a sysctl right now legitimately is. Therefore
folding both into domctl would at least be bending the rules of what
should be sysctl and what domctl. It would need clarifying what (fake)
domain such a (folded) domctl ought to operate on for the case that's
currently a sysctl. That then may (or may not) be justification for such
folding.

Jan

> @Stefano: Since I am not 100% if I understand the whole story behind 
> this feature, would you mind checking if I am providing correct 
> information above and sharing your opinions on this? Thank you very much!
> 
> [1] 
> https://lore.kernel.org/xen-devel/13240b69-f7bb-6a64-b89c-b7c2cbb7e465@xxxxxxx/
> 
> Kind regards,
> Henry
> 
>> Jan
>