Re: [PATCH v6 5/6] arm/dom0less: assign dom0less guests to cpupools

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.04.2022 12:20, Luca Fancellu wrote:
> 
> 
>> On 11 Apr 2022, at 10:08, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 11.04.2022 10:54, Luca Fancellu wrote:
>>>> On 8 Apr 2022, at 13:10, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 08.04.2022 13:15, Luca Fancellu wrote:
>>>>>> On 8 Apr 2022, at 11:24, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>> On 08.04.2022 11:39, Luca Fancellu wrote:
>>>>>>>> On 8 Apr 2022, at 10:10, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>>>> On 08.04.2022 10:45, Luca Fancellu wrote:
>>>>>>>>> @@ -106,6 +106,8 @@ struct xen_domctl_createdomain {
>>>>>>>>> /* Per-vCPU buffer size in bytes. 0 to disable. */
>>>>>>>>> uint32_t vmtrace_size;
>>>>>>>>>
>>>>>>>>> + uint32_t cpupool_id;
>>>>>>>>
>>>>>>>> This could do with a comment explaining default behavior. In particular
>>>>>>>> I wonder what 0 means: Looking at cpupool_destroy() I can't see that it
>>>>>>>> would be impossible to delete pool 0 (but there may of course be
>>>>>>>> reasons elsewhere, e.g. preventing pool 0 to ever go empty) - Jürgen?
>>>>>>>> Yet if pool 0 can be removed, zero being passed in here should imo not
>>>>>>>> lead to failure of VM creation. Otoh I understand that this would
>>>>>>>> already happen ahead of your change, preventing of which would
>>>>>>>> apparently possible only via passing CPUPOOLID_NONE here.
>>>>>>>
>>>>>>> Pool-0 can’t be emptied because Dom0 is sitting there (the patch is 
>>>>>>> modifying
>>>>>>> cpupool_id only for DomUs).
>>>>>>
>>>>>> But we're talking about dom0less as per the subject of the patch here.
>>>>>
>>>>> Domains started using dom0less feature are not privileged and can’t do 
>>>>> any operation
>>>>> on cpu pools, that’s why I thought about Dom0.
>>>>
>>>> It's all a matter of XSM policy what a domain may or may not be able
>>>> to carry out.
>>>
>>> Yes you are right, however I didn’t see so far this use case with a domU 
>>> and the tool stack,
>>> probably because it would need also xenstore etc… I’m aware that there is 
>>> some work going
>>> on to enable it also for dom0less domUs, so my question is:
>>>
>>> Do you see this as a blocker for this patch? Are you ok if I send this 
>>> patch with just the comment
>>> below or in your opinion this patch requires some other work?
>>
>> Agreement looks to be that there should be precautionary code added
>> to prevent the deleting of pool 0. This imo wants to be a prereq
>> change to the one here.
> 
> Since we have the requirement of having cpu0 in pool-0, I’m thinking about a 
> check to don’t allow
> Cpu0 to be removed from pool-0, that will cover also the destroy case because 
> we can’t destroy
> a cpupool that is not empty.
> 
> In your opinion is it ok to proceed with a separate patch as prereq work 
> having this change?

Well, I did already say so (see context above).

Jan