* [PATCH v4 1/4] xen/arm: Move dom0 creation in domain_build.c
2021-04-14 9:14 [PATCH v4 0/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
@ 2021-04-14 9:14 ` Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
2021-04-14 9:14 ` [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL Luca Fancellu
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 9:14 UTC (permalink / raw)
To: xen-devel
Cc: bertrand.marquis, wei.chen, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk
Move dom0 create and start from setup.c to a dedicated
function in domain_build.c.
With this change, the function construct_dom0() is not
used outside of domain_build.c anymore.
So it is now a static function.
No functional changes intended.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
v4 changes:
- changes to the commit message
v3 changes:
- move create_dom0 function after construct_dom0 and
make construct_dom0 static
---
xen/arch/arm/domain_build.c | 38 ++++++++++++++++++++++++++++++++++++-
xen/arch/arm/setup.c | 29 +---------------------------
xen/include/asm-arm/setup.h | 2 +-
3 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 374bf655ee..359957dc1b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -21,6 +21,7 @@
#include <asm/device.h>
#include <asm/kernel.h>
#include <asm/setup.h>
+#include <asm/tee/tee.h>
#include <asm/platform.h>
#include <asm/psci.h>
#include <asm/setup.h>
@@ -2520,7 +2521,7 @@ void __init create_domUs(void)
}
}
-int __init construct_dom0(struct domain *d)
+static int __init construct_dom0(struct domain *d)
{
struct kernel_info kinfo = {};
int rc;
@@ -2578,6 +2579,41 @@ int __init construct_dom0(struct domain *d)
return construct_domain(d, &kinfo);
}
+struct domain* __init create_dom0(void)
+{
+ struct domain *dom0;
+ struct xen_domctl_createdomain dom0_cfg = {
+ .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
+ .max_evtchn_port = -1,
+ .max_grant_frames = gnttab_dom0_frames(),
+ .max_maptrack_frames = -1,
+ };
+
+ /* The vGIC for DOM0 is exactly emulating the hardware GIC */
+ dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
+ /*
+ * Xen vGIC supports a maximum of 992 interrupt lines.
+ * 32 are substracted to cover local IRQs.
+ */
+ dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32;
+ if ( gic_number_lines() > 992 )
+ printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
+ dom0_cfg.arch.tee_type = tee_get_type();
+ dom0_cfg.max_vcpus = dom0_max_vcpus();
+
+ if ( iommu_enabled )
+ dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
+
+ dom0 = domain_create(0, &dom0_cfg, true);
+ if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
+ panic("Error creating domain 0\n");
+
+ if ( construct_dom0(dom0) != 0)
+ panic("Could not set up DOM0 guest OS\n");
+
+ return dom0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2532ec9739..b405f58996 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -51,7 +51,6 @@
#include <asm/platform.h>
#include <asm/procinfo.h>
#include <asm/setup.h>
-#include <asm/tee/tee.h>
#include <xsm/xsm.h>
#include <asm/acpi.h>
@@ -805,12 +804,6 @@ void __init start_xen(unsigned long boot_phys_offset,
const char *cmdline;
struct bootmodule *xen_bootmodule;
struct domain *dom0;
- struct xen_domctl_createdomain dom0_cfg = {
- .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
- .max_evtchn_port = -1,
- .max_grant_frames = gnttab_dom0_frames(),
- .max_maptrack_frames = -1,
- };
int rc;
dcache_line_bytes = read_dcache_line_bytes();
@@ -965,27 +958,7 @@ void __init start_xen(unsigned long boot_phys_offset,
enable_errata_workarounds();
/* Create initial domain 0. */
- /* The vGIC for DOM0 is exactly emulating the hardware GIC */
- dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
- /*
- * Xen vGIC supports a maximum of 992 interrupt lines.
- * 32 are substracted to cover local IRQs.
- */
- dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32;
- if ( gic_number_lines() > 992 )
- printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
- dom0_cfg.arch.tee_type = tee_get_type();
- dom0_cfg.max_vcpus = dom0_max_vcpus();
-
- if ( iommu_enabled )
- dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
-
- dom0 = domain_create(0, &dom0_cfg, true);
- if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
- panic("Error creating domain 0\n");
-
- if ( construct_dom0(dom0) != 0)
- panic("Could not set up DOM0 guest OS\n");
+ dom0 = create_dom0();
heap_init_late();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 28bf622aa1..5283244015 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -93,8 +93,8 @@ void acpi_create_efi_mmap_table(struct domain *d,
int acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
-int construct_dom0(struct domain *d);
void create_domUs(void);
+struct domain* create_dom0(void);
void discard_initial_modules(void);
void fw_unreserved_regions(paddr_t s, paddr_t e,
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v4 1/4] xen/arm: Move dom0 creation in domain_build.c
2021-04-14 9:14 ` [PATCH v4 1/4] xen/arm: Move dom0 creation in domain_build.c Luca Fancellu
@ 2021-04-14 10:14 ` Bertrand Marquis
0 siblings, 0 replies; 16+ messages in thread
From: Bertrand Marquis @ 2021-04-14 10:14 UTC (permalink / raw)
To: Luca Fancellu
Cc: Xen-devel, Wei Chen, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Hi Luca,
> On 14 Apr 2021, at 10:14, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>
> Move dom0 create and start from setup.c to a dedicated
> function in domain_build.c.
>
> With this change, the function construct_dom0() is not
> used outside of domain_build.c anymore.
> So it is now a static function.
>
> No functional changes intended.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers,
Bertrand
> ---
> v4 changes:
> - changes to the commit message
> v3 changes:
> - move create_dom0 function after construct_dom0 and
> make construct_dom0 static
> ---
> xen/arch/arm/domain_build.c | 38 ++++++++++++++++++++++++++++++++++++-
> xen/arch/arm/setup.c | 29 +---------------------------
> xen/include/asm-arm/setup.h | 2 +-
> 3 files changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 374bf655ee..359957dc1b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -21,6 +21,7 @@
> #include <asm/device.h>
> #include <asm/kernel.h>
> #include <asm/setup.h>
> +#include <asm/tee/tee.h>
> #include <asm/platform.h>
> #include <asm/psci.h>
> #include <asm/setup.h>
> @@ -2520,7 +2521,7 @@ void __init create_domUs(void)
> }
> }
>
> -int __init construct_dom0(struct domain *d)
> +static int __init construct_dom0(struct domain *d)
> {
> struct kernel_info kinfo = {};
> int rc;
> @@ -2578,6 +2579,41 @@ int __init construct_dom0(struct domain *d)
> return construct_domain(d, &kinfo);
> }
>
> +struct domain* __init create_dom0(void)
> +{
> + struct domain *dom0;
> + struct xen_domctl_createdomain dom0_cfg = {
> + .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> + .max_evtchn_port = -1,
> + .max_grant_frames = gnttab_dom0_frames(),
> + .max_maptrack_frames = -1,
> + };
> +
> + /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> + dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> + /*
> + * Xen vGIC supports a maximum of 992 interrupt lines.
> + * 32 are substracted to cover local IRQs.
> + */
> + dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32;
> + if ( gic_number_lines() > 992 )
> + printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
> + dom0_cfg.arch.tee_type = tee_get_type();
> + dom0_cfg.max_vcpus = dom0_max_vcpus();
> +
> + if ( iommu_enabled )
> + dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> +
> + dom0 = domain_create(0, &dom0_cfg, true);
> + if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
> + panic("Error creating domain 0\n");
> +
> + if ( construct_dom0(dom0) != 0)
> + panic("Could not set up DOM0 guest OS\n");
> +
> + return dom0;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 2532ec9739..b405f58996 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -51,7 +51,6 @@
> #include <asm/platform.h>
> #include <asm/procinfo.h>
> #include <asm/setup.h>
> -#include <asm/tee/tee.h>
> #include <xsm/xsm.h>
> #include <asm/acpi.h>
>
> @@ -805,12 +804,6 @@ void __init start_xen(unsigned long boot_phys_offset,
> const char *cmdline;
> struct bootmodule *xen_bootmodule;
> struct domain *dom0;
> - struct xen_domctl_createdomain dom0_cfg = {
> - .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> - .max_evtchn_port = -1,
> - .max_grant_frames = gnttab_dom0_frames(),
> - .max_maptrack_frames = -1,
> - };
> int rc;
>
> dcache_line_bytes = read_dcache_line_bytes();
> @@ -965,27 +958,7 @@ void __init start_xen(unsigned long boot_phys_offset,
> enable_errata_workarounds();
>
> /* Create initial domain 0. */
> - /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> - dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> - /*
> - * Xen vGIC supports a maximum of 992 interrupt lines.
> - * 32 are substracted to cover local IRQs.
> - */
> - dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32;
> - if ( gic_number_lines() > 992 )
> - printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
> - dom0_cfg.arch.tee_type = tee_get_type();
> - dom0_cfg.max_vcpus = dom0_max_vcpus();
> -
> - if ( iommu_enabled )
> - dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> -
> - dom0 = domain_create(0, &dom0_cfg, true);
> - if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
> - panic("Error creating domain 0\n");
> -
> - if ( construct_dom0(dom0) != 0)
> - panic("Could not set up DOM0 guest OS\n");
> + dom0 = create_dom0();
>
> heap_init_late();
>
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 28bf622aa1..5283244015 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -93,8 +93,8 @@ void acpi_create_efi_mmap_table(struct domain *d,
>
> int acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
>
> -int construct_dom0(struct domain *d);
> void create_domUs(void);
> +struct domain* create_dom0(void);
>
> void discard_initial_modules(void);
> void fw_unreserved_regions(paddr_t s, paddr_t e,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 9:14 [PATCH v4 0/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
2021-04-14 9:14 ` [PATCH v4 1/4] xen/arm: Move dom0 creation in domain_build.c Luca Fancellu
@ 2021-04-14 9:14 ` Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
2021-04-14 11:16 ` Julien Grall
2021-04-14 9:14 ` [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs() Luca Fancellu
2021-04-14 9:14 ` [PATCH v4 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
3 siblings, 2 replies; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 9:14 UTC (permalink / raw)
To: xen-devel
Cc: bertrand.marquis, wei.chen, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Rahul Singh
Among the common and arm codebase there are few cases where
the hardware_domain variable is checked to see if the current
domain is equal to the hardware_domain, change this cases to
use is_hardware_domain() function instead.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
v4 changes:
- removed unneeded check for domain NULL from is_hardware_domain
introduced in v3
v3 changes:
- removed unneeded parenthesis for macro is_domain_direct_mapped
- is_hardware_domain() checks for the passed domain and if it is
NULL, it returns false.
- reverted back checks in the function late_hwdom_init
---
xen/arch/arm/irq.c | 2 +-
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 2 +-
xen/drivers/passthrough/arm/smmu-v3.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 2 +-
xen/include/asm-arm/domain.h | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b71b099e6f..b761d90c40 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -412,7 +412,7 @@ bool is_assignable_irq(unsigned int irq)
*/
bool irq_type_set_by_domain(const struct domain *d)
{
- return (d == hardware_domain);
+ return is_hardware_domain(d);
}
/*
diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index aef358d880..8b8e3a00ba 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -1168,7 +1168,7 @@ static int ipmmu_reassign_device(struct domain *s, struct domain *t,
int ret = 0;
/* Don't allow remapping on other domain than hwdom */
- if ( t && t != hardware_domain )
+ if ( t && !is_hardware_domain(t) )
return -EPERM;
if ( t == s )
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index 53d150cdb6..d115df7320 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -3366,7 +3366,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
int ret = 0;
/* Don't allow remapping on other domain than hwdom */
- if (t && t != hardware_domain)
+ if ( t && !is_hardware_domain(t) )
return -EPERM;
if (t == s)
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 3e8aa37866..932fdfd6dd 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2670,7 +2670,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
int ret = 0;
/* Don't allow remapping on other domain than hwdom */
- if (t && t != hardware_domain)
+ if ( t && !is_hardware_domain(t) )
return -EPERM;
if (t == s)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 1da90f207d..0a74df9931 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -30,7 +30,7 @@ enum domain_type {
#endif
/* The hardware domain has always its memory direct mapped. */
-#define is_domain_direct_mapped(d) ((d) == hardware_domain)
+#define is_domain_direct_mapped(d) is_hardware_domain(d)
struct vtimer {
struct vcpu *v;
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 9:14 ` [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL Luca Fancellu
@ 2021-04-14 10:14 ` Bertrand Marquis
2021-04-14 11:16 ` Julien Grall
1 sibling, 0 replies; 16+ messages in thread
From: Bertrand Marquis @ 2021-04-14 10:14 UTC (permalink / raw)
To: Luca Fancellu
Cc: xen-devel, Wei Chen, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Rahul Singh
Hi Luca,
> On 14 Apr 2021, at 10:14, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>
> Among the common and arm codebase there are few cases where
> the hardware_domain variable is checked to see if the current
> domain is equal to the hardware_domain, change this cases to
> use is_hardware_domain() function instead.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers,
Bertrand
> ---
> v4 changes:
> - removed unneeded check for domain NULL from is_hardware_domain
> introduced in v3
> v3 changes:
> - removed unneeded parenthesis for macro is_domain_direct_mapped
> - is_hardware_domain() checks for the passed domain and if it is
> NULL, it returns false.
> - reverted back checks in the function late_hwdom_init
> ---
> xen/arch/arm/irq.c | 2 +-
> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 2 +-
> xen/drivers/passthrough/arm/smmu-v3.c | 2 +-
> xen/drivers/passthrough/arm/smmu.c | 2 +-
> xen/include/asm-arm/domain.h | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index b71b099e6f..b761d90c40 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -412,7 +412,7 @@ bool is_assignable_irq(unsigned int irq)
> */
> bool irq_type_set_by_domain(const struct domain *d)
> {
> - return (d == hardware_domain);
> + return is_hardware_domain(d);
> }
>
> /*
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index aef358d880..8b8e3a00ba 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -1168,7 +1168,7 @@ static int ipmmu_reassign_device(struct domain *s, struct domain *t,
> int ret = 0;
>
> /* Don't allow remapping on other domain than hwdom */
> - if ( t && t != hardware_domain )
> + if ( t && !is_hardware_domain(t) )
> return -EPERM;
>
> if ( t == s )
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index 53d150cdb6..d115df7320 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -3366,7 +3366,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
> int ret = 0;
>
> /* Don't allow remapping on other domain than hwdom */
> - if (t && t != hardware_domain)
> + if ( t && !is_hardware_domain(t) )
> return -EPERM;
>
> if (t == s)
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index 3e8aa37866..932fdfd6dd 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2670,7 +2670,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
> int ret = 0;
>
> /* Don't allow remapping on other domain than hwdom */
> - if (t && t != hardware_domain)
> + if ( t && !is_hardware_domain(t) )
> return -EPERM;
>
> if (t == s)
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 1da90f207d..0a74df9931 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -30,7 +30,7 @@ enum domain_type {
> #endif
>
> /* The hardware domain has always its memory direct mapped. */
> -#define is_domain_direct_mapped(d) ((d) == hardware_domain)
> +#define is_domain_direct_mapped(d) is_hardware_domain(d)
>
> struct vtimer {
> struct vcpu *v;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 9:14 ` [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
@ 2021-04-14 11:16 ` Julien Grall
2021-04-14 11:29 ` Luca Fancellu
1 sibling, 1 reply; 16+ messages in thread
From: Julien Grall @ 2021-04-14 11:16 UTC (permalink / raw)
To: Luca Fancellu, xen-devel
Cc: bertrand.marquis, wei.chen, Stefano Stabellini,
Volodymyr Babchuk, Rahul Singh
Hi Luca,
On 14/04/2021 10:14, Luca Fancellu wrote:
> Among the common and arm codebase there are few cases where
> the hardware_domain variable is checked to see if the current
> domain is equal to the hardware_domain, change this cases to
> use is_hardware_domain() function instead. >
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> v4 changes:
> - removed unneeded check for domain NULL from is_hardware_domain
> introduced in v3
After this change, this patch is only avoid to open-code
is_hardware_domain(). Although, it adds an extra speculation barrier.
I am not against the change, however I think the commit message needs to
updated to match what the patch is doing.
Can you propose a new commit message?
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 11:16 ` Julien Grall
@ 2021-04-14 11:29 ` Luca Fancellu
2021-04-14 13:45 ` Julien Grall
0 siblings, 1 reply; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 11:29 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Bertrand Marquis, wei.chen, Stefano Stabellini,
Volodymyr Babchuk, Rahul Singh
> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
>
> Hi Luca,
>
> On 14/04/2021 10:14, Luca Fancellu wrote:
>> Among the common and arm codebase there are few cases where
>> the hardware_domain variable is checked to see if the current
>> domain is equal to the hardware_domain, change this cases to
>> use is_hardware_domain() function instead. >
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> v4 changes:
>> - removed unneeded check for domain NULL from is_hardware_domain
>> introduced in v3
>
> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
>
> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
>
> Can you propose a new commit message?
Hi Julien,
Yes I agree, what about:
xen/arm: Reinforce use of is_hardware_domain
Among the common and arm codebase there are few cases where
the hardware_domain variable is checked to see if the current
domain is equal to the hardware_domain, change this cases to
use is_hardware_domain() function instead.
In the eventuality that hardware_domain is NULL, is_hardware_domain
will return false because an analysis of the common and arm codebase
shows that is_hardware_domain is called always with a non NULL
domain pointer.
Cheers,
Luca
>
> Cheers,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 11:29 ` Luca Fancellu
@ 2021-04-14 13:45 ` Julien Grall
2021-04-14 13:47 ` Luca Fancellu
0 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2021-04-14 13:45 UTC (permalink / raw)
To: Luca Fancellu
Cc: xen-devel, Bertrand Marquis, wei.chen, Stefano Stabellini,
Volodymyr Babchuk, Rahul Singh
Hi Luca,
On 14/04/2021 12:29, Luca Fancellu wrote:
>> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Luca,
>>
>> On 14/04/2021 10:14, Luca Fancellu wrote:
>>> Among the common and arm codebase there are few cases where
>>> the hardware_domain variable is checked to see if the current
>>> domain is equal to the hardware_domain, change this cases to
>>> use is_hardware_domain() function instead. >
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> v4 changes:
>>> - removed unneeded check for domain NULL from is_hardware_domain
>>> introduced in v3
>>
>> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
>>
>> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
>>
>> Can you propose a new commit message?
>
> Hi Julien,
>
> Yes I agree, what about:
>
> xen/arm: Reinforce use of is_hardware_domain
>
> Among the common and arm codebase there are few cases where
I would drop 'common' because you are only modifying the arm codebase.
> the hardware_domain variable is checked to see if the current
> domain is equal to the hardware_domain, change this cases to
> use is_hardware_domain() function instead.
> In the eventuality that hardware_domain is NULL, is_hardware_domain
> will return false because an analysis of the common and arm codebase
> shows that is_hardware_domain is called always with a non NULL
> domain pointer.
This paragraph seems to come out of the blue. I would drop it.
How about:
"
There are a few places on Arm where we use pretty much an open-coded
version of is_hardware_domain(). The main difference, is the helper will
also block speculation (not yet implemented on Arm).
The existing users are not in hot path, so blocking speculation would
not hurt when it is implemented. So remove the open-coded version within
the arm codebase.
"
If you are happy with the commit message, I will commit it the series
tomorrow (to give an opportunity to Stefano to review).
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 13:45 ` Julien Grall
@ 2021-04-14 13:47 ` Luca Fancellu
2021-04-14 20:35 ` Stefano Stabellini
0 siblings, 1 reply; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 13:47 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Bertrand Marquis, wei.chen, Stefano Stabellini,
Volodymyr Babchuk, Rahul Singh
> On 14 Apr 2021, at 14:45, Julien Grall <julien@xen.org> wrote:
>
> Hi Luca,
>
> On 14/04/2021 12:29, Luca Fancellu wrote:
>>> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
>>>
>>> Hi Luca,
>>>
>>> On 14/04/2021 10:14, Luca Fancellu wrote:
>>>> Among the common and arm codebase there are few cases where
>>>> the hardware_domain variable is checked to see if the current
>>>> domain is equal to the hardware_domain, change this cases to
>>>> use is_hardware_domain() function instead. >
>>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>>> ---
>>>> v4 changes:
>>>> - removed unneeded check for domain NULL from is_hardware_domain
>>>> introduced in v3
>>>
>>> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
>>>
>>> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
>>>
>>> Can you propose a new commit message?
>> Hi Julien,
>> Yes I agree, what about:
>> xen/arm: Reinforce use of is_hardware_domain
>> Among the common and arm codebase there are few cases where
>
> I would drop 'common' because you are only modifying the arm codebase.
>
>> the hardware_domain variable is checked to see if the current
>> domain is equal to the hardware_domain, change this cases to
>> use is_hardware_domain() function instead.
>
>
>> In the eventuality that hardware_domain is NULL, is_hardware_domain
>> will return false because an analysis of the common and arm codebase
>> shows that is_hardware_domain is called always with a non NULL
>> domain pointer.
>
> This paragraph seems to come out of the blue. I would drop it.
>
> How about:
>
> "
> There are a few places on Arm where we use pretty much an open-coded version of is_hardware_domain(). The main difference, is the helper will also block speculation (not yet implemented on Arm).
>
> The existing users are not in hot path, so blocking speculation would not hurt when it is implemented. So remove the open-coded version within the arm codebase.
> "
>
> If you are happy with the commit message, I will commit it the series tomorrow (to give an opportunity to Stefano to review).
>
Hi Julien,
Yes your version is much better, thank you very much!
Cheers,
Luca
> Cheers,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 13:47 ` Luca Fancellu
@ 2021-04-14 20:35 ` Stefano Stabellini
2021-04-15 17:17 ` Julien Grall
0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2021-04-14 20:35 UTC (permalink / raw)
To: Luca Fancellu
Cc: Julien Grall, xen-devel, Bertrand Marquis, wei.chen,
Stefano Stabellini, Volodymyr Babchuk, Rahul Singh
On Wed, 14 Apr 2021, Luca Fancellu wrote:
> > On 14 Apr 2021, at 14:45, Julien Grall <julien@xen.org> wrote:
> >
> > Hi Luca,
> >
> > On 14/04/2021 12:29, Luca Fancellu wrote:
> >>> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
> >>>
> >>> Hi Luca,
> >>>
> >>> On 14/04/2021 10:14, Luca Fancellu wrote:
> >>>> Among the common and arm codebase there are few cases where
> >>>> the hardware_domain variable is checked to see if the current
> >>>> domain is equal to the hardware_domain, change this cases to
> >>>> use is_hardware_domain() function instead. >
> >>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> >>>> ---
> >>>> v4 changes:
> >>>> - removed unneeded check for domain NULL from is_hardware_domain
> >>>> introduced in v3
> >>>
> >>> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
> >>>
> >>> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
> >>>
> >>> Can you propose a new commit message?
> >> Hi Julien,
> >> Yes I agree, what about:
> >> xen/arm: Reinforce use of is_hardware_domain
> >> Among the common and arm codebase there are few cases where
> >
> > I would drop 'common' because you are only modifying the arm codebase.
> >
> >> the hardware_domain variable is checked to see if the current
> >> domain is equal to the hardware_domain, change this cases to
> >> use is_hardware_domain() function instead.
> >
> >
> >> In the eventuality that hardware_domain is NULL, is_hardware_domain
> >> will return false because an analysis of the common and arm codebase
> >> shows that is_hardware_domain is called always with a non NULL
> >> domain pointer.
> >
> > This paragraph seems to come out of the blue. I would drop it.
> >
> > How about:
> >
> > "
> > There are a few places on Arm where we use pretty much an open-coded version of is_hardware_domain(). The main difference, is the helper will also block speculation (not yet implemented on Arm).
> >
> > The existing users are not in hot path, so blocking speculation would not hurt when it is implemented. So remove the open-coded version within the arm codebase.
> > "
> >
> > If you are happy with the commit message, I will commit it the series tomorrow (to give an opportunity to Stefano to review).
> >
>
> Hi Julien,
>
> Yes your version is much better, thank you very much!
It looks great, thanks for your work on this!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL
2021-04-14 20:35 ` Stefano Stabellini
@ 2021-04-15 17:17 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2021-04-15 17:17 UTC (permalink / raw)
To: Stefano Stabellini, Luca Fancellu
Cc: xen-devel, Bertrand Marquis, wei.chen, Volodymyr Babchuk, Rahul Singh
Hi,
On 14/04/2021 21:35, Stefano Stabellini wrote:
> On Wed, 14 Apr 2021, Luca Fancellu wrote:
>>> On 14 Apr 2021, at 14:45, Julien Grall <julien@xen.org> wrote:
>>>
>>> Hi Luca,
>>>
>>> On 14/04/2021 12:29, Luca Fancellu wrote:
>>>>> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
>>>>>
>>>>> Hi Luca,
>>>>>
>>>>> On 14/04/2021 10:14, Luca Fancellu wrote:
>>>>>> Among the common and arm codebase there are few cases where
>>>>>> the hardware_domain variable is checked to see if the current
>>>>>> domain is equal to the hardware_domain, change this cases to
>>>>>> use is_hardware_domain() function instead. >
>>>>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>>>>> ---
>>>>>> v4 changes:
>>>>>> - removed unneeded check for domain NULL from is_hardware_domain
>>>>>> introduced in v3
>>>>>
>>>>> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
>>>>>
>>>>> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
>>>>>
>>>>> Can you propose a new commit message?
>>>> Hi Julien,
>>>> Yes I agree, what about:
>>>> xen/arm: Reinforce use of is_hardware_domain
>>>> Among the common and arm codebase there are few cases where
>>>
>>> I would drop 'common' because you are only modifying the arm codebase.
>>>
>>>> the hardware_domain variable is checked to see if the current
>>>> domain is equal to the hardware_domain, change this cases to
>>>> use is_hardware_domain() function instead.
>>>
>>>
>>>> In the eventuality that hardware_domain is NULL, is_hardware_domain
>>>> will return false because an analysis of the common and arm codebase
>>>> shows that is_hardware_domain is called always with a non NULL
>>>> domain pointer.
>>>
>>> This paragraph seems to come out of the blue. I would drop it.
>>>
>>> How about:
>>>
>>> "
>>> There are a few places on Arm where we use pretty much an open-coded version of is_hardware_domain(). The main difference, is the helper will also block speculation (not yet implemented on Arm).
>>>
>>> The existing users are not in hot path, so blocking speculation would not hurt when it is implemented. So remove the open-coded version within the arm codebase.
>>> "
>>>
>>> If you are happy with the commit message, I will commit it the series tomorrow (to give an opportunity to Stefano to review).
>>>
>>
>> Hi Julien,
>>
>> Yes your version is much better, thank you very much!
>
> It looks great, thanks for your work on this!
I have committed the series. Thanks for the work!
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs()
2021-04-14 9:14 [PATCH v4 0/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
2021-04-14 9:14 ` [PATCH v4 1/4] xen/arm: Move dom0 creation in domain_build.c Luca Fancellu
2021-04-14 9:14 ` [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL Luca Fancellu
@ 2021-04-14 9:14 ` Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
2021-04-14 11:17 ` Julien Grall
2021-04-14 9:14 ` [PATCH v4 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
3 siblings, 2 replies; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 9:14 UTC (permalink / raw)
To: xen-devel
Cc: bertrand.marquis, wei.chen, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk
This patch adds a comment in create_domUs() right before
domain_create() to explain the importance of the pre-increment
operator on the variable max_init_domid, to ensure that the
domid 0 is allocated only during start_xen() function by the
create_dom0() and not on any other possible code path to the
domain_create() function.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes in v4:
- Change to the commit title
Changes in v3:
- removed check introduced in v2.
---
xen/arch/arm/domain_build.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 359957dc1b..b1d7b9849f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2508,6 +2508,11 @@ void __init create_domUs(void)
GUEST_VPL011_SPI - 32 + 1);
}
+ /*
+ * The variable max_init_domid is initialized with zero, so here it's
+ * very important to use the pre-increment operator to call
+ * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
+ */
d = domain_create(++max_init_domid, &d_cfg, false);
if ( IS_ERR(d) )
panic("Error creating domain %s\n", dt_node_name(node));
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs()
2021-04-14 9:14 ` [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs() Luca Fancellu
@ 2021-04-14 10:14 ` Bertrand Marquis
2021-04-14 11:17 ` Julien Grall
1 sibling, 0 replies; 16+ messages in thread
From: Bertrand Marquis @ 2021-04-14 10:14 UTC (permalink / raw)
To: Luca Fancellu
Cc: xen-devel, Wei Chen, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Hi Luca,
> On 14 Apr 2021, at 10:14, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>
> This patch adds a comment in create_domUs() right before
> domain_create() to explain the importance of the pre-increment
> operator on the variable max_init_domid, to ensure that the
> domid 0 is allocated only during start_xen() function by the
> create_dom0() and not on any other possible code path to the
> domain_create() function.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers,
Bertrand
> ---
> Changes in v4:
> - Change to the commit title
> Changes in v3:
> - removed check introduced in v2.
> ---
> xen/arch/arm/domain_build.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 359957dc1b..b1d7b9849f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2508,6 +2508,11 @@ void __init create_domUs(void)
> GUEST_VPL011_SPI - 32 + 1);
> }
>
> + /*
> + * The variable max_init_domid is initialized with zero, so here it's
> + * very important to use the pre-increment operator to call
> + * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
> + */
> d = domain_create(++max_init_domid, &d_cfg, false);
> if ( IS_ERR(d) )
> panic("Error creating domain %s\n", dt_node_name(node));
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs()
2021-04-14 9:14 ` [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs() Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
@ 2021-04-14 11:17 ` Julien Grall
1 sibling, 0 replies; 16+ messages in thread
From: Julien Grall @ 2021-04-14 11:17 UTC (permalink / raw)
To: Luca Fancellu, xen-devel
Cc: bertrand.marquis, wei.chen, Stefano Stabellini, Volodymyr Babchuk
On 14/04/2021 10:14, Luca Fancellu wrote:
> This patch adds a comment in create_domUs() right before
> domain_create() to explain the importance of the pre-increment
> operator on the variable max_init_domid, to ensure that the
> domid 0 is allocated only during start_xen() function by the
> create_dom0() and not on any other possible code path to the
> domain_create() function.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
> ---
> Changes in v4:
> - Change to the commit title
> Changes in v3:
> - removed check introduced in v2.
> ---
> xen/arch/arm/domain_build.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 359957dc1b..b1d7b9849f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2508,6 +2508,11 @@ void __init create_domUs(void)
> GUEST_VPL011_SPI - 32 + 1);
> }
>
> + /*
> + * The variable max_init_domid is initialized with zero, so here it's
> + * very important to use the pre-increment operator to call
> + * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
> + */
> d = domain_create(++max_init_domid, &d_cfg, false);
> if ( IS_ERR(d) )
> panic("Error creating domain %s\n", dt_node_name(node));
>
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less
2021-04-14 9:14 [PATCH v4 0/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
` (2 preceding siblings ...)
2021-04-14 9:14 ` [PATCH v4 3/4] xen/arm: Clarify how the domid is decided in create_domUs() Luca Fancellu
@ 2021-04-14 9:14 ` Luca Fancellu
2021-04-14 10:14 ` Bertrand Marquis
3 siblings, 1 reply; 16+ messages in thread
From: Luca Fancellu @ 2021-04-14 9:14 UTC (permalink / raw)
To: xen-devel
Cc: bertrand.marquis, wei.chen, Andrew Cooper, George Dunlap,
Ian Jackson, Jan Beulich, Julien Grall, Stefano Stabellini,
Wei Liu, Volodymyr Babchuk
This patch prevents the dom0 to be loaded skipping its
building and going forward to build domUs when the dom0
kernel is not found and at least one domU is present.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
v3 changes:
- Rephrase documentation
---
docs/features/dom0less.pandoc | 7 +++---
xen/arch/arm/setup.c | 42 ++++++++++++++++++++++++++++++++---
2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/docs/features/dom0less.pandoc b/docs/features/dom0less.pandoc
index d798596cdf..c9edb529e1 100644
--- a/docs/features/dom0less.pandoc
+++ b/docs/features/dom0less.pandoc
@@ -16,9 +16,10 @@ Multiboot specification has been extended to allow for multiple domains
to be passed to Xen. See docs/misc/arm/device-tree/booting.txt for more
information about the Multiboot specification and how to use it.
-Currently, a control domain ("dom0") is still required, but in the
-future it will become unnecessary when all domains are created
-directly from Xen. Instead of waiting for the control domain to be fully
+Currently, a control domain ("dom0") is still required to manage the DomU
+domains, but the system can start also without dom0 if the Device Tree
+doesn't specify the dom0 kernel and it declares one or more domUs.
+Instead of waiting for the control domain (when declared) to be fully
booted and the Xen tools to become available, domains created by Xen
this way are started right away in parallel. Hence, their boot time is
typically much shorter.
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index b405f58996..ecc4f0ae98 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -793,6 +793,38 @@ static void __init setup_mm(void)
}
#endif
+static bool __init is_dom0less_mode(void)
+{
+ struct bootmodules *mods = &bootinfo.modules;
+ struct bootmodule *mod;
+ unsigned int i;
+ bool dom0found = false;
+ bool domUfound = false;
+
+ /* Look into the bootmodules */
+ for ( i = 0 ; i < mods->nr_mods ; i++ )
+ {
+ mod = &mods->module[i];
+ /* Find if dom0 and domU kernels are present */
+ if ( mod->kind == BOOTMOD_KERNEL )
+ {
+ if ( mod->domU == false )
+ {
+ dom0found = true;
+ break;
+ }
+ else
+ domUfound = true;
+ }
+ }
+
+ /*
+ * If there is no dom0 kernel but at least one domU, then we are in
+ * dom0less mode
+ */
+ return ( !dom0found && domUfound );
+}
+
size_t __read_mostly dcache_line_bytes;
/* C entry point for boot CPU */
@@ -803,7 +835,7 @@ void __init start_xen(unsigned long boot_phys_offset,
int cpus, i;
const char *cmdline;
struct bootmodule *xen_bootmodule;
- struct domain *dom0;
+ struct domain *dom0 = NULL;
int rc;
dcache_line_bytes = read_dcache_line_bytes();
@@ -958,7 +990,10 @@ void __init start_xen(unsigned long boot_phys_offset,
enable_errata_workarounds();
/* Create initial domain 0. */
- dom0 = create_dom0();
+ if ( !is_dom0less_mode() )
+ dom0 = create_dom0();
+ else
+ printk(XENLOG_INFO "Xen dom0less mode detected\n");
heap_init_late();
@@ -976,7 +1011,8 @@ void __init start_xen(unsigned long boot_phys_offset,
if ( acpi_disabled )
create_domUs();
- domain_unpause_by_systemcontroller(dom0);
+ if ( dom0 )
+ domain_unpause_by_systemcontroller(dom0);
/* Switch on to the dynamically allocated stack for the idle vcpu
* since the static one we're running on is about to be freed. */
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v4 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less
2021-04-14 9:14 ` [PATCH v4 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less Luca Fancellu
@ 2021-04-14 10:14 ` Bertrand Marquis
0 siblings, 0 replies; 16+ messages in thread
From: Bertrand Marquis @ 2021-04-14 10:14 UTC (permalink / raw)
To: Luca Fancellu
Cc: Xen-devel, Wei Chen, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu,
Volodymyr Babchuk
Hi Luca,
> On 14 Apr 2021, at 10:14, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>
> This patch prevents the dom0 to be loaded skipping its
> building and going forward to build domUs when the dom0
> kernel is not found and at least one domU is present.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers,
Bertrand
> ---
> v3 changes:
> - Rephrase documentation
> ---
> docs/features/dom0less.pandoc | 7 +++---
> xen/arch/arm/setup.c | 42 ++++++++++++++++++++++++++++++++---
> 2 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/docs/features/dom0less.pandoc b/docs/features/dom0less.pandoc
> index d798596cdf..c9edb529e1 100644
> --- a/docs/features/dom0less.pandoc
> +++ b/docs/features/dom0less.pandoc
> @@ -16,9 +16,10 @@ Multiboot specification has been extended to allow for multiple domains
> to be passed to Xen. See docs/misc/arm/device-tree/booting.txt for more
> information about the Multiboot specification and how to use it.
>
> -Currently, a control domain ("dom0") is still required, but in the
> -future it will become unnecessary when all domains are created
> -directly from Xen. Instead of waiting for the control domain to be fully
> +Currently, a control domain ("dom0") is still required to manage the DomU
> +domains, but the system can start also without dom0 if the Device Tree
> +doesn't specify the dom0 kernel and it declares one or more domUs.
> +Instead of waiting for the control domain (when declared) to be fully
> booted and the Xen tools to become available, domains created by Xen
> this way are started right away in parallel. Hence, their boot time is
> typically much shorter.
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index b405f58996..ecc4f0ae98 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -793,6 +793,38 @@ static void __init setup_mm(void)
> }
> #endif
>
> +static bool __init is_dom0less_mode(void)
> +{
> + struct bootmodules *mods = &bootinfo.modules;
> + struct bootmodule *mod;
> + unsigned int i;
> + bool dom0found = false;
> + bool domUfound = false;
> +
> + /* Look into the bootmodules */
> + for ( i = 0 ; i < mods->nr_mods ; i++ )
> + {
> + mod = &mods->module[i];
> + /* Find if dom0 and domU kernels are present */
> + if ( mod->kind == BOOTMOD_KERNEL )
> + {
> + if ( mod->domU == false )
> + {
> + dom0found = true;
> + break;
> + }
> + else
> + domUfound = true;
> + }
> + }
> +
> + /*
> + * If there is no dom0 kernel but at least one domU, then we are in
> + * dom0less mode
> + */
> + return ( !dom0found && domUfound );
> +}
> +
> size_t __read_mostly dcache_line_bytes;
>
> /* C entry point for boot CPU */
> @@ -803,7 +835,7 @@ void __init start_xen(unsigned long boot_phys_offset,
> int cpus, i;
> const char *cmdline;
> struct bootmodule *xen_bootmodule;
> - struct domain *dom0;
> + struct domain *dom0 = NULL;
> int rc;
>
> dcache_line_bytes = read_dcache_line_bytes();
> @@ -958,7 +990,10 @@ void __init start_xen(unsigned long boot_phys_offset,
> enable_errata_workarounds();
>
> /* Create initial domain 0. */
> - dom0 = create_dom0();
> + if ( !is_dom0less_mode() )
> + dom0 = create_dom0();
> + else
> + printk(XENLOG_INFO "Xen dom0less mode detected\n");
>
> heap_init_late();
>
> @@ -976,7 +1011,8 @@ void __init start_xen(unsigned long boot_phys_offset,
> if ( acpi_disabled )
> create_domUs();
>
> - domain_unpause_by_systemcontroller(dom0);
> + if ( dom0 )
> + domain_unpause_by_systemcontroller(dom0);
>
> /* Switch on to the dynamically allocated stack for the idle vcpu
> * since the static one we're running on is about to be freed. */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread