[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH RFC 09/15] xen/arm: refactor construct_dom0
Hi Stefano,
On 13/06/18 23:15, Stefano Stabellini wrote:
Move generic initializations out of construct_dom0 so that they can be
reused.
No functional changes in this patch.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
xen/arch/arm/domain_build.c | 124 ++++++++++++++++++++++++--------------------
1 file changed, 67 insertions(+), 57 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4e4cd19..b31c563 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2092,73 +2092,27 @@ static void __init find_gnttab_region(struct domain *d,
kinfo->gnttab_start, kinfo->gnttab_start + kinfo->gnttab_size);
}
-int __init construct_dom0(struct domain *d)
+int __init __construct_domain(struct domain *d, struct kernel_info *kinfo)
{
- struct kernel_info kinfo = {};
struct vcpu *saved_current;
- int rc, i, cpu;
+ int i, cpu;
struct vcpu *v = d->vcpu[0];
struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
- /* Sanity! */
- BUG_ON(d->domain_id != 0);
- BUG_ON(d->vcpu[0] == NULL);
- BUG_ON(v->is_initialised);
I think we want to keep the last two BUG_ON here. "d->vcpu[0] == NULL"
is making sure that the value used when setting current is sane.
"v->is_initalised" is making sure this function is not called twice by
mistake. Indeed the variable will be set at the end of __construct_domain.
-
- printk("*** LOADING DOMAIN 0 ***\n");
- if ( dom0_mem <= 0 )
- {
- warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR
NOW\n");
- dom0_mem = MB(512);
- }
-
-
- iommu_hwdom_init(d);
-
- d->max_pages = ~0U;
-
- kinfo.unassigned_mem = dom0_mem;
- kinfo.d = d;
-
- rc = kernel_probe(&kinfo);
- if ( rc < 0 )
- return rc;
-
#ifdef CONFIG_ARM_64
/* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
- if ( !(cpu_has_el1_32) && kinfo.type == DOMAIN_32BIT )
+ if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )
{
printk("Platform does not support 32-bit domain\n");
return -EINVAL;
}
- d->arch.type = kinfo.type;
Any reason to move this out?
if ( is_64bit_domain(d) )
vcpu_switch_to_aarch64_mode(v);
#endif
- allocate_memory(d, &kinfo);
I think this could stay here.
- find_gnttab_region(d, &kinfo);
-
- /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
- rc = gic_map_hwdom_extra_mappings(d);
- if ( rc < 0 )
- return rc;
-
- rc = platform_specific_mapping(d);
- if ( rc < 0 )
- return rc;
-
- if ( acpi_disabled )
- rc = prepare_dtb(d, &kinfo);
- else
- rc = prepare_acpi(d, &kinfo);
I think this is probably a call to add "hwdom" in the name of the 2
functions.
-
- if ( rc < 0 )
- return rc;
-
/*
* The following loads use the domain's p2m and require current to
* be a vcpu of the domain, temporarily switch
@@ -2171,20 +2125,18 @@ int __init construct_dom0(struct domain *d)
* kernel_load will determine the placement of the kernel as well
* as the initrd & fdt in RAM, so call it first.
*/
- kernel_load(&kinfo);
+ kernel_load(kinfo);
/* initrd_load will fix up the fdt, so call it before dtb_load */
- initrd_load(&kinfo);
- dtb_load(&kinfo);
+ initrd_load(kinfo);
+ dtb_load(kinfo);
/* Now that we are done restore the original p2m and current. */
set_current(saved_current);
p2m_restore_state(saved_current);
- discard_initial_modules();
-
memset(regs, 0, sizeof(*regs));
- regs->pc = (register_t)kinfo.entry;
+ regs->pc = (register_t)kinfo->entry;
if ( is_32bit_domain(d) )
{
@@ -2202,14 +2154,14 @@ int __init construct_dom0(struct domain *d)
*/
regs->r0 = 0; /* SBZ */
regs->r1 = 0xffffffff; /* We use DTB therefore no machine id */
- regs->r2 = kinfo.dtb_paddr;
+ regs->r2 = kinfo->dtb_paddr;
}
#ifdef CONFIG_ARM_64
else
{
regs->cpsr = PSR_GUEST64_INIT;
/* From linux/Documentation/arm64/booting.txt */
- regs->x0 = kinfo.dtb_paddr;
+ regs->x0 = kinfo->dtb_paddr;
regs->x1 = 0; /* Reserved for future use */
regs->x2 = 0; /* Reserved for future use */
regs->x3 = 0; /* Reserved for future use */
@@ -2235,6 +2187,64 @@ int __init construct_dom0(struct domain *d)
return 0;
}
+int __init construct_dom0(struct domain *d)
+{
+ struct kernel_info kinfo = {};
+ int rc;
+
+ struct vcpu *v = d->vcpu[0];
+
+ /* Sanity! */
+ BUG_ON(d->domain_id != 0);
+ BUG_ON(d->vcpu[0] == NULL);
+ BUG_ON(v->is_initialised);
+
+ printk("*** LOADING DOMAIN 0 ***\n");
+ if ( dom0_mem <= 0 )
+ {
+ warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR
NOW\n");
+ dom0_mem = MB(512);
+ }
+
+
No need to copy the second newline :).
+ iommu_hwdom_init(d);
+
+ d->max_pages = ~0U;
+
+ kinfo.unassigned_mem = dom0_mem;
+ kinfo.d = d;
+
+ rc = kernel_probe(&kinfo);
+ if ( rc < 0 )
+ return rc;
+
+ allocate_memory(d, &kinfo);
+ find_gnttab_region(d, &kinfo);
+
+ /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
+ rc = gic_map_hwdom_extra_mappings(d);
+ if ( rc < 0 )
+ return rc;
+
+ rc = platform_specific_mapping(d);
+ if ( rc < 0 )
+ return rc;
+
+ d->arch.type = kinfo.type;
+
+ if ( acpi_disabled )
+ rc = prepare_dtb(d, &kinfo);
+ else
+ rc = prepare_acpi(d, &kinfo);
+
+ if ( rc < 0 )
+ return rc;
+
+ discard_initial_modules();
You say "no functional change" in this patch. But this is one. The
module are now discard much earlier. This imply that memory baking the
Image/Initrd will be free to be re-used at any time.
I don't think this is what we want. Unless you can promise no memory is
allocated in __construct_domain().
+
+ return __construct_domain(d, &kinfo);
+}
+
/*
* Local variables:
* mode: C
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
