[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v11 03/12] xen/arm: permit non direct-mapped Dom0 construction



On Fri, Dec 13, 2024 at 11:56 AM Michal Orzel <michal.orzel@xxxxxxx> wrote:
>
>
>
> On 13/12/2024 11:26, Carlo Nonato wrote:
> >
> >
> > Hi Michal,
> >
> > On Fri, Dec 13, 2024 at 10:46 AM Michal Orzel <michal.orzel@xxxxxxx> wrote:
> >>
> >> Hi Carlo, Andrea,
> >>
> >> On 12/12/2024 19:22, Andrea Bastoni wrote:
> >>>
> >>>
> >>> On 12/12/2024 18:48, Carlo Nonato wrote:
> >>>> Hi,
> >>>>
> >>>> On Mon, Dec 9, 2024 at 8:17 PM Julien Grall <julien@xxxxxxx> wrote:
> >>>>>
> >>>>> Hi Michal,
> >>>>>
> >>>>> On 07/12/2024 15:04, Michal Orzel wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 06/12/2024 19:37, Julien Grall wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> Sorry for the late answer.
> >>>>>>>
> >>>>>>> On 05/12/2024 09:40, Michal Orzel wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 02/12/2024 17:59, Carlo Nonato wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Cache coloring requires Dom0 not to be direct-mapped because of its 
> >>>>>>>>> non
> >>>>>>>>> contiguous mapping nature, so allocate_memory() is needed in this 
> >>>>>>>>> case.
> >>>>>>>>> 8d2c3ab18cc1 ("arm/dom0less: put dom0less feature code in a 
> >>>>>>>>> separate module")
> >>>>>>>>> moved allocate_memory() in dom0less_build.c. In order to use it
> >>>>>>>>> in Dom0 construction bring it back to domain_build.c and declare it 
> >>>>>>>>> in
> >>>>>>>>> domain_build.h.
> >>>>>>>>>
> >>>>>>>>> Take the opportunity to adapt the implementation of 
> >>>>>>>>> allocate_memory() so
> >>>>>>>>> that it uses the host layout when called on the hwdom, via
> >>>>>>>>> find_unallocated_memory().
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
> >>>>>>>>> ---
> >>>>>>>>> v11:
> >>>>>>>>> - GUEST_RAM_BANKS instead of hardcoding the number of banks in 
> >>>>>>>>> allocate_memory()
> >>>>>>>>> - hwdom_ext_regions -> hwdom_free_mem in allocate_memory()
> >>>>>>>>> - added a comment in allocate_memory() when skipping small banks
> >>>>>>>>> v10:
> >>>>>>>>> - fixed a compilation bug that happened when dom0less support was 
> >>>>>>>>> disabled
> >>>>>>>>> v9:
> >>>>>>>>> - no changes
> >>>>>>>>> v8:
> >>>>>>>>> - patch adapted to new changes to allocate_memory()
> >>>>>>>>> v7:
> >>>>>>>>> - allocate_memory() now uses the host layout when called on the 
> >>>>>>>>> hwdom
> >>>>>>>>> v6:
> >>>>>>>>> - new patch
> >>>>>>>>> ---
> >>>>>>>>>    xen/arch/arm/dom0less-build.c           | 44 -----------
> >>>>>>>>>    xen/arch/arm/domain_build.c             | 97 
> >>>>>>>>> ++++++++++++++++++++++++-
> >>>>>>>>>    xen/arch/arm/include/asm/domain_build.h |  1 +
> >>>>>>>>>    3 files changed, 94 insertions(+), 48 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/xen/arch/arm/dom0less-build.c 
> >>>>>>>>> b/xen/arch/arm/dom0less-build.c
> >>>>>>>>> index d93a85434e..67b1503647 100644
> >>>>>>>>> --- a/xen/arch/arm/dom0less-build.c
> >>>>>>>>> +++ b/xen/arch/arm/dom0less-build.c
> >>>>>>>>> @@ -49,50 +49,6 @@ bool __init is_dom0less_mode(void)
> >>>>>>>>>        return ( !dom0found && domUfound );
> >>>>>>>>>    }
> >>>>>>>>>
> >>>>>>>>> -static void __init allocate_memory(struct domain *d, struct 
> >>>>>>>>> kernel_info *kinfo)
> >>>>>>>>> -{
> >>>>>>>>> -    struct membanks *mem = kernel_info_get_mem(kinfo);
> >>>>>>>>> -    unsigned int i;
> >>>>>>>>> -    paddr_t bank_size;
> >>>>>>>>> -
> >>>>>>>>> -    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for 
> >>>>>>>>> %pd:\n",
> >>>>>>>>> -           /* Don't want format this as PRIpaddr (16 digit hex) */
> >>>>>>>>> -           (unsigned long)(kinfo->unassigned_mem >> 20), d);
> >>>>>>>>> -
> >>>>>>>>> -    mem->nr_banks = 0;
> >>>>>>>>> -    bank_size = MIN(GUEST_RAM0_SIZE, kinfo->unassigned_mem);
> >>>>>>>>> -    if ( !allocate_bank_memory(kinfo, 
> >>>>>>>>> gaddr_to_gfn(GUEST_RAM0_BASE),
> >>>>>>>>> -                               bank_size) )
> >>>>>>>>> -        goto fail;
> >>>>>>>>> -
> >>>>>>>>> -    bank_size = MIN(GUEST_RAM1_SIZE, kinfo->unassigned_mem);
> >>>>>>>>> -    if ( !allocate_bank_memory(kinfo, 
> >>>>>>>>> gaddr_to_gfn(GUEST_RAM1_BASE),
> >>>>>>>>> -                               bank_size) )
> >>>>>>>>> -        goto fail;
> >>>>>>>>> -
> >>>>>>>>> -    if ( kinfo->unassigned_mem )
> >>>>>>>>> -        goto fail;
> >>>>>>>>> -
> >>>>>>>>> -    for( i = 0; i < mem->nr_banks; i++ )
> >>>>>>>>> -    {
> >>>>>>>>> -        printk(XENLOG_INFO "%pd BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" 
> >>>>>>>>> (%ldMB)\n",
> >>>>>>>>> -               d,
> >>>>>>>>> -               i,
> >>>>>>>>> -               mem->bank[i].start,
> >>>>>>>>> -               mem->bank[i].start + mem->bank[i].size,
> >>>>>>>>> -               /* Don't want format this as PRIpaddr (16 digit 
> >>>>>>>>> hex) */
> >>>>>>>>> -               (unsigned long)(mem->bank[i].size >> 20));
> >>>>>>>>> -    }
> >>>>>>>>> -
> >>>>>>>>> -    return;
> >>>>>>>>> -
> >>>>>>>>> -fail:
> >>>>>>>>> -    panic("Failed to allocate requested domain memory."
> >>>>>>>>> -          /* Don't want format this as PRIpaddr (16 digit hex) */
> >>>>>>>>> -          " %ldKB unallocated. Fix the VMs configurations.\n",
> >>>>>>>>> -          (unsigned long)kinfo->unassigned_mem >> 10);
> >>>>>>>>> -}
> >>>>>>>>> -
> >>>>>>>>>    #ifdef CONFIG_VGICV2
> >>>>>>>>>    static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
> >>>>>>>>>    {
> >>>>>>>>> diff --git a/xen/arch/arm/domain_build.c 
> >>>>>>>>> b/xen/arch/arm/domain_build.c
> >>>>>>>>> index 2c30792de8..2b8cba9b2f 100644
> >>>>>>>>> --- a/xen/arch/arm/domain_build.c
> >>>>>>>>> +++ b/xen/arch/arm/domain_build.c
> >>>>>>>>> @@ -416,7 +416,6 @@ static void __init allocate_memory_11(struct 
> >>>>>>>>> domain *d,
> >>>>>>>>>        }
> >>>>>>>>>    }
> >>>>>>>>>
> >>>>>>>>> -#ifdef CONFIG_DOM0LESS_BOOT
> >>>>>>>>>    bool __init allocate_domheap_memory(struct domain *d, paddr_t 
> >>>>>>>>> tot_size,
> >>>>>>>>>                                        alloc_domheap_mem_cb cb, 
> >>>>>>>>> void *extra)
> >>>>>>>>>    {
> >>>>>>>>> @@ -508,7 +507,6 @@ bool __init allocate_bank_memory(struct 
> >>>>>>>>> kernel_info *kinfo, gfn_t sgfn,
> >>>>>>>>>
> >>>>>>>>>        return true;
> >>>>>>>>>    }
> >>>>>>>>> -#endif
> >>>>>>>>>
> >>>>>>>>>    /*
> >>>>>>>>>     * When PCI passthrough is available we want to keep the
> >>>>>>>>> @@ -1003,6 +1001,94 @@ out:
> >>>>>>>>>        return res;
> >>>>>>>>>    }
> >>>>>>>>>
> >>>>>>>>> +void __init allocate_memory(struct domain *d, struct kernel_info 
> >>>>>>>>> *kinfo)
> >>>>>>>>> +{
> >>>>>>>>> +    struct membanks *mem = kernel_info_get_mem(kinfo);
> >>>>>>>>> +    unsigned int i, nr_banks = GUEST_RAM_BANKS;
> >>>>>>>>> +    paddr_t bank_start, bank_size;
> >>>>>>>> Limit the scope
> >>>>>>>>
> >>>>>>>>> +    struct membanks *hwdom_free_mem = NULL;
> >>>>>>>>> +    const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
> >>>>>>>>> +    const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
> >>>>>>>> Limit the scope
> >>>>>>>>
> >>>>>>>>> +
> >>>>>>>>> +    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for 
> >>>>>>>>> %pd:\n",
> >>>>>>>>> +           /* Don't want format this as PRIpaddr (16 digit hex) */
> >>>>>>>>> +           (unsigned long)(kinfo->unassigned_mem >> 20), d);
> >>>>>>>>> +
> >>>>>>>>> +    mem->nr_banks = 0;
> >>>>>>>>> +    /*
> >>>>>>>>> +     * Use host memory layout for hwdom. Only case for this is 
> >>>>>>>>> when LLC coloring
> >>>>>>>>> +     * is enabled.
> >>>>>>>>> +     */
> >>>>>>>>> +    if ( is_hardware_domain(d) )
> >>>>>>>>> +    {
> >>>>>>>>> +        ASSERT(llc_coloring_enabled);
> >>>>>>>> This patch does not build because of declaration not being visible. 
> >>>>>>>> You must include <xen/llc-coloring.h>.
> >>>>>>>
> >>>>>>> Piggying back on this comment. AFAICT, the code below would work also 
> >>>>>>> in
> >>>>>>> the non cache coloring case. So what's the assert is for?
> >>>>>>>
> >>>>>>>>
> >>>>>>>>> +
> >>>>>>>>> +        hwdom_free_mem = xzalloc_flex_struct(struct membanks, bank,
> >>>>>>>>> +                                             NR_MEM_BANKS);
> >>>>>>>>> +        if ( !hwdom_free_mem )
> >>>>>>>>> +            goto fail;
> >>>>>>>>> +
> >>>>>>>>> +        hwdom_free_mem->max_banks = NR_MEM_BANKS;
> >>>>>>>>> +
> >>>>>>>>> +        if ( find_unallocated_memory(kinfo, hwdom_free_mem) )
> >>>>>>>> My remarks for the use of find_unallocated_memory() 1:1 have not 
> >>>>>>>> been addressed. You did not even
> >>>>>>>> change the comments inside the function. The problem is that the 
> >>>>>>>> function is specifically designed
> >>>>>>>> for finding extended regions and assumes being called at certain 
> >>>>>>>> point i.e. dom0 RAM allocated, gnttab
> >>>>>>>> region allocated, etc.
> >>>>>>>
> >>>>>>> So I agree that the function should be updated if we plan to use it 
> >>>>>>> for
> >>>>>>> other purpose.
> >>>>>>>
> >>>>>>> My opinion is that we should attempt to make the function generic so
> >>>>>>> that in your
> >>>>>>>> case you can choose which regions to exclude, define even your own 
> >>>>>>>> function to grab free regions (at the moment
> >>>>>>>> add_ext_regions grabs banks >= 64M but you still discards banks >= 
> >>>>>>>> 128M, so it's a bit wasteful.
> >>>>>>>>
> >>>>>>>> My very short attempt to make the function as generic as possible in 
> >>>>>>>> the first iteration:
> >>>>>>>> https://paste.debian.net/1338334/
> >>>>>>>
> >>>>>>> This looks better, but I wonder why we need still need to exclude the
> >>>>>>> static regions? Wouldn't it be sufficient to exclude just reserved 
> >>>>>>> regions?
> >>>>>> Static shared memory banks are not part of reserved memory (i.e. 
> >>>>>> bootinfo.reserved_mem) if that's what you're asking.
> >>>>>> They are stored in bootinfo.shmem, hence we need to take them into 
> >>>>>> account when searching for unused address space.
> >>>>>
> >>>>> Oh I missed the fact you now pass "mem_banks" as a parameter. I thought
> >>>>> they would still get excluded for cache coloring case.
> >>>>>
> >>>>>>
> >>>>>> If you and Carlo are ok with my proposed solution for making the 
> >>>>>> function generic, I can send a patch as a prerequisite
> >>>>>> patch for Carlo series.
> >>>>>
> >>>>> I am fine with the approach.
> >>>>>
> >>>>> Cheers,
> >>>>>
> >>>>> --
> >>>>> Julien Grall
> >>>>>
> >>>>
> >>>>> @@ -2152,7 +2238,10 @@ static int __init construct_dom0(struct domain 
> >>>>> *d)
> >>>>>      /* type must be set before allocate_memory */
> >>>>>      d->arch.type = kinfo.type;
> >>>>>  #endif
> >>>>> -    allocate_memory_11(d, &kinfo);
> >>>>> +    if ( is_domain_direct_mapped(d) )
> >>>>> +        allocate_memory_11(d, &kinfo);
> >>>>> +    else
> >>>>> +        allocate_memory(d, &kinfo);
> >>>>>      find_gnttab_region(d, &kinfo);
> >>>>
> >>>> Since find_gnttab_region() is called after allocate_memory(), 
> >>>> kinfo->gnttab_*
> >>>> fields aren't initialized and the call to find_unallocated_memory() with
> >>>> gnttab as the region to exclude, fails ending in a crash since memory for
> >>>> dom0 can't be allocated.
> >>>>
> >>>> Can the solution be to call find_gnttab_region() before the above if?
> >>>
> >>> The function is called find, but currently it only initializes 
> >>> kinfo->gnttab_start
> >>> and kinfo->gnttab_size and we tested that moving it before 
> >>> allocate_memory* doesn't
> >>> cause fallouts.
> >>>
> >>> If moving before allocate_memory*, would it be better to rename it e.g., 
> >>> init_gnttab_region()?
> >>>
> >>> Thanks,
> >>> Andrea
> >>>
> >>>> Or should I just call it before allocate_memory() in one case, but still 
> >>>> after
> >>>> allocate_memory_11() in the other?
> >>>>
> >>>> Thanks.
> >>>
> >>
> >> AFAICT there is nothing stopping us from moving find_gnttab_region() 
> >> before allocate_*. This function initializes
> >> gnttab region with PA of Xen. In normal case, because Xen is added as 
> >> bootmodule, it will never be mapped in dom0 memory map
> >> and the placement does not matter. In LLC case, it will point to relocated 
> >> address of Xen and it needs to be known before
> >> calling find_unallocated_memory. Don't rename it, leave as is, just move 
> >> before allocate_*.
> >>
> >> @Carlo:
> >> My prerequisite patch has been merged, so you're good to respin a series 
> >> (unless you wait for some feedback in which case do let me know).
> >> To prevent too many respins, you're going to call find_unallocated_memory 
> >> for LLC passing resmem and gnttab to be excluded. If you're going
> >> to reuse add_ext_regions, you need to rename it and fix comments to make 
> >> it more generic. As for the size, the decision is yours. One solution
> >> would be to modify add_ext_regions to take min bank size as parameter 
> >> (64MB for extended regions, X for LLC dom0). In your code, you write that
> >> the first bank must contain dom0, dtb, ramdisk and you chose 128MB. 
> >> However, looking at the code, you seem to discard banks < 128 for all the 
> >> banks,
> >> not only for the first one. This is the part that I don't have a ready 
> >> solution. Maybe you could define your own add_free_region function and sort
> >> the banks, so that you take the largest possible bank first for dom0. This 
> >> could simplify things.
> >
> > For the moment I added a __add_ext_regions() helper that also takes a 
> > skip_size
> I'm not sure if MISRA and our guidelines are happy with prefixing with 
> function with __.
> I don't understand the skip_size parameter. In which scenario do you want to 
> use it? Not for
> extended regions and for LLC, even with your current solution, you also want 
> to find banks bigger than
> some size.
>
> > parameter. This is called by add_ext_regions() and by a new
> > add_hwdom_free_regions() callback used in allocate_memory().
> > I still use 128MB for all the banks. Do you think this is acceptable, maybe
> > with a FIXME comment cause we should skip only the first bank, or not?
> First of all, I'm not convinced with 128MB. This is definitely not a 
> requirement for arm64.
> allocate_memory_11 uses it but the algorithm of finding banks is completely 
> different.
>
> AFAICT, with my suggested solution i.e. sorting banks in a helper like 
> add_ext_regions used only
> for LLC case, you no longer need to worry about size. You simply start with 
> the biggest possible bank
> as the first bank.
>
> ~Michal
>

Here's my current patch:

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index d93a85434e..67b1503647 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -49,50 +49,6 @@ bool __init is_dom0less_mode(void)
     return ( !dom0found && domUfound );
 }

-static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
-{
-    struct membanks *mem = kernel_info_get_mem(kinfo);
-    unsigned int i;
-    paddr_t bank_size;
-
-    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for %pd:\n",
-           /* Don't want format this as PRIpaddr (16 digit hex) */
-           (unsigned long)(kinfo->unassigned_mem >> 20), d);
-
-    mem->nr_banks = 0;
-    bank_size = MIN(GUEST_RAM0_SIZE, kinfo->unassigned_mem);
-    if ( !allocate_bank_memory(kinfo, gaddr_to_gfn(GUEST_RAM0_BASE),
-                               bank_size) )
-        goto fail;
-
-    bank_size = MIN(GUEST_RAM1_SIZE, kinfo->unassigned_mem);
-    if ( !allocate_bank_memory(kinfo, gaddr_to_gfn(GUEST_RAM1_BASE),
-                               bank_size) )
-        goto fail;
-
-    if ( kinfo->unassigned_mem )
-        goto fail;
-
-    for( i = 0; i < mem->nr_banks; i++ )
-    {
-        printk(XENLOG_INFO "%pd BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" (%ldMB)\n",
-               d,
-               i,
-               mem->bank[i].start,
-               mem->bank[i].start + mem->bank[i].size,
-               /* Don't want format this as PRIpaddr (16 digit hex) */
-               (unsigned long)(mem->bank[i].size >> 20));
-    }
-
-    return;
-
-fail:
-    panic("Failed to allocate requested domain memory."
-          /* Don't want format this as PRIpaddr (16 digit hex) */
-          " %ldKB unallocated. Fix the VMs configurations.\n",
-          (unsigned long)kinfo->unassigned_mem >> 10);
-}
-
 #ifdef CONFIG_VGICV2
 static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
 {
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index adf26f2778..59ac45c4e0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2,6 +2,7 @@
 #include <xen/init.h>
 #include <xen/compile.h>
 #include <xen/lib.h>
+#include <xen/llc-coloring.h>
 #include <xen/mm.h>
 #include <xen/param.h>
 #include <xen/domain_page.h>
@@ -416,7 +417,6 @@ static void __init allocate_memory_11(struct domain *d,
     }
 }

