* [PATCH v3 1/3] tools/libxl: Rename libxl__arch_domain_build_info_acpi_setdefault to...
2018-10-01 18:57 [PATCH v3 0/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
@ 2018-10-01 18:57 ` Julien Grall
2018-10-01 18:57 ` [PATCH v2 1/2] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:57 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Julien Grall, sstabellini, ian.jackson, roger.pau
libxl__arch_domain_build_info_setdefault
A follow-up will require to modify default of multiple fields of
build_info. So rename the function accordingly.
No functional change.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in v3:
- Add Roger's reviewed-by
- Add Wei's acked-by
---
tools/libxl/libxl_arch.h | 3 +--
tools/libxl/libxl_arm.c | 4 ++--
tools/libxl/libxl_create.c | 2 +-
tools/libxl/libxl_x86.c | 3 +--
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index c8ccaaf14c..5ab0c95974 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -65,8 +65,7 @@ _hidden
int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);
_hidden
-void libxl__arch_domain_build_info_acpi_setdefault(
- libxl_domain_build_info *b_info);
+void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info);
_hidden
int libxl__arch_extra_memory(libxl__gc *gc,
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index baa0d38e01..699fd9ddc6 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1110,9 +1110,9 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq)
return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
}
-void libxl__arch_domain_build_info_acpi_setdefault(
- libxl_domain_build_info *b_info)
+void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
{
+ /* ACPI is disabled by default */
libxl_defbool_setdefault(&b_info->acpi, false);
}
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index dcfde7787e..580320d272 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -215,7 +215,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (!b_info->event_channels)
b_info->event_channels = 1023;
- libxl__arch_domain_build_info_acpi_setdefault(b_info);
+ libxl__arch_domain_build_info_setdefault(b_info);
libxl_defbool_setdefault(&b_info->dm_restrict, false);
switch (b_info->type) {
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 6f670b03b5..81523a568f 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -613,8 +613,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
return rc;
}
-void libxl__arch_domain_build_info_acpi_setdefault(
- libxl_domain_build_info *b_info)
+void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
{
libxl_defbool_setdefault(&b_info->acpi, true);
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 1/2] xen/arm: vgic-v3: Delay the initialization of the domain information
2018-10-01 18:57 [PATCH v3 0/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
2018-10-01 18:57 ` [PATCH v3 1/3] tools/libxl: Rename libxl__arch_domain_build_info_acpi_setdefault to Julien Grall
@ 2018-10-01 18:57 ` Julien Grall
2018-10-01 18:58 ` Julien Grall
2018-10-01 18:57 ` [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline Julien Grall
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:57 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, Andrew Cooper, ian.jackson, Julien Grall,
roger.pau
A follow-up patch will require to know the number of vCPUs when
initializating the vGICv3 domain structure. However this information is
not available at domain creation. This is only known once
XEN_DOMCTL_max_vpus is called for that domain.
In order to get the max vCPUs around, delay the domain part of the vGIC
v3 initialization until the first vCPU of the domain is initialized.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
This is nasty but I can't find a better way for Xen 4.11 and older. We
still need it for unstable because the number of vCPUs is not known in
arch_domain_init. There are discussion to rework the domain creation a
bit further but I would hope to fix the bug first.
Andrew, I have CCed you to know whether you have a better idea where to
place this call on Xen 4.11 and older.
Changes in v2:
- The patch is also needed for the time being on unstable
- Add Stefano's recently invented tag
- Add Shameer's tested tag
---
xen/arch/arm/vgic-v3.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 4b42739a52..df1bab3a35 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1573,9 +1573,11 @@ static const struct mmio_handler_ops vgic_distr_mmio_handler = {
.write = vgic_v3_distr_mmio_write,
};
+static int vgic_v3_real_domain_init(struct domain *d);
+
static int vgic_v3_vcpu_init(struct vcpu *v)
{
- int i;
+ int i, rc;
paddr_t rdist_base;
struct vgic_rdist_region *region;
unsigned int last_cpu;
@@ -1584,6 +1586,19 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
struct domain *d = v->domain;
/*
+ * This is the earliest place where the number of vCPUs is
+ * known. This is required to initialize correctly the vGIC v3
+ * domain structure. We only to do that when vCPU 0 is
+ * initilialized.
+ */
+ if ( v->vcpu_id == 0 )
+ {
+ rc = vgic_v3_real_domain_init(d);
+ if ( rc )
+ return rc;
+ }
+
+ /*
* Find the region where the re-distributor lives. For this purpose,
* we look one region ahead as we have only the first CPU in hand.
*/
@@ -1641,7 +1656,7 @@ static inline unsigned int vgic_v3_rdist_count(struct domain *d)
GUEST_GICV3_RDIST_REGIONS;
}
-static int vgic_v3_domain_init(struct domain *d)
+static int vgic_v3_real_domain_init(struct domain *d)
{
struct vgic_rdist_region *rdist_regions;
int rdist_count, i, ret;
@@ -1733,6 +1748,16 @@ static int vgic_v3_domain_init(struct domain *d)
return 0;
}
+static int vgic_v3_domain_init(struct domain *d)
+{
+ /*
+ * The domain initialization for vGIC v3 is delayed until the first vCPU
+ * is created. This because the initialization may require to know the
+ * number of vCPUs that is not known when creating the domain.
+ */
+ return 0;
+}
+
static void vgic_v3_domain_free(struct domain *d)
{
vgic_v3_its_free_domain(d);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/2] xen/arm: vgic-v3: Delay the initialization of the domain information
2018-10-01 18:57 ` [PATCH v2 1/2] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall
@ 2018-10-01 18:58 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:58 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, sstabellini, ian.jackson, Andrew Cooper, roger.pau
Hi,
I forgot to remove patch from the previous series before sending the new
one.
Please ignore that patch. Sorry for the noise.
Cheers,
On 10/01/2018 07:57 PM, Julien Grall wrote:
> A follow-up patch will require to know the number of vCPUs when
> initializating the vGICv3 domain structure. However this information is
> not available at domain creation. This is only known once
> XEN_DOMCTL_max_vpus is called for that domain.
>
> In order to get the max vCPUs around, delay the domain part of the vGIC
> v3 initialization until the first vCPU of the domain is initialized.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org>
>
> ---
>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>
> This is nasty but I can't find a better way for Xen 4.11 and older. We
> still need it for unstable because the number of vCPUs is not known in
> arch_domain_init. There are discussion to rework the domain creation a
> bit further but I would hope to fix the bug first.
>
> Andrew, I have CCed you to know whether you have a better idea where to
> place this call on Xen 4.11 and older.
>
> Changes in v2:
> - The patch is also needed for the time being on unstable
> - Add Stefano's recently invented tag
> - Add Shameer's tested tag
> ---
> xen/arch/arm/vgic-v3.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 4b42739a52..df1bab3a35 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1573,9 +1573,11 @@ static const struct mmio_handler_ops vgic_distr_mmio_handler = {
> .write = vgic_v3_distr_mmio_write,
> };
>
> +static int vgic_v3_real_domain_init(struct domain *d);
> +
> static int vgic_v3_vcpu_init(struct vcpu *v)
> {
> - int i;
> + int i, rc;
> paddr_t rdist_base;
> struct vgic_rdist_region *region;
> unsigned int last_cpu;
> @@ -1584,6 +1586,19 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
> struct domain *d = v->domain;
>
> /*
> + * This is the earliest place where the number of vCPUs is
> + * known. This is required to initialize correctly the vGIC v3
> + * domain structure. We only to do that when vCPU 0 is
> + * initilialized.
> + */
> + if ( v->vcpu_id == 0 )
> + {
> + rc = vgic_v3_real_domain_init(d);
> + if ( rc )
> + return rc;
> + }
> +
> + /*
> * Find the region where the re-distributor lives. For this purpose,
> * we look one region ahead as we have only the first CPU in hand.
> */
> @@ -1641,7 +1656,7 @@ static inline unsigned int vgic_v3_rdist_count(struct domain *d)
> GUEST_GICV3_RDIST_REGIONS;
> }
>
> -static int vgic_v3_domain_init(struct domain *d)
> +static int vgic_v3_real_domain_init(struct domain *d)
> {
> struct vgic_rdist_region *rdist_regions;
> int rdist_count, i, ret;
> @@ -1733,6 +1748,16 @@ static int vgic_v3_domain_init(struct domain *d)
> return 0;
> }
>
> +static int vgic_v3_domain_init(struct domain *d)
> +{
> + /*
> + * The domain initialization for vGIC v3 is delayed until the first vCPU
> + * is created. This because the initialization may require to know the
> + * number of vCPUs that is not known when creating the domain.
> + */
> + return 0;
> +}
> +
> static void vgic_v3_domain_free(struct domain *d)
> {
> vgic_v3_its_free_domain(d);
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline
2018-10-01 18:57 [PATCH v3 0/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
2018-10-01 18:57 ` [PATCH v3 1/3] tools/libxl: Rename libxl__arch_domain_build_info_acpi_setdefault to Julien Grall
2018-10-01 18:57 ` [PATCH v2 1/2] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall
@ 2018-10-01 18:57 ` Julien Grall
2018-10-02 8:22 ` Roger Pau Monné
2018-10-03 10:35 ` Wei Liu
2018-10-01 18:57 ` [PATCH v2 2/2] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall
2018-10-01 18:57 ` [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
4 siblings, 2 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:57 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Julien Grall, sstabellini, ian.jackson, roger.pau
The PV fields kernel, ramdisk, cmdline are only there for compatibility
with old toolstack. Instead of manually copying them over to there new
field, use the deprecated_by attribute in the IDL.
Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
- Patch added
---
tools/libxl/libxl_create.c | 19 -------------------
tools/libxl/libxl_types.idl | 6 +++---
2 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 580320d272..fe97eebdea 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -368,25 +368,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
b_info->shadow_memkb = 0;
if (b_info->u.pv.slack_memkb == LIBXL_MEMKB_DEFAULT)
b_info->u.pv.slack_memkb = 0;
-
- /* For compatibility, fill in b_info->kernel|ramdisk|cmdline
- * with the value in u.pv, later processing will use
- * b_info->kernel|ramdisk|cmdline only.
- * User with old APIs that passes u.pv.kernel|ramdisk|cmdline
- * is not affected.
- */
- if (!b_info->kernel && b_info->u.pv.kernel) {
- b_info->kernel = b_info->u.pv.kernel;
- b_info->u.pv.kernel = NULL;
- }
- if (!b_info->ramdisk && b_info->u.pv.ramdisk) {
- b_info->ramdisk = b_info->u.pv.ramdisk;
- b_info->u.pv.ramdisk = NULL;
- }
- if (!b_info->cmdline && b_info->u.pv.cmdline) {
- b_info->cmdline = b_info->u.pv.cmdline;
- b_info->u.pv.cmdline = NULL;
- }
break;
case LIBXL_DOMAIN_TYPE_PVH:
libxl_defbool_setdefault(&b_info->u.pvh.pvshim, false);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2cceb8c057..3b8f967651 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -588,12 +588,12 @@ libxl_domain_build_info = Struct("domain_build_info",[
("rdm_mem_boundary_memkb", MemKB),
("mca_caps", uint64),
])),
- ("pv", Struct(None, [("kernel", string),
+ ("pv", Struct(None, [("kernel", string, {'deprecated_by': 'kernel'}),
("slack_memkb", MemKB),
("bootloader", string, {'deprecated_by': 'bootloader'}),
("bootloader_args", libxl_string_list, {'deprecated_by': 'bootloader_args'}),
- ("cmdline", string),
- ("ramdisk", string),
+ ("cmdline", string, {'deprecated_by': 'cmdline'}),
+ ("ramdisk", string, {'deprecated_by': 'ramdisk'}),
("features", string, {'const': True}),
# Use host's E820 for PCI passthrough.
("e820_host", libxl_defbool),
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline
2018-10-01 18:57 ` [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline Julien Grall
@ 2018-10-02 8:22 ` Roger Pau Monné
2018-10-03 10:35 ` Wei Liu
1 sibling, 0 replies; 16+ messages in thread
From: Roger Pau Monné @ 2018-10-02 8:22 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, ian.jackson, wei.liu2
On Mon, Oct 01, 2018 at 07:57:19PM +0100, Julien Grall wrote:
> The PV fields kernel, ramdisk, cmdline are only there for compatibility
> with old toolstack. Instead of manually copying them over to there new
> field, use the deprecated_by attribute in the IDL.
>
> Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline
2018-10-01 18:57 ` [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline Julien Grall
2018-10-02 8:22 ` Roger Pau Monné
@ 2018-10-03 10:35 ` Wei Liu
1 sibling, 0 replies; 16+ messages in thread
From: Wei Liu @ 2018-10-03 10:35 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, ian.jackson, wei.liu2, roger.pau
On Mon, Oct 01, 2018 at 07:57:19PM +0100, Julien Grall wrote:
> The PV fields kernel, ramdisk, cmdline are only there for compatibility
> with old toolstack. Instead of manually copying them over to there new
> field, use the deprecated_by attribute in the IDL.
>
> Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] xen/arm: vgic-v3: Don't create empty re-distributor regions
2018-10-01 18:57 [PATCH v3 0/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
` (2 preceding siblings ...)
2018-10-01 18:57 ` [PATCH v3 2/3] tools/libxl: Deprecate PV fields kernel, ramdisk, cmdline Julien Grall
@ 2018-10-01 18:57 ` Julien Grall
2018-10-01 18:59 ` Julien Grall
2018-10-01 18:57 ` [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
4 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:57 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Julien Grall, sstabellini, ian.jackson, roger.pau
At the moment, Xen is assuming the hardware domain will have the same
number of re-distributor regions as the host. However, as the
number of CPUs or the stride (e.g on GICv4) may be different we end up
exposing regions which does not contain any re-distributors.
When booting, Linux will go through all the re-distributor region to
check whether a property (e.g vPLIs) is available accross all the
re-distributors. This will result to a data abort on empty regions
because there are no underlying re-distributor.
So we need to limit the number of regions exposed to the hardware
domain. The code reworked to only expose the minimun number of regions
required by the hardware domain. It is assumed the regions will be
populated starting from the first one.
Lastly, rename vgic_v3_rdist_count to reflect the value return by the
helper.
Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
Changes in v2:
- Rename vgic_v3_rdist_count to vgic_v3_max_rdist_count
- Fixup #re-distributors
- Fix typoes
- Add Shameer's tested tag
---
xen/arch/arm/gic-v3.c | 14 +++++++++++---
xen/arch/arm/vgic-v3.c | 21 ++++++++++++++++++---
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index c98a163ee7..2c1454f425 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1265,7 +1265,8 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
if ( res )
return res;
- res = fdt_property_cell(fdt, "#redistributor-regions", gicv3.rdist_count);
+ res = fdt_property_cell(fdt, "#redistributor-regions",
+ d->arch.vgic.nr_regions);
if ( res )
return res;
@@ -1274,8 +1275,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
* GIC has two memory regions: Distributor + rdist regions
* CPU interface and virtual cpu interfaces accessesed as System registers
* So cells are created only for Distributor and rdist regions
+ * The hardware domain may not use all the regions. So only copy
+ * what is necessary.
*/
- new_len = new_len * (gicv3.rdist_count + 1);
+ new_len = new_len * (d->arch.vgic.nr_regions + 1);
hw_reg = dt_get_property(gic, "reg", &len);
if ( !hw_reg )
@@ -1466,6 +1469,7 @@ static inline bool gic_dist_supports_dvis(void)
}
static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
+
{
struct acpi_subtable_header *header;
struct acpi_madt_generic_interrupt *host_gicc, *gicc;
@@ -1503,7 +1507,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
/* Add Generic Redistributor */
size = sizeof(struct acpi_madt_generic_redistributor);
- for ( i = 0; i < gicv3.rdist_count; i++ )
+ /*
+ * The hardware domain may not used all the regions. So only copy
+ * what is necessary.
+ */
+ for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
{
gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + table_len);
gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index df1bab3a35..efe824c6fb 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1640,7 +1640,11 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
return 0;
}
-static inline unsigned int vgic_v3_rdist_count(struct domain *d)
+/*
+ * Return the maximum number possible of re-distributor regions for
+ * a given domain.
+ */
+static inline unsigned int vgic_v3_max_rdist_count(struct domain *d)
{
/*
* Normally there is only one GICv3 redistributor region.
@@ -1662,7 +1666,7 @@ static int vgic_v3_real_domain_init(struct domain *d)
int rdist_count, i, ret;
/* Allocate memory for Re-distributor regions */
- rdist_count = vgic_v3_rdist_count(d);
+ rdist_count = vgic_v3_max_rdist_count(d);
rdist_regions = xzalloc_array(struct vgic_rdist_region, rdist_count);
if ( !rdist_regions )
@@ -1695,8 +1699,19 @@ static int vgic_v3_real_domain_init(struct domain *d)
d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
first_cpu += size / GICV3_GICR_SIZE;
+
+ if ( first_cpu >= d->max_vcpus )
+ break;
}
+ /*
+ * The hardware domain may not use all the re-distributors
+ * regions (e.g when the number of vCPUs does not match the
+ * number of pCPUs). Update the number of regions to avoid
+ * exposing unused region as they will not get emulated.
+ */
+ d->arch.vgic.nr_regions = i + 1;
+
d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
}
else
@@ -1825,7 +1840,7 @@ int vgic_v3_init(struct domain *d, int *mmio_count)
}
/* GICD region + number of Redistributors */
- *mmio_count = vgic_v3_rdist_count(d) + 1;
+ *mmio_count = vgic_v3_max_rdist_count(d) + 1;
/* one region per ITS */
*mmio_count += vgic_v3_its_count(d);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/2] xen/arm: vgic-v3: Don't create empty re-distributor regions
2018-10-01 18:57 ` [PATCH v2 2/2] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall
@ 2018-10-01 18:59 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:59 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, sstabellini, ian.jackson, roger.pau
Hi,
I forgot to remove patch from the previous series before sending the new
one.
Please ignore that patch. Sorry for the noise.
Cheers,
On 10/01/2018 07:57 PM, Julien Grall wrote:
> At the moment, Xen is assuming the hardware domain will have the same
> number of re-distributor regions as the host. However, as the
> number of CPUs or the stride (e.g on GICv4) may be different we end up
> exposing regions which does not contain any re-distributors.
>
> When booting, Linux will go through all the re-distributor region to
> check whether a property (e.g vPLIs) is available accross all the
> re-distributors. This will result to a data abort on empty regions
> because there are no underlying re-distributor.
>
> So we need to limit the number of regions exposed to the hardware
> domain. The code reworked to only expose the minimun number of regions
> required by the hardware domain. It is assumed the regions will be
> populated starting from the first one.
>
> Lastly, rename vgic_v3_rdist_count to reflect the value return by the
> helper.
>
> Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>
> ---
> Changes in v2:
> - Rename vgic_v3_rdist_count to vgic_v3_max_rdist_count
> - Fixup #re-distributors
> - Fix typoes
> - Add Shameer's tested tag
> ---
> xen/arch/arm/gic-v3.c | 14 +++++++++++---
> xen/arch/arm/vgic-v3.c | 21 ++++++++++++++++++---
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index c98a163ee7..2c1454f425 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1265,7 +1265,8 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
> if ( res )
> return res;
>
> - res = fdt_property_cell(fdt, "#redistributor-regions", gicv3.rdist_count);
> + res = fdt_property_cell(fdt, "#redistributor-regions",
> + d->arch.vgic.nr_regions);
> if ( res )
> return res;
>
> @@ -1274,8 +1275,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
> * GIC has two memory regions: Distributor + rdist regions
> * CPU interface and virtual cpu interfaces accessesed as System registers
> * So cells are created only for Distributor and rdist regions
> + * The hardware domain may not use all the regions. So only copy
> + * what is necessary.
> */
> - new_len = new_len * (gicv3.rdist_count + 1);
> + new_len = new_len * (d->arch.vgic.nr_regions + 1);
>
> hw_reg = dt_get_property(gic, "reg", &len);
> if ( !hw_reg )
> @@ -1466,6 +1469,7 @@ static inline bool gic_dist_supports_dvis(void)
> }
>
> static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> +
> {
> struct acpi_subtable_header *header;
> struct acpi_madt_generic_interrupt *host_gicc, *gicc;
> @@ -1503,7 +1507,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>
> /* Add Generic Redistributor */
> size = sizeof(struct acpi_madt_generic_redistributor);
> - for ( i = 0; i < gicv3.rdist_count; i++ )
> + /*
> + * The hardware domain may not used all the regions. So only copy
> + * what is necessary.
> + */
> + for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
> {
> gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + table_len);
> gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index df1bab3a35..efe824c6fb 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1640,7 +1640,11 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
> return 0;
> }
>
> -static inline unsigned int vgic_v3_rdist_count(struct domain *d)
> +/*
> + * Return the maximum number possible of re-distributor regions for
> + * a given domain.
> + */
> +static inline unsigned int vgic_v3_max_rdist_count(struct domain *d)
> {
> /*
> * Normally there is only one GICv3 redistributor region.
> @@ -1662,7 +1666,7 @@ static int vgic_v3_real_domain_init(struct domain *d)
> int rdist_count, i, ret;
>
> /* Allocate memory for Re-distributor regions */
> - rdist_count = vgic_v3_rdist_count(d);
> + rdist_count = vgic_v3_max_rdist_count(d);
>
> rdist_regions = xzalloc_array(struct vgic_rdist_region, rdist_count);
> if ( !rdist_regions )
> @@ -1695,8 +1699,19 @@ static int vgic_v3_real_domain_init(struct domain *d)
> d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
>
> first_cpu += size / GICV3_GICR_SIZE;
> +
> + if ( first_cpu >= d->max_vcpus )
> + break;
> }
>
> + /*
> + * The hardware domain may not use all the re-distributors
> + * regions (e.g when the number of vCPUs does not match the
> + * number of pCPUs). Update the number of regions to avoid
> + * exposing unused region as they will not get emulated.
> + */
> + d->arch.vgic.nr_regions = i + 1;
> +
> d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
> }
> else
> @@ -1825,7 +1840,7 @@ int vgic_v3_init(struct domain *d, int *mmio_count)
> }
>
> /* GICD region + number of Redistributors */
> - *mmio_count = vgic_v3_rdist_count(d) + 1;
> + *mmio_count = vgic_v3_max_rdist_count(d) + 1;
>
> /* one region per ITS */
> *mmio_count += vgic_v3_its_count(d);
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH
2018-10-01 18:57 [PATCH v3 0/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
` (3 preceding siblings ...)
2018-10-01 18:57 ` [PATCH v2 2/2] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall
@ 2018-10-01 18:57 ` Julien Grall
2018-10-02 8:56 ` Roger Pau Monné
2018-10-03 10:48 ` Wei Liu
4 siblings, 2 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-01 18:57 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Julien Grall, sstabellini, ian.jackson, roger.pau
Currently, the toolstack is considering Arm guest always PV. However,
they are very similar to PVH because HW virtualization extension are used
and QEMU is not started. So switch Arm guest type to PVH.
To keep compatibility with toolstack creating Arm guest with PV type
(e.g libvirt), libxl will now convert those guests to PVH.
Furthermore, the default type for Arm in xl will now be PVH to allow
smooth transition for user.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
This was discussed at Xen Summit and also in various thread on
xen-devel. The latest one was when Andrew sent a patch to deny guest creation
on Arm with XEN_DOMCTL_CDF_hap unset.
I suspect we first implemented Arm guest as PV in libxl because PVH was
non-existent and the type was easier to avoid spawning QEMU. Note that
Linux and Xen are already considering Arm guest as PVH.
Changes in v3:
- Properly reset u.pvh
- Update documentation and print
- Return ERROR_INVAL rather than ERROR_FAIL
Changes in v2:
- Rather than denying PV guest, convert them to PVH
---
docs/man/xl.cfg.pod.5.in | 5 +++--
tools/libxl/libxl_arch.h | 3 ++-
tools/libxl/libxl_arm.c | 26 ++++++++++++++++++++++++--
tools/libxl/libxl_create.c | 2 +-
tools/libxl/libxl_x86.c | 3 ++-
tools/xl/xl_parse.c | 4 ++++
6 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index b72718151b..b1c0be14cd 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -80,13 +80,14 @@ single host must be unique.
=item B<type="pv">
Specifies that this is to be a PV domain, suitable for hosting Xen-aware
-guest operating systems. This is the default.
+guest operating systems. This is the default on x86.
=item B<type="pvh">
Specifies that this is to be an PVH domain. That is a lightweight HVM-like
guest without a device model and without many of the emulated devices
-available to HVM guests. Note that this mode requires a PVH aware kernel.
+available to HVM guests. Note that this mode requires a PVH aware kernel on
+x86. This is the default on Arm.
=item B<type="hvm">
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 5ab0c95974..930570ef1e 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -65,7 +65,8 @@ _hidden
int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);
_hidden
-void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info);
+void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
+ libxl_domain_build_info *b_info);
_hidden
int libxl__arch_extra_memory(libxl__gc *gc,
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 699fd9ddc6..25dc3defc6 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -953,7 +953,11 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
int rc;
uint64_t val;
- assert(info->type == LIBXL_DOMAIN_TYPE_PV);
+ if (info->type != LIBXL_DOMAIN_TYPE_PVH) {
+ LOG(ERROR, "Unsupported Arm guest type %s",
+ libxl_domain_type_to_string(info->type));
+ return ERROR_INVAL;
+ }
/* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
val = MASK_INSR(HVM_PARAM_CALLBACK_TYPE_PPI,
@@ -1110,10 +1114,28 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq)
return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
}
-void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
+void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
+ libxl_domain_build_info *b_info)
{
/* ACPI is disabled by default */
libxl_defbool_setdefault(&b_info->acpi, false);
+
+ /*
+ * Arm guest are now considered as PVH by the toolstack. To allow
+ * compatibility with previous toolstack, PV guest are automatically
+ * converted to PVH.
+ */
+ if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
+ return;
+
+ LOG(WARN, "Converting PV guest to PVH.");
+ LOG(WARN, "Arm guest are now PVH.");
+ LOG(WARN, "Please fix your configuration file/toolstack.");
+
+ /* Re-initialize type to PVH and all associated fields to defaults. */
+ memset(&b_info->u, '\0', sizeof(b_info->u));
+ b_info->type = LIBXL_DOMAIN_TYPE_INVALID;
+ libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
}
/*
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index fe97eebdea..320dbed3c6 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -215,7 +215,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (!b_info->event_channels)
b_info->event_channels = 1023;
- libxl__arch_domain_build_info_setdefault(b_info);
+ libxl__arch_domain_build_info_setdefault(gc, b_info);
libxl_defbool_setdefault(&b_info->dm_restrict, false);
switch (b_info->type) {
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 81523a568f..8b6759c089 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -613,7 +613,8 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
return rc;
}
-void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
+void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
+ libxl_domain_build_info *b_info)
{
libxl_defbool_setdefault(&b_info->acpi, true);
}
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 971ec1bc56..0bda28152b 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1286,7 +1286,11 @@ void parse_config_data(const char *config_source,
}
if (c_info->type == LIBXL_DOMAIN_TYPE_INVALID)
+#if defined(__arm__) || defined(__aarch64__)
+ c_info->type = LIBXL_DOMAIN_TYPE_PVH;
+#else
c_info->type = LIBXL_DOMAIN_TYPE_PV;
+#endif
xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH
2018-10-01 18:57 ` [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
@ 2018-10-02 8:56 ` Roger Pau Monné
2018-10-02 10:09 ` Julien Grall
2018-10-03 10:48 ` Wei Liu
1 sibling, 1 reply; 16+ messages in thread
From: Roger Pau Monné @ 2018-10-02 8:56 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, ian.jackson, wei.liu2
On Mon, Oct 01, 2018 at 07:57:21PM +0100, Julien Grall wrote:
> Currently, the toolstack is considering Arm guest always PV. However,
> they are very similar to PVH because HW virtualization extension are used
> and QEMU is not started. So switch Arm guest type to PVH.
>
> To keep compatibility with toolstack creating Arm guest with PV type
> (e.g libvirt), libxl will now convert those guests to PVH.
>
> Furthermore, the default type for Arm in xl will now be PVH to allow
> smooth transition for user.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> ---
>
> This was discussed at Xen Summit and also in various thread on
> xen-devel. The latest one was when Andrew sent a patch to deny guest creation
> on Arm with XEN_DOMCTL_CDF_hap unset.
>
> I suspect we first implemented Arm guest as PV in libxl because PVH was
> non-existent and the type was easier to avoid spawning QEMU. Note that
> Linux and Xen are already considering Arm guest as PVH.
>
> Changes in v3:
> - Properly reset u.pvh
> - Update documentation and print
> - Return ERROR_INVAL rather than ERROR_FAIL
>
> Changes in v2:
> - Rather than denying PV guest, convert them to PVH
> ---
> docs/man/xl.cfg.pod.5.in | 5 +++--
> tools/libxl/libxl_arch.h | 3 ++-
> tools/libxl/libxl_arm.c | 26 ++++++++++++++++++++++++--
> tools/libxl/libxl_create.c | 2 +-
> tools/libxl/libxl_x86.c | 3 ++-
> tools/xl/xl_parse.c | 4 ++++
> 6 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index b72718151b..b1c0be14cd 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -80,13 +80,14 @@ single host must be unique.
> =item B<type="pv">
>
> Specifies that this is to be a PV domain, suitable for hosting Xen-aware
> -guest operating systems. This is the default.
> +guest operating systems. This is the default on x86.
>
> =item B<type="pvh">
>
> Specifies that this is to be an PVH domain. That is a lightweight HVM-like
> guest without a device model and without many of the emulated devices
> -available to HVM guests. Note that this mode requires a PVH aware kernel.
> +available to HVM guests. Note that this mode requires a PVH aware kernel on
> +x86. This is the default on Arm.
>
> =item B<type="hvm">
>
> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
> index 5ab0c95974..930570ef1e 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -65,7 +65,8 @@ _hidden
> int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);
>
> _hidden
> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info);
> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> + libxl_domain_build_info *b_info);
>
> _hidden
> int libxl__arch_extra_memory(libxl__gc *gc,
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 699fd9ddc6..25dc3defc6 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -953,7 +953,11 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> int rc;
> uint64_t val;
>
> - assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> + if (info->type != LIBXL_DOMAIN_TYPE_PVH) {
> + LOG(ERROR, "Unsupported Arm guest type %s",
> + libxl_domain_type_to_string(info->type));
> + return ERROR_INVAL;
> + }
>
> /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
> val = MASK_INSR(HVM_PARAM_CALLBACK_TYPE_PPI,
> @@ -1110,10 +1114,28 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq)
> return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
> }
>
> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> + libxl_domain_build_info *b_info)
> {
> /* ACPI is disabled by default */
> libxl_defbool_setdefault(&b_info->acpi, false);
> +
> + /*
> + * Arm guest are now considered as PVH by the toolstack. To allow
> + * compatibility with previous toolstack, PV guest are automatically
> + * converted to PVH.
> + */
> + if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
> + return;
> +
> + LOG(WARN, "Converting PV guest to PVH.");
> + LOG(WARN, "Arm guest are now PVH.");
> + LOG(WARN, "Please fix your configuration file/toolstack.");
> +
> + /* Re-initialize type to PVH and all associated fields to defaults. */
> + memset(&b_info->u, '\0', sizeof(b_info->u));
I don't really like the zeroing done here, but I think it's fine as
long as it's only done when converting from PV -> PVH and after
copying the deprecated fields.
Thansk, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH
2018-10-02 8:56 ` Roger Pau Monné
@ 2018-10-02 10:09 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-10-02 10:09 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, sstabellini, ian.jackson, wei.liu2
Hi Roger,
On 02/10/2018 09:56, Roger Pau Monné wrote:
> On Mon, Oct 01, 2018 at 07:57:21PM +0100, Julien Grall wrote:
>> Currently, the toolstack is considering Arm guest always PV. However,
>> they are very similar to PVH because HW virtualization extension are used
>> and QEMU is not started. So switch Arm guest type to PVH.
>>
>> To keep compatibility with toolstack creating Arm guest with PV type
>> (e.g libvirt), libxl will now convert those guests to PVH.
>>
>> Furthermore, the default type for Arm in xl will now be PVH to allow
>> smooth transition for user.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
>>
>> ---
>>
>> This was discussed at Xen Summit and also in various thread on
>> xen-devel. The latest one was when Andrew sent a patch to deny guest creation
>> on Arm with XEN_DOMCTL_CDF_hap unset.
>>
>> I suspect we first implemented Arm guest as PV in libxl because PVH was
>> non-existent and the type was easier to avoid spawning QEMU. Note that
>> Linux and Xen are already considering Arm guest as PVH.
>>
>> Changes in v3:
>> - Properly reset u.pvh
>> - Update documentation and print
>> - Return ERROR_INVAL rather than ERROR_FAIL
>>
>> Changes in v2:
>> - Rather than denying PV guest, convert them to PVH
>> ---
>> docs/man/xl.cfg.pod.5.in | 5 +++--
>> tools/libxl/libxl_arch.h | 3 ++-
>> tools/libxl/libxl_arm.c | 26 ++++++++++++++++++++++++--
>> tools/libxl/libxl_create.c | 2 +-
>> tools/libxl/libxl_x86.c | 3 ++-
>> tools/xl/xl_parse.c | 4 ++++
>> 6 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
>> index b72718151b..b1c0be14cd 100644
>> --- a/docs/man/xl.cfg.pod.5.in
>> +++ b/docs/man/xl.cfg.pod.5.in
>> @@ -80,13 +80,14 @@ single host must be unique.
>> =item B<type="pv">
>>
>> Specifies that this is to be a PV domain, suitable for hosting Xen-aware
>> -guest operating systems. This is the default.
>> +guest operating systems. This is the default on x86.
>>
>> =item B<type="pvh">
>>
>> Specifies that this is to be an PVH domain. That is a lightweight HVM-like
>> guest without a device model and without many of the emulated devices
>> -available to HVM guests. Note that this mode requires a PVH aware kernel.
>> +available to HVM guests. Note that this mode requires a PVH aware kernel on
>> +x86. This is the default on Arm.
>>
>> =item B<type="hvm">
>>
>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
>> index 5ab0c95974..930570ef1e 100644
>> --- a/tools/libxl/libxl_arch.h
>> +++ b/tools/libxl/libxl_arch.h
>> @@ -65,7 +65,8 @@ _hidden
>> int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);
>>
>> _hidden
>> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info);
>> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>> + libxl_domain_build_info *b_info);
>>
>> _hidden
>> int libxl__arch_extra_memory(libxl__gc *gc,
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index 699fd9ddc6..25dc3defc6 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -953,7 +953,11 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>> int rc;
>> uint64_t val;
>>
>> - assert(info->type == LIBXL_DOMAIN_TYPE_PV);
>> + if (info->type != LIBXL_DOMAIN_TYPE_PVH) {
>> + LOG(ERROR, "Unsupported Arm guest type %s",
>> + libxl_domain_type_to_string(info->type));
>> + return ERROR_INVAL;
>> + }
>>
>> /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
>> val = MASK_INSR(HVM_PARAM_CALLBACK_TYPE_PPI,
>> @@ -1110,10 +1114,28 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq)
>> return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
>> }
>>
>> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info *b_info)
>> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>> + libxl_domain_build_info *b_info)
>> {
>> /* ACPI is disabled by default */
>> libxl_defbool_setdefault(&b_info->acpi, false);
>> +
>> + /*
>> + * Arm guest are now considered as PVH by the toolstack. To allow
>> + * compatibility with previous toolstack, PV guest are automatically
>> + * converted to PVH.
>> + */
>> + if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
>> + return;
>> +
>> + LOG(WARN, "Converting PV guest to PVH.");
>> + LOG(WARN, "Arm guest are now PVH.");
>> + LOG(WARN, "Please fix your configuration file/toolstack.");
>> +
>> + /* Re-initialize type to PVH and all associated fields to defaults. */
>> + memset(&b_info->u, '\0', sizeof(b_info->u));
>
> I don't really like the zeroing done here, but I think it's fine as
> long as it's only done when converting from PV -> PVH and after
> copying the deprecated fields.
I don't think we can safely avoid to re-initialize b_info->u. Some
fields in u.pv are not initialized to 0 (e.g slack_memkb is initialized
to ~0ULL). So you will transfer that value over to pvh union and
altering the init values for PVH.
Furthermore, PVH fields may not be initialized to 0 (so far it is not
the case). So the memset here will ensure that all fields are 0, then
libxl_domain_build_info_init_type() will setup init value for non-zero
fields.
libxl__arch_domain_build_info_setdefault is called after deprecated
fields are copied and soon enough to not handle specific Guest type default.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH
2018-10-01 18:57 ` [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH Julien Grall
2018-10-02 8:56 ` Roger Pau Monné
@ 2018-10-03 10:48 ` Wei Liu
1 sibling, 0 replies; 16+ messages in thread
From: Wei Liu @ 2018-10-03 10:48 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, ian.jackson, wei.liu2, roger.pau
On Mon, Oct 01, 2018 at 07:57:21PM +0100, Julien Grall wrote:
> Currently, the toolstack is considering Arm guest always PV. However,
> they are very similar to PVH because HW virtualization extension are used
> and QEMU is not started. So switch Arm guest type to PVH.
>
> To keep compatibility with toolstack creating Arm guest with PV type
> (e.g libvirt), libxl will now convert those guests to PVH.
>
> Furthermore, the default type for Arm in xl will now be PVH to allow
> smooth transition for user.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread