[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 18/25] xen/arm: generate a simple device tree for domUs
Hi Stefano,
On 01/08/18 00:28, Stefano Stabellini wrote:
Introduce functions to generate a basic domU device tree, similar to the
existing functions in tools/libxl/libxl_arm.c.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v3:
- remove CONFIG_ACPI for make_chosen_node
- remove make_hypervisor_node until all Xen related functionalities
(evtchns, grant table, etc.) work correctly
Changes in v2:
- move prepare_dtb rename to previous patch
- use switch for the gic version
- use arm,gic-400 instead of arm,cortex-a15-gic
- add @unit-address in the gic node name
- add comment on DOMU_DTB_SIZE
---
xen/arch/arm/domain_build.c | 211 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 209 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index dfa74e4..167a56e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1057,7 +1057,6 @@ static int __init make_timer_node(const struct domain *d,
void *fdt,
return res;
}
-#ifdef CONFIG_ACPI
/*
* This function is used as part of the device tree generation for Dom0
* on ACPI systems, and DomUs started directly from Xen based on device
@@ -1103,7 +1102,6 @@ static int __init make_chosen_node(const struct
kernel_info *kinfo)
return res;
}
-#endif
static int __init map_irq_to_domain(struct domain *d, unsigned int irq,
bool need_mapping, const char *devname)
@@ -1496,6 +1494,211 @@ static int __init handle_node(struct domain *d, struct
kernel_info *kinfo,
return res;
}
+static int __init make_gic_domU_node(const struct domain *d, void *fdt, int addrcells, int sizecells)
+{
+ int res = 0;
+ int reg_size = addrcells + sizecells;
+ int nr_cells = reg_size * 2;
+ __be32 reg[nr_cells];
+ __be32 *cells;
You are trying to be save a few lines by handling both GICv2 and GICv3
node in a single function. But this only thing it adds is confusion when
reading the code.
Please have two functions (one for each GIC version) to generate the DT.
This would also help to add ITS/GICv2m support in the future.
+
+ switch ( gic_hw_version() )
+ {
+ case GIC_V3:
+ res = fdt_begin_node(fdt,
"interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE));
+ break;
+ case GIC_V2:
+ res = fdt_begin_node(fdt,
"interrupt-controller@"__stringify(GUEST_GICD_BASE));
+ break;
+ default:
+ panic("Unsupported GIC version");
+ }
+
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "#address-cells", 0);
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "#interrupt-cells", 3);
+ if ( res )
+ return res;
+
+ res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+ if ( res )
+ return res;
+
+ switch ( gic_hw_version() )
+ {
+ case GIC_V3:
+ {
+ const uint64_t gicd_base = GUEST_GICV3_GICD_BASE;
+ const uint64_t gicd_size = GUEST_GICV3_GICD_SIZE;
+ const uint64_t gicr0_base = GUEST_GICV3_GICR0_BASE;
+ const uint64_t gicr0_size = GUEST_GICV3_GICR0_SIZE;
I am not entirely convinced of the usefulness of those variables.
+
+ res = fdt_property_string(fdt, "compatible", "arm,gic-v3");
+ if ( res )
+ return res;
+
+ cells = ®[0];
+ dt_child_set_range(&cells, addrcells, sizecells, gicd_base, gicd_size);
+ dt_child_set_range(&cells, addrcells, sizecells, gicr0_base,
gicr0_size);
+ res = fdt_property(fdt, "reg", reg, sizeof(reg));
+ break;
+ }
+ case GIC_V2:
+ {
+ const uint64_t gicd_base = GUEST_GICD_BASE;
+ const uint64_t gicd_size = GUEST_GICD_SIZE;
+ const uint64_t gicc_base = GUEST_GICC_BASE;
+ const uint64_t gicc_size = GUEST_GICC_SIZE;
Same here.
+
+ res = fdt_property_string(fdt, "compatible", "arm,gic-400");
+ if ( res )
+ return res;
+
+ cells = ®[0];
+ dt_child_set_range(&cells, addrcells, sizecells, gicd_base, gicd_size);
+ dt_child_set_range(&cells, addrcells, sizecells, gicc_base, gicc_size);
+ break;
+ }
+ default:
+ break;
+ }
+
+ res = fdt_property(fdt, "reg", reg, sizeof(reg));
+ if (res)
+ return res;
+
+ res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
+ if (res)
+ return res;
+
+ res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
+ if (res)
+ return res;
+
+ res = fdt_end_node(fdt);
+
+ return res;
+}
+
+static int __init make_timer_domU_node(const struct domain *d, void *fdt)
+{
+ int res;
+ gic_interrupt_t intrs[3];
+
+ res = fdt_begin_node(fdt, "timer");
+ if ( res )
+ return res;
+
+ if (!is_64bit_domain(d))
Coding style:
if ( ... )
+ {
+ res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
+ if ( res )
+ return res;
+ } else {
Coding style:
}
else
{
+ res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
+ if ( res )
+ return res;
+ }
+
+ set_interrupt_ppi(intrs[0], GUEST_TIMER_PHYS_S_PPI, 0xf,
DT_IRQ_TYPE_LEVEL_LOW);
+ set_interrupt_ppi(intrs[1], GUEST_TIMER_PHYS_NS_PPI, 0xf,
DT_IRQ_TYPE_LEVEL_LOW);
+ set_interrupt_ppi(intrs[2], GUEST_TIMER_VIRT_PPI, 0xf,
DT_IRQ_TYPE_LEVEL_LOW);
+
+ res = fdt_property(fdt, "interrupts", intrs, sizeof (intrs[0]) * 3);
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "interrupt-parent",
+ GUEST_PHANDLE_GIC);
+ if (res)
+ return res;
+
+ res = fdt_end_node(fdt);
Newline here please.
+ return res;
+}
+
+/*
+ * The max size for DT is 2MB. However, the generated DT is small, 4KB
+ * are enough for now, but we might have to increase it in the feature.
s/feature/future/
+ */
+#define DOMU_DTB_SIZE 4096
+static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
+{
+ int addrcells, sizecells;
+ int ret;
+
+ addrcells = dt_child_n_addr_cells(dt_host); > + sizecells =
dt_child_n_size_cells(dt_host);
Why do you use the host DT to find out the the #address-cells and
#size-cells? Should not this be independent?
+
+ kinfo->fdt = xmalloc_bytes(DOMU_DTB_SIZE);
+ if ( kinfo->fdt == NULL )
+ return -ENOMEM;
+
+ ret = fdt_create(kinfo->fdt, DOMU_DTB_SIZE);
+ if ( ret < 0 )
+ goto err;
+
+ ret = fdt_finish_reservemap(kinfo->fdt);
+ if ( ret < 0 )
+ goto err;
+
+ ret = fdt_begin_node(kinfo->fdt, "/");
+ if ( ret < 0 )
+ goto err;
+
+ ret = fdt_property_cell(kinfo->fdt, "#address-cells", addrcells);
+ if ( ret )
+ goto err;
+
+ ret = fdt_property_cell(kinfo->fdt, "#size-cells", sizecells);
+ if ( ret )
+ goto err;
+
+ ret = make_chosen_node(kinfo);
+ if ( ret )
+ goto err;
+
+ ret = make_psci_node(kinfo->fdt, NULL);
+ if ( ret )
+ goto err;
+
+ ret = make_cpus_node(d, kinfo->fdt, NULL);
+ if ( ret )
+ goto err;
+
+ ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
+ if ( ret )
+ goto err;
+
+ ret = make_gic_domU_node(d, kinfo->fdt, addrcells, sizecells);
+ if ( ret )
+ goto err;
+
+ ret = make_timer_domU_node(d, kinfo->fdt);
+ if ( ret )
+ goto err;
+
+ ret = fdt_end_node(kinfo->fdt);
+ if ( ret < 0 )
+ goto err;
+
+ ret = fdt_finish(kinfo->fdt);
+ if ( ret < 0 )
+ goto err;
+
+ return 0;
+
+ err:
+ printk("Device tree generation failed (%d).\n", ret);
+ xfree(kinfo->fdt);
Newline here please.
+ return -EINVAL;
+}
+
static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info
*kinfo)
{
const p2m_type_t default_p2mt = p2m_mmio_direct_c;
@@ -2366,6 +2569,10 @@ static int __init construct_domU(struct domain *d,
struct dt_device_node *node)
#endif
allocate_memory(d, &kinfo);
+ rc = prepare_dtb_domU(d, &kinfo);
+ if ( rc < 0 )
+ return rc;
+
return __construct_domain(d, &kinfo);
}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