-#ifdef CONFIG_DOM0LESS_BOOT
 bool __init allocate_domheap_memory(struct domain *d, paddr_t tot_size,
                                     alloc_domheap_mem_cb cb, void *extra)
 {
@@ -508,7 +508,6 @@ bool __init allocate_bank_memory(struct
kernel_info *kinfo, gfn_t sgfn,

     return true;
 }
-#endif

 /*
  * When PCI passthrough is available we want to keep the
@@ -859,8 +858,8 @@ int __init make_memory_node(const struct
kernel_info *kinfo, int addrcells,
     return res;
 }

-int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
-                           void *data)
+static int __init __add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
+                                    void *data, paddr_t skip_size)
 {
     struct membanks *ext_regions = data;
     paddr_t start, size;
@@ -885,12 +884,7 @@ int __init add_ext_regions(unsigned long s_gfn,
unsigned long e_gfn,
     e += 1;
     size = (e - start) & ~(SZ_2M - 1);

-    /*
-     * Reasonable size. Not too little to pick up small ranges which are
-     * not quite useful but increase bookkeeping and not too large
-     * to skip a large proportion of unused address space.
-     */
-    if ( size < MB(64) )
+    if ( size < skip_size )
         return 0;

     ext_regions->bank[ext_regions->nr_banks].start = start;
@@ -900,6 +894,28 @@ int __init add_ext_regions(unsigned long s_gfn,
unsigned long e_gfn,
     return 0;
 }

+static int __init add_hwdom_free_regions(unsigned long s_gfn,
+                                         unsigned long e_gfn, void *data)
+{
+    /*
+     * Skip banks that are too small. The first bank must contain dom0 kernel +
+     * ramdisk + dtb and 128 MB is the same limit used in allocate_memory_11().
+     */
+    return __add_ext_regions(s_gfn, e_gfn, data, MB(128));
+}
+
+
+int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
+                           void *data)
+{
+    /*
+     * Reasonable size. Not too little to pick up small ranges which are
+     * not quite useful but increase bookkeeping and not too large
+     * to skip a large proportion of unused address space.
+     */
+    return __add_ext_regions(s_gfn, e_gfn, data, MB(64));
+}
+
 /*
  * Find unused regions of Host address space which can be exposed to domain
  * using the host memory layout. In order to calculate regions we exclude every
@@ -977,6 +993,109 @@ out:
     return res;
 }

+void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
+{
+    struct membanks *mem = kernel_info_get_mem(kinfo);
+    unsigned int i, nr_banks = GUEST_RAM_BANKS;
+    struct membanks *hwdom_free_mem = NULL;
+
+    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for %pd:\n",
+           /* Don't want format this as PRIpaddr (16 digit hex) */
+           (unsigned long)(kinfo->unassigned_mem >> 20), d);
+
+    mem->nr_banks = 0;
+    /*
+     * Use host memory layout for hwdom. Only case for this is when
LLC coloring
+     * is enabled.
+     */
+    if ( is_hardware_domain(d) )
+    {
+        struct membanks *gnttab = xzalloc_flex_struct(struct
membanks, bank, 1);
+        /*
+         * Exclude the following regions:
+         * 1) Remove reserved memory
+         * 2) Grant table assigned to Dom0
+         */
+        const struct membanks *mem_banks[] = {
+            bootinfo_get_reserved_mem(),
+            gnttab,
+        };
+
+        ASSERT(llc_coloring_enabled);
+
+        if ( !gnttab )
+            goto fail;
+
+        gnttab->nr_banks = 1;
+        gnttab->bank[0].start = kinfo->gnttab_start;
+        gnttab->bank[0].size = kinfo->gnttab_start + kinfo->gnttab_size;
+
+        hwdom_free_mem = xzalloc_flex_struct(struct membanks, bank,
+                                             NR_MEM_BANKS);
+        if ( !hwdom_free_mem )
+            goto fail;
+
+        hwdom_free_mem->max_banks = NR_MEM_BANKS;
+
+        if ( find_unallocated_memory(kinfo, mem_banks, ARRAY_SIZE(mem_banks),
+                                     hwdom_free_mem, add_hwdom_free_regions) )
+            goto fail;
+
+        nr_banks = hwdom_free_mem->nr_banks;
+        xfree(gnttab);
+    }
+
+    for ( i = 0; kinfo->unassigned_mem > 0 && nr_banks > 0; i++, nr_banks-- )
+    {
+        paddr_t bank_start, bank_size;
+
+        if ( is_hardware_domain(d) )
+        {
+            bank_start = hwdom_free_mem->bank[i].start;
+            bank_size = hwdom_free_mem->bank[i].size;
+            ASSERT(bank_size >= MB(128));
+        }
+        else
+        {
+            const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
+            const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
+
+            if ( i >= GUEST_RAM_BANKS )
+                goto fail;
+
+            bank_start = bankbase[i];
+            bank_size = banksize[i];
+        }
+
+        bank_size = MIN(bank_size, kinfo->unassigned_mem);
+        if ( !allocate_bank_memory(kinfo, gaddr_to_gfn(bank_start),
bank_size) )
+            goto fail;
+    }
+
+    if ( kinfo->unassigned_mem )
+        goto fail;
+
+    for( i = 0; i < mem->nr_banks; i++ )
+    {
+        printk(XENLOG_INFO "%pd BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" (%ldMB)\n",
+               d,
+               i,
+               mem->bank[i].start,
+               mem->bank[i].start + mem->bank[i].size,
+               /* Don't want format this as PRIpaddr (16 digit hex) */
+               (unsigned long)(mem->bank[i].size >> 20));
+    }
+
+    xfree(hwdom_free_mem);
+    return;
+
+fail:
+    panic("Failed to allocate requested domain memory."
+          /* Don't want format this as PRIpaddr (16 digit hex) */
+          " %ldKB unallocated. Fix the VMs configurations.\n",
+          (unsigned long)kinfo->unassigned_mem >> 10);
+}
+
 static int __init handle_pci_range(const struct dt_device_node *dev,
                                    uint64_t addr, uint64_t len, void *data)
 {
@@ -1235,7 +1354,7 @@ int __init make_hypervisor_node(struct domain *d,

         ext_regions->max_banks = NR_MEM_BANKS;

-        if ( is_domain_direct_mapped(d) )
+        if ( domain_use_host_layout(d) )
         {
             if ( !is_iommu_enabled(d) )
                 res = find_host_extended_regions(kinfo, ext_regions);
@@ -2164,8 +2283,11 @@ static int __init construct_dom0(struct domain *d)
     /* type must be set before allocate_memory */
     d->arch.type = kinfo.type;
 #endif
-    allocate_memory_11(d, &kinfo);
     find_gnttab_region(d, &kinfo);
+    if ( is_domain_direct_mapped(d) )
+        allocate_memory_11(d, &kinfo);
+    else
+        allocate_memory(d, &kinfo);

     rc = process_shm_chosen(d, &kinfo);
     if ( rc < 0 )
diff --git a/xen/arch/arm/include/asm/domain_build.h
b/xen/arch/arm/include/asm/domain_build.h
index e712afbc7f..b0d646e173 100644
--- a/xen/arch/arm/include/asm/domain_build.h
+++ b/xen/arch/arm/include/asm/domain_build.h
@@ -11,6 +11,7 @@ bool allocate_domheap_memory(struct domain *d,
paddr_t tot_size,
                              alloc_domheap_mem_cb cb, void *extra);
 bool allocate_bank_memory(struct kernel_info *kinfo, gfn_t sgfn,
                           paddr_t tot_size);
+void allocate_memory(struct domain *d, struct kernel_info *kinfo);
 int construct_domain(struct domain *d, struct kernel_info *kinfo);
 int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
 int make_chosen_node(const struct kernel_info *kinfo);
@@ -54,6 +55,9 @@ static inline int prepare_acpi(struct domain *d,
struct kernel_info *kinfo)
 int prepare_acpi(struct domain *d, struct kernel_info *kinfo);
 #endif

+typedef int (*add_free_regions_fn)(unsigned long s_gfn, unsigned long e_gfn,
+                                   void *data);
+
 int add_ext_regions(unsigned long s_gfn, unsigned long e_gfn, void *data);

 #endif

skip_size can be renamed to threshold_size to make it more clear.

Anyway I'm not following you on the suggested solution: when should I sort the
banks, how can I do it in the callback of find_unallocated_memory() and
what if the first biggest bank is lower than 128MB, I should not care for that?

Thanks.

- Carlo



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.