Re: [PATCH V4 03/10] xen/arm: handle static memory in dt_unreserved_regions

[Julien Grall <julien@xxxxxxx>]

Hi Penny,

On 28/07/2021 11:27, Penny Zheng wrote:
> static memory regions overlap with memory nodes. The
> overlapping memory is reserved-memory and should be
> handled accordingly:
> dt_unreserved_regions should skip these regions the
> same way they are already skipping mem-reserved regions.
>
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
>   xen/arch/arm/setup.c | 47 ++++++++++++++++++++++++++++----------------
>   1 file changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 63a908e325..f569134317 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -200,6 +200,13 @@ static void __init dt_unreserved_regions(paddr_t s, 
> paddr_t e,
>                                            int first)
>   {
>       int i, nr = fdt_num_mem_rsv(device_tree_flattened);
> +    /*
> +     * There are two types of reserved memory stored in bootinfo, one defines
> +     * in /reserved-memory node, the other refers to domain on static 
> allocation
> +     * through "xen,static-mem" property.
> +     */
> +    int nr_rsv_type = 2, t = 0, prev_nr;
> +    struct meminfo *rsv_type[2] = {&bootinfo.reserved_mem, 
> &bootinfo.static_mem};
Looking at the rest of the series, it doesn't look like there is a real 
benefits to have the static memory and reserved memory in separate 
arrays as they are walked only a few times and they are both meant to be 
small. In fact, I think this code is lot more difficult to read.
So it would be best to merge the two arrays in one. We can add a flag in 
the structure to differentiate between "static" and "reserved" memory.
Cheers,

--
Julien Grall