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

Re: [Xen-devel] [PATCH v5 3/7] xen/arm: keep track of reserved-memory regions


On 8/13/19 3:23 PM, Volodymyr Babchuk wrote:

Stefano Stabellini writes:

As we parse the device tree in Xen, keep track of the reserved-memory
regions as they need special treatment (follow-up patches will make use
of the stored information.)

Reuse process_memory_node to add reserved-memory regions to the
bootinfo.reserved_mem array.

Refuse to continue once we reach the max number of reserved memory
regions to avoid accidentally mapping any portions of them into a VM.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>

Changes in v5:
- remove unneeded cast
- remove unneeded strlen check
- don't pass address_cells, size_cells, depth to device_tree_for_each_node

Changes in v4:
- depth + 1 in process_reserved_memory_node
- pass address_cells and size_cells to device_tree_for_each_node
- pass struct meminfo * instead of a boolean to process_memory_node
- improve in-code comment
- use a separate process_reserved_memory_node (separate from
   process_memory_node) function wrapper to have different error handling

Changes in v3:
- match only /reserved-memory
- put the warning back in place for reg not present on a normal memory
- refuse to continue once we reach the max number of reserved memory

Changes in v2:
- call process_memory_node from process_reserved_memory_node to avoid
  xen/arch/arm/bootfdt.c      | 41 +++++++++++++++++++++++++++++++------
  xen/include/asm-arm/setup.h |  1 +
  2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 590b14304c..0b0e22a3d0 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -136,6 +136,7 @@ static int __init process_memory_node(const void *fdt, int 
      const __be32 *cell;
      paddr_t start, size;
      u32 reg_cells = address_cells + size_cells;
+    struct meminfo *mem = data;

      if ( address_cells < 1 || size_cells < 1 )
          return -ENOENT;
@@ -147,21 +148,46 @@ static int __init process_memory_node(const void *fdt, 
int node,
      cell = (const __be32 *)prop->data;
      banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));

-    for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ )
+    for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
What is logic behind the second part of the loop condition?

You know that if (banks > NR_MEM_BANKS) then you will exit with error. Do
you really need to iterate over loop in this case?

Well, the error is ignored in the case of memory banks. So iterating over the first banks allows you to fill up bootinfo with as much as possible as RAM. The rest of the RAM will not be used by Xen.

          device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
          if ( !size )
-        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
-        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
-        bootinfo.mem.nr_banks++;
+        mem->bank[mem->nr_banks].start = start;
+        mem->bank[mem->nr_banks].size = size;
+        mem->nr_banks++;

-    if ( bootinfo.mem.nr_banks == NR_MEM_BANKS )
+    if ( mem->nr_banks == NR_MEM_BANKS )
Looks like you have the same off-by-one error, as in previous patch.
I can see that it was there earlier. But it is good time to fix it.

I don't think there was an off-by-one error before this series. So what do you mean?


Julien Grall

Xen-devel mailing list



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