[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 3/8] xen/arm: introduce kinfo->guest_phandle_gic
Hi Stefano,
On 8/21/19 4:53 AM, Stefano Stabellini wrote:
Instead of always hard-coding the GIC phandle (GUEST_PHANDLE_GIC), store
it in a variable under kinfo. This way it can be dynamically chosen per
domain.
Initialize guest_phandle_gic to GUEST_PHANDLE_GIC at the beginning of
prepare_dtb_domU. Later patches will change the value of
guest_phandle_gic depending on user provided information.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v4:
- new patch
---
xen/arch/arm/domain_build.c | 42 ++++++++++++++++++++----------------
xen/include/asm-arm/kernel.h | 3 +++
2 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f92069c85f..cd585f05ca 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1510,8 +1510,9 @@ static int __init handle_node(struct domain *d, struct
kernel_info *kinfo,
return res;
}
-static int __init make_gicv2_domU_node(const struct domain *d, void *fdt)
+static int __init make_gicv2_domU_node(struct kernel_info *kinfo)
It is a bit unclear why the first argument is dropped but not replace by
the declaration of a variable. If it was never used before, then please
mention it in the commit message.
Similarly, please mention why the prototype has been changed.
{
+ void *fdt = kinfo->fdt;
int res = 0;
__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
__be32 *cells;
@@ -1546,11 +1547,11 @@ static int __init make_gicv2_domU_node(const struct
domain *d, void *fdt)
if (res)
return res;
- res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
+ res = fdt_property_cell(fdt, "linux,phandle", kinfo->guest_phandle_gic);
if (res)
return res;
- res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
+ res = fdt_property_cell(fdt, "phandle", kinfo->guest_phandle_gic);
if (res)
return res;
@@ -1559,8 +1560,9 @@ static int __init make_gicv2_domU_node(const struct domain *d, void *fdt)
return res;
}
-static int __init make_gicv3_domU_node(const struct domain *d, void *fdt)
+static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
Ditto.
{
+ void *fdt = kinfo->fdt;
int res = 0;
__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
__be32 *cells;
@@ -1595,11 +1597,11 @@ static int __init make_gicv3_domU_node(const struct
domain *d, void *fdt)
if (res)
return res;
- res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
+ res = fdt_property_cell(fdt, "linux,phandle", kinfo->guest_phandle_gic);
if (res)
return res;
- res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
+ res = fdt_property_cell(fdt, "phandle", kinfo->guest_phandle_gic);
if (res)
return res;
@@ -1608,21 +1610,22 @@ static int __init make_gicv3_domU_node(const struct domain *d, void *fdt)
return res;
}
[...]
-static int __init make_timer_domU_node(const struct domain *d, void *fdt)
+static int __init make_timer_domU_node(struct kernel_info *kinfo)
This definitely needs some rebase on the latest staging to pick up the
new prototype. Actually, the prototype was modified beginning of August
but this was sent end of August... Please make sure you are using the
latest staging at the time when sending a series.
{
+ void *fdt = kinfo->fdt;
int res;
gic_interrupt_t intrs[3];
@@ -1630,7 +1633,7 @@ static int __init make_timer_domU_node(const struct domain *d, void *fdt)
if ( res )
return res;
- if ( !is_64bit_domain(d) )
+ if ( !is_64bit_domain(kinfo->d) )
{
res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
if ( res )
@@ -1652,7 +1655,7 @@ static int __init make_timer_domU_node(const struct
domain *d, void *fdt)
return res;
res = fdt_property_cell(fdt, "interrupt-parent",
- GUEST_PHANDLE_GIC);
+ kinfo->guest_phandle_gic);
if (res)
return res;
@@ -1662,8 +1665,9 @@ static int __init make_timer_domU_node(const struct domain *d, void *fdt)
}
#ifdef CONFIG_SBSA_VUART_CONSOLE
-static int __init make_vpl011_uart_node(const struct domain *d, void *fdt)
+static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
{
+ void *fdt = kinfo->fdt;
int res;
gic_interrupt_t intr;
__be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
@@ -1694,7 +1698,7 @@ static int __init make_vpl011_uart_node(const struct
domain *d, void *fdt)
return res;
res = fdt_property_cell(fdt, "interrupt-parent",
- GUEST_PHANDLE_GIC);
+ kinfo->guest_phandle_gic);
if ( res )
return res;
@@ -1719,6 +1723,8 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
int addrcells, sizecells;
int ret;
+ kinfo->guest_phandle_gic = GUEST_PHANDLE_GIC;
+
addrcells = GUEST_ROOT_ADDRESS_CELLS;
sizecells = GUEST_ROOT_SIZE_CELLS;
@@ -1762,11 +1768,11 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
if ( ret )
goto err;
- ret = make_gic_domU_node(d, kinfo->fdt);
+ ret = make_gic_domU_node(kinfo);
if ( ret )
goto err;
- ret = make_timer_domU_node(d, kinfo->fdt);
+ ret = make_timer_domU_node(kinfo);
if ( ret )
goto err;
@@ -1774,7 +1780,7 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
{
ret = -EINVAL;
#ifdef CONFIG_SBSA_VUART_CONSOLE
- ret = make_vpl011_uart_node(d, kinfo->fdt);
+ ret = make_vpl011_uart_node(kinfo);
#endif
if ( ret )
goto err;
diff --git a/xen/include/asm-arm/kernel.h b/xen/include/asm-arm/kernel.h
index 33f3e72b11..760434369b 100644
--- a/xen/include/asm-arm/kernel.h
+++ b/xen/include/asm-arm/kernel.h
@@ -36,6 +36,9 @@ struct kernel_info {
/* Enable pl011 emulation */
bool vpl011;
+ /* GIC phandle */
+ uint32_t guest_phandle_gic;
Looking at the usage, I think this should be fdt32_t because you are
directly passing the value to the FDT calls.
This makes me realize that we consistently use wrongly GUEST_PHANDLE_GIC
in both Xen and libxl. Indeed, as we pass the value directly the guest
will not see 65000 but 3908894720 as it will do the conversion from
big-endian to little-endian.
I can see two solution to fix this:
1) define GUEST_PHANDLE_GIC as cpu_to_be32(65000)
2) Use cpu_to_be32(GUEST_PHANDLE_GIC)
It would be good to agree how GUEST_PHANDLE_GIC is used so we have the
same behavior when the DT is created by Xen and libxl.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
