Re: [PATCH for-4.19? v6 3/9] xen: Refactor altp2m options into a structured format
On Tue, Jun 11, 2024 at 11:36 AM Jan Beulich wrote: > > On 11.06.2024 11:34, Petr Beneš wrote: > > On Tue, Jun 11, 2024 at 11:14 AM Jan Beulich wrote: > >> On 11.06.2024 10:00, Petr Beneš wrote: > >>> On Tue, Jun 11, 2024 at 8:41 AM Jan Beulich wrote: > >>>> On 10.06.2024 19:10, Petr Beneš wrote: > >>>>> From: Petr Beneš > >>>>> > >>>>> Encapsulate the altp2m options within a struct. This change is > >>>>> preparatory > >>>>> and sets the groundwork for introducing additional parameter in > >>>>> subsequent > >>>>> commit. > >>>>> > >>>>> Signed-off-by: Petr Beneš > >>>>> Acked-by: Julien Grall # arm > >>>>> Reviewed-by: Jan Beulich # hypervisor > >>>> > >>>> Looks like you lost Christian's ack for ... > >>>> > >>>>> --- > >>>>> tools/libs/light/libxl_create.c | 6 +++--- > >>>>> tools/ocaml/libs/xc/xenctrl_stubs.c | 4 +++- > >>>> > >>>> ... the adjustment of this file? > >>> > >>> In the cover email, Christian only acked: > >>> > >>>> tools/ocaml/libs/xc/xenctrl.ml | 2 + > >>>> tools/ocaml/libs/xc/xenctrl.mli | 2 + > >>>> tools/ocaml/libs/xc/xenctrl_stubs.c | 40 +++--- > >> > >> Right, but above I was talking about the last of these three files. > >> > >> Jan > > > > Ouch. It didn't occur to me that Ack on cover email acks each of the > > files in every separate patch. My thinking was it acks only the > > patches where those three are together. Anyway, it makes sense. I'll > > resend v7. > > Well, no need to just for this. Yet if a v7 turns out necessary, please > make sure you have the ack recorded. > > Jan Noted. P.
Re: [PATCH for-4.19? v6 3/9] xen: Refactor altp2m options into a structured format
On Tue, Jun 11, 2024 at 11:14 AM Jan Beulich wrote: > > On 11.06.2024 10:00, Petr Beneš wrote: > > On Tue, Jun 11, 2024 at 8:41 AM Jan Beulich wrote: > >> > >> On 10.06.2024 19:10, Petr Beneš wrote: > >>> From: Petr Beneš > >>> > >>> Encapsulate the altp2m options within a struct. This change is preparatory > >>> and sets the groundwork for introducing additional parameter in subsequent > >>> commit. > >>> > >>> Signed-off-by: Petr Beneš > >>> Acked-by: Julien Grall # arm > >>> Reviewed-by: Jan Beulich # hypervisor > >> > >> Looks like you lost Christian's ack for ... > >> > >>> --- > >>> tools/libs/light/libxl_create.c | 6 +++--- > >>> tools/ocaml/libs/xc/xenctrl_stubs.c | 4 +++- > >> > >> ... the adjustment of this file? > > > > In the cover email, Christian only acked: > > > >> tools/ocaml/libs/xc/xenctrl.ml | 2 + > >> tools/ocaml/libs/xc/xenctrl.mli | 2 + > >> tools/ocaml/libs/xc/xenctrl_stubs.c | 40 +++--- > > Right, but above I was talking about the last of these three files. > > Jan Ouch. It didn't occur to me that Ack on cover email acks each of the files in every separate patch. My thinking was it acks only the patches where those three are together. Anyway, it makes sense. I'll resend v7. P.
Re: [PATCH for-4.19? v6 3/9] xen: Refactor altp2m options into a structured format
On Tue, Jun 11, 2024 at 8:41 AM Jan Beulich wrote: > > On 10.06.2024 19:10, Petr Beneš wrote: > > From: Petr Beneš > > > > Encapsulate the altp2m options within a struct. This change is preparatory > > and sets the groundwork for introducing additional parameter in subsequent > > commit. > > > > Signed-off-by: Petr Beneš > > Acked-by: Julien Grall # arm > > Reviewed-by: Jan Beulich # hypervisor > > Looks like you lost Christian's ack for ... > > > --- > > tools/libs/light/libxl_create.c | 6 +++--- > > tools/ocaml/libs/xc/xenctrl_stubs.c | 4 +++- > > ... the adjustment of this file? In the cover email, Christian only acked: > tools/ocaml/libs/xc/xenctrl.ml | 2 + > tools/ocaml/libs/xc/xenctrl.mli | 2 + > tools/ocaml/libs/xc/xenctrl_stubs.c | 40 +++--- P.
[PATCH for-4.19? v6 5/9] docs/man: Add altp2m_count parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the altp2m_count configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index ac3f88fd57..ff03b43884 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
[PATCH for-4.19? v6 3/9] xen: Refactor altp2m options into a structured format
From: Petr Beneš
Encapsulate the altp2m options within a struct. This change is preparatory
and sets the groundwork for introducing additional parameter in subsequent
commit.
Signed-off-by: Petr Beneš
Acked-by: Julien Grall # arm
Reviewed-by: Jan Beulich # hypervisor
---
tools/libs/light/libxl_create.c | 6 +++---
tools/ocaml/libs/xc/xenctrl_stubs.c | 4 +++-
xen/arch/arm/domain.c | 2 +-
xen/arch/x86/domain.c | 4 ++--
xen/arch/x86/hvm/hvm.c | 2 +-
xen/include/public/domctl.h | 4 +++-
6 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index edeadd57ef..569e3d21ed 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -680,17 +680,17 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
LOG(DETAIL, "altp2m: %s", libxl_altp2m_mode_to_string(b_info->altp2m));
switch(b_info->altp2m) {
case LIBXL_ALTP2M_MODE_MIXED:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_mixed);
break;
case LIBXL_ALTP2M_MODE_EXTERNAL:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_external);
break;
case LIBXL_ALTP2M_MODE_LIMITED:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_limited);
break;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index a529080129..e6c977521f 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -231,7 +231,9 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
- .altp2m_opts = Int32_val(VAL_ALTP2M_OPTS),
+ .altp2m = {
+ .opts = Int32_val(VAL_ALTP2M_OPTS),
+ },
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8bde2f730d..5234b627d0 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -688,7 +688,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m_opts )
+if ( config->altp2m.opts )
{
dprintk(XENLOG_INFO, "Altp2m not supported\n");
return -EINVAL;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ccadfe0c9e..a4f2e7bad1 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -637,7 +637,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
bool hap = config->flags & XEN_DOMCTL_CDF_hap;
bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
unsigned int max_vcpus;
-unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
+unsigned int altp2m_mode = MASK_EXTR(config->altp2m.opts,
XEN_DOMCTL_ALTP2M_mode_mask);
if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
@@ -717,7 +717,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
+if ( config->altp2m.opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
{
dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
config->flags);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8334ab1711..a66ebaaceb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -659,7 +659,7 @@ int hvm_domain_initialise(struct domain *d,
d->arch.hvm.params[HVM_PARAM_TRIPLE_FAULT_REASON] = SHUTDOWN_reboot;
/* Set altp2m based on domctl flags. */
-switch ( MASK_EXTR(config->altp2m_opts, XEN_DOMCTL_ALTP2M_mode_mask) )
+switch ( MASK_EXTR(config->altp2m.opts, XEN_DOMCTL_ALTP2M_mode_mask) )
{
case XEN_DOMCTL_ALTP2M_mixed:
d->arch.hvm.params[HVM_PARAM_ALTP2M] = XEN_ALTP2M_mixed;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 2a49fe46ce..dea399aa8e 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -86,6 +86,7 @@ struct xen_domctl_createdomain {
uint32_t grant_opts;
+struct {
/*
* Enable altp2m mixed mode.
*
@@ -102,7 +103,8 @@ struct xen_domctl_createdomain {
/* Altp2m mode signaling uses bits [0, 1]. */
#define XEN_DOMCTL_ALTP2M_mode_mask (0x3U)
#define XEN_DOMCTL_ALTP2M_mode(
[PATCH for-4.19? v6 7/9] tools/libxl: Activate the altp2m_count feature
From: Petr Beneš
This commit activates the previously introduced altp2m_count parameter,
establishing the connection between libxl and Xen.
Signed-off-by: Petr Beneš
---
tools/libs/light/libxl_create.c | 4
1 file changed, 4 insertions(+)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 11d2f282f5..5ad552c4ec 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -656,6 +656,10 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
.max_grant_frames = b_info->max_grant_frames,
.max_maptrack_frames = b_info->max_maptrack_frames,
.grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
+.altp2m = {
+.opts = 0, /* .opts will be set below */
+.nr = b_info->altp2m_count,
+},
.vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10,
XC_PAGE_SHIFT),
.cpupool_id = info->poolid,
};
--
2.34.1
[PATCH for-4.19? v6 1/9] tools/ocaml: Fix mixed tabs/spaces
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
tools/ocaml/libs/xc/xenctrl_stubs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index c6da9bb091..e86c455802 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -138,12 +138,12 @@ static void
domain_handle_of_uuid_string(xen_domain_handle_t h,
* integers in the Ocaml ABI for more idiomatic handling.
*/
static value c_bitmap_to_ocaml_list
- /* ! */
- /*
+/* ! */
+/*
* All calls to this function must be in a form suitable
* for xenctrl_abi_check. The parsing there is ad-hoc.
*/
- (unsigned int bitmap)
+(unsigned int bitmap)
{
CAMLparam0();
CAMLlocal2(list, tmp);
@@ -180,8 +180,8 @@ static value c_bitmap_to_ocaml_list
}
static unsigned int ocaml_list_to_c_bitmap(value l)
- /* ! */
- /*
+/* ! */
+/*
* All calls to this function must be in a form suitable
* for xenctrl_abi_check. The parsing there is ad-hoc.
*/
@@ -259,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
/* Quick & dirty check for ABI changes. */
BUILD_BUG_ON(sizeof(cfg) != 68);
-/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
+ /* Mnemonics for the named fields inside
xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
#define VAL_MISC_FLAGS Field(arch_domconfig, 1)
@@ -351,7 +351,7 @@ static value dom_op(value xch_val, value domid,
caml_enter_blocking_section();
result = fn(xch, c_domid);
caml_leave_blocking_section();
-if (result)
+ if (result)
failwith_xc(xch);
CAMLreturn(Val_unit);
}
@@ -383,7 +383,7 @@ CAMLprim value stub_xc_domain_resume_fast(value xch_val,
value domid)
caml_enter_blocking_section();
result = xc_domain_resume(xch, c_domid, 1);
caml_leave_blocking_section();
-if (result)
+ if (result)
failwith_xc(xch);
CAMLreturn(Val_unit);
}
@@ -426,7 +426,7 @@ static value alloc_domaininfo(xc_domaininfo_t * info)
Store_field(result, 13, Val_int(info->max_vcpu_id));
Store_field(result, 14, caml_copy_int32(info->ssidref));
-tmp = caml_alloc_small(16, 0);
+ tmp = caml_alloc_small(16, 0);
for (i = 0; i < 16; i++) {
Field(tmp, i) = Val_int(info->handle[i]);
}
--
2.34.1
[PATCH for-4.19? v6 9/9] tools/ocaml: Add altp2m_count parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the altp2m_count parameter.
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 19 +++
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 2690f9a923..a3e50ac394 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -86,6 +86,7 @@ type domctl_create_config =
max_maptrack_frames: int;
max_grant_version: int;
altp2m_opts: int32;
+altp2m_count: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index febbe1f6ae..b97021d3d2 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -78,6 +78,7 @@ type domctl_create_config = {
max_maptrack_frames: int;
max_grant_version: int;
altp2m_opts: int32;
+ altp2m_count: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index e6c977521f..78ae4967e6 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -211,13 +211,22 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
#define VAL_ALTP2M_OPTS Field(config, 9)
-#define VAL_VMTRACE_BUF_KB Field(config, 10)
-#define VAL_CPUPOOL_ID Field(config, 11)
-#define VAL_ARCHField(config, 12)
+#define VAL_ALTP2M_COUNTField(config, 10)
+#define VAL_VMTRACE_BUF_KB Field(config, 11)
+#define VAL_CPUPOOL_ID Field(config, 12)
+#define VAL_ARCHField(config, 13)
uint32_t domid = Int_val(wanted_domid);
+ uint32_t altp2m_opts = Int32_val(VAL_ALTP2M_OPTS);
+ uint32_t altp2m_nr = Int32_val(VAL_ALTP2M_COUNT);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
+ if ( altp2m_opts != (uint16_t)altp2m_opts )
+ caml_invalid_argument("altp2m_opts");
+
+ if ( altp2m_nr != (uint16_t)altp2m_nr )
+ caml_invalid_argument("altp2m_count");
+
vmtrace_size = ROUNDUP(vmtrace_size << 10, XC_PAGE_SHIFT);
if ( vmtrace_size != (uint32_t)vmtrace_size )
caml_invalid_argument("vmtrace_buf_kb");
@@ -232,7 +241,8 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
.altp2m = {
- .opts = Int32_val(VAL_ALTP2M_OPTS),
+ .opts = altp2m_opts,
+ .nr = altp2m_nr,
},
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
@@ -292,6 +302,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_COUNT
#undef VAL_ALTP2M_OPTS
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
--
2.34.1
[PATCH for-4.19? v6 6/9] xen: Make the maximum number of altp2m views configurable for x86
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
views for the domain during its creation. Previously, the limits were hardcoded
to a maximum of 10. This change allows for greater flexibility in environments
that require more or fewer altp2m views.
The maximum configurable limit for nr_altp2m on x86 is now set to
MAX_NR_ALTP2M (which currently holds the MAX_EPTP value - 512). This cap is
linked to the architectural limit of the EPTP-switching VMFUNC, which supports
up to 512 entries. Despite there being no inherent need for limiting nr_altp2m
in scenarios not utilizing VMFUNC, decoupling these components would necessitate
substantial code changes.
xen_domctl_createdomain::altp2m is extended for a new field `nr`, that will
configure this limit for a domain. Additionally, previous altp2m.opts value
has been reduced from uint32_t to uint16_t so that both of these fields occupy
as little space as possible.
Accesses to the altp2m_p2m array are modified to respect the new nr_altp2m
value. Accesses to the altp2m_(visible_)eptp arrays are unmodified, since
these arrays always have fixed size of MAX_EPTP.
A dummy hvm_altp2m_supported() function is introduced for non-HVM builds, so
that the compilation won't fail for them.
Additional sanitization is introduced in the x86/arch_sanitise_domain_config
to fix the altp2m.nr value to 10 if it is previously set to 0. This behavior
is only temporary and immediately removed in the upcoming commit (which will
disallow creating a domain with enabled altp2m with zero nr_altp2m).
The reason for this temporary workaround is to retain the legacy behavior
until the feature is fully activated in libxl.
Also, arm/arch_sanitise_domain_config is extended to not allow requesting
non-zero altp2ms.
Signed-off-by: Petr Beneš
---
xen/arch/arm/domain.c | 2 +-
xen/arch/x86/domain.c | 40 +++
xen/arch/x86/hvm/hvm.c | 8 +++-
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
xen/arch/x86/include/asm/domain.h | 9 +++--
xen/arch/x86/include/asm/hvm/hvm.h | 5 +++
xen/arch/x86/include/asm/p2m.h | 4 +-
xen/arch/x86/mm/altp2m.c | 64 +++---
xen/arch/x86/mm/hap/hap.c | 6 +--
xen/arch/x86/mm/mem_access.c | 14 +++
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 7 ++--
xen/arch/x86/mm/p2m.c | 8 ++--
xen/common/domain.c| 1 +
xen/include/public/domctl.h| 5 ++-
xen/include/xen/sched.h| 2 +
16 files changed, 121 insertions(+), 58 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 5234b627d0..e5785d2d96 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -688,7 +688,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m.opts )
+if ( config->altp2m.opts || config->altp2m.nr )
{
dprintk(XENLOG_INFO, "Altp2m not supported\n");
return -EINVAL;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a4f2e7bad1..faec09e15e 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -724,16 +724,42 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( altp2m_mode && nested_virt )
+if ( altp2m_mode )
{
-dprintk(XENLOG_INFO,
-"Nested virt and altp2m are not supported together\n");
-return -EINVAL;
-}
+if ( nested_virt )
+{
+dprintk(XENLOG_INFO,
+"Nested virt and altp2m are not supported together\n");
+return -EINVAL;
+}
+
+if ( !hap )
+{
+dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
+return -EINVAL;
+}
+
+if ( !hvm_altp2m_supported() )
+{
+dprintk(XENLOG_INFO, "altp2m is not supported\n");
+return -EINVAL;
+}
+
+if ( !config->altp2m.nr )
+{
+/* Fix the value to the legacy default */
+config->altp2m.nr = 10;
+}
-if ( altp2m_mode && !hap )
+if ( config->altp2m.nr > MAX_NR_ALTP2M )
+{
+dprintk(XENLOG_INFO, "altp2m.nr must be <= %lu\n", MAX_NR_ALTP2M);
+return -EINVAL;
+}
+}
+else if ( config->altp2m.nr )
{
-dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
+dprintk(XENLOG_INFO, "altp2m.nr must be zero when altp2m is off\n");
return -EINVAL;
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a66ebaaceb..3d0357a0f8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4657,6 +4657,12 @@ static
[PATCH for-4.19? v6 8/9] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0
From: Petr Beneš
Since libxl finally sends the altp2m.nr value, we can remove the previously
introduced temporary workaround.
Creating domain with enabled altp2m while setting altp2m.nr == 0 doesn't
make sense and it's probably not what user wants.
Signed-off-by: Petr Beneš
Acked-by: Jan Beulich
---
xen/arch/x86/domain.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index faec09e15e..721d753c95 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -747,8 +747,9 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
if ( !config->altp2m.nr )
{
-/* Fix the value to the legacy default */
-config->altp2m.nr = 10;
+dprintk(XENLOG_INFO,
+"altp2m must be requested with altp2m.nr > 0\n");
+return -EINVAL;
}
if ( config->altp2m.nr > MAX_NR_ALTP2M )
--
2.34.1
[PATCH for-4.19? v6 0/9] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Changes since v5: - Reverted "Introduction of accessor functions for altp2m arrays and refactoring the code to use them." - Reason is minimizing the code changes, and save the code consistency. - I've addressed (hopefully all) issues with long lines and mismatched _nospec replacements mentioned in previous reviews. - Removed "struct domain *d" from altp2m_vcpu_initialise/destroy. Changes since v4: - Rebased on top of staging (applying Roger's changes). - Fix mixed tabs/spaces in xenctrl_stubs.c. - Add missing OCaml bindings for altp2m_opts. - Substitute altp2m_opts into an unnamed structure. (This is a preparation for the next patch that will introduce the `nr` field.) - altp2m.opts is then shortened to uint16_t and a new field altp2m.nr is added - also uint16_t. This value is then verified by libxl to not exceed the maximum uint16_t value. This puts a hard limit to number of altp2m to 65535, which is enough, at least for the time being. Also, altp2m.opts currently uses only 2 bits. Therefore I believe this change is justified. - Introduction of accessor functions for altp2m arrays and refactoring the code to use them. - Added a check to arm/arch_sanitise_domain_config() to disallow creating domains with altp2m.nr != 0. - Added dummy hvm_altp2m_supported() to avoid build errors when CONFIG_HVM is disabled. - Finally, expose altp2m_count to OCaml bindings (and verify both altp2m_opts and altp2m_count fit uint16_t). - I also removed Christian Lindig from the Acked-by, since I think this change is significant enough to require a re-review. Changes since v3: - Rebased on top of staging (some functions were moved to altp2m.c). - Re-added the array_index_nospec() where it was removed. Changes since v2: - Changed max_altp2m to nr_altp2m. - Moved arch-dependent check from xen/common/domain.c to xen/arch/x86/domain.c. - Replaced min(d->nr_altp2m, MAX_EPTP) occurences for just d->nr_altp2m. - Replaced array_index_nospec(altp2m_idx, ...) for just altp2m_idx. - Shortened long lines. - Removed unnecessary comments in altp2m_vcpu_initialise/destroy. - Moved nr_altp2m field after max_ fields in xen_domctl_createdomain. - Removed the commit that adjusted the initial allocation of pages from 256 to 1024. This means that after these patches, technically, the nr_altp2m will be capped to (256 - 1 - vcpus - MAX_NESTEDP2M) instead of MAX_EPTP (512). Future work will be needed to fix this. Petr Beneš (9): tools/ocaml: Fix mixed tabs/spaces tools/ocaml: Add missing ocaml bindings for altp2m_opts xen: Refactor altp2m options into a structured format tools/xl: Add altp2m_count parameter docs/man: Add altp2m_count parameter to the xl.cfg manual xen: Make the maximum number of altp2m views configurable for x86 tools/libxl: Activate the altp2m_count feature xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0 tools/ocaml: Add altp2m_count parameter docs/man/xl.cfg.5.pod.in | 14 ++ tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go | 1 + tools/include/libxl.h| 8 tools/libs/light/libxl_create.c | 19 +++-- tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml | 2 + tools/ocaml/libs/xc/xenctrl.mli | 2 + tools/ocaml/libs/xc/xenctrl_stubs.c | 40 +++-- tools/xl/xl_parse.c | 9 xen/arch/arm/domain.c| 2 +- xen/arch/x86/domain.c| 45 +++ xen/arch/x86/hvm/hvm.c | 10 - xen/arch/x86/hvm/vmx/vmx.c | 2 +- xen/arch/x86/include/asm/domain.h| 9 ++-- xen/arch/x86/include/asm/hvm/hvm.h | 5 +++ xen/arch/x86/include/asm/p2m.h | 4 +- xen/arch/x86/mm/altp2m.c | 64 ++-- xen/arch/x86/mm/hap/hap.c| 6 +-- xen/arch/x86/mm/mem_access.c | 14 +++--- xen/arch/x86/mm/mem_sharing.c| 2 +- xen/arch/x86/mm/p2m-ept.c| 7 +-- xen/arch/x86/mm/p2m.c| 8 ++-- xen/common/domain.c | 1 + xen/include/public/domctl.h | 7 ++- xen/include/xen/sched.h | 2 + 26 files changed, 210 insertions(+), 76 deletions(-) -- 2.34.1
[PATCH for-4.19? v6 2/9] tools/ocaml: Add missing ocaml bindings for altp2m_opts
From: Petr Beneš
Fixes: 0291089f6ea8 ("xen: enable altp2m at create domain domctl")
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 9 ++---
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..2690f9a923 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+altp2m_opts: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..febbe1f6ae 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ altp2m_opts: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index e86c455802..a529080129 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,9 +210,10 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_ALTP2M_OPTS Field(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -230,6 +231,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .altp2m_opts = Int32_val(VAL_ALTP2M_OPTS),
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -288,6 +290,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_OPTS
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
[PATCH for-4.19? v6 4/9] tools/xl: Add altp2m_count parameter
From: Petr Beneš
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š
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 9 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 9 +
6 files changed, 30 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index fe5110474d..0449c55f31 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1159,6 +1159,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_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();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1676,6 +1677,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_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(); 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 c9e45b306f..54607758d3 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -603,6 +603,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+Altp2MCount uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f5c7167742..bfa06caad2 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1250,6 +1250,14 @@ 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
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
+
/*
* 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 569e3d21ed..11d2f282f5 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -482,6 +482,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(_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 4e65e6fda5..2963c5e250 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -729,6 +729,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 e3a4800f6e..a82b8fe6e4 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2063,6 +2063,15 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "altp2m_count", , 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", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
Re: [PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
> (when booted with altp2m=1) Sorry, forgot to remove this statement before sending the email, it's not really true. I wanted to add that as I wrote in a previous email exchange - altp2m should be ideally initialized only when it's requested - as opposed to the current situation, when it's initialized in the domain_create. However, that is more suited for 4.20. P.
Re: [PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
On Mon, Jun 10, 2024 at 1:21 PM Jan Beulich wrote:
>
> On 10.06.2024 12:34, Petr Beneš wrote:
> > On Mon, Jun 10, 2024 at 12:16 PM Jan Beulich wrote:
> >>
> >> On 10.06.2024 11:10, Petr Beneš wrote:
> >>> On Mon, Jun 10, 2024 at 9:30 AM Jan Beulich wrote:
> >>>>
> >>>> On 09.06.2024 01:06, Petr Beneš wrote:
> >>>>> On Thu, Jun 6, 2024 at 9:24 AM Jan Beulich wrote:
> >>>>>>> @@ -122,7 +131,12 @@ int p2m_init_altp2m(struct domain *d)
> >>>>>>> struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> >>>>>>>
> >>>>>>> mm_lock_init(>arch.altp2m_list_lock);
> >>>>>>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>>>>>> +d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *,
> >>>>>>> d->nr_altp2m);
> >>>>>>> +
> >>>>>>> +if ( !d->arch.altp2m_p2m )
> >>>>>>> +return -ENOMEM;
> >>>>>>
> >>>>>> This isn't really needed, is it? Both ...
> >>>>>>
> >>>>>>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>>>>>
> >>>>>> ... this and ...
> >>>>>>
> >>>>>>> {
> >>>>>>> d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
> >>>>>>> if ( p2m == NULL )
> >>>>>>> @@ -143,7 +157,10 @@ void p2m_teardown_altp2m(struct domain *d)
> >>>>>>> unsigned int i;
> >>>>>>> struct p2m_domain *p2m;
> >>>>>>>
> >>>>>>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>>>>>> +if ( !d->arch.altp2m_p2m )
> >>>>>>> +return;
> >>>>
> >>>> I'm sorry, the question was meant to be on this if() instead.
> >>>>
> >>>>>>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>>>>>> {
> >>>>>>> if ( !d->arch.altp2m_p2m[i] )
> >>>>>>> continue;
> >>>>>>> @@ -151,6 +168,8 @@ void p2m_teardown_altp2m(struct domain *d)
> >>>>>>> d->arch.altp2m_p2m[i] = NULL;
> >>>>>>> p2m_free_one(p2m);
> >>>>>>> }
> >>>>>>> +
> >>>>>>> +XFREE(d->arch.altp2m_p2m);
> >>>>>>> }
> >>>>>>
> >>>>>> ... this ought to be fine without?
> >>>>>
> >>>>> Could you, please, elaborate? I honestly don't know what you mean here
> >>>>> (by "this isn't needed").
> >>>>
> >>>> I hope the above correction is enough?
> >>>
> >>> I'm sorry, but not really? I feel like I'm blind but I can't see
> >>> anything I could remove without causing (or risking) crash.
> >>
> >> The loop is going to do nothing when d->nr_altp2m == 0, and the XFREE() is
> >> going to do nothing when d->arch.altp2m_p2m == NULL. Hence what does the
> >> if() guard against? IOW what possible crashes are you seeing that I don't
> >> see?
> >
> > I see now. I was seeing ghosts - my thinking was that if
> > p2m_init_altp2m fails to allocate altp2m_p2m, it will call
> > p2m_teardown_altp2m - which, without the if(), would start to iterate
> > through an array that is not allocated. It doesn't happen, it just
> > returns -ENOMEM.
> >
> > So to reiterate:
> >
> > if ( !d->arch.altp2m_p2m )
> > return;
> >
> > ... are we talking that this condition inside p2m_teardown_altp2m
> > isn't needed?
>
> I'm not sure about "isn't" vs "shouldn't". The call from p2m_final_teardown()
> also needs to remain safe to make. Which may require that upon allocation
> failure you zap d->nr_altp2m. Or which alternatively may mean that the if()
> actually needs to stay.
True, p2m_final_teardown is called whenever p2m_init (and by extension
p2m_init_altp2m) fails. Which means that condition must stay - or, as
you suggested, reset nr_altp2m to 0.
I would rather leave the code as is. Modifying nr_altp2m would (in my
opinion) feel semantically incorrect, as that value should behave more
or less as const, that is initialized once.
> > Or is there anything else?
>
> There was also the question of whether to guard the allocation, to avoid a
> de-generate xmalloc_array() of zero size. Yet in the interest of avoiding
> not strictly necessary conditionals, that may well want to remain as is.
True, nr_altp2m would mean zero-sized allocation, as p2m_init_altp2m
is called unconditionally (when booted with altp2m=1). Is it a
problem, though?
P.
Re: [PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
On Mon, Jun 10, 2024 at 12:16 PM Jan Beulich wrote:
>
> On 10.06.2024 11:10, Petr Beneš wrote:
> > On Mon, Jun 10, 2024 at 9:30 AM Jan Beulich wrote:
> >>
> >> On 09.06.2024 01:06, Petr Beneš wrote:
> >>> On Thu, Jun 6, 2024 at 9:24 AM Jan Beulich wrote:
> >>>>> @@ -122,7 +131,12 @@ int p2m_init_altp2m(struct domain *d)
> >>>>> struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> >>>>>
> >>>>> mm_lock_init(>arch.altp2m_list_lock);
> >>>>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>>>> +d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *,
> >>>>> d->nr_altp2m);
> >>>>> +
> >>>>> +if ( !d->arch.altp2m_p2m )
> >>>>> +return -ENOMEM;
> >>>>
> >>>> This isn't really needed, is it? Both ...
> >>>>
> >>>>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>>>
> >>>> ... this and ...
> >>>>
> >>>>> {
> >>>>> d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
> >>>>> if ( p2m == NULL )
> >>>>> @@ -143,7 +157,10 @@ void p2m_teardown_altp2m(struct domain *d)
> >>>>> unsigned int i;
> >>>>> struct p2m_domain *p2m;
> >>>>>
> >>>>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>>>> +if ( !d->arch.altp2m_p2m )
> >>>>> +return;
> >>
> >> I'm sorry, the question was meant to be on this if() instead.
> >>
> >>>>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>>>> {
> >>>>> if ( !d->arch.altp2m_p2m[i] )
> >>>>> continue;
> >>>>> @@ -151,6 +168,8 @@ void p2m_teardown_altp2m(struct domain *d)
> >>>>> d->arch.altp2m_p2m[i] = NULL;
> >>>>> p2m_free_one(p2m);
> >>>>> }
> >>>>> +
> >>>>> +XFREE(d->arch.altp2m_p2m);
> >>>>> }
> >>>>
> >>>> ... this ought to be fine without?
> >>>
> >>> Could you, please, elaborate? I honestly don't know what you mean here
> >>> (by "this isn't needed").
> >>
> >> I hope the above correction is enough?
> >
> > I'm sorry, but not really? I feel like I'm blind but I can't see
> > anything I could remove without causing (or risking) crash.
>
> The loop is going to do nothing when d->nr_altp2m == 0, and the XFREE() is
> going to do nothing when d->arch.altp2m_p2m == NULL. Hence what does the
> if() guard against? IOW what possible crashes are you seeing that I don't
> see?
I see now. I was seeing ghosts - my thinking was that if
p2m_init_altp2m fails to allocate altp2m_p2m, it will call
p2m_teardown_altp2m - which, without the if(), would start to iterate
through an array that is not allocated. It doesn't happen, it just
returns -ENOMEM.
So to reiterate:
if ( !d->arch.altp2m_p2m )
return;
... are we talking that this condition inside p2m_teardown_altp2m
isn't needed? Or is there anything else?
P.
Re: [PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
On Mon, Jun 10, 2024 at 9:30 AM Jan Beulich wrote:
>
> On 09.06.2024 01:06, Petr Beneš wrote:
> > On Thu, Jun 6, 2024 at 9:24 AM Jan Beulich wrote:
> >>> @@ -122,7 +131,12 @@ int p2m_init_altp2m(struct domain *d)
> >>> struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> >>>
> >>> mm_lock_init(>arch.altp2m_list_lock);
> >>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>> +d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *,
> >>> d->nr_altp2m);
> >>> +
> >>> +if ( !d->arch.altp2m_p2m )
> >>> +return -ENOMEM;
> >>
> >> This isn't really needed, is it? Both ...
> >>
> >>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>
> >> ... this and ...
> >>
> >>> {
> >>> d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
> >>> if ( p2m == NULL )
> >>> @@ -143,7 +157,10 @@ void p2m_teardown_altp2m(struct domain *d)
> >>> unsigned int i;
> >>> struct p2m_domain *p2m;
> >>>
> >>> -for ( i = 0; i < MAX_ALTP2M; i++ )
> >>> +if ( !d->arch.altp2m_p2m )
> >>> +return;
>
> I'm sorry, the question was meant to be on this if() instead.
>
> >>> +for ( i = 0; i < d->nr_altp2m; i++ )
> >>> {
> >>> if ( !d->arch.altp2m_p2m[i] )
> >>> continue;
> >>> @@ -151,6 +168,8 @@ void p2m_teardown_altp2m(struct domain *d)
> >>> d->arch.altp2m_p2m[i] = NULL;
> >>> p2m_free_one(p2m);
> >>> }
> >>> +
> >>> +XFREE(d->arch.altp2m_p2m);
> >>> }
> >>
> >> ... this ought to be fine without?
> >
> > Could you, please, elaborate? I honestly don't know what you mean here
> > (by "this isn't needed").
>
> I hope the above correction is enough?
I'm sorry, but not really? I feel like I'm blind but I can't see
anything I could remove without causing (or risking) crash.
P.
Re: [PATCH for-4.19? v5 09/10] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0
The flag isn't bool. It can hold one of 3 values (besides 0). P. On Thu, Jun 6, 2024 at 9:57 AM Jan Beulich wrote: > > On 02.06.2024 22:04, Petr Beneš wrote: > > From: Petr Beneš > > > > Since libxl finally sends the altp2m.nr value, we can remove the previously > > introduced temporary workaround. > > > > Creating domain with enabled altp2m while setting altp2m.nr == 0 doesn't > > make sense and it's probably not what user wants. > > Yet: Do we need separate count and flag anymore? Can't "nr != 0" > take the place of the flag being true? > > Jan
Re: [PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
On Thu, Jun 6, 2024 at 9:24 AM Jan Beulich wrote:
> > @@ -122,7 +131,12 @@ int p2m_init_altp2m(struct domain *d)
> > struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> >
> > mm_lock_init(>arch.altp2m_list_lock);
> > -for ( i = 0; i < MAX_ALTP2M; i++ )
> > +d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *, d->nr_altp2m);
> > +
> > +if ( !d->arch.altp2m_p2m )
> > +return -ENOMEM;
>
> This isn't really needed, is it? Both ...
>
> > +for ( i = 0; i < d->nr_altp2m; i++ )
>
> ... this and ...
>
> > {
> > d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
> > if ( p2m == NULL )
> > @@ -143,7 +157,10 @@ void p2m_teardown_altp2m(struct domain *d)
> > unsigned int i;
> > struct p2m_domain *p2m;
> >
> > -for ( i = 0; i < MAX_ALTP2M; i++ )
> > +if ( !d->arch.altp2m_p2m )
> > +return;
> > +
> > +for ( i = 0; i < d->nr_altp2m; i++ )
> > {
> > if ( !d->arch.altp2m_p2m[i] )
> > continue;
> > @@ -151,6 +168,8 @@ void p2m_teardown_altp2m(struct domain *d)
> > d->arch.altp2m_p2m[i] = NULL;
> > p2m_free_one(p2m);
> > }
> > +
> > +XFREE(d->arch.altp2m_p2m);
> > }
>
> ... this ought to be fine without?
Could you, please, elaborate? I honestly don't know what you mean here
(by "this isn't needed").
P.
[PATCH for-4.19? v5 10/10] tools/ocaml: Add altp2m_count parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the altp2m_count parameter.
Signed-off-by: Petr Beneš
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 19 +++
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 2690f9a923..a3e50ac394 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -86,6 +86,7 @@ type domctl_create_config =
max_maptrack_frames: int;
max_grant_version: int;
altp2m_opts: int32;
+altp2m_count: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index febbe1f6ae..b97021d3d2 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -78,6 +78,7 @@ type domctl_create_config = {
max_maptrack_frames: int;
max_grant_version: int;
altp2m_opts: int32;
+ altp2m_count: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index e6c977521f..78ae4967e6 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -211,13 +211,22 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
#define VAL_ALTP2M_OPTS Field(config, 9)
-#define VAL_VMTRACE_BUF_KB Field(config, 10)
-#define VAL_CPUPOOL_ID Field(config, 11)
-#define VAL_ARCHField(config, 12)
+#define VAL_ALTP2M_COUNTField(config, 10)
+#define VAL_VMTRACE_BUF_KB Field(config, 11)
+#define VAL_CPUPOOL_ID Field(config, 12)
+#define VAL_ARCHField(config, 13)
uint32_t domid = Int_val(wanted_domid);
+ uint32_t altp2m_opts = Int32_val(VAL_ALTP2M_OPTS);
+ uint32_t altp2m_nr = Int32_val(VAL_ALTP2M_COUNT);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
+ if ( altp2m_opts != (uint16_t)altp2m_opts )
+ caml_invalid_argument("altp2m_opts");
+
+ if ( altp2m_nr != (uint16_t)altp2m_nr )
+ caml_invalid_argument("altp2m_count");
+
vmtrace_size = ROUNDUP(vmtrace_size << 10, XC_PAGE_SHIFT);
if ( vmtrace_size != (uint32_t)vmtrace_size )
caml_invalid_argument("vmtrace_buf_kb");
@@ -232,7 +241,8 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
.altp2m = {
- .opts = Int32_val(VAL_ALTP2M_OPTS),
+ .opts = altp2m_opts,
+ .nr = altp2m_nr,
},
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
@@ -292,6 +302,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_COUNT
#undef VAL_ALTP2M_OPTS
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
--
2.34.1
[PATCH for-4.19? v5 06/10] x86/altp2m: Introduce accessor functions for safer array indexing
From: Petr Beneš
This patch introduces a set of accessor functions for altp2m array operations,
and refactoring direct array accesses with them.
This approach aims on improving the code clarity and also future-proofing the
codebase against potential speculative execution attacks.
Signed-off-by: Petr Beneš
---
xen/arch/x86/hvm/vmx/vmx.c| 4 +--
xen/arch/x86/include/asm/altp2m.h | 32 +
xen/arch/x86/include/asm/p2m.h| 7 ++--
xen/arch/x86/mm/altp2m.c | 60 +--
xen/arch/x86/mm/hap/hap.c | 8 ++---
xen/arch/x86/mm/mem_access.c | 17 -
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 14
xen/arch/x86/mm/p2m.c | 16 -
9 files changed, 91 insertions(+), 69 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index f16faa6a61..a420d452b3 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4887,10 +4887,10 @@ bool asmlinkage vmx_vmenter_helper(const struct
cpu_user_regs *regs)
for ( i = 0; i < MAX_ALTP2M; ++i )
{
-if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
+if ( altp2m_get_eptp(currd, i) == mfn_x(INVALID_MFN) )
continue;
-ept = >arch.altp2m_p2m[i]->ept;
+ept = _get_p2m(currd, i)->ept;
if ( cpumask_test_cpu(cpu, ept->invalidate) )
{
cpumask_clear_cpu(cpu, ept->invalidate);
diff --git a/xen/arch/x86/include/asm/altp2m.h
b/xen/arch/x86/include/asm/altp2m.h
index e5e59cbd68..2f064c61a2 100644
--- a/xen/arch/x86/include/asm/altp2m.h
+++ b/xen/arch/x86/include/asm/altp2m.h
@@ -19,6 +19,38 @@ static inline bool altp2m_active(const struct domain *d)
return d->arch.altp2m_active;
}
+static inline struct p2m_domain *altp2m_get_p2m(const struct domain* d,
+unsigned int idx)
+{
+return d->arch.altp2m_p2m[array_index_nospec(idx, MAX_ALTP2M)];
+}
+
+static inline uint64_t altp2m_get_eptp(const struct domain* d,
+ unsigned int idx)
+{
+return d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)];
+}
+
+static inline void altp2m_set_eptp(const struct domain* d,
+ unsigned int idx,
+ uint64_t eptp)
+{
+d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] = eptp;
+}
+
+static inline uint64_t altp2m_get_visible_eptp(const struct domain* d,
+ unsigned int idx)
+{
+return d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)];
+}
+
+static inline void altp2m_set_visible_eptp(const struct domain* d,
+ unsigned int idx,
+ uint64_t eptp)
+{
+d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)] = eptp;
+}
+
/* Alternate p2m VCPU */
void altp2m_vcpu_initialise(struct vcpu *v);
void altp2m_vcpu_destroy(struct vcpu *v);
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index c1478ffc36..e6f7764f9f 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -18,6 +18,7 @@
#include
#include
#include /* for pagetable_t */
+#include
/* Debugging and auditing of the P2M code? */
#if !defined(NDEBUG) && defined(CONFIG_HVM)
@@ -888,13 +889,14 @@ static inline struct p2m_domain *p2m_get_altp2m(struct
vcpu *v)
BUG_ON(index >= MAX_ALTP2M);
-return v->domain->arch.altp2m_p2m[index];
+return altp2m_get_p2m(v->domain, index);
}
/* set current alternate p2m table */
static inline bool p2m_set_altp2m(struct vcpu *v, unsigned int idx)
{
struct p2m_domain *orig;
+struct p2m_domain *ap2m;
BUG_ON(idx >= MAX_ALTP2M);
@@ -906,7 +908,8 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned
int idx)
atomic_dec(>active_vcpus);
vcpu_altp2m(v).p2midx = idx;
-atomic_inc(>domain->arch.altp2m_p2m[idx]->active_vcpus);
+ap2m = altp2m_get_p2m(v->domain, idx);
+atomic_inc(>active_vcpus);
return true;
}
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 6fe1e9ed6b..7fb1738376 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -205,7 +205,7 @@ bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned
int idx)
altp2m_list_lock(d);
-if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
+if ( altp2m_get_eptp(d, idx) != mfn_x(INVALID_MFN) )
{
if ( p2m_set_altp2m(v, idx) )
altp2m_vcpu_update_p2m(v);
@@ -307,7 +307,7 @@ static void p2m_reset_altp2m(struct domain *d, unsigned int
idx,
struct p2m_domain *p2m;
ASSERT(idx < MAX_ALTP2M);
-p2m = array_acc
[PATCH for-4.19? v5 04/10] tools/xl: Add altp2m_count parameter
From: Petr Beneš
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š
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 9 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 9 +
6 files changed, 30 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index fe5110474d..0449c55f31 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1159,6 +1159,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_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();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1676,6 +1677,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_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(); 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 c9e45b306f..54607758d3 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -603,6 +603,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+Altp2MCount uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f5c7167742..bfa06caad2 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1250,6 +1250,14 @@ 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
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
+
/*
* 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 569e3d21ed..11d2f282f5 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -482,6 +482,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(_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 4e65e6fda5..2963c5e250 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -729,6 +729,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 e3a4800f6e..a82b8fe6e4 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2063,6 +2063,15 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "altp2m_count", , 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", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
[PATCH for-4.19? v5 09/10] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0
From: Petr Beneš
Since libxl finally sends the altp2m.nr value, we can remove the previously
introduced temporary workaround.
Creating domain with enabled altp2m while setting altp2m.nr == 0 doesn't
make sense and it's probably not what user wants.
Signed-off-by: Petr Beneš
---
xen/arch/x86/domain.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 011cffb07e..52bfeafe3f 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -747,8 +747,9 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
if ( !config->altp2m.nr )
{
-/* Fix the value to the legacy default */
-config->altp2m.nr = 10;
+dprintk(XENLOG_INFO,
+"altp2m must be requested with altp2m.nr > 0\n");
+return -EINVAL;
}
if ( config->altp2m.nr > MAX_NR_ALTP2M )
--
2.34.1
[PATCH for-4.19? v5 01/10] tools/ocaml: Fix mixed tabs/spaces
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
---
tools/ocaml/libs/xc/xenctrl_stubs.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index c6da9bb091..e86c455802 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -138,12 +138,12 @@ static void
domain_handle_of_uuid_string(xen_domain_handle_t h,
* integers in the Ocaml ABI for more idiomatic handling.
*/
static value c_bitmap_to_ocaml_list
- /* ! */
- /*
+/* ! */
+/*
* All calls to this function must be in a form suitable
* for xenctrl_abi_check. The parsing there is ad-hoc.
*/
- (unsigned int bitmap)
+(unsigned int bitmap)
{
CAMLparam0();
CAMLlocal2(list, tmp);
@@ -180,8 +180,8 @@ static value c_bitmap_to_ocaml_list
}
static unsigned int ocaml_list_to_c_bitmap(value l)
- /* ! */
- /*
+/* ! */
+/*
* All calls to this function must be in a form suitable
* for xenctrl_abi_check. The parsing there is ad-hoc.
*/
@@ -259,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
/* Quick & dirty check for ABI changes. */
BUILD_BUG_ON(sizeof(cfg) != 68);
-/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
+ /* Mnemonics for the named fields inside
xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
#define VAL_MISC_FLAGS Field(arch_domconfig, 1)
@@ -351,7 +351,7 @@ static value dom_op(value xch_val, value domid,
caml_enter_blocking_section();
result = fn(xch, c_domid);
caml_leave_blocking_section();
-if (result)
+ if (result)
failwith_xc(xch);
CAMLreturn(Val_unit);
}
@@ -383,7 +383,7 @@ CAMLprim value stub_xc_domain_resume_fast(value xch_val,
value domid)
caml_enter_blocking_section();
result = xc_domain_resume(xch, c_domid, 1);
caml_leave_blocking_section();
-if (result)
+ if (result)
failwith_xc(xch);
CAMLreturn(Val_unit);
}
@@ -426,7 +426,7 @@ static value alloc_domaininfo(xc_domaininfo_t * info)
Store_field(result, 13, Val_int(info->max_vcpu_id));
Store_field(result, 14, caml_copy_int32(info->ssidref));
-tmp = caml_alloc_small(16, 0);
+ tmp = caml_alloc_small(16, 0);
for (i = 0; i < 16; i++) {
Field(tmp, i) = Val_int(info->handle[i]);
}
--
2.34.1
[PATCH for-4.19? v5 02/10] tools/ocaml: Add missing ocaml bindings for altp2m_opts
From: Petr Beneš
Fixes: 0291089f6ea8 ("xen: enable altp2m at create domain domctl")
Signed-off-by: Petr Beneš
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 9 ++---
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..2690f9a923 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+altp2m_opts: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..febbe1f6ae 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ altp2m_opts: int32;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index e86c455802..a529080129 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,9 +210,10 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_ALTP2M_OPTS Field(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -230,6 +231,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .altp2m_opts = Int32_val(VAL_ALTP2M_OPTS),
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -288,6 +290,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_OPTS
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
[PATCH for-4.19? v5 08/10] tools/libxl: Activate the altp2m_count feature
From: Petr Beneš
This commit activates the previously introduced altp2m_count parameter,
establishing the connection between libxl and Xen.
Signed-off-by: Petr Beneš
---
tools/libs/light/libxl_create.c | 4
1 file changed, 4 insertions(+)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 11d2f282f5..5ad552c4ec 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -656,6 +656,10 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
.max_grant_frames = b_info->max_grant_frames,
.max_maptrack_frames = b_info->max_maptrack_frames,
.grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
+.altp2m = {
+.opts = 0, /* .opts will be set below */
+.nr = b_info->altp2m_count,
+},
.vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10,
XC_PAGE_SHIFT),
.cpupool_id = info->poolid,
};
--
2.34.1
[PATCH for-4.19? v5 05/10] docs/man: Add altp2m_count parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the altp2m_count configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index ac3f88fd57..ff03b43884 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
[PATCH for-4.19? v5 00/10] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Changes since v4: - Rebased on top of staging (applying Roger's changes). - Fix mixed tabs/spaces in xenctrl_stubs.c. - Add missing OCaml bindings for altp2m_opts. - Substitute altp2m_opts into an unnamed structure. (This is a preparation for the next patch that will introduce the `nr` field.) - altp2m.opts is then shortened to uint16_t and a new field altp2m.nr is added - also uint16_t. This value is then verified by libxl to not exceed the maximum uint16_t value. This puts a hard limit to number of altp2m to 65535, which is enough, at least for the time being. Also, altp2m.opts currently uses only 2 bits. Therefore I believe this change is justified. - Introduction of accessor functions for altp2m arrays and refactoring the code to use them. - Added a check to arm/arch_sanitise_domain_config() to disallow creating domains with altp2m.nr != 0. - Added dummy hvm_altp2m_supported() to avoid build errors when CONFIG_HVM is disabled. - Finally, expose altp2m_count to OCaml bindings (and verify both altp2m_opts and altp2m_count fit uint16_t). - I also removed Christian Lindig from the Acked-by, since I think this change is significant enough to require a re-review. Changes since v3: - Rebased on top of staging (some functions were moved to altp2m.c). - Re-added the array_index_nospec() where it was removed. Changes since v2: - Changed max_altp2m to nr_altp2m. - Moved arch-dependent check from xen/common/domain.c to xen/arch/x86/domain.c. - Replaced min(d->nr_altp2m, MAX_EPTP) occurences for just d->nr_altp2m. - Replaced array_index_nospec(altp2m_idx, ...) for just altp2m_idx. - Shortened long lines. - Removed unnecessary comments in altp2m_vcpu_initialise/destroy. - Moved nr_altp2m field after max_ fields in xen_domctl_createdomain. - Removed the commit that adjusted the initial allocation of pages from 256 to 1024. This means that after these patches, technically, the nr_altp2m will be capped to (256 - 1 - vcpus - MAX_NESTEDP2M) instead of MAX_EPTP (512). Future work will be needed to fix this. Petr Beneš (10): tools/ocaml: Fix mixed tabs/spaces tools/ocaml: Add missing ocaml bindings for altp2m_opts xen: Refactor altp2m options into a structured format tools/xl: Add altp2m_count parameter docs/man: Add altp2m_count parameter to the xl.cfg manual x86/altp2m: Introduce accessor functions for safer array indexing xen: Make the maximum number of altp2m views configurable for x86 tools/libxl: Activate the altp2m_count feature xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0 tools/ocaml: Add altp2m_count parameter docs/man/xl.cfg.5.pod.in | 14 tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go | 1 + tools/include/libxl.h| 8 ++ tools/libs/light/libxl_create.c | 19 - tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml | 2 + tools/ocaml/libs/xc/xenctrl.mli | 2 + tools/ocaml/libs/xc/xenctrl_stubs.c | 40 +++--- tools/xl/xl_parse.c | 9 +++ xen/arch/arm/domain.c| 2 +- xen/arch/x86/domain.c| 45 --- xen/arch/x86/hvm/hvm.c | 10 ++- xen/arch/x86/hvm/vmx/vmx.c | 6 +- xen/arch/x86/include/asm/altp2m.h| 32 xen/arch/x86/include/asm/domain.h| 9 ++- xen/arch/x86/include/asm/hvm/hvm.h | 5 ++ xen/arch/x86/include/asm/p2m.h | 11 ++- xen/arch/x86/mm/altp2m.c | 110 ++- xen/arch/x86/mm/hap/hap.c| 16 ++-- xen/arch/x86/mm/mem_access.c | 25 +++--- xen/arch/x86/mm/mem_sharing.c| 4 +- xen/arch/x86/mm/p2m-ept.c| 18 ++--- xen/arch/x86/mm/p2m.c| 24 +++--- xen/common/domain.c | 1 + xen/include/public/domctl.h | 7 +- xen/include/xen/sched.h | 2 + 27 files changed, 290 insertions(+), 135 deletions(-) -- 2.34.1
[PATCH for-4.19? v5 03/10] xen: Refactor altp2m options into a structured format
From: Petr Beneš
Encapsulate the altp2m options within a struct. This change is preparatory
and sets the groundwork for introducing additional parameter in subsequent
commit.
Signed-off-by: Petr Beneš
---
tools/libs/light/libxl_create.c | 6 +++---
tools/ocaml/libs/xc/xenctrl_stubs.c | 4 +++-
xen/arch/arm/domain.c | 2 +-
xen/arch/x86/domain.c | 4 ++--
xen/arch/x86/hvm/hvm.c | 2 +-
xen/include/public/domctl.h | 4 +++-
6 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index edeadd57ef..569e3d21ed 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -680,17 +680,17 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
LOG(DETAIL, "altp2m: %s", libxl_altp2m_mode_to_string(b_info->altp2m));
switch(b_info->altp2m) {
case LIBXL_ALTP2M_MODE_MIXED:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_mixed);
break;
case LIBXL_ALTP2M_MODE_EXTERNAL:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_external);
break;
case LIBXL_ALTP2M_MODE_LIMITED:
-create.altp2m_opts |=
+create.altp2m.opts |=
XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_limited);
break;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index a529080129..e6c977521f 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -231,7 +231,9 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
- .altp2m_opts = Int32_val(VAL_ALTP2M_OPTS),
+ .altp2m = {
+ .opts = Int32_val(VAL_ALTP2M_OPTS),
+ },
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8bde2f730d..5234b627d0 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -688,7 +688,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m_opts )
+if ( config->altp2m.opts )
{
dprintk(XENLOG_INFO, "Altp2m not supported\n");
return -EINVAL;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 536542841e..bb5ba8fc1e 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -637,7 +637,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
bool hap = config->flags & XEN_DOMCTL_CDF_hap;
bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
unsigned int max_vcpus;
-unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
+unsigned int altp2m_mode = MASK_EXTR(config->altp2m.opts,
XEN_DOMCTL_ALTP2M_mode_mask);
if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
@@ -717,7 +717,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
+if ( config->altp2m.opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
{
dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
config->flags);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8334ab1711..a66ebaaceb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -659,7 +659,7 @@ int hvm_domain_initialise(struct domain *d,
d->arch.hvm.params[HVM_PARAM_TRIPLE_FAULT_REASON] = SHUTDOWN_reboot;
/* Set altp2m based on domctl flags. */
-switch ( MASK_EXTR(config->altp2m_opts, XEN_DOMCTL_ALTP2M_mode_mask) )
+switch ( MASK_EXTR(config->altp2m.opts, XEN_DOMCTL_ALTP2M_mode_mask) )
{
case XEN_DOMCTL_ALTP2M_mixed:
d->arch.hvm.params[HVM_PARAM_ALTP2M] = XEN_ALTP2M_mixed;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 2a49fe46ce..dea399aa8e 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -86,6 +86,7 @@ struct xen_domctl_createdomain {
uint32_t grant_opts;
+struct {
/*
* Enable altp2m mixed mode.
*
@@ -102,7 +103,8 @@ struct xen_domctl_createdomain {
/* Altp2m mode signaling uses bits [0, 1]. */
#define XEN_DOMCTL_ALTP2M_mode_mask (0x3U)
#define XEN_DOMCTL_ALTP2M_mode(m)((m) & XEN_DOMCTL_ALTP2M_mode_mask)
-uint32_t altp2m_opt
[PATCH for-4.19? v5 07/10] xen: Make the maximum number of altp2m views configurable for x86
From: Petr Beneš
x86: Make the maximum number of altp2m views configurable
This commit introduces the ability to configure the maximum number of altp2m
views for the domain during its creation. Previously, the limits were hardcoded
to a maximum of 10. This change allows for greater flexibility in environments
that require more or fewer altp2m views.
The maximum configurable limit for nr_altp2m on x86 is now set to
MAX_NR_ALTP2M (which currently holds the MAX_EPTP value - 512). This cap is
linked to the architectural limit of the EPTP-switching VMFUNC, which supports
up to 512 entries. Despite there being no inherent need for limiting nr_altp2m
in scenarios not utilizing VMFUNC, decoupling these components would necessitate
substantial code changes.
xen_domctl_createdomain::altp2m is extended for a new field `nr`, that will
configure this limit for a domain. Additionally, previous altp2m.opts value
has been reduced from uint32_t to uint16_t so that both of these fields occupy
as little space as possible.
altp2m_get_p2m() function is modified to respect the new nr_altp2m value.
Accessor functions that operate on EPT arrays are unmodified, since these
arrays always have fixed size of MAX_EPTP.
A dummy hvm_altp2m_supported() function is introduced for non-HVM builds, so
that the compilation won't fail for them.
Additional sanitization is introduced in the x86/arch_sanitise_domain_config
to fix the altp2m.nr value to 10 if it is previously set to 0. This behavior
is only temporary and immediately removed in the upcoming commit (which will
disallow creating a domain with enabled altp2m with zero nr_altp2m).
The reason for this temporary workaround is to retain the legacy behavior
until the feature is fully activated in libxl.
Also, arm/arch_sanitise_domain_config is extended to not allow requesting
non-zero altp2ms.
Signed-off-by: Petr Beneš
---
xen/arch/arm/domain.c | 2 +-
xen/arch/x86/domain.c | 40 ++
xen/arch/x86/hvm/hvm.c | 8 -
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
xen/arch/x86/include/asm/altp2m.h | 2 +-
xen/arch/x86/include/asm/domain.h | 9 ++---
xen/arch/x86/include/asm/hvm/hvm.h | 5 +++
xen/arch/x86/include/asm/p2m.h | 4 +--
xen/arch/x86/mm/altp2m.c | 54 --
xen/arch/x86/mm/hap/hap.c | 8 ++---
xen/arch/x86/mm/mem_access.c | 8 ++---
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 4 +--
xen/arch/x86/mm/p2m.c | 8 ++---
xen/common/domain.c| 1 +
xen/include/public/domctl.h| 5 ++-
xen/include/xen/sched.h| 2 ++
17 files changed, 113 insertions(+), 51 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 5234b627d0..e5785d2d96 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -688,7 +688,7 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( config->altp2m.opts )
+if ( config->altp2m.opts || config->altp2m.nr )
{
dprintk(XENLOG_INFO, "Altp2m not supported\n");
return -EINVAL;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index bb5ba8fc1e..011cffb07e 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -724,16 +724,42 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
-if ( altp2m_mode && nested_virt )
+if ( altp2m_mode )
{
-dprintk(XENLOG_INFO,
-"Nested virt and altp2m are not supported together\n");
-return -EINVAL;
-}
+if ( nested_virt )
+{
+dprintk(XENLOG_INFO,
+"Nested virt and altp2m are not supported together\n");
+return -EINVAL;
+}
+
+if ( !hap )
+{
+dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
+return -EINVAL;
+}
+
+if ( !hvm_altp2m_supported() )
+{
+dprintk(XENLOG_INFO, "altp2m is not supported\n");
+return -EINVAL;
+}
+
+if ( !config->altp2m.nr )
+{
+/* Fix the value to the legacy default */
+config->altp2m.nr = 10;
+}
-if ( altp2m_mode && !hap )
+if ( config->altp2m.nr > MAX_NR_ALTP2M )
+{
+dprintk(XENLOG_INFO, "altp2m.nr must be <= %lu\n", MAX_NR_ALTP2M);
+return -EINVAL;
+}
+}
+else if ( config->altp2m.nr )
{
-dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
+dprintk(XENLOG_INFO, "altp2m.nr must be zero when altp2m is off\n");
return -EINVAL;
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a66ebaaceb..3d0357a0f8 100644
Re: [PATCH for-4.19? v4 4/6] x86: Make the maximum number of altp2m views configurable
On Mon, May 27, 2024 at 8:19 AM Jan Beulich wrote:
>
>
> This is suspicious: You compare against one value but log another. This
> isn't EPT-specific, so shouldn't use MAX_EPTP.
Sorry, I copy-pasted a snippet and didn't edit it correctly. Of
course, it should have been:
if ( config->nr_altp2m > MAX_NR_ALTP2M )
{
dprintk(XENLOG_INFO, "nr_altp2m must be <= %lu\n", MAX_NR_ALTP2M);
return -EINVAL;
}
> > ... should I be consistent and also replace these accesses with
> > altp2m_get_eptp/altp2m_get_p2m (which will internally use
> > array_index_nospec), or should I leave them as they are?
>
> Perhaps leave them as they are, unless you can technically justify the
> adjustment.
If we can avoid speculative execution, why just not do it? The
performance overhead array_index_nospec is negligible compared to the
whole VMEXIT handling. It will also serve as future-proofing, since
nobody will be confused whether they should directly access the array,
but instead use the accessor function.
Currently, the idea seems to be that array_index_nospec() is used when
the index is user-controlled, and not used when the index is under
xen's control (i.e. in loops). But I found at least 2 instances where
the index _is_ user controlled and the nospec access is not used -
further proving my previous point.
That being said, if there are no protests, I would replace all array
index accesses with the newly introduced accessor functions, which
will unconditionally use array_index_nospec().
P.
Re: [PATCH for-4.19? v4 4/6] x86: Make the maximum number of altp2m views configurable
On Tue, May 21, 2024 at 12:59 PM Jan Beulich wrote:
>
> The compared entities don't really fit together. I think we want a new
> MAX_NR_ALTP2M, which - for the time being - could simply be
>
> #define MAX_NR_ALTP2M MAX_EPTP
>
> in the header. That would then be a suitable replacement for the
> min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) that you're adjusting
> elsewhere. Which however raises the question whether in EPT-specific
> code the min() wouldn't better survive, as min(d->nr_altp2m, MAX_EPTP).
>
As you mentioned in a previous email, I've removed all the min(...,
MAX_EPTP) invocations from the code, since nr_altp2m is validated to
be no greater than that value. The only remaining places where this
value occurs are:
- In my newly introduced condition in arch_sanitise_domain_config:
if ( config->nr_altp2m > MAX_EPTP )
{
dprintk(XENLOG_INFO, "nr_altp2m must be <= %lu\n", MAX_NR_ALTP2M);
return -EINVAL;
}
- In hap_enable():
for ( i = 0; i < MAX_EPTP; i++ )
{
d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
}
Note that altp2m_eptp/altp2m_visible_eptp is never accessed beyond
nr_altp2m. From what you're saying, it sounds to me like I should only
replace the first mentioned occurrence with MAX_NR_ALTP2M. Correct me
if I'm wrong.
> > @@ -5228,7 +5234,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t
> > p2midx)
> > if ( !hvm_is_singlestep_supported() )
> > return;
> >
> > -if ( p2midx >= MAX_ALTP2M )
> > +if ( p2midx >= v->domain->nr_altp2m )
> > return;
>
> You don't introduce a new local variable here. I'd like to ask that you also
> don't ...
>
> > @@ -403,12 +403,12 @@ long p2m_set_mem_access_multi(struct domain *d,
> > /* altp2m view 0 is treated as the hostp2m */
> > if ( altp2m_idx )
> > {
> > -if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> > - d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)]
> > ==
> > - mfn_x(INVALID_MFN) )
> > +if ( altp2m_idx >= d->nr_altp2m ||
> > + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx,
> > d->nr_altp2m)]
> > + == mfn_x(INVALID_MFN) )
>
> Please don't break previously correct style: Binary operators (here: == )
> belong onto the end of the earlier line. That'll render the line too long
> again, but you want to deal with that e.g. thus:
>
> d->arch.altp2m_eptp[array_index_nospec(altp2m_idx,
> d->nr_altp2m)] ==
> mfn_x(INVALID_MFN) )
>
Roger suggested introducing the altp2m_get_p2m() function, which I
like. I think introducing altp2m_get_eptp/visible_eptp and
altp2m_set_eptp/visible_eptp would also elegantly solve the issue of
overly long lines. My question is: if I go this route, should I
strictly replace with these functions only accesses that use
array_index_nospec()? Or should I replace all array accesses? For
example:
for ( i = 0; i < d->nr_altp2m; i++ )
{
struct p2m_domain *p2m;
if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
p2m = d->arch.altp2m_p2m[i];
p2m_lock(p2m);
p2m->ept.ad = value;
p2m_unlock(p2m);
}
... should I be consistent and also replace these accesses with
altp2m_get_eptp/altp2m_get_p2m (which will internally use
array_index_nospec), or should I leave them as they are?
P.
[PATCH for-4.19? v4 5/6] tools/libxl: Activate the altp2m_count feature
From: Petr Beneš This commit activates the previously introduced altp2m_count parameter, establishing the connection between libxl and Xen. Signed-off-by: Petr Beneš --- tools/libs/light/libxl_create.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index 236b8c1965..f5eb16d0dc 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -657,6 +657,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, .max_grant_frames = b_info->max_grant_frames, .max_maptrack_frames = b_info->max_maptrack_frames, .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version), +.nr_altp2m = b_info->altp2m_count, .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT), .cpupool_id = info->poolid, }; -- 2.34.1
[PATCH for-4.19? v4 6/6] tools/ocaml: Add altp2m_count parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the altp2m_count parameter.
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 11 +++
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..dfb3d331c9 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+altp2m_count: int;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..ff0e309c56 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ altp2m_count: int;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2b6d3c09df..1f544cd2e4 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,9 +210,10 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_ALTP2M_COUNTField(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -230,6 +231,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .nr_altp2m = Int_val(VAL_ALTP2M_COUNT),
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -257,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#if defined(__i386__) || defined(__x86_64__)
/* Quick & dirty check for ABI changes. */
- BUILD_BUG_ON(sizeof(cfg) != 64);
+ BUILD_BUG_ON(sizeof(cfg) != 68);
/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
@@ -288,6 +290,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_COUNT
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
[PATCH for-4.19? v4 0/6] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. Adjustments include: - "Prepare" commits with style changes. - Adding a new `altp2m_count` parameter to xl. - Adding a new `altp2m_count` parameter to the OCaml bindings. - Adding a new `altp2m_count` parameter to the xl.cfg manual. - Adding a new `nr_altp2m` field into the `xen_domctl_createdomain`, which, after sanity checks, is stored in newly introduced `nr_altp2m` field of `struct domain` - leaving room for other architectures to implement the altp2m feature. - Replacing MAX_ALTP2M macro occurrences with `domain->nr_altp2m`. This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Changes since v2: - Changed max_altp2m to nr_altp2m. - Moved arch-dependent check from xen/common/domain.c to xen/arch/x86/domain.c. - Replaced min(d->nr_altp2m, MAX_EPTP) occurences for just d->nr_altp2m. - Replaced array_index_nospec(altp2m_idx, ...) for just altp2m_idx. - Shortened long lines. - Removed unnecessary comments in altp2m_vcpu_initialise/destroy. - Moved nr_altp2m field after max_ fields in xen_domctl_createdomain. - Removed the commit that adjusted the initial allocation of pages from 256 to 1024. This means that after these patches, technically, the nr_altp2m will be capped to (256 - 1 - vcpus - MAX_NESTEDP2M) instead of MAX_EPTP (512). Future work will be needed to fix this. Changes since v3: - Rebased on top of staging (some functions were moved to altp2m.c). - Re-added the array_index_nospec() where it was removed. Petr Beneš (6): x86/p2m: Add braces for better code clarity tools/xl: Add altp2m_count parameter docs/man: Add altp2m_count parameter to the xl.cfg manual x86: Make the maximum number of altp2m views configurable tools/libxl: Activate the altp2m_count feature tools/ocaml: Add altp2m_count parameter docs/man/xl.cfg.5.pod.in | 14 + tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go | 1 + tools/include/libxl.h| 8 +++ tools/libs/light/libxl_create.c | 10 +++ tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml | 1 + tools/ocaml/libs/xc/xenctrl.mli | 1 + tools/ocaml/libs/xc/xenctrl_stubs.c | 11 ++-- tools/xl/xl_parse.c | 4 ++ xen/arch/x86/domain.c| 12 xen/arch/x86/hvm/hvm.c | 8 ++- xen/arch/x86/hvm/vmx/vmx.c | 2 +- xen/arch/x86/include/asm/domain.h| 7 +-- xen/arch/x86/include/asm/p2m.h | 6 +- xen/arch/x86/mm/altp2m.c | 91 +--- xen/arch/x86/mm/hap/hap.c| 6 +- xen/arch/x86/mm/mem_access.c | 24 xen/arch/x86/mm/mem_sharing.c| 2 +- xen/arch/x86/mm/p2m-ept.c| 12 ++-- xen/arch/x86/mm/p2m.c| 12 ++-- xen/common/domain.c | 1 + xen/include/public/domctl.h | 5 +- xen/include/xen/sched.h | 2 + 24 files changed, 169 insertions(+), 74 deletions(-) -- 2.34.1
[PATCH for-4.19? v4 1/6] x86/p2m: Add braces for better code clarity
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
Reviewed-by: Stefano Stabellini
---
xen/arch/x86/mm/p2m.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 4a4620e870..db5d9b6c2a 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
change_entry_type_global(altp2m, ot, nt);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
@@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
_memory_type_changed(altp2m);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
--
2.34.1
[PATCH for-4.19? v4 2/6] tools/xl: Add altp2m_count parameter
From: Petr Beneš
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š
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 9 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 4
6 files changed, 25 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 78bdb08b15..40c247a0d0 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1158,6 +1158,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_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();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_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(); 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 ccfe18019e..a3ae8ef35a 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -602,6 +602,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+Altp2MCount uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..1f149a8015 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1239,6 +1239,14 @@ 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
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
+
/*
* 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 41252ec553..236b8c1965 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -483,6 +483,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(_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 79e9c656cc..eb306fedf5 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -728,6 +728,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 c504ab3711..048ab6be0d 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2063,6 +2063,10 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "altp2m_count", , 1)) {
+b_info->altp2m_count = l;
+}
+
if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
[PATCH for-4.19? v4 3/6] docs/man: Add altp2m_count parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the altp2m_count configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 8f2b375ce9..5c09610cf4 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
[PATCH for-4.19? v4 4/6] x86: Make the maximum number of altp2m views configurable
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
views for the domain during its creation. Previously, the limits were hardcoded
to a maximum of 10. This change allows for greater flexibility in environments
that require more or fewer altp2m views.
The maximum configurable limit for max_altp2m on x86 is now set to MAX_EPTP
(512). This cap is linked to the architectural limit of the EPTP-switching
VMFUNC, which supports up to 512 entries. Despite there being no inherent need
for limiting max_altp2m in scenarios not utilizing VMFUNC, decoupling these
components would necessitate substantial code changes.
Signed-off-by: Petr Beneš
---
xen/arch/x86/domain.c | 12
xen/arch/x86/hvm/hvm.c| 8 ++-
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/include/asm/domain.h | 7 +--
xen/arch/x86/include/asm/p2m.h| 6 +-
xen/arch/x86/mm/altp2m.c | 91 +++
xen/arch/x86/mm/hap/hap.c | 6 +-
xen/arch/x86/mm/mem_access.c | 24
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 12 ++--
xen/arch/x86/mm/p2m.c | 8 +--
xen/common/domain.c | 1 +
xen/include/public/domctl.h | 5 +-
xen/include/xen/sched.h | 2 +
14 files changed, 116 insertions(+), 70 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 00a3aaa576..3bd18cb2d0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -685,6 +685,18 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+if ( config->nr_altp2m && !hvm_altp2m_supported() )
+{
+dprintk(XENLOG_INFO, "altp2m requested but not available\n");
+return -EINVAL;
+}
+
+if ( config->nr_altp2m > MAX_EPTP )
+{
+dprintk(XENLOG_INFO, "nr_altp2m must be <= %lu\n", MAX_EPTP);
+return -EINVAL;
+}
+
if ( config->vmtrace_size )
{
unsigned int size = config->vmtrace_size;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9594e0a5c5..77e4016bdb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4639,6 +4639,12 @@ static int do_altp2m_op(
goto out;
}
+if ( d->nr_altp2m == 0 )
+{
+rc = -EINVAL;
+goto out;
+}
+
if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
goto out;
@@ -5228,7 +5234,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
if ( !hvm_is_singlestep_supported() )
return;
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= v->domain->nr_altp2m )
return;
v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5f67a48592..76ee09b701 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4888,7 +4888,7 @@ bool asmlinkage vmx_vmenter_helper(const struct
cpu_user_regs *regs)
{
unsigned int i;
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < currd->nr_altp2m; ++i )
{
if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
diff --git a/xen/arch/x86/include/asm/domain.h
b/xen/arch/x86/include/asm/domain.h
index f5daeb182b..3935328781 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,10 @@ struct paging_vcpu {
struct shadow_vcpu shadow;
};
-#define MAX_NESTEDP2M 10
-
-#define MAX_ALTP2M 10 /* arbitrary */
#define INVALID_ALTP2M 0x
#define MAX_EPTP(PAGE_SIZE / sizeof(uint64_t))
+#define MAX_NESTEDP2M 10
+
struct p2m_domain;
struct time_scale {
int shift;
@@ -353,7 +352,7 @@ struct arch_domain
/* altp2m: allow multiple copies of host p2m */
bool altp2m_active;
-struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
+struct p2m_domain **altp2m_p2m;
mm_lock_t altp2m_list_lock;
uint64_t *altp2m_eptp;
uint64_t *altp2m_visible_eptp;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..e66c081149 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -881,7 +881,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu
*v)
if ( index == INVALID_ALTP2M )
return NULL;
-BUG_ON(index >= MAX_ALTP2M);
+BUG_ON(index >= v->domain->nr_altp2m);
return v->domain->arch.altp2m_p2m[index];
}
@@ -891,7 +891,7 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned
int idx)
{
struct p2m_domain *orig;
-BUG_ON(idx >= MAX_ALTP2M);
+BUG_ON(idx >= v->domain->nr_altp2m);
if ( idx == vcpu_altp2m(v).p2midx )
return false;
@@ -943,7 +943,7 @@ int p2m_altp2m_propagate_change(s
Re: [PATCH for-4.19? v3 4/6] x86: Make the maximum number of altp2m views configurable
On Sat, May 18, 2024 at 3:18 AM Tamas K Lengyel wrote: > > > -ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx); > > +ap2m = d->arch.altp2m_p2m[altp2m_idx]; > > Why is it no longer necessary to use array_access_nospec? > > Tamas I was under the impression that when the domain_create guarantees the length of the array, and before each access to the altp2m_p2m there is an explicit check if idx is within bounds of nr_altp2m, that the array_access_nospec is not needed anymore. But apparently I misunderstood how speculative execution works, when I now read about it. I'll put it back. P.
[PATCH for-4.19? v3 4/6] x86: Make the maximum number of altp2m views configurable
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
views for the domain during its creation. Previously, the limits were hardcoded
to a maximum of 10. This change allows for greater flexibility in environments
that require more or fewer altp2m views.
The maximum configurable limit for max_altp2m on x86 is now set to MAX_EPTP
(512). This cap is linked to the architectural limit of the EPTP-switching
VMFUNC, which supports up to 512 entries. Despite there being no inherent need
for limiting max_altp2m in scenarios not utilizing VMFUNC, decoupling these
components would necessitate substantial code changes.
Signed-off-by: Petr Beneš
---
xen/arch/x86/domain.c | 12 +
xen/arch/x86/hvm/hvm.c| 8 +++-
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/include/asm/domain.h | 7 ++-
xen/arch/x86/include/asm/p2m.h| 4 +-
xen/arch/x86/mm/altp2m.c | 23 +-
xen/arch/x86/mm/hap/hap.c | 6 +--
xen/arch/x86/mm/mem_access.c | 23 --
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 10 ++--
xen/arch/x86/mm/p2m.c | 76 +++
xen/common/domain.c | 1 +
xen/include/public/domctl.h | 5 +-
xen/include/xen/sched.h | 2 +
14 files changed, 108 insertions(+), 73 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 00a3aaa576..3bd18cb2d0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -685,6 +685,18 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+if ( config->nr_altp2m && !hvm_altp2m_supported() )
+{
+dprintk(XENLOG_INFO, "altp2m requested but not available\n");
+return -EINVAL;
+}
+
+if ( config->nr_altp2m > MAX_EPTP )
+{
+dprintk(XENLOG_INFO, "nr_altp2m must be <= %lu\n", MAX_EPTP);
+return -EINVAL;
+}
+
if ( config->vmtrace_size )
{
unsigned int size = config->vmtrace_size;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9594e0a5c5..77e4016bdb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4639,6 +4639,12 @@ static int do_altp2m_op(
goto out;
}
+if ( d->nr_altp2m == 0 )
+{
+rc = -EINVAL;
+goto out;
+}
+
if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
goto out;
@@ -5228,7 +5234,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
if ( !hvm_is_singlestep_supported() )
return;
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= v->domain->nr_altp2m )
return;
v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5f67a48592..76ee09b701 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4888,7 +4888,7 @@ bool asmlinkage vmx_vmenter_helper(const struct
cpu_user_regs *regs)
{
unsigned int i;
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < currd->nr_altp2m; ++i )
{
if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
diff --git a/xen/arch/x86/include/asm/domain.h
b/xen/arch/x86/include/asm/domain.h
index f5daeb182b..3935328781 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,10 @@ struct paging_vcpu {
struct shadow_vcpu shadow;
};
-#define MAX_NESTEDP2M 10
-
-#define MAX_ALTP2M 10 /* arbitrary */
#define INVALID_ALTP2M 0x
#define MAX_EPTP(PAGE_SIZE / sizeof(uint64_t))
+#define MAX_NESTEDP2M 10
+
struct p2m_domain;
struct time_scale {
int shift;
@@ -353,7 +352,7 @@ struct arch_domain
/* altp2m: allow multiple copies of host p2m */
bool altp2m_active;
-struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
+struct p2m_domain **altp2m_p2m;
mm_lock_t altp2m_list_lock;
uint64_t *altp2m_eptp;
uint64_t *altp2m_visible_eptp;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..6faa105baf 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -881,7 +881,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu
*v)
if ( index == INVALID_ALTP2M )
return NULL;
-BUG_ON(index >= MAX_ALTP2M);
+BUG_ON(index >= v->domain->nr_altp2m);
return v->domain->arch.altp2m_p2m[index];
}
@@ -891,7 +891,7 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned
int idx)
{
struct p2m_domain *orig;
-BUG_ON(idx >= MAX_ALTP2M);
+BUG_ON(idx >= v->domain->nr_altp2m);
if ( idx == vcpu_altp2m(v).p2midx )
return false;
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arc
[PATCH for-4.19? v3 6/6] tools/ocaml: Add altp2m_count parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the altp2m_count parameter.
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 11 +++
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..dfb3d331c9 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+altp2m_count: int;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..ff0e309c56 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ altp2m_count: int;
vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2b6d3c09df..1f544cd2e4 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,9 +210,10 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_ALTP2M_COUNTField(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -230,6 +231,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .nr_altp2m = Int_val(VAL_ALTP2M_COUNT),
.vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -257,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#if defined(__i386__) || defined(__x86_64__)
/* Quick & dirty check for ABI changes. */
- BUILD_BUG_ON(sizeof(cfg) != 64);
+ BUILD_BUG_ON(sizeof(cfg) != 68);
/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
@@ -288,6 +290,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
#undef VAL_VMTRACE_BUF_KB
+#undef VAL_ALTP2M_COUNT
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
[PATCH for-4.19? v3 1/6] x86/p2m: Add braces for better code clarity
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
Reviewed-by: Stefano Stabellini
---
xen/arch/x86/mm/p2m.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ce742c12e0..eb7996170d 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
change_entry_type_global(altp2m, ot, nt);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
@@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
_memory_type_changed(altp2m);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
--
2.34.1
[PATCH for-4.19? v3 2/6] tools/xl: Add altp2m_count parameter
From: Petr Beneš
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š
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 9 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 4
6 files changed, 25 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 78bdb08b15..40c247a0d0 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1158,6 +1158,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_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();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_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(); 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 ccfe18019e..a3ae8ef35a 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -602,6 +602,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+Altp2MCount uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..1f149a8015 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1239,6 +1239,14 @@ 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
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
+
/*
* 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 41252ec553..236b8c1965 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -483,6 +483,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(_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 79e9c656cc..eb306fedf5 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -728,6 +728,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 c504ab3711..048ab6be0d 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2063,6 +2063,10 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "altp2m_count", , 1)) {
+b_info->altp2m_count = l;
+}
+
if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
[PATCH for-4.19? v3 5/6] tools/libxl: Activate the altp2m_count feature
From: Petr Beneš This commit activates the previously introduced altp2m_count parameter, establishing the connection between libxl and Xen. Signed-off-by: Petr Beneš --- tools/libs/light/libxl_create.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index 236b8c1965..f5eb16d0dc 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -657,6 +657,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, .max_grant_frames = b_info->max_grant_frames, .max_maptrack_frames = b_info->max_maptrack_frames, .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version), +.nr_altp2m = b_info->altp2m_count, .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT), .cpupool_id = info->poolid, }; -- 2.34.1
[PATCH for-4.19? v3 3/6] docs/man: Add altp2m_count parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the altp2m_count configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 8f2b375ce9..5c09610cf4 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
[PATCH for-4.19? v3 0/6] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. Adjustments include: - "Prepare" commits with style changes. - Adding a new `altp2m_count` parameter to xl. - Adding a new `altp2m_count` parameter to the OCaml bindings. - Adding a new `altp2m_count` parameter to the xl.cfg manual. - Adding a new `nr_altp2m` field into the `xen_domctl_createdomain`, which, after sanity checks, is stored in newly introduced `nr_altp2m` field of `struct domain` - leaving room for other architectures to implement the altp2m feature. - Replacing MAX_ALTP2M macro occurrences with `domain->nr_altp2m`. This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Changes since v2: - Changed max_altp2m to nr_altp2m. - Moved arch-dependent check from xen/common/domain.c to xen/arch/x86/domain.c. - Replaced min(d->nr_altp2m, MAX_EPTP) occurences for just d->nr_altp2m. - Replaced array_index_nospec(altp2m_idx, ...) for just altp2m_idx. - Shortened long lines. - Removed unnecessary comments in altp2m_vcpu_initialise/destroy. - Moved nr_altp2m field after max_ fields in xen_domctl_createdomain. - Removed the commit that adjusted the initial allocation of pages from 256 to 1024. This means that after these patches, technically, the nr_altp2m will be capped to (256 - 1 + vcpus + MAX_NESTEDP2M) instead of MAX_EPTP (512). Future work will be needed to fix this. Petr Beneš (6): x86/p2m: Add braces for better code clarity tools/xl: Add altp2m_count parameter docs/man: Add altp2m_count parameter to the xl.cfg manual x86: Make the maximum number of altp2m views configurable tools/libxl: Activate the altp2m_count feature tools/ocaml: Add altp2m_count parameter docs/man/xl.cfg.5.pod.in | 14 + tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go | 1 + tools/include/libxl.h| 8 +++ tools/libs/light/libxl_create.c | 10 tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml | 1 + tools/ocaml/libs/xc/xenctrl.mli | 1 + tools/ocaml/libs/xc/xenctrl_stubs.c | 11 ++-- tools/xl/xl_parse.c | 4 ++ xen/arch/x86/domain.c| 12 + xen/arch/x86/hvm/hvm.c | 8 ++- xen/arch/x86/hvm/vmx/vmx.c | 2 +- xen/arch/x86/include/asm/domain.h| 7 ++- xen/arch/x86/include/asm/p2m.h | 4 +- xen/arch/x86/mm/altp2m.c | 23 +++- xen/arch/x86/mm/hap/hap.c| 6 +-- xen/arch/x86/mm/mem_access.c | 23 xen/arch/x86/mm/mem_sharing.c| 2 +- xen/arch/x86/mm/p2m-ept.c| 10 ++-- xen/arch/x86/mm/p2m.c| 80 ++-- xen/common/domain.c | 1 + xen/include/public/domctl.h | 5 +- xen/include/xen/sched.h | 2 + 24 files changed, 161 insertions(+), 77 deletions(-) -- 2.34.1
Re: [PATCH for-4.19 v2 2/3] xen/x86: enable altp2m at create domain domctl
On Wed, May 8, 2024 at 9:38 PM Andrew Cooper wrote: > Both fields can reasonably share uint32_t, but could you work with Petr > to make both halfs of this land cleanly. Hi, I think creating a new anonymous struct "altp2m" within `struct domain` would be a good fit. uint16_t for my MAX_ALTP2M replacement field will be enough, so the other uint16_t could be used for various altp2m modes/flags. P.
Re: [PATCH v2 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages
On Thu, May 2, 2024 at 8:36 AM Jan Beulich wrote:
>
> On 30.04.2024 17:40, Petr Beneš wrote:
> > On Tue, Apr 30, 2024 at 4:47 PM Jan Beulich wrote:
> >>
> >> On 28.04.2024 18:52, Petr Beneš wrote:
> >>> From: Petr Beneš
> >>>
> >>> This change anticipates scenarios where `max_altp2m` is set to its maximum
> >>> supported value (i.e., 512), ensuring sufficient memory is allocated
> >>> upfront
> >>> to accommodate all altp2m tables without initialization failure.
> >>
> >> And guests with fewer or even no altp2m-s still need the same bump? You
> >> know the number of altp2m-s upon domain creation, so why bump by any more
> >> than what's strictly needed for that?
> >
> > I have to admit I've considered computing the value which goes to
> > hap_set_allocation
> > by simply adding 256 + max_altp2m, but that felt so arbitrary - the
> > 256 value itself
> > feels arbitrary, as I haven't found any reasoning for it anywhere.
> >
> > I have also tried to make code changes to make the initial allocation
> > size configurable
> > via libxl (possibly reusing the shadow_memkb) - which seemed to me
> > like the "correct"
> > solution, but those changes were more complicated than I had
> > anticipated and I would
> > definitely not make it till the 4.19 deadline.
> >
> > Question is, what to do now? Should I change it to 256 + max_altp2m?
>
> Counter question: Is accounting for just the root page table really
> enough? Meaning to say: I'm not convinced that minimum would really
> be appropriate for altp2m use even before your changes. You growing
> the number of root page tables _always_ required just makes things
> worse even without considering how (many) altp2m-s are then going
> to be used. Such an issue, if I'm right with this, would imo want
> addressing up front, in a separate patch.
It is enough - at least based on my experiments. I'll try to explain it below.
> >> Also isn't there at least one more place where the tool stack (libxl I
> >> think) would need changing, where Dom0 ballooning needs are calculated?
> >> And/or doesn't the pool size have a default calculation in the tool
> >> stack, too?
> >
> > I have found places in libxl where the mempool_size is calculated, but
> > that mempool
> > size is then set AFTER the domain is created via xc_set_paging_mempool_size.
> >
> > In my opinion it doesn't necessarily require change, since it's
> > expected by the user
> > to manually set it via shadow_memkb. The only current problem is (which this
> > commit is trying to fix) that setting shadow_memkb doesn't help when
> > max_altp2m > (256 - 1 + vcpus + MAX_NESTEDP2M), since the initial mempool
> > size is hardcoded.
>
> Wait - are you saying the guest config value isn't respected in certain
> cases? That would be another thing wanting to be fixed separately, up
> front.
The xc_set_paging_mempool_size is still called within domain_create.
So the value of shadow_memkb is respected before any of the guest code
is run. My point was that shadow_memkb isn't respected here:
>>> --- a/xen/arch/x86/mm/hap/hap.c
>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>> @@ -468,7 +468,7 @@ int hap_enable(struct domain *d, u32 mode)
>>> if ( old_pages == 0 )
>>> {
>>> paging_lock(d);
>>> -rv = hap_set_allocation(d, 256, NULL);
>>> +rv = hap_set_allocation(d, 1024, NULL);
This code (+ the root altp2ms allocation) is executed before the libxl
sends the shadow_memkb.
In another words, the sequence is following:
libxl:
--
do_domain_create
initiate_domain_create
libxl__domain_make
xc_domain_create // MAX_ALTP2M is passed here
// and hap_enable is called
dcs->bl.callback = domcreate_bootloader_done
domcreate_bootloader_done
libxl__domain_build
libxl__build_pre
libxl__arch_domain_create
libxl__domain_set_paging_mempool_size
xc_set_paging_mempool_size(shadow_mem)
xen (xc_domain_create cont.):
-
domain_create
arch_domain_create
hvm_domain_initialise
paging_enable
hap_enable
// note that we shadow_mem (from config) hasn't been
// provided yet
hap_set_allocation(d, 1024, NULL);
p2m_alloc_table(p2m_get_hostp2m(d));
p2m_alloc_table(d->arch.nested_p2m[i..MAX_NESTEDP2M]);
p2m_alloc_table(d->arch.altp2m_p2m[i..MAX_ALTP2M]);
(I hope the em
Re: [PATCH v2 4/7] x86: Make the maximum number of altp2m views configurable
On Tue, Apr 30, 2024 at 4:27 PM Jan Beulich wrote:
>
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -685,6 +685,12 @@ int arch_sanitise_domain_config(struct
> > xen_domctl_createdomain *config)
> > return -EINVAL;
> > }
> >
> > +if ( config->max_altp2m > MAX_EPTP )
> > +{
> > +dprintk(XENLOG_INFO, "max_altp2m must be <= %u\n", MAX_EPTP);
> > +return -EINVAL;
> > +}
>
> ... using MAX_EPTP here feels like a layering violation to me. Imo there want
> to be separate constants, tied together with a suitably placed BUILD_BUG_ON().
>
> Furthermore comparisons like this (there are further ones elsewhere) suggest
> there is a (continued) naming issue: A max_ or MAX_ prefix ought to name a
> "maximum valid value", not "number of permitted values". This is not a
> request to alter MAX_EPTP, but one to make sure the new struct fields really
> have matching names and purposes.
Do you have any proposals? I was considering nr_altp2m. Another
question is what it should be named in xl.cfg - also nr_altp2m? I was
too hesitant to name it like that, since there aren't any nr_* fields
currently.
> > --- a/xen/arch/x86/include/asm/domain.h
> > +++ b/xen/arch/x86/include/asm/domain.h
> > @@ -258,11 +258,10 @@ struct paging_vcpu {
> > struct shadow_vcpu shadow;
> > };
> >
> > +#define INVALID_ALTP2M 0x
> > +#define MAX_EPTP((unsigned int)(PAGE_SIZE / sizeof(uint64_t)))
>
> Aiui you add this cast because of the various min() uses. However, besides
> wanting to avoid such casts (or in fact any, whenever possible), I don't
> see why you need to retain all those min(): You bound d->max_altp2m against
> MAX_EPTP during domain creation anyway.
Fair. I considered removing the min()s altogether. I left them in
place because I was too paranoid.
>
> > @@ -28,8 +34,13 @@ altp2m_vcpu_initialise(struct vcpu *v)
> > void
> > altp2m_vcpu_destroy(struct vcpu *v)
> > {
> > +struct domain *d = v->domain;
> > struct p2m_domain *p2m;
> >
> > +/* Skip destruction if no altp2m was used. */
> > +if ( d->max_altp2m == 0 )
> > +return;
>
> ... this one doesn't look quite right: Even if altp2m-s were initialized,
> none may have been used (yet).
Fair. Bad choice of words.
>
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -602,6 +602,8 @@ struct domain
> > unsigned int guest_request_sync : 1;
> > } monitor;
> >
> > +unsigned int max_altp2m; /* Maximum number of altp2m tables */
> > +
> > unsigned int vmtrace_size; /* Buffer size in bytes, or 0 to disable. */
>
> ... this suggest you're confident other architectures will also want
> to support altp2m. Yet nothing like that is said in the description.
> In the absence of that common code should not require touching (much).
Depends on the definition of "want". altp2m patches for arm/aarch64
were sent to Xen some 6 years ago but were unfortunately rejected. I
would very much welcome them.
I have added the field to domain instead of arch_domain simply because
it is not an arch-bound feature - similarly to vmtrace below, which
also doesn't have an equivalent implementation on other archs than x86
(yet).
As far as other comments/nits go - understood.
P.
Re: [PATCH v2 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages
On Tue, Apr 30, 2024 at 4:47 PM Jan Beulich wrote:
>
> On 28.04.2024 18:52, Petr Beneš wrote:
> > From: Petr Beneš
> >
> > This change anticipates scenarios where `max_altp2m` is set to its maximum
> > supported value (i.e., 512), ensuring sufficient memory is allocated upfront
> > to accommodate all altp2m tables without initialization failure.
>
> And guests with fewer or even no altp2m-s still need the same bump? You
> know the number of altp2m-s upon domain creation, so why bump by any more
> than what's strictly needed for that?
I have to admit I've considered computing the value which goes to
hap_set_allocation
by simply adding 256 + max_altp2m, but that felt so arbitrary - the
256 value itself
feels arbitrary, as I haven't found any reasoning for it anywhere.
I have also tried to make code changes to make the initial allocation
size configurable
via libxl (possibly reusing the shadow_memkb) - which seemed to me
like the "correct"
solution, but those changes were more complicated than I had
anticipated and I would
definitely not make it till the 4.19 deadline.
Question is, what to do now? Should I change it to 256 + max_altp2m?
> > The necessity for this increase arises from the current mechanism where
> > altp2m
> > tables are allocated at initialization, requiring one page from the mempool
> > for each altp2m view.
>
> So that's the p2m_alloc_table() out of hap_enable()? If you're permitting
> up to 512 altp2m-s, I think it needs considering to not waste up to 2Mb
> without knowing how many of the altp2m-s are actually going to be used.
Yes and I ultimately agree.
> How complicate on-demand allocation would be I can't tell though, I have
> to admit.
That's also a fix I've been trying to work on - to make whole altp2m allocations
on-demand. Unfortunately, I didn't make it in time.
> > --- a/tools/tests/paging-mempool/test-paging-mempool.c
> > +++ b/tools/tests/paging-mempool/test-paging-mempool.c
> > @@ -35,7 +35,7 @@ static struct xen_domctl_createdomain create = {
> >
> > static uint64_t default_mempool_size_bytes =
> > #if defined(__x86_64__) || defined(__i386__)
> > -256 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
> > +1024 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
>
> I also can't derive from the description why we'd need to go from 256 to
> 1024 here and ...
It's explained in the code few lines below:
/*
* Check that the domain has the expected default allocation size. This
* will fail if the logic in Xen is altered without an equivalent
* adjustment here.
*/
I have verified that the default_mempool_size_bytes must reflect the number
of pages in the initial hap_set_allocation() call.
Is it something I should include in the commit message, too?
> > --- a/xen/arch/x86/mm/hap/hap.c
> > +++ b/xen/arch/x86/mm/hap/hap.c
> > @@ -468,7 +468,7 @@ int hap_enable(struct domain *d, u32 mode)
> > if ( old_pages == 0 )
> > {
> > paging_lock(d);
> > -rv = hap_set_allocation(d, 256, NULL);
> > +rv = hap_set_allocation(d, 1024, NULL);
>
> ... here. You talk of (up to) 512 pages there only.
>
> Also isn't there at least one more place where the tool stack (libxl I
> think) would need changing, where Dom0 ballooning needs are calculated?
> And/or doesn't the pool size have a default calculation in the tool
> stack, too?
I have found places in libxl where the mempool_size is calculated, but
that mempool
size is then set AFTER the domain is created via xc_set_paging_mempool_size.
In my opinion it doesn't necessarily require change, since it's
expected by the user
to manually set it via shadow_memkb. The only current problem is (which this
commit is trying to fix) that setting shadow_memkb doesn't help when
max_altp2m > (256 - 1 + vcpus + MAX_NESTEDP2M), since the initial mempool
size is hardcoded.
I didn't find any other places that would require reflecting the
current "256" value.
P.
Re: [PATCH v2 1/7] x86/p2m: Add braces for better code clarity
On Mon, Apr 29, 2024 at 9:07 AM Jan Beulich wrote: > > On 28.04.2024 18:52, Petr Beneš wrote: > > From: Petr Beneš > > > > No functional change. > > > > Signed-off-by: Petr Beneš > > Where did Stefano's R-b go? > > Jan Oh no, I missed that one. Should I do v3?
[PATCH v2 2/7] tools/xl: Add max_altp2m parameter
From: Petr Beneš
Introduce a new max_altp2m parameter to control the maximum number of altp2m
views a domain can use. By default, if max_altp2m 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š
---
Changed since v1:
* Removed the change of xen/include/public/domctl.h (moved into future commit)
* Removed corresponding assignment of xen_domctl_createdomain::max_altp2m
in libxl_create.c (moved into future commit)
* Clarify in commit message that this change is preparatory
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 8
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 4
6 files changed, 24 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 78bdb08b15..4458d5bcbb 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1158,6 +1158,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_x86.msr_relaxed);err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
x.Altp2M = Altp2MMode(xc.altp2m)
+x.MaxAltp2M = uint32(xc.max_altp2m)
x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
if err := x.Vpmu.fromC();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_x86.msr_relaxed); err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.max_altp2m = C.uint32_t(x.MaxAltp2M)
xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
if err := x.Vpmu.toC(); 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 ccfe18019e..7139bcf324 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -602,6 +602,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+MaxAltp2M uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..c73d9f2ff1 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1239,6 +1239,14 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_ALTP2M 1
+/*
+ * LIBXL_HAVE_MAX_ALTP2M
+ * If this is defined, then libxl supports setting the maximum number of
+ * alternate p2m tables.
+ */
+#define LIBXL_HAVE_MAX_ALTP2M 1
+#define LIBXL_MAX_ALTP2M_DEFAULT (~(uint32_t)0)
+
/*
* 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 41252ec553..801f704a02 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -483,6 +483,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
return -ERROR_INVAL;
}
+if (b_info->max_altp2m == LIBXL_MAX_ALTP2M_DEFAULT) {
+if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
+b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
+b_info->max_altp2m = 10;
+else
+b_info->max_altp2m = 0;
+}
+
/* Assume that providing a bootloader user implies enabling restrict. */
libxl_defbool_setdefault(_info->bootloader_restrict,
!!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 470122e768..c887d8ea8c 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -728,6 +728,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),
+("max_altp2m", uint32, {'init_val': 'LIBXL_MAX_ALTP2M_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 ab09d0288b..741668e10a 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2061,6 +2061,10 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "max_altp2m", , 1)) {
+b_info->max_altp2m = l;
+}
+
if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
--
2.34.1
[PATCH v2 6/7] tools/ocaml: Add max_altp2m parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the max_altp2m parameter.
Signed-off-by: Petr Beneš
Acked-by: Christian Lindig
---
Changed since v1:
* Moved this commit AFTER Xen changes (where
xen_domctl_createdomain::max_altp2m field
is introduced) to avoid breaking the build
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++---
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..ed851bb071 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -82,6 +82,7 @@ type domctl_create_config =
iommu_opts: domain_create_iommu_opts list;
max_vcpus: int;
max_evtchn_port: int;
+max_altp2m: int;
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..971b269d85 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -74,6 +74,7 @@ type domctl_create_config = {
iommu_opts: domain_create_iommu_opts list;
max_vcpus: int;
max_evtchn_port: int;
+ max_altp2m: int;
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2b6d3c09df..0b70cc9b08 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -207,12 +207,13 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_IOMMU_OPTS Field(config, 3)
#define VAL_MAX_VCPUS Field(config, 4)
#define VAL_MAX_EVTCHN_PORT Field(config, 5)
-#define VAL_MAX_GRANT_FRAMESField(config, 6)
-#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
-#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_MAX_ALTP2M Field(config, 6)
+#define VAL_MAX_GRANT_FRAMESField(config, 7)
+#define VAL_MAX_MAPTRACK_FRAMES Field(config, 8)
+#define VAL_MAX_GRANT_VERSION Field(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -226,6 +227,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.ssidref = Int32_val(VAL_SSIDREF),
.max_vcpus = Int_val(VAL_MAX_VCPUS),
.max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT),
+ .max_altp2m = Int_val(VAL_MAX_ALTP2M),
.max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES),
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
@@ -257,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#if defined(__i386__) || defined(__x86_64__)
/* Quick & dirty check for ABI changes. */
- BUILD_BUG_ON(sizeof(cfg) != 64);
+ BUILD_BUG_ON(sizeof(cfg) != 68);
/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
@@ -291,6 +293,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
+#undef VAL_MAX_ALTP2M
#undef VAL_MAX_EVTCHN_PORT
#undef VAL_MAX_VCPUS
#undef VAL_IOMMU_OPTS
--
2.34.1
[PATCH v2 0/7] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. Adjustments include: - "Prepare" commits with style changes. - Adding a new `max_altp2m` parameter to xl. - Adding a new `max_altp2m` parameter to the OCaml bindings. - Adding a new `max_altp2m` parameter to the xl.cfg manual. - Adding a new `max_altp2m` field into the `xen_domctl_createdomain`, which, after sanity checks, is stored in newly introduced `max_altp2m` field of `struct domain` - leaving room for other architectures to implement the altp2m feature. - Replacing MAX_ALTP2M macro occurrences with `domain->max_altp2m`. - Finally, adjusting the initial allocation of pages in `hap_enable` from 256 to 1024 pages to accommodate potentially larger `max_altp2m` values (i.e., maximum of 512). This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Petr Beneš (7): x86/p2m: Add braces for better code clarity tools/xl: Add max_altp2m parameter docs/man: Add max_altp2m parameter to the xl.cfg manual x86: Make the maximum number of altp2m views configurable tools/libxl: Activate the max_altp2m feature tools/ocaml: Add max_altp2m parameter x86/hap: Increase the number of initial mempool_size to 1024 pages docs/man/xl.cfg.5.pod.in | 14 + tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go| 1 + tools/include/libxl.h | 8 +++ tools/libs/light/libxl_create.c | 9 tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml| 1 + tools/ocaml/libs/xc/xenctrl.mli | 1 + tools/ocaml/libs/xc/xenctrl_stubs.c | 17 +++--- .../paging-mempool/test-paging-mempool.c | 2 +- tools/xl/xl_parse.c | 4 ++ xen/arch/x86/domain.c | 6 +++ xen/arch/x86/hvm/hvm.c| 8 ++- xen/arch/x86/hvm/vmx/vmx.c| 2 +- xen/arch/x86/include/asm/domain.h | 7 ++- xen/arch/x86/include/asm/p2m.h| 4 +- xen/arch/x86/mm/altp2m.c | 27 +- xen/arch/x86/mm/hap/hap.c | 8 +-- xen/arch/x86/mm/mem_access.c | 14 ++--- xen/arch/x86/mm/mem_sharing.c | 2 +- xen/arch/x86/mm/p2m-ept.c | 6 +-- xen/arch/x86/mm/p2m.c | 54 ++- xen/common/domain.c | 7 +++ xen/include/public/domctl.h | 3 +- xen/include/xen/sched.h | 2 + 25 files changed, 151 insertions(+), 59 deletions(-) -- 2.34.1
[PATCH v2 5/7] tools/libxl: Activate the max_altp2m feature
From: Petr Beneš This commit activates the previously introduced max_altp2m parameter, establishing the connection between libxl and Xen. Signed-off-by: Petr Beneš --- Changed since v1: * This is a new commit in the series tools/libs/light/libxl_create.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index 801f704a02..6ccc1fa158 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -653,6 +653,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, .ssidref = info->ssidref, .max_vcpus = b_info->max_vcpus, .max_evtchn_port = b_info->event_channels, +.max_altp2m = b_info->max_altp2m, .max_grant_frames = b_info->max_grant_frames, .max_maptrack_frames = b_info->max_maptrack_frames, .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version), -- 2.34.1
[PATCH v2 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages
From: Petr Beneš
This change anticipates scenarios where `max_altp2m` is set to its maximum
supported value (i.e., 512), ensuring sufficient memory is allocated upfront
to accommodate all altp2m tables without initialization failure.
The necessity for this increase arises from the current mechanism where altp2m
tables are allocated at initialization, requiring one page from the mempool
for each altp2m view.
Signed-off-by: Petr Beneš
---
tools/tests/paging-mempool/test-paging-mempool.c | 2 +-
xen/arch/x86/mm/hap/hap.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/tests/paging-mempool/test-paging-mempool.c
b/tools/tests/paging-mempool/test-paging-mempool.c
index 1ebc13455a..91b06fa0cf 100644
--- a/tools/tests/paging-mempool/test-paging-mempool.c
+++ b/tools/tests/paging-mempool/test-paging-mempool.c
@@ -35,7 +35,7 @@ static struct xen_domctl_createdomain create = {
static uint64_t default_mempool_size_bytes =
#if defined(__x86_64__) || defined(__i386__)
-256 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
+1024 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
#elif defined (__arm__) || defined(__aarch64__)
16 << 12;
#endif
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 7aff5fa664..fab7e256a4 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -468,7 +468,7 @@ int hap_enable(struct domain *d, u32 mode)
if ( old_pages == 0 )
{
paging_lock(d);
-rv = hap_set_allocation(d, 256, NULL);
+rv = hap_set_allocation(d, 1024, NULL);
if ( rv != 0 )
{
hap_set_allocation(d, 0, NULL);
--
2.34.1
[PATCH v2 4/7] x86: Make the maximum number of altp2m views configurable
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
tables during domain creation. Previously, the limits were hardcoded to a
maximum of 10. This change allows for greater flexibility in environments that
require more or fewer altp2m views.
The maximum configurable limit for max_altp2m on x86 is now set to MAX_EPTP
(512). This cap is linked to the architectural limit of the EPTP-switching
VMFUNC, which supports up to 512 entries. Despite there being no inherent need
for limiting max_altp2m in scenarios not utilizing VMFUNC, decoupling these
components would necessitate substantial code changes.
Signed-off-by: Petr Beneš
---
Changed since v1:
* Added xen_domctl_createdomain::max_altp2m field to
xen/include/public/domctl.h
* Bumped the XEN_DOMCTL_INTERFACE_VERSION
* More elaborate commit message
xen/arch/x86/domain.c | 6
xen/arch/x86/hvm/hvm.c| 8 -
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/include/asm/domain.h | 7 ++---
xen/arch/x86/include/asm/p2m.h| 4 +--
xen/arch/x86/mm/altp2m.c | 27 +++--
xen/arch/x86/mm/hap/hap.c | 6 ++--
xen/arch/x86/mm/mem_access.c | 14 -
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 6 ++--
xen/arch/x86/mm/p2m.c | 50 +++
xen/common/domain.c | 7 +
xen/include/public/domctl.h | 3 +-
xen/include/xen/sched.h | 2 ++
14 files changed, 94 insertions(+), 50 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 20e83cf38b..6b458f330c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -685,6 +685,12 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+if ( config->max_altp2m > MAX_EPTP )
+{
+dprintk(XENLOG_INFO, "max_altp2m must be <= %u\n", MAX_EPTP);
+return -EINVAL;
+}
+
if ( config->vmtrace_size )
{
unsigned int size = config->vmtrace_size;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0ce45b177c..9b70fe7cfc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4633,6 +4633,12 @@ static int do_altp2m_op(
goto out;
}
+if ( d->max_altp2m == 0 )
+{
+rc = -EINVAL;
+goto out;
+}
+
if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
goto out;
@@ -5222,7 +5228,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
if ( !hvm_is_singlestep_supported() )
return;
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= v->domain->max_altp2m )
return;
v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5f67a48592..8f57f3a7f5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4888,7 +4888,7 @@ bool asmlinkage vmx_vmenter_helper(const struct
cpu_user_regs *regs)
{
unsigned int i;
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < currd->max_altp2m; ++i )
{
if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
diff --git a/xen/arch/x86/include/asm/domain.h
b/xen/arch/x86/include/asm/domain.h
index f5daeb182b..5bb0bcae81 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,10 @@ struct paging_vcpu {
struct shadow_vcpu shadow;
};
+#define INVALID_ALTP2M 0x
+#define MAX_EPTP((unsigned int)(PAGE_SIZE / sizeof(uint64_t)))
#define MAX_NESTEDP2M 10
-#define MAX_ALTP2M 10 /* arbitrary */
-#define INVALID_ALTP2M 0x
-#define MAX_EPTP(PAGE_SIZE / sizeof(uint64_t))
struct p2m_domain;
struct time_scale {
int shift;
@@ -353,7 +352,7 @@ struct arch_domain
/* altp2m: allow multiple copies of host p2m */
bool altp2m_active;
-struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
+struct p2m_domain **altp2m_p2m;
mm_lock_t altp2m_list_lock;
uint64_t *altp2m_eptp;
uint64_t *altp2m_visible_eptp;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..2086bcb633 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -881,7 +881,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu
*v)
if ( index == INVALID_ALTP2M )
return NULL;
-BUG_ON(index >= MAX_ALTP2M);
+BUG_ON(index >= v->domain->max_altp2m);
return v->domain->arch.altp2m_p2m[index];
}
@@ -891,7 +891,7 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned
int idx)
{
struct p2m_domain *orig;
-BUG_ON(idx >= MAX_ALTP2M);
+BUG_ON(idx >= v->domain->max_altp2m);
if ( idx == vcpu_altp2m(v).p2
[PATCH v2 1/7] x86/p2m: Add braces for better code clarity
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
---
xen/arch/x86/mm/p2m.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ce742c12e0..eb7996170d 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
change_entry_type_global(altp2m, ot, nt);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
@@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
_memory_type_changed(altp2m);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
--
2.34.1
[PATCH v2 3/7] docs/man: Add max_altp2m parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the max_altp2m configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 8f2b375ce9..2d4ea35736 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
Re: [PATCH 3/7] tools/xl: Add max_altp2m parameter
On Thu, Apr 25, 2024 at 8:19 AM Jan Beulich wrote:
>
> On 24.04.2024 22:42, Petr Beneš wrote:
> > Introduce a new max_altp2m parameter to control the maximum number of altp2m
> > views a domain can use. By default, if max_altp2m is unspecified and altp2m
> > is
> > enabled, the value is set to 10, reflecting the legacy behavior.
>
> Apart from the public header you don't even touch hypervisor code here. What
> you say above therefore is limited in scope to the tool stack. I question
> though that adding a new field without respecting it constitutes an overall
> consistent change. In particular I'm curious how you mean to deal with this
> for other than x86, where altp2m as a concept doesn't exist (yet).
>
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -77,6 +77,7 @@ struct xen_domctl_createdomain {
> > */
> > uint32_t max_vcpus;
> > uint32_t max_evtchn_port;
> > +uint32_t max_altp2m;
> > int32_t max_grant_frames;
> > int32_t max_maptrack_frames;
>
> Such a struct layout change needs to be accompanied by an interface version
> bump (which hasn't happened so far in this release cycle, afaics).
>
> Jan
So I should not include domctl.h changes in this commit and instead
edit the commit message that it's merely a preparation for the
following commit.
Then, move the xen_domctl_createdomain field creation to the commit
with the hypervisor changes (+ include interface version bump) and
then create an additional commit that ties xl and xen together.
Do I understand it correctly?
Re: [PATCH 1/7] x86/p2m: Add braces for better code clarity
On Thu, Apr 25, 2024 at 8:21 AM Jan Beulich wrote:
>
> On 24.04.2024 22:41, Petr Beneš wrote:
> > From: Petr Beneš
> >
> > No functional change.
> >
> > Signed-off-by: Petr Beneš
>
> Hmm. I don't really mind the extra braces, but I also don't really see a need.
> IOW this is not an objection, but it'll want to be someone else (if anyone) to
> ack this.
>
> Jan
>
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
> > unsigned int i;
> >
> > for ( i = 0; i < MAX_ALTP2M; i++ )
> > +{
> > if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> > {
> > struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> > @@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
> > change_entry_type_global(altp2m, ot, nt);
> > p2m_unlock(altp2m);
> > }
> > +}
> > }
> >
> > p2m_unlock(hostp2m);
> > @@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
> > unsigned int i;
> >
> > for ( i = 0; i < MAX_ALTP2M; i++ )
> > +{
> > if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> > {
> > struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> > @@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
> > _memory_type_changed(altp2m);
> > p2m_unlock(altp2m);
> > }
> > +}
> > }
> >
> > p2m_unlock(hostp2m);
>
I should have specified that it builds on commit
5205bda5f11cc03ca62ad2bb6c34bf738bbb3247, which did some coding style
cleanup and added braces to several places too, but missed these two.
[PATCH 6/7] x86: Make the maximum number of altp2m views configurable
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
tables during domain creation. Previously, the limits were hardcoded to a
maximum of 10. This change allows for greater flexibility in environments that
require more or fewer altp2m views.
Signed-off-by: Petr Beneš
---
xen/arch/x86/domain.c | 6
xen/arch/x86/hvm/hvm.c| 8 -
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/include/asm/domain.h | 7 ++---
xen/arch/x86/include/asm/p2m.h| 4 +--
xen/arch/x86/mm/altp2m.c | 27 +++--
xen/arch/x86/mm/hap/hap.c | 6 ++--
xen/arch/x86/mm/mem_access.c | 14 -
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 6 ++--
xen/arch/x86/mm/p2m.c | 50 +++
xen/common/domain.c | 7 +
xen/include/xen/sched.h | 2 ++
13 files changed, 92 insertions(+), 49 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 4f851aa81f..95ae675ad0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -679,6 +679,12 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+if ( config->max_altp2m > MAX_EPTP )
+{
+dprintk(XENLOG_INFO, "max_altp2m must be <= %u\n", MAX_EPTP);
+return -EINVAL;
+}
+
if ( config->vmtrace_size )
{
unsigned int size = config->vmtrace_size;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0ce45b177c..9b70fe7cfc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4633,6 +4633,12 @@ static int do_altp2m_op(
goto out;
}
+if ( d->max_altp2m == 0 )
+{
+rc = -EINVAL;
+goto out;
+}
+
if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
goto out;
@@ -5222,7 +5228,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
if ( !hvm_is_singlestep_supported() )
return;
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= v->domain->max_altp2m )
return;
v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0935762378..eadde4dbcb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4888,7 +4888,7 @@ bool asmlinkage vmx_vmenter_helper(const struct
cpu_user_regs *regs)
{
unsigned int i;
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < currd->max_altp2m; ++i )
{
if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
diff --git a/xen/arch/x86/include/asm/domain.h
b/xen/arch/x86/include/asm/domain.h
index f5daeb182b..5bb0bcae81 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,10 @@ struct paging_vcpu {
struct shadow_vcpu shadow;
};
+#define INVALID_ALTP2M 0x
+#define MAX_EPTP((unsigned int)(PAGE_SIZE / sizeof(uint64_t)))
#define MAX_NESTEDP2M 10
-#define MAX_ALTP2M 10 /* arbitrary */
-#define INVALID_ALTP2M 0x
-#define MAX_EPTP(PAGE_SIZE / sizeof(uint64_t))
struct p2m_domain;
struct time_scale {
int shift;
@@ -353,7 +352,7 @@ struct arch_domain
/* altp2m: allow multiple copies of host p2m */
bool altp2m_active;
-struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
+struct p2m_domain **altp2m_p2m;
mm_lock_t altp2m_list_lock;
uint64_t *altp2m_eptp;
uint64_t *altp2m_visible_eptp;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..2086bcb633 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -881,7 +881,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu
*v)
if ( index == INVALID_ALTP2M )
return NULL;
-BUG_ON(index >= MAX_ALTP2M);
+BUG_ON(index >= v->domain->max_altp2m);
return v->domain->arch.altp2m_p2m[index];
}
@@ -891,7 +891,7 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned
int idx)
{
struct p2m_domain *orig;
-BUG_ON(idx >= MAX_ALTP2M);
+BUG_ON(idx >= v->domain->max_altp2m);
if ( idx == vcpu_altp2m(v).p2midx )
return false;
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index a04297b646..c91e0fbfd1 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -13,6 +13,12 @@
void
altp2m_vcpu_initialise(struct vcpu *v)
{
+struct domain *d = v->domain;
+
+/* Skip initialisation if no altp2m will be used. */
+if ( d->max_altp2m == 0 )
+return;
+
if ( v != current )
vcpu_pause(v);
@@ -28,8 +34,13 @@ altp2m_vcpu_initialise(struct vcpu *v)
void
altp2m_vcpu_destroy(struct vcpu *v)
{
+struct
[PATCH 3/7] tools/xl: Add max_altp2m parameter
From: Petr Beneš
Introduce a new max_altp2m parameter to control the maximum number of altp2m
views a domain can use. By default, if max_altp2m is unspecified and altp2m is
enabled, the value is set to 10, reflecting the legacy behavior.
Signed-off-by: Petr Beneš
---
tools/golang/xenlight/helpers.gen.go | 2 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h| 8
tools/libs/light/libxl_create.c | 9 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 4
xen/include/public/domctl.h | 1 +
7 files changed, 26 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 78bdb08b15..4458d5bcbb 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1158,6 +1158,7 @@ if err :=
x.ArchX86.MsrRelaxed.fromC(_x86.msr_relaxed);err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
x.Altp2M = Altp2MMode(xc.altp2m)
+x.MaxAltp2M = uint32(xc.max_altp2m)
x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
if err := x.Vpmu.fromC();err != nil {
return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err :=
x.ArchX86.MsrRelaxed.toC(_x86.msr_relaxed); err != nil {
return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.max_altp2m = C.uint32_t(x.MaxAltp2M)
xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
if err := x.Vpmu.toC(); 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 ccfe18019e..7139bcf324 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -602,6 +602,7 @@ ArchX86 struct {
MsrRelaxed Defbool
}
Altp2M Altp2MMode
+MaxAltp2M uint32
VmtraceBufKb int
Vpmu Defbool
}
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..c73d9f2ff1 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1239,6 +1239,14 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_ALTP2M 1
+/*
+ * LIBXL_HAVE_MAX_ALTP2M
+ * If this is defined, then libxl supports setting the maximum number of
+ * alternate p2m tables.
+ */
+#define LIBXL_HAVE_MAX_ALTP2M 1
+#define LIBXL_MAX_ALTP2M_DEFAULT (~(uint32_t)0)
+
/*
* 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 41252ec553..6ccc1fa158 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -483,6 +483,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
return -ERROR_INVAL;
}
+if (b_info->max_altp2m == LIBXL_MAX_ALTP2M_DEFAULT) {
+if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
+b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
+b_info->max_altp2m = 10;
+else
+b_info->max_altp2m = 0;
+}
+
/* Assume that providing a bootloader user implies enabling restrict. */
libxl_defbool_setdefault(_info->bootloader_restrict,
!!b_info->bootloader_user);
@@ -645,6 +653,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config
*d_config,
.ssidref = info->ssidref,
.max_vcpus = b_info->max_vcpus,
.max_evtchn_port = b_info->event_channels,
+.max_altp2m = b_info->max_altp2m,
.max_grant_frames = b_info->max_grant_frames,
.max_maptrack_frames = b_info->max_maptrack_frames,
.grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 470122e768..c887d8ea8c 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -728,6 +728,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),
+("max_altp2m", uint32, {'init_val': 'LIBXL_MAX_ALTP2M_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 ab09d0288b..741668e10a 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2061,6 +2061,10 @@ void parse_config_data(const char *config_source,
}
}
+if (!xlu_cfg_get_long(config, "max_altp2m", , 1)) {
+b_info->max_altp2m = l;
+}
+
if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", , 1) && l) {
b_info->vmtrace_buf_kb = l;
}
diff --git a/xen/include/public/domctl.h b/xen/includ
[PATCH 4/7] tools/ocaml: Add max_altp2m parameter
From: Petr Beneš
Allow developers using the OCaml bindings to set the max_altp2m parameter.
Signed-off-by: Petr Beneš
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++---
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..ed851bb071 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -82,6 +82,7 @@ type domctl_create_config =
iommu_opts: domain_create_iommu_opts list;
max_vcpus: int;
max_evtchn_port: int;
+max_altp2m: int;
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..971b269d85 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -74,6 +74,7 @@ type domctl_create_config = {
iommu_opts: domain_create_iommu_opts list;
max_vcpus: int;
max_evtchn_port: int;
+ max_altp2m: int;
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2b6d3c09df..0b70cc9b08 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -207,12 +207,13 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_IOMMU_OPTS Field(config, 3)
#define VAL_MAX_VCPUS Field(config, 4)
#define VAL_MAX_EVTCHN_PORT Field(config, 5)
-#define VAL_MAX_GRANT_FRAMESField(config, 6)
-#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
-#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_VMTRACE_BUF_KB Field(config, 9)
-#define VAL_CPUPOOL_ID Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_MAX_ALTP2M Field(config, 6)
+#define VAL_MAX_GRANT_FRAMESField(config, 7)
+#define VAL_MAX_MAPTRACK_FRAMES Field(config, 8)
+#define VAL_MAX_GRANT_VERSION Field(config, 9)
+#define VAL_VMTRACE_BUF_KB Field(config, 10)
+#define VAL_CPUPOOL_ID Field(config, 11)
+#define VAL_ARCHField(config, 12)
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -226,6 +227,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.ssidref = Int32_val(VAL_SSIDREF),
.max_vcpus = Int_val(VAL_MAX_VCPUS),
.max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT),
+ .max_altp2m = Int_val(VAL_MAX_ALTP2M),
.max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES),
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
@@ -257,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#if defined(__i386__) || defined(__x86_64__)
/* Quick & dirty check for ABI changes. */
- BUILD_BUG_ON(sizeof(cfg) != 64);
+ BUILD_BUG_ON(sizeof(cfg) != 68);
/* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
#define VAL_EMUL_FLAGS Field(arch_domconfig, 0)
@@ -291,6 +293,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
+#undef VAL_MAX_ALTP2M
#undef VAL_MAX_EVTCHN_PORT
#undef VAL_MAX_VCPUS
#undef VAL_IOMMU_OPTS
--
2.34.1
[PATCH 1/7] x86/p2m: Add braces for better code clarity
From: Petr Beneš
No functional change.
Signed-off-by: Petr Beneš
---
xen/arch/x86/mm/p2m.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ce742c12e0..eb7996170d 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
change_entry_type_global(altp2m, ot, nt);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
@@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
unsigned int i;
for ( i = 0; i < MAX_ALTP2M; i++ )
+{
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
_memory_type_changed(altp2m);
p2m_unlock(altp2m);
}
+}
}
p2m_unlock(hostp2m);
--
2.34.1
[PATCH 2/7] x86/hap: Refactor boolean field assignments
From: Petr Beneš No functional change. Signed-off-by: Petr Beneš --- xen/arch/x86/mm/hap/hap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index 9f964c1d87..d2011fde24 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -522,7 +522,7 @@ int hap_enable(struct domain *d, u32 mode) goto out; } -d->arch.altp2m_active = 0; +d->arch.altp2m_active = false; } /* Now let other users see the new mode */ @@ -585,7 +585,7 @@ void hap_teardown(struct domain *d, bool *preempted) for_each_vcpu ( d, v ) altp2m_vcpu_disable_ve(v); -d->arch.altp2m_active = 0; +d->arch.altp2m_active = false; FREE_XENHEAP_PAGE(d->arch.altp2m_eptp); FREE_XENHEAP_PAGE(d->arch.altp2m_visible_eptp); -- 2.34.1
[PATCH 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages
From: Petr Beneš
This change anticipates scenarios where `max_altp2m` is set to its maximum
supported value (i.e., 512), ensuring sufficient memory is allocated upfront
to accommodate all altp2m tables without initialization failure.
Signed-off-by: Petr Beneš
---
tools/tests/paging-mempool/test-paging-mempool.c | 2 +-
xen/arch/x86/mm/hap/hap.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/tests/paging-mempool/test-paging-mempool.c
b/tools/tests/paging-mempool/test-paging-mempool.c
index 1ebc13455a..91b06fa0cf 100644
--- a/tools/tests/paging-mempool/test-paging-mempool.c
+++ b/tools/tests/paging-mempool/test-paging-mempool.c
@@ -35,7 +35,7 @@ static struct xen_domctl_createdomain create = {
static uint64_t default_mempool_size_bytes =
#if defined(__x86_64__) || defined(__i386__)
-256 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
+1024 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
#elif defined (__arm__) || defined(__aarch64__)
16 << 12;
#endif
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 7aff5fa664..fab7e256a4 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -468,7 +468,7 @@ int hap_enable(struct domain *d, u32 mode)
if ( old_pages == 0 )
{
paging_lock(d);
-rv = hap_set_allocation(d, 256, NULL);
+rv = hap_set_allocation(d, 1024, NULL);
if ( rv != 0 )
{
hap_set_allocation(d, 0, NULL);
--
2.34.1
[PATCH 5/7] docs/man: Add max_altp2m parameter to the xl.cfg manual
From: Petr Beneš Update manual pages to include detailed information about the max_altp2m configuration parameter. Signed-off-by: Petr Beneš --- docs/man/xl.cfg.5.pod.in | 14 ++ 1 file changed, 14 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 8f2b375ce9..2d4ea35736 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for x86 systems will continue to work using it. +=item B + +Specifies the maximum number of alternate-p2m views available to the guest. +This setting is crucial in domain introspection scenarios that require +multiple physical-to-machine (p2m) memory mappings to be established +simultaneously. + +Enabling multiple p2m views may increase memory usage. It is advisable to +review and adjust the B setting as necessary to accommodate +the additional memory requirements. + +B: This option is ignored if B is disabled. The default value +is 10. + =item B Enable or disables guest access to hardware virtualisation features, -- 2.34.1
[PATCH 0/7] x86: Make MAX_ALTP2M configurable
From: Petr Beneš This series introduces the ability to configure the maximum number of altp2m tables during domain creation. Previously, the limits were hardcoded to a maximum of 10. This change allows for greater flexibility in environments that require more or fewer altp2m views. Adjustments include: - "Prepare" commits with style changes. - Adding a new `max_altp2m` parameter to xl. - Adding a new `max_altp2m` parameter to the OCaml bindings. - Adding a new `max_altp2m` parameter to the xl.cfg manual. - Introducing `max_altp2m` into `xen_domctl_createdomain`, which, after sanity checks, is stored in newly introduced `max_altp2m` field of `struct domain` - leaving room for other architectures to implement the altp2m feature. - Replacing MAX_ALTP2M macro occurrences with `domain->max_altp2m`. - Finally, adjusting the initial allocation of pages in `hap_enable` from 256 to 1024 pages to accommodate potentially larger `max_altp2m` values (i.e., maximum of 512). This enhancement is particularly relevant for users leveraging Xen's features for virtual machine introspection. Petr Beneš (7): x86/p2m: Add braces for better code clarity x86/hap: Refactor boolean field assignments tools/xl: Add max_altp2m parameter tools/ocaml: Add max_altp2m parameter docs/man: Add max_altp2m parameter to the xl.cfg manual x86: Make the maximum number of altp2m views configurable x86/hap: Increase the number of initial mempool_size to 1024 pages docs/man/xl.cfg.5.pod.in | 14 + tools/golang/xenlight/helpers.gen.go | 2 + tools/golang/xenlight/types.gen.go| 1 + tools/include/libxl.h | 8 +++ tools/libs/light/libxl_create.c | 9 tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml| 1 + tools/ocaml/libs/xc/xenctrl.mli | 1 + tools/ocaml/libs/xc/xenctrl_stubs.c | 17 +++--- .../paging-mempool/test-paging-mempool.c | 2 +- tools/xl/xl_parse.c | 4 ++ xen/arch/x86/domain.c | 6 +++ xen/arch/x86/hvm/hvm.c| 8 ++- xen/arch/x86/hvm/vmx/vmx.c| 2 +- xen/arch/x86/include/asm/domain.h | 7 ++- xen/arch/x86/include/asm/p2m.h| 4 +- xen/arch/x86/mm/altp2m.c | 27 +- xen/arch/x86/mm/hap/hap.c | 12 ++--- xen/arch/x86/mm/mem_access.c | 14 ++--- xen/arch/x86/mm/mem_sharing.c | 2 +- xen/arch/x86/mm/p2m-ept.c | 6 +-- xen/arch/x86/mm/p2m.c | 54 ++- xen/common/domain.c | 7 +++ xen/include/public/domctl.h | 1 + xen/include/xen/sched.h | 2 + 25 files changed, 152 insertions(+), 60 deletions(-) -- 2.34.1
[PATCH] x86/monitor: allow fast-singlestepping without enabling singlestep monitor
From: Petr Beneš
Reorder the condition checks within the HVM_MONITOR_SINGLESTEP_BREAKPOINT
case to enable fast singlestepping independently of the singlestep monitor
being enabled. Previously, fast singlestepping required the singlestep
monitor to be explicitly enabled through xc_monitor_singlestep, even though
it operates entirely within Xen and does not generate external events.
Signed-off-by: Petr Beneš
---
xen/arch/x86/hvm/monitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 4f500beaf5..2a8ff07ec9 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -178,8 +178,6 @@ int hvm_monitor_debug(unsigned long rip, enum
hvm_monitor_debug_type type,
break;
case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
-if ( !ad->monitor.singlestep_enabled )
-return 0;
if ( curr->arch.hvm.fast_single_step.enabled )
{
p2m_altp2m_check(curr, curr->arch.hvm.fast_single_step.p2midx);
@@ -188,6 +186,8 @@ int hvm_monitor_debug(unsigned long rip, enum
hvm_monitor_debug_type type,
curr->arch.hvm.fast_single_step.p2midx = 0;
return 0;
}
+if ( !ad->monitor.singlestep_enabled )
+return 0;
req.reason = VM_EVENT_REASON_SINGLESTEP;
req.u.singlestep.gfn = gfn_of_rip(rip);
sync = true;
--
2.34.1
Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
On Wed, Feb 14, 2024 at 8:12 AM Jan Beulich wrote:
>
> On 08.02.2024 10:13, Christian Lindig wrote:
> >> On 7 Feb 2024, at 22:04, Petr Beneš wrote:
> >> Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
> >> vm.cfg configuration, correcting an oversight from its initial
> >> introduction.
> >>
> >> Signed-off-by: Petr Beneš
> >
> > Acked-by: Christian Lindig
> >
> > This looks correct from an OCaml perspective. Why was the new field added
> > in the middle of the record type domctl_create_config and thus forcing
> > changes to the index of fields coming later in the record versus just
> > appending the new field to the record type?
> >
> > The critical bit is using the correct type in
> > "Int32_val(VAL_VMTRACE_BUF_KB)” that matches the type "vmtrace_buf_kb:
> > int32;” - which it does.
>
> Is this then perhaps also lacking a
>
> Fixes: 45ba9a7d7688 ("tools/[lib]xl: Add vmtrace_buf_size parameter")
>
> and hence wanting backporting?
>
> Jan
In my opinion, yes.
P.
Re: [PATCH v2] libxl: Fix comment for LIBXL_HAVE_VMTRACE_BUF_KB
> On Tue, Feb 13, 2024 at 4:02 PM Jan Beulich wrote: > > On 13.02.2024 15:56, Petr Beneš wrote: > > From: Petr Beneš > > > > It's located in libxl_domain_build_info, not libxl_domain_create_info. > > > > Signed-off-by: Petr Beneš > > Acked-by: Anthony PERARD > > Any reason you didn't also add the Fixes: tag that Anthony suggested > to put there (for me to recognize that this may need backporting; > didn't check yet when that earlier commit went in)? > > Jan Apologies. No other reason that it's my first experience with creating v2 patch and I honestly didn't know that the "Fixes: tag" suggestion was directed at me. How should I proceed from here? P.
[PATCH v2] libxl: Fix comment for LIBXL_HAVE_VMTRACE_BUF_KB
From: Petr Beneš It's located in libxl_domain_build_info, not libxl_domain_create_info. Signed-off-by: Petr Beneš Acked-by: Anthony PERARD --- tools/include/libxl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/include/libxl.h b/tools/include/libxl.h index f1652b1664..46bc774126 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -519,7 +519,7 @@ #define LIBXL_HAVE_PHYSINFO_CAP_VMTRACE 1 /* - * LIBXL_HAVE_VMTRACE_BUF_KB indicates that libxl_domain_create_info has a + * LIBXL_HAVE_VMTRACE_BUF_KB indicates that libxl_domain_build_info has a * vmtrace_buf_kb parameter, which allows to enable pre-allocation of * processor tracing buffers of given size. */ -- 2.34.1
Re: [PATCH] x86/hvm: Fix fast singlestep state persistence
> On Fri, Feb 9, 2024 at 3:58 PM Andrew Cooper
> wrote:
>
> On 08/02/2024 9:20 pm, Petr Beneš wrote:
> > From: Petr Beneš
> >
> > This patch addresses an issue where the fast singlestep setting would
> > persist
> > despite xc_domain_debug_control being called with
> > XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF.
> > Specifically, if fast singlestep was enabled in a VMI session and that
> > session
> > stopped before the MTF trap occurred, the fast singlestep setting remained
> > active even though MTF itself was disabled. This led to a situation where,
> > upon
> > starting a new VMI session, the first event to trigger an EPT violation
> > would
> > cause the corresponding EPT event callback to be skipped due to the
> > lingering
> > fast singlestep setting.
> >
> > The fix ensures that the fast singlestep setting is properly reset when
> > disabling single step debugging operations.
> >
> > Signed-off-by: Petr Beneš
> > ---
> > xen/arch/x86/hvm/hvm.c | 32 +++-
> > 1 file changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index e8deeb0222..4f988de4c1 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -5160,26 +5160,40 @@ long do_hvm_op(unsigned long op,
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >
> > int hvm_debug_op(struct vcpu *v, int32_t op)
> > {
> > -int rc;
> > +int rc = 0;
> >
> > switch ( op )
> > {
> > case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
> > case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
> > -rc = -EOPNOTSUPP;
> > if ( !cpu_has_monitor_trap_flag )
> > -break;
> > -rc = 0;
> > -vcpu_pause(v);
> > -v->arch.hvm.single_step =
> > -(op == XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON);
> > -vcpu_unpause(v); /* guest will latch new state */
> > +return -EOPNOTSUPP;
> > break;
> > default:
> > -rc = -ENOSYS;
> > +return -ENOSYS;
> > +}
> > +
> > +vcpu_pause(v);
> > +
> > +switch ( op )
> > +{
> > +case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
> > +v->arch.hvm.single_step = true;
> > +break;
> > +
> > +case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
> > +v->arch.hvm.single_step = false;
> > +v->arch.hvm.fast_single_step.enabled = false;
> > +v->arch.hvm.fast_single_step.p2midx = 0;
> > break;
> > +
> > +default:
> > +ASSERT_UNREACHABLE();
>
> Two things.
>
> First, this reads as if it's reachable, and therefore wrong. You
> probably want an /* Excluded above */ comment to point out why it's safe
> in this case.
>
> Second, I know you're copying the existing switch(), but it wasn't
> compliant with Xen's coding style. The cases and their clauses should
> have one fewer indentation level.
>
> I'm happy to fix up both on commit.
>
> ~Andrew
Thanks for the feedback. If it's not too much of a hassle, I'll be
happy if you fix it.
P.
Re: [PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
> On Thu, Feb 8, 2024 at 10:14 AM Christian Lindig > wrote: > > > On 7 Feb 2024, at 22:04, Petr Beneš wrote: > > > > > > Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the > > vm.cfg configuration, correcting an oversight from its initial introduction. > > > > Signed-off-by: Petr Beneš > > Acked-by: Christian Lindig > > This looks correct from an OCaml perspective. Why was the new field added in > the middle of the record type domctl_create_config and thus forcing changes > to the index of fields coming later in the record versus just appending the > new field to the record type? > To match the position of the field inside of xen_domctl_createdomain. P.
[PATCH] x86/hvm: Fix fast singlestep state persistence
From: Petr Beneš
This patch addresses an issue where the fast singlestep setting would persist
despite xc_domain_debug_control being called with
XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF.
Specifically, if fast singlestep was enabled in a VMI session and that session
stopped before the MTF trap occurred, the fast singlestep setting remained
active even though MTF itself was disabled. This led to a situation where, upon
starting a new VMI session, the first event to trigger an EPT violation would
cause the corresponding EPT event callback to be skipped due to the lingering
fast singlestep setting.
The fix ensures that the fast singlestep setting is properly reset when
disabling single step debugging operations.
Signed-off-by: Petr Beneš
---
xen/arch/x86/hvm/hvm.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e8deeb0222..4f988de4c1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5160,26 +5160,40 @@ long do_hvm_op(unsigned long op,
XEN_GUEST_HANDLE_PARAM(void) arg)
int hvm_debug_op(struct vcpu *v, int32_t op)
{
-int rc;
+int rc = 0;
switch ( op )
{
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
-rc = -EOPNOTSUPP;
if ( !cpu_has_monitor_trap_flag )
-break;
-rc = 0;
-vcpu_pause(v);
-v->arch.hvm.single_step =
-(op == XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON);
-vcpu_unpause(v); /* guest will latch new state */
+return -EOPNOTSUPP;
break;
default:
-rc = -ENOSYS;
+return -ENOSYS;
+}
+
+vcpu_pause(v);
+
+switch ( op )
+{
+case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
+v->arch.hvm.single_step = true;
+break;
+
+case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
+v->arch.hvm.single_step = false;
+v->arch.hvm.fast_single_step.enabled = false;
+v->arch.hvm.fast_single_step.p2midx = 0;
break;
+
+default:
+ASSERT_UNREACHABLE();
+return -ENOSYS;
}
+vcpu_unpause(v); /* guest will latch new state */
+
return rc;
}
--
2.34.1
[PATCH] tools/ocaml: Add missing vmtrace_buf_kb field
From: Petr Beneš
Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
vm.cfg configuration, correcting an oversight from its initial introduction.
Signed-off-by: Petr Beneš
---
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++--
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index d6c6eb73db..55923857ec 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -85,6 +85,7 @@ type domctl_create_config =
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 3bfc16edba..9b4b45db3a 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -77,6 +77,7 @@ type domctl_create_config = {
max_grant_frames: int;
max_maptrack_frames: int;
max_grant_version: int;
+ vmtrace_buf_kb: int32;
cpupool_id: int32;
arch: arch_domainconfig;
}
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 3703f48c74..2b6d3c09df 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -210,10 +210,17 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#define VAL_MAX_GRANT_FRAMESField(config, 6)
#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
#define VAL_MAX_GRANT_VERSION Field(config, 8)
-#define VAL_CPUPOOL_ID Field(config, 9)
-#define VAL_ARCHField(config, 10)
+#define VAL_VMTRACE_BUF_KB Field(config, 9)
+#define VAL_CPUPOOL_ID Field(config, 10)
+#define VAL_ARCHField(config, 11)
uint32_t domid = Int_val(wanted_domid);
+ uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
+
+ vmtrace_size = ROUNDUP(vmtrace_size << 10, XC_PAGE_SHIFT);
+ if ( vmtrace_size != (uint32_t)vmtrace_size )
+ caml_invalid_argument("vmtrace_buf_kb");
+
int result;
struct xen_domctl_createdomain cfg = {
.ssidref = Int32_val(VAL_SSIDREF),
@@ -223,6 +230,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
+ .vmtrace_size = vmtrace_size,
.cpupool_id = Int32_val(VAL_CPUPOOL_ID),
};
@@ -279,6 +287,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value
wanted_domid, value co
#undef VAL_ARCH
#undef VAL_CPUPOOL_ID
+#undef VAL_VMTRACE_BUF_KB
#undef VAL_MAX_GRANT_VERSION
#undef VAL_MAX_MAPTRACK_FRAMES
#undef VAL_MAX_GRANT_FRAMES
--
2.34.1
[PATCH] libxl: Fix comment for LIBXL_HAVE_VMTRACE_BUF_KB
From: Petr Beneš It's located in libxl_domain_build_info, not libxl_domain_create_info. --- tools/include/libxl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/include/libxl.h b/tools/include/libxl.h index 907aa0a330..14f69823e0 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -519,7 +519,7 @@ #define LIBXL_HAVE_PHYSINFO_CAP_VMTRACE 1 /* - * LIBXL_HAVE_VMTRACE_BUF_KB indicates that libxl_domain_create_info has a + * LIBXL_HAVE_VMTRACE_BUF_KB indicates that libxl_domain_build_info has a * vmtrace_buf_kb parameter, which allows to enable pre-allocation of * processor tracing buffers of given size. */ -- 2.34.1
[PATCH] x86: Add configuration options for max_altp2m and max_nestedp2m limits
From: Petr Beneš
This commit introduces the ability to configure the maximum number of altp2m
and nestedp2m tables through boot-time parameters. Previously, the limits were
hardcoded to a maximum of 10 for both. This change allows for greater
flexibility in environments that require more or fewer tables, enhancing Xen's
adaptability to different workloads and scenarios.
Adjustments include:
- Adding boot parameters `max_altp2m` and `max_nestedp2m` to allow setting
these limits at boot time.
- Modifying various parts of the code to use these configurable limits instead
of the previous hardcoded values.
- Ensuring that if the configured values exceed the maximum supported EPTP
entries, they are adjusted down with a warning logged.
- Adjusting the initial allocation of pages in `hap_enable` from 256 to 2048
pages when `old_pages == 0`.
This change anticipates scenarios where `max_altp2m` and `max_nestedp2m`
are set to their maximum supported values (i.e., 512), ensuring sufficient
memory is allocated upfront to accommodate all altp2m and nestedp2m tables
without initialization failure.
This enhancement is particularly relevant for users leveraging Xen's features
for virtual machine introspection or nested virtualization support.
Signed-off-by: Petr Beneš
---
.../paging-mempool/test-paging-mempool.c | 2 +-
xen/arch/x86/hvm/hvm.c| 22 +++-
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/include/asm/domain.h | 8 +--
xen/arch/x86/include/asm/p2m.h| 4 +-
xen/arch/x86/mm/altp2m.c | 16 +-
xen/arch/x86/mm/hap/hap.c | 14 ++---
xen/arch/x86/mm/mem_access.c | 14 ++---
xen/arch/x86/mm/mem_sharing.c | 6 +-
xen/arch/x86/mm/nested.c | 16 +-
xen/arch/x86/mm/p2m-ept.c | 6 +-
xen/arch/x86/mm/p2m.c | 56 +--
12 files changed, 105 insertions(+), 61 deletions(-)
diff --git a/tools/tests/paging-mempool/test-paging-mempool.c
b/tools/tests/paging-mempool/test-paging-mempool.c
index 1ebc13455a..b94d4a4fe1 100644
--- a/tools/tests/paging-mempool/test-paging-mempool.c
+++ b/tools/tests/paging-mempool/test-paging-mempool.c
@@ -35,7 +35,7 @@ static struct xen_domctl_createdomain create = {
static uint64_t default_mempool_size_bytes =
#if defined(__x86_64__) || defined(__i386__)
-256 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
+2048 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */
#elif defined (__arm__) || defined(__aarch64__)
16 << 12;
#endif
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 482eebbabf..cb5e190083 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -105,6 +105,12 @@ static const char __initconst warning_hvm_fep[] =
static bool_t __initdata opt_altp2m_enabled = 0;
boolean_param("altp2m", opt_altp2m_enabled);
+unsigned long __read_mostly max_altp2m = 10;
+integer_param("max_altp2m", max_altp2m);
+
+unsigned long __read_mostly max_nestedp2m = 10;
+integer_param("max_nestedp2m", max_nestedp2m);
+
static int cf_check cpu_callback(
struct notifier_block *nfb, unsigned long action, void *hcpu)
{
@@ -191,6 +197,20 @@ static int __init cf_check hvm_enable(void)
if ( !opt_altp2m_enabled )
hvm_funcs.altp2m_supported = 0;
+else
+{
+if ( max_altp2m > MAX_EPTP )
+{
+printk(XENLOG_WARNING "HVM: max_altp2m must be <= %lu,
adjusting\n", MAX_EPTP);
+max_altp2m = MAX_EPTP;
+}
+
+if ( max_nestedp2m > MAX_EPTP )
+{
+printk(XENLOG_WARNING "HVM: max_nestedp2m must be <= %lu,
adjusting\n", MAX_EPTP);
+max_nestedp2m = MAX_EPTP;
+}
+}
if ( opt_hvm_fep )
warning_add(warning_hvm_fep);
@@ -5207,7 +5227,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
if ( !hvm_is_singlestep_supported() )
return;
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= max_altp2m )
return;
v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 1edc7f1e91..71a9269f1d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4857,7 +4857,7 @@ bool vmx_vmenter_helper(const struct cpu_user_regs *regs)
{
unsigned int i;
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < max_altp2m; ++i )
{
if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
continue;
diff --git a/xen/arch/x86/include/asm/domain.h
b/xen/arch/x86/include/asm/domain.h
index 619e667938..429f0d6668 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/as
[PATCH] x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access
From: Petr Beneš
This patch addresses a behavior discrepancy in the handling of altp2m views,
where upon the creation and subsequent EPT violation, the page access
permissions were incorrectly inherited from the hostp2m instead of respecting
the altp2m default_access.
Previously, when a new altp2m view was established with restrictive
default_access permissions and activated via xc_altp2m_switch_to_view(),
it failed to trigger an event on the first access violation. This behavior
diverged from the intended mechanism, where the altp2m's default_access
should dictate the initial permissions, ensuring proper event triggering on
access violations.
The correction involves modifying the handling mechanism to respect the
altp2m view's default_access upon its activation, eliminating the need for
setting memory access permissions for the entire altp2m range (e.g. within
xen-access.c). This change not only aligns the behavior with the expected
access control logic but also results in a significant performance improvement
by reducing the overhead associated with setting memory access permissions
across the altp2m range.
Signed-off-by: Petr Beneš
---
tools/misc/xen-access.c | 14 --
xen/arch/x86/mm/p2m.c | 3 +++
2 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/tools/misc/xen-access.c b/tools/misc/xen-access.c
index 4097eebe6f..6cf3b6a42c 100644
--- a/tools/misc/xen-access.c
+++ b/tools/misc/xen-access.c
@@ -517,9 +517,6 @@ int main(int argc, char *argv[])
/* With altp2m we just create a new, restricted view of the memory */
if ( memaccess && altp2m )
{
-xen_pfn_t gfn = 0;
-unsigned long perm_set = 0;
-
if( altp2m_write_no_gpt )
{
rc = xc_monitor_inguest_pagefault(xch, domain_id, 1);
@@ -551,17 +548,6 @@ int main(int argc, char *argv[])
}
DPRINTF("altp2m view created with id %u\n", altp2m_view_id);
-DPRINTF("Setting altp2m mem_access permissions.. ");
-
-for(; gfn < xenaccess->max_gpfn; ++gfn)
-{
-rc = xc_altp2m_set_mem_access( xch, domain_id, altp2m_view_id, gfn,
- default_access);
-if ( !rc )
-perm_set++;
-}
-
-DPRINTF("done! Permissions set on %lu pages.\n", perm_set);
rc = xc_altp2m_switch_to_view( xch, domain_id, altp2m_view_id );
if ( rc < 0 )
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 0983bd71d9..4251144704 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1849,6 +1849,9 @@ bool p2m_altp2m_get_or_propagate(struct p2m_domain *ap2m,
unsigned long gfn_l,
amfn = _mfn(mfn_x(*mfn) & mask);
gfn = _gfn(gfn_l & mask);
+/* Override the altp2m entry with its default access. */
+*p2ma = ap2m->default_access;
+
rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
p2m_unlock(ap2m);
--
2.34.1
