On 16/08/2021 07:00, Penny Zheng wrote:
Hi Julien
Hi Penny,
-----Original Message-----
From: Julien Grall <jul...@xen.org>
Sent: Wednesday, August 11, 2021 9:48 PM
To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen
<wei.c...@arm.com>; nd <n...@arm.com>
Subject: Re: [PATCH V4 03/10] xen/arm: handle static memory in
dt_unreserved_regions
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.zh...@arm.com>
---
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.
How about adding a "static" flag in "struct meminfo" to tell. See the below
example:
"
struct meminfo {
int nr_banks;
struct membank bank[NR_MEM_BANKS];
bool static; /* whether memory is reserved as static memory. */
};
"
And I will delete "struct meminfo static_mem" array, all "static" and
"reserved" memory
will be stored in one "struct meminfo reserved_mem" array.
Did you intend to suggest to add the new member in struct membank rather
than struct meminfo?
If not, then I don't understand how this would help have the static and
reserved region in a single array and avoid the extra loop you added.
Cheers,
--
Julien Grall
