Message ID | 20230824090640.25338-1-michal.orzel@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/arm: Handle empty grant table region in find_unallocated_memory() | expand |
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,
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
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,
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,
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;
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(-)