This is preparatory work for the upcoming commits that implement the
standard PV time interface (ARM DEN 0057A).

No functional changes intended.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 xen/arch/arm/domain_build.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4ff161887ec3..9d44b6fa9470 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1070,6 +1070,23 @@ static int __init find_host_extended_regions(const struct kernel_info *kinfo,
     return res;
 }
 
+static int __init find_unused_regions(struct domain *d,
+                                      const struct kernel_info *kinfo,
+                                      struct membanks *ext_regions)
+{
+    if ( domain_use_host_layout(d) )
+    {
+        if ( !is_iommu_enabled(d) )
+            return find_host_extended_regions(kinfo, ext_regions);
+        else
+            return find_memory_holes(kinfo, ext_regions);
+    }
+    else
+    {
+        return find_domU_holes(kinfo, ext_regions);
+    }
+}
+
 int __init make_hypervisor_node(struct domain *d,
                                 const struct kernel_info *kinfo,
                                 int addrcells, int sizecells)
@@ -1121,17 +1138,7 @@ int __init make_hypervisor_node(struct domain *d,
         if ( !ext_regions )
             return -ENOMEM;
 
-        if ( domain_use_host_layout(d) )
-        {
-            if ( !is_iommu_enabled(d) )
-                res = find_host_extended_regions(kinfo, ext_regions);
-            else
-                res = find_memory_holes(kinfo, ext_regions);
-        }
-        else
-        {
-            res = find_domU_holes(kinfo, ext_regions);
-        }
+        res = find_unused_regions(d, kinfo, ext_regions);
 
         if ( res )
             printk(XENLOG_WARNING "%pd: failed to allocate extended regions\n",
-- 
2.48.1

On 21/06/2025 17:11, Koichiro Den wrote:
> This is preparatory work for the upcoming commits that implement the
> standard PV time interface (ARM DEN 0057A).
I personally don't find such messages useful. If at all, reasoning should be
given first and then this message could appear.

> 
> No functional changes intended.
> 
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
>  xen/arch/arm/domain_build.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4ff161887ec3..9d44b6fa9470 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1070,6 +1070,23 @@ static int __init find_host_extended_regions(const struct kernel_info *kinfo,
>      return res;
>  }
>  
> +static int __init find_unused_regions(struct domain *d,
> +                                      const struct kernel_info *kinfo,
Let's not propagate unjustified (IMO) behavior of passing both domain and kinfo
pointers. The latter contains the domain member and therefore is sufficient as
argument.

> +                                      struct membanks *ext_regions)
> +{
> +    if ( domain_use_host_layout(d) )
> +    {
> +        if ( !is_iommu_enabled(d) )
> +            return find_host_extended_regions(kinfo, ext_regions);
> +        else
> +            return find_memory_holes(kinfo, ext_regions);
> +    }
> +    else
> +    {
You can take opportunity to drop unneeded braces

Otherwise, LGTM.

~Michal

On Mon, Jun 23, 2025 at 03:33:55PM +0200, Orzel, Michal wrote:
> 

Thank you for the review + apologies for my delayed response.

> 
> On 21/06/2025 17:11, Koichiro Den wrote:
> > This is preparatory work for the upcoming commits that implement the
> > standard PV time interface (ARM DEN 0057A).
> I personally don't find such messages useful. If at all, reasoning should be
> given first and then this message could appear.

Right, I'll rewrite the commit message.

> 
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> >  xen/arch/arm/domain_build.c | 29 ++++++++++++++++++-----------
> >  1 file changed, 18 insertions(+), 11 deletions(-)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 4ff161887ec3..9d44b6fa9470 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1070,6 +1070,23 @@ static int __init find_host_extended_regions(const struct kernel_info *kinfo,
> >      return res;
> >  }
> >  
> > +static int __init find_unused_regions(struct domain *d,
> > +                                      const struct kernel_info *kinfo,
> Let's not propagate unjustified (IMO) behavior of passing both domain and kinfo
> pointers. The latter contains the domain member and therefore is sufficient as
> argument.

Thanks for pointing that out, I'll do so in the next take.

> 
> > +                                      struct membanks *ext_regions)
> > +{
> > +    if ( domain_use_host_layout(d) )
> > +    {
> > +        if ( !is_iommu_enabled(d) )
> > +            return find_host_extended_regions(kinfo, ext_regions);
> > +        else
> > +            return find_memory_holes(kinfo, ext_regions);
> > +    }
> > +    else
> > +    {
> You can take opportunity to drop unneeded braces

Will update this part in v2. Thank you!

> 
> Otherwise, LGTM.
> 
> ~Michal
>