From: Petr Beneš <w1benny@gmail.com>
Introduce a new altp2m_count parameter to control the maximum number of altp2m
views a domain can use. By default, if altp2m_count is unspecified and altp2m
is enabled, the value is set to 10, reflecting the legacy behavior.
This change is preparatory; it establishes the groundwork for the feature but
does not activate it.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h | 7 +++++++
tools/libs/light/libxl_create.c | 9 +++++++++
tools/libs/light/libxl_internal.h | 1 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 9 +++++++++
7 files changed, 30 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 191be87297..515a6ef10a 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1167,6 +1167,7 @@ if err := x.ArchX86.MsrRelaxed.fromC(&xc.arch_x86.msr_relaxed);err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
x.Altp2M = Altp2MMode(xc.altp2m)
+x.Altp2MCount = uint32(xc.altp2m_count)
x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
if err := x.Vpmu.fromC(&xc.vpmu);err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1695,6 +1696,7 @@ if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.altp2m_count = C.uint32_t(x.Altp2MCount)
xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
if err := x.Vpmu.toC(&xc.vpmu); err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 656933c6c9..1a4f076bd2 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -604,6 +604,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+Altp2MCount uint32
VmtraceBufKb int
Vpmu Defbool
TrapUnmappedAccesses Defbool
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index a8704e0268..d6b6e5d2dd 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1257,6 +1257,13 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_ALTP2M 1
+/*
+ * LIBXL_HAVE_ALTP2M_COUNT
+ * If this is defined, then libxl supports setting the maximum number of
+ * alternate p2m tables.
+ */
+#define LIBXL_HAVE_ALTP2M_COUNT 1
+
/*
* LIBXL_HAVE_REMUS
* If this is defined, then libxl supports remus.
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 8a85fba1cf..acf7fd9837 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
return -ERROR_INVAL;
}
+ if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
+ if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
+ b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
+ /* Reflect the default legacy count */
+ b_info->altp2m_count = 10;
+ else
+ b_info->altp2m_count = 0;
+ }
+
/* Assume that providing a bootloader user implies enabling restrict. */
libxl_defbool_setdefault(&b_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 75bb0b94cf..2a5b233caa 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -111,6 +111,7 @@
#define LIBXL_MIN_DOM0_MEM (128*1024)
#define LIBXL_INVALID_GFN (~(uint64_t)0)
#define LIBXL_VGA_HOLE_SIZE 0x20
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
/* use 0 as the domid of the toolstack domain for now */
#define LIBXL_TOOLSTACK_DOMID 0
#define QEMU_SIGNATURE "DeviceModelRecord0002"
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index a3a79d12b2..fe251649f3 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -731,6 +731,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
# Alternate p2m is not bound to any architecture or guest type, as it is
# supported by x86 HVM and ARM support is planned.
("altp2m", libxl_altp2m_mode),
+ ("altp2m_count", uint32, {'init_val': 'LIBXL_ALTP2M_COUNT_DEFAULT'}),
# Size of preallocated vmtrace trace buffers (in KBYTES).
# Use zero value to disable this feature.
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 7e11c62ba0..28cdbf07c2 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2099,6 +2099,15 @@ void parse_config_data(const char *config_source,
}
}
+ if (!xlu_cfg_get_long(config, "altp2m_count", &l, 1)) {
+ if (l != (uint16_t)l) {
+ fprintf(stderr, "ERROR: invalid value %ld for \"altp2m_count\"\n", l);
+ exit (1);
+ }
+
+ b_info->altp2m_count = l;
+ }
+
if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", &l, 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
On Tue, Jul 01, 2025 at 07:54:24PM +0000, Petr Beneš wrote:
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 8a85fba1cf..acf7fd9837 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> return -ERROR_INVAL;
> }
>
> + if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
> + if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
This access turned out to be an issue. "hvm.altp2m" is only set to a
default value for hvm guests on x86, in
libxl__arch_domain_build_info_setdefault() in ibxl_x86.c. So trying to
create a PV guest will fail here.
It seems that altp2m_count is going to be used for the creation of all
guest, right? That is in addition to HVM, it will be also used for PV
guest and on Arm, and any other architectures that could be added. What
should be the value of altp2m_count in all this case, if altp2m is only
set on x86 HVM guest?
> + b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
> + /* Reflect the default legacy count */
> + b_info->altp2m_count = 10;
> + else
> + b_info->altp2m_count = 0;
Cheers,
--
Anthony PERARD
On Mon, Jul 7, 2025 at 3:35 PM Anthony PERARD <anthony@xenproject.org> wrote:
>
> It seems that altp2m_count is going to be used for the creation of all
> guest, right? That is in addition to HVM, it will be also used for PV
> guest and on Arm, and any other architectures that could be added.
>
> Anthony PERARD
I'm suggesting to use what is used elsewhere in the libxl_create.c:
(https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1233
and https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1241)
((d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
libxl_defbool_val(d_config->b_info.u.hvm.altp2m))
In other words, add check for the LIBXL_DOMAIN_TYPE_HVM type before
calling that offending libxl_defbool_val(). Would that be okay?
> What should be the value of altp2m_count in all this case, if altp2m is only
> set on x86 HVM guest?
0. Similarly to other fields that are implemented only for x86, but
are available (and technically implementable) elsewhere - like altp2m
field, vmtrace_buf_kb, etc.
P.
On Mon, Jul 07, 2025 at 04:52:04PM +0200, Petr Beneš wrote: > On Mon, Jul 7, 2025 at 3:35 PM Anthony PERARD <anthony@xenproject.org> wrote: > > > > It seems that altp2m_count is going to be used for the creation of all > > guest, right? That is in addition to HVM, it will be also used for PV > > guest and on Arm, and any other architectures that could be added. > > I'm suggesting to use what is used elsewhere in the libxl_create.c: > > (https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1233 > and https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1241) > > ((d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM && > libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) > > In other words, add check for the LIBXL_DOMAIN_TYPE_HVM type before > calling that offending libxl_defbool_val(). Would that be okay? Yes, that would be fine. > > What should be the value of altp2m_count in all this case, if altp2m is only > > set on x86 HVM guest? > > 0. Similarly to other fields that are implemented only for x86, but > are available (and technically implementable) elsewhere - like altp2m > field, vmtrace_buf_kb, etc. Ok. Thanks, -- Anthony PERARD
On 01.07.2025 21:54, Petr Beneš wrote:
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> return -ERROR_INVAL;
> }
>
> + if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
> + if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
Is it possible that this is what causes
Assertion failed: !libxl_defbool_is_default(db) (libxl.c: libxl_defbool_val: 337)
observable in CI? Unless we can get a fix pretty quickly, I expect at least two
of the three patches that were committed from this series will need reverting.
Jan
> + b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
> + /* Reflect the default legacy count */
> + b_info->altp2m_count = 10;
> + else
> + b_info->altp2m_count = 0;
> + }
> +
> /* Assume that providing a bootloader user implies enabling restrict. */
> libxl_defbool_setdefault(&b_info->bootloader_restrict,
> !!b_info->bootloader_user);
On Tue, Jul 1, 2025 at 3:54 PM Petr Beneš <w1benny@gmail.com> wrote: > > From: Petr Beneš <w1benny@gmail.com> > > Introduce a new altp2m_count parameter to control the maximum number of altp2m > views a domain can use. By default, if altp2m_count is unspecified and altp2m > is enabled, the value is set to 10, reflecting the legacy behavior. > > This change is preparatory; it establishes the groundwork for the feature but > does not activate it. > > Signed-off-by: Petr Beneš <w1benny@gmail.com> > Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> For the tools/golang changes, Acked-by: Nick Rosbrook <enr0n@ubuntu.com>
© 2016 - 2025 Red Hat, Inc.