Hi,
On 13/10/2021 17:44, Oleksandr wrote:
On 13.10.21 19:24, Julien Grall wrote:
On 13/10/2021 16:49, Oleksandr wrote:
On 13.10.21 18:15, Julien Grall wrote:
On 13/10/2021 14:46, Oleksandr wrote:
Hi Julien
Hi Oleksandr,
Hi Julien
Thank you for the prompt response.
On 13.10.21 00:43, Oleksandr wrote:
diff --git a/tools/libs/light/libxl_arm.c
b/tools/libs/light/libxl_arm.c
index e3140a6..53ae0f3 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -615,9 +615,12 @@ static int make_hypervisor_node(libxl__gc *gc,
void *fdt,
"xen,xen");
if (res) return res;
- /* reg 0 is grant table space */
+ /*
+ * reg 0 is a placeholder for grant table space, reg 1...N are
+ * the placeholders for extended regions.
+ */
res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
GUEST_ROOT_SIZE_CELLS,
- 1,GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
+ GUEST_RAM_BANKS + 1, 0, 0, 0, 0, 0, 0);
Here you are relying on GUEST_RAM_BANKS == 2. I think this is pretty
fragile as this may change in the future.
fdt_property_regs() is not really suitable for here. I would suggest
to create a new helper fdt_property_placeholder() which takes the
address cells, size cells and the number of ranges. The function
will then create N range that are zeroed.
You are right. Probably, we could add an assert here for now to be
triggered if "GUEST_RAM_BANKS != 2"?
But, if you still think we need to make it with the helper right now,
I will. However, I don't know how long this will take.
I would prefer if we introduce the helper now. Below a potential
version (not compiled):
You wouldn't believe)))
I decided to not wait for the answer and re-check. So, I ended up with
the similar to what you suggested below. Thank you.
Yes, it will work if add missing locals. However, I initially named it
exactly as was suggested (fdt_property_placeholder) and got a
compilation error, since fdt_property_placeholder is already present.
So, I was thinking to choose another name or to even open-code it, but I
see you already proposed new name fdt_property_reg_placeholder.
Ah I didn't realize that libfdt already implemented
fdt_property_placeholder(). The function sounds perfect for us (and the
code is nicer :)), but unfortunately this was introdcued only in 2017.
So it is possible that some distros may not ship the last libfdt.
So for now, I think we need to implement our own. In a follow-up we
could try to provide a compat layer as we did for other missing fdt_*
helpers.
Cheers,
--
Julien Grall
