This patch adds a comment in create_domUs() right before
domain_create() to explain the importance of the pre-increment
operator on the variable max_init_domid, to ensure that the
domid 0 is allocated only during start_xen() function by the
create_dom0() and not on any other possible code path to the
domain_create() function.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes in v3:
- removed check introduced in v2.
---
 xen/arch/arm/domain_build.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 359957dc1b..b1d7b9849f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2508,6 +2508,11 @@ void __init create_domUs(void)
                                          GUEST_VPL011_SPI - 32 + 1);
         }
 
+        /*
+         * The variable max_init_domid is initialized with zero, so here it's
+         * very important to use the pre-increment operator to call
+         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
+         */
         d = domain_create(++max_init_domid, &d_cfg, false);
         if ( IS_ERR(d) )
             panic("Error creating domain %s\n", dt_node_name(node));
-- 
2.17.1

Hi Luca,

The title probably wants to be updated as you don't really reserve domid 
0. How about:

xen/arm: Clarify how the domid is decided in create_domUs()

On 12/04/2021 11:52, Luca Fancellu wrote:
> This patch adds a comment in create_domUs() right before
> domain_create() to explain the importance of the pre-increment
> operator on the variable max_init_domid, to ensure that the
> domid 0 is allocated only during start_xen() function by the
> create_dom0() and not on any other possible code path to the
> domain_create() function.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> Changes in v3:
> - removed check introduced in v2.
> ---
>   xen/arch/arm/domain_build.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 359957dc1b..b1d7b9849f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2508,6 +2508,11 @@ void __init create_domUs(void)
>                                            GUEST_VPL011_SPI - 32 + 1);
>           }
>   
> +        /*
> +         * The variable max_init_domid is initialized with zero, so here it's
> +         * very important to use the pre-increment operator to call
> +         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
> +         */
>           d = domain_create(++max_init_domid, &d_cfg, false);
>           if ( IS_ERR(d) )
>               panic("Error creating domain %s\n", dt_node_name(node));
> 

-- 
Julien Grall

> On 13 Apr 2021, at 18:00, Julien Grall <julien@xen.org> wrote:
> 
> Hi Luca,
> 
> The title probably wants to be updated as you don't really reserve domid 0. How about:
> 
> xen/arm: Clarify how the domid is decided in create_domUs()

Sure I’ll update in the v4 patch I will send soon

Cheers,
Luca

> 
> On 12/04/2021 11:52, Luca Fancellu wrote:
>> This patch adds a comment in create_domUs() right before
>> domain_create() to explain the importance of the pre-increment
>> operator on the variable max_init_domid, to ensure that the
>> domid 0 is allocated only during start_xen() function by the
>> create_dom0() and not on any other possible code path to the
>> domain_create() function.
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> Changes in v3:
>> - removed check introduced in v2.
>> ---
>>  xen/arch/arm/domain_build.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 359957dc1b..b1d7b9849f 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2508,6 +2508,11 @@ void __init create_domUs(void)
>>                                           GUEST_VPL011_SPI - 32 + 1);
>>          }
>>  +        /*
>> +         * The variable max_init_domid is initialized with zero, so here it's
>> +         * very important to use the pre-increment operator to call
>> +         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
>> +         */
>>          d = domain_create(++max_init_domid, &d_cfg, false);
>>          if ( IS_ERR(d) )
>>              panic("Error creating domain %s\n", dt_node_name(node));
> 
> -- 
> Julien Grall