1. Patchew
  2. Xen
  3. View series

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

Michal Orzel posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230824090640.25338-1-michal.orzel@amd.com
xen/arch/arm/domain_build.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
[PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()
Posted by Michal Orzel 8 months, 2 weeks ago 
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"

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@amd.com>
---
 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;
-- 
2.25.1
Re: [PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()
Posted by Julien Grall 8 months, 2 weeks ago 
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@amd.com>

The change itself looks fine. So with the stack trace add:

Reviewed-by: Julien Grall <jgrall@amazon.com>

> ---
>   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
Re: [PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()
Posted by Michal Orzel 8 months, 2 weeks ago 
Hi Julien,

On 24/08/2023 11:10, Julien Grall wrote:
> 
> 
> 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().
Here you go:
(XEN) Xen call trace:
(XEN)    [<0000020000218568>] rangeset_remove_range+0xbc/0x2cc (PC)
(XEN)    [<00000200002c76bc>] domain_build.c#make_hypervisor_node+0x294/0x7c4 (LR)
(XEN)    [<00000200002ca240>] domain_build.c#handle_node+0x7ec/0x924
(XEN)    [<00000200002ca7ac>] domain_build.c#construct_dom0+0x434/0x4d8

Can you append this to the commit msg while committing or do you want a respin?

~Michal
Re: [PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()
Posted by Julien Grall 8 months, 2 weeks ago 

On 24/08/2023 10:17, Michal Orzel wrote:
> On 24/08/2023 11:10, Julien Grall wrote:
>> 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().
> Here you go:
> (XEN) Xen call trace:
> (XEN)    [<0000020000218568>] rangeset_remove_range+0xbc/0x2cc (PC)
> (XEN)    [<00000200002c76bc>] domain_build.c#make_hypervisor_node+0x294/0x7c4 (LR)
> (XEN)    [<00000200002ca240>] domain_build.c#handle_node+0x7ec/0x924
> (XEN)    [<00000200002ca7ac>] domain_build.c#construct_dom0+0x434/0x4d8
> 
> Can you append this to the commit msg while committing or do you want a respin?

Thanks. It can be done on commit.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Handle empty grant table region in find_unallocated_memory()
Posted by Julien Grall 7 months, 3 weeks ago 

On 24/08/2023 11:04, Julien Grall wrote:
> 
> 
> On 24/08/2023 10:17, Michal Orzel wrote:
>> On 24/08/2023 11:10, Julien Grall wrote:
>>> 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().
>> Here you go:
>> (XEN) Xen call trace:
>> (XEN)    [<0000020000218568>] rangeset_remove_range+0xbc/0x2cc (PC)
>> (XEN)    [<00000200002c76bc>] 
>> domain_build.c#make_hypervisor_node+0x294/0x7c4 (LR)
>> (XEN)    [<00000200002ca240>] domain_build.c#handle_node+0x7ec/0x924
>> (XEN)    [<00000200002ca7ac>] domain_build.c#construct_dom0+0x434/0x4d8
>>
>> Can you append this to the commit msg while committing or do you want 
>> a respin?
> 
> Thanks. It can be done on commit.

I have now committed the patch. Sorry for the delay.

Cheers,

-- 
Julien Grall

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.