Hi MIchal,
> On 28 Feb 2023, at 12:10, Michal Orzel <michal.or...@amd.com> wrote:
>
> Hi Bertrand,
>
> On 28/02/2023 09:08, Bertrand Marquis wrote:
>>
>>
>> Before trying to create a dom0less guest, check that max_init_domid
>> increment will generate a valid domain ID, lower than
>> DOMID_FIRST_RESERVED.
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
>> ---
>> xen/arch/arm/domain_build.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index edca23b986d2..9707eb7b1bb1 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -3879,6 +3879,9 @@ void __init create_domUs(void)
>> if ( !dt_device_is_compatible(node, "xen,domain") )
>> continue;
>>
>> + if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
>> + panic("No more domain IDs available\n");
> Here are some of my thoughts:
> 1. The check if domid is >= DOMID_FIRST_RESERVED is used in quite a lot of
> places in the Xen code. We might want to introduce a global function for that
> purpose
> instead of repeating this check all over the codebase.
We could introduce something but looking at the code i think that the first
thing to do
would be to use is_system_domain where possible (ie when there is a domain
structure)
and then cleanup a bit domctl.c where there are some double check of
DOMID_FIRST_RESERVED (in the hypercall code and in is_free_domid).
Once that is done, there would be a lot less usage of this.
>
> 2. This check is something that could be moved to be generic. At the moment
> we do have
> an ASSERT with is_system_domain in domain_create. I know domain_create can be
> called for
> domids in special range so this would need to be thought through.
I do not think that domain_create is the right place to have this check as it
is correct to call it to
create system domains.
>
> 3. The placement of this check at the top of the function before starting to
> parse dt properties
> might be problematic in the future if we decide to allow specifying static
> domids for dom0less domUs.
> In a static configuration, most of the time, we do not have xenstore (either
> because of lack of xenstore
> support or because of lack of dom0). AFAIKT, in Xen a domain can get to know
> its domid only through xenstore
> (DOMID_SELF is not working in all the cases). Also, in a static
> configuration, it makes the life of an integrator
> easy to know all the domids upfront to easily set up some communication,
> grant tables, etc.
Right now the idea is to fail as early as possible to prevent doing any
operation that is not needed.
Having a way to statically define the dom id in configuration does make sense
and the check will have
to be modified once the support for this will be added.
>
> Let me know your thoughts.
There is some improvement possible in the overall code but the goal here is
just to solve a possible issue so this
patch could be merged and other changes could be done in a following patch if
wanted.
Cheers
Bertrand
>
> ~Michal
