Re: [Xen-devel] [PATCH v3 12/25] xen/arm: refactor construct_dom0
On Mon, 13 Aug 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 01/08/18 00:27, Stefano Stabellini wrote:
> > Move generic initializations out of construct_dom0 so that they can be
> > reused.
> >
> > Rename prepare_dtb to prepare_dtb_hwdom to avoid confusion.
> >
> > No functional changes in this patch.
> >
> > Signed-off-by: Stefano Stabellini
> >
> > ---
> > Changes in v3:
> > - move setting type before allocate_memory
> > - add ifdef around it and a comment
> > Changes in v2:
> > - move discard_initial_modules() after __construct_domain()
> > - remove useless blank line
> > - leave safety BUG_ONs in __construct_domain
> > - rename prepare_dtb to prepare_dtb_hwdom
> > ---
> > xen/arch/arm/domain_build.c | 128
> >
> > 1 file changed, 70 insertions(+), 58 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 8d82849..0835340 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1371,7 +1371,7 @@ static int __init handle_node(struct domain *d, struct
> > kernel_info *kinfo,
> > return res;
> > }
> > -static int __init prepare_dtb(struct domain *d, struct kernel_info
> > *kinfo)
> > +static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info
> > *kinfo)
> > {
> > const p2m_type_t default_p2mt = p2m_mmio_direct_c;
> > const void *fdt;
> > @@ -2106,75 +2106,31 @@ 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)
> > +static int __init __construct_domain(struct domain *d, struct kernel_info
> > *kinfo)
> > {
> > -const struct bootcmdline *kernel =
> > boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
> > -struct kernel_info kinfo = {};
> > struct vcpu *saved_current;
> > -int rc, i, cpu;
> > -
> > +int i, cpu;
> > +struct cpu_user_regs *regs;
> > struct vcpu *v = d->vcpu[0];
> > -struct cpu_user_regs *regs = >arch.cpu_info->guest_cpu_user_regs;
>
> There is no need to rewrite that line in
>
> struct cpu_user_regs *regs;
>
> regs = ...;
OK
> > -/* 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);
> > -}
> > -
> > -
> > -iommu_hwdom_init(d);
> > -
> > -d->max_pages = ~0U;
> > -
> > -kinfo.unassigned_mem = dom0_mem;
> > -kinfo.d = d;
> > -
> > -rc = kernel_probe(, NULL);
> > -if ( rc < 0 )
> > -return rc;
> > +regs = >arch.cpu_info->guest_cpu_user_regs;
> > #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;
> > if ( is_64bit_domain(d) )
> > vcpu_switch_to_aarch64_mode(v);
> > #endif
> > -kinfo.cmdline = kernel != NULL ? >cmdline[0] : NULL;
> > -allocate_memory(d, );
> > -find_gnttab_region(d, );
> > -
> > -/* 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, );
> > -else
> > -rc = prepare_acpi(d, );
> > -
> > -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
> > @@ -2187,20 +2143,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();
> > +kernel_load(kinfo);
> > /* initrd_load will fix up the fdt, so call it before dtb_load */
> > -initrd_load();
> > -dtb_load();
> > +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) )
> > {
> > @@ -2218,14 +2172,14 @@ int __init
Re: [Xen-devel] [PATCH v3 12/25] xen/arm: refactor construct_dom0
Hi Stefano,
On 01/08/18 00:27, Stefano Stabellini wrote:
Move generic initializations out of construct_dom0 so that they can be
reused.
Rename prepare_dtb to prepare_dtb_hwdom to avoid confusion.
No functional changes in this patch.
Signed-off-by: Stefano Stabellini
---
Changes in v3:
- move setting type before allocate_memory
- add ifdef around it and a comment
Changes in v2:
- move discard_initial_modules() after __construct_domain()
- remove useless blank line
- leave safety BUG_ONs in __construct_domain
- rename prepare_dtb to prepare_dtb_hwdom
---
xen/arch/arm/domain_build.c | 128
1 file changed, 70 insertions(+), 58 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 8d82849..0835340 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1371,7 +1371,7 @@ static int __init handle_node(struct domain *d, struct
kernel_info *kinfo,
return res;
}
-static int __init prepare_dtb(struct domain *d, struct kernel_info *kinfo)
+static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info
*kinfo)
{
const p2m_type_t default_p2mt = p2m_mmio_direct_c;
const void *fdt;
@@ -2106,75 +2106,31 @@ 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)
+static int __init __construct_domain(struct domain *d, struct kernel_info
*kinfo)
{
-const struct bootcmdline *kernel =
boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
-struct kernel_info kinfo = {};
struct vcpu *saved_current;
-int rc, i, cpu;
-
+int i, cpu;
+struct cpu_user_regs *regs;
struct vcpu *v = d->vcpu[0];
-struct cpu_user_regs *regs = >arch.cpu_info->guest_cpu_user_regs;
There is no need to rewrite that line in
struct cpu_user_regs *regs;
regs = ...;
-/* 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);
-}
-
-
-iommu_hwdom_init(d);
-
-d->max_pages = ~0U;
-
-kinfo.unassigned_mem = dom0_mem;
-kinfo.d = d;
-
-rc = kernel_probe(, NULL);
-if ( rc < 0 )
-return rc;
+regs = >arch.cpu_info->guest_cpu_user_regs;
#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;
if ( is_64bit_domain(d) )
vcpu_switch_to_aarch64_mode(v);
#endif
-kinfo.cmdline = kernel != NULL ? >cmdline[0] : NULL;
-allocate_memory(d, );
-find_gnttab_region(d, );
-
-/* 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, );
-else
-rc = prepare_acpi(d, );
-
-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
@@ -2187,20 +2143,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();
+kernel_load(kinfo);
/* initrd_load will fix up the fdt, so call it before dtb_load */
-initrd_load();
-dtb_load();
+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) )
{
@@ -2218,14 +2172,14 @@ int __init construct_dom0(struct domain *d)
*/
regs->r0 = 0; /* SBZ */
regs->r1 = 0x; /* 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
[Xen-devel] [PATCH v3 12/25] xen/arm: refactor construct_dom0
Move generic initializations out of construct_dom0 so that they can be
reused.
Rename prepare_dtb to prepare_dtb_hwdom to avoid confusion.
No functional changes in this patch.
Signed-off-by: Stefano Stabellini
---
Changes in v3:
- move setting type before allocate_memory
- add ifdef around it and a comment
Changes in v2:
- move discard_initial_modules() after __construct_domain()
- remove useless blank line
- leave safety BUG_ONs in __construct_domain
- rename prepare_dtb to prepare_dtb_hwdom
---
xen/arch/arm/domain_build.c | 128
1 file changed, 70 insertions(+), 58 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 8d82849..0835340 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1371,7 +1371,7 @@ static int __init handle_node(struct domain *d, struct
kernel_info *kinfo,
return res;
}
-static int __init prepare_dtb(struct domain *d, struct kernel_info *kinfo)
+static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info
*kinfo)
{
const p2m_type_t default_p2mt = p2m_mmio_direct_c;
const void *fdt;
@@ -2106,75 +2106,31 @@ 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)
+static int __init __construct_domain(struct domain *d, struct kernel_info
*kinfo)
{
-const struct bootcmdline *kernel =
boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
-struct kernel_info kinfo = {};
struct vcpu *saved_current;
-int rc, i, cpu;
-
+int i, cpu;
+struct cpu_user_regs *regs;
struct vcpu *v = d->vcpu[0];
-struct cpu_user_regs *regs = >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);
-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(, NULL);
-if ( rc < 0 )
-return rc;
+regs = >arch.cpu_info->guest_cpu_user_regs;
#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;
if ( is_64bit_domain(d) )
vcpu_switch_to_aarch64_mode(v);
#endif
-kinfo.cmdline = kernel != NULL ? >cmdline[0] : NULL;
-allocate_memory(d, );
-find_gnttab_region(d, );
-
-/* 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, );
-else
-rc = prepare_acpi(d, );
-
-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
@@ -2187,20 +2143,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();
+kernel_load(kinfo);
/* initrd_load will fix up the fdt, so call it before dtb_load */
-initrd_load();
-dtb_load();
+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) )
{
@@ -2218,14 +2172,14 @@ int __init construct_dom0(struct domain *d)
*/
regs->r0 = 0; /* SBZ */
regs->r1 = 0x; /* 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 */
@@ -2251,6 +2205,64 @@ int __init construct_dom0(struct domain *d)
return 0;
}
+int __init construct_dom0(struct domain *d)
+{
+const struct bootcmdline *kernel =
