Re: [PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 24/08/2023 10:06, Michal Orzel wrote:
> When creating dom0 with grant table support disabled in Xen and no IOMMU,
> the following assert is triggered (debug build):
> "Assertion 's <= e' failed at common/rangeset.c:189"
A partial stack trace would have been handy. This help the reader to 
understand how you came to the conclusion that the issue was in 
find_unallocated_memory().
> This is because find_unallocated_memory() (used to find memory holes
> for extended regions) calls rangeset_remove_range() for an empty
> grant table region. Fix it by checking if the size of region is not 0.
>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
The change itself looks fine. So with the stack trace add:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

> ---
>   xen/arch/arm/domain_build.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 54bf5623c889..2e899458acdf 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1633,14 +1633,18 @@ static int __init find_unallocated_memory(const struct 
> kernel_info *kinfo,
>       }
>   
>       /* Remove grant table region */
> -    start = kinfo->gnttab_start;
> -    end = kinfo->gnttab_start + kinfo->gnttab_size;
> -    res = rangeset_remove_range(unalloc_mem, PFN_DOWN(start), PFN_DOWN(end - 
> 1));
> -    if ( res )
> +    if ( kinfo->gnttab_size )
>       {
> -        printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n",
> -               start, end);
> -        goto out;
> +        start = kinfo->gnttab_start;
> +        end = kinfo->gnttab_start + kinfo->gnttab_size;
> +        res = rangeset_remove_range(unalloc_mem, PFN_DOWN(start),
> +                                    PFN_DOWN(end - 1));
> +        if ( res )
> +        {
> +            printk(XENLOG_ERR "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr"\n",
> +                   start, end);
> +            goto out;
> +        }
>       }
>   
>       start = 0;
Cheers,

--
Julien Grall