[PATCH 0/7] x86: Make MAX_ALTP2M configurable

[PATCH 0/7] x86: Make MAX_ALTP2M configurable

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

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 1/7] x86/p2m: Add braces for better code clarity

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

No functional change.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 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

[Petr Beneš]

From: Petr Beneš <w.benny@outlook.com>

No functional change.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 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 3/7] tools/xl: Add max_altp2m parameter

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

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š <w1benny@gmail.com>
---
 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(&xc.arch_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(&xc.vpmu);err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
 xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.max_altp2m = C.uint32_t(x.MaxAltp2M)
 xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
 if err := x.Vpmu.toC(&xc.vpmu); err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 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(&b_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", &l, 1)) {
+        b_info->max_altp2m = l;
+    }
+
     if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", &l, 1) && l) {
         b_info->vmtrace_buf_kb = l;
     }
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a33f9ec32b..7a34465c21 100644
--- 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;
 
-- 
2.34.1

[PATCH 4/7] tools/ocaml: Add max_altp2m parameter

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

Allow developers using the OCaml bindings to set the max_altp2m parameter.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 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_FRAMES    Field(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_ARCH                Field(config, 11)
+#define VAL_MAX_ALTP2M          Field(config, 6)
+#define VAL_MAX_GRANT_FRAMES    Field(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_ARCH                Field(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 5/7] docs/man: Add max_altp2m parameter to the xl.cfg manual

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

Update manual pages to include detailed information about the max_altp2m
configuration parameter.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 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<This option is deprecated, use the option
 B<Note>: While the option "altp2mhvm" is deprecated, legacy applications for
 x86 systems will continue to work using it.
 
+=item B<max_altp2m=NUMBER>
+
+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<shadow_memory> setting as necessary to accommodate
+the additional memory requirements.
+
+B<Note>: This option is ignored if B<altp2m> is disabled. The default value
+is 10.
+
 =item B<nestedhvm=BOOLEAN>
 
 Enable or disables guest access to hardware virtualisation features,
-- 
2.34.1

[PATCH 6/7] x86: Make the maximum number of altp2m views configurable

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

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š <w1benny@gmail.com>
---
 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  0xffff
+#define MAX_EPTP        ((unsigned int)(PAGE_SIZE / sizeof(uint64_t)))
 #define MAX_NESTEDP2M 10
 
-#define MAX_ALTP2M      10 /* arbitrary */
-#define INVALID_ALTP2M  0xffff
-#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 domain *d = v->domain;
     struct p2m_domain *p2m;
 
+    /* Skip destruction if no altp2m was used. */
+    if ( d->max_altp2m == 0 )
+        return;
+
     if ( v != current )
         vcpu_pause(v);
 
@@ -120,7 +131,13 @@ int p2m_init_altp2m(struct domain *d)
     struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
 
     mm_lock_init(&d->arch.altp2m_list_lock);
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+
+    if ( (d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *, d->max_altp2m)) == NULL )
+    {
+        return -ENOMEM;
+    }
+
+    for ( i = 0; i < d->max_altp2m; i++ )
     {
         d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
         if ( p2m == NULL )
@@ -141,7 +158,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->max_altp2m; i++ )
     {
         if ( !d->arch.altp2m_p2m[i] )
             continue;
@@ -149,6 +169,9 @@ void p2m_teardown_altp2m(struct domain *d)
         d->arch.altp2m_p2m[i] = NULL;
         p2m_free_one(p2m);
     }
+
+    xfree(d->arch.altp2m_p2m);
+    d->arch.altp2m_p2m = NULL;
 }
 
 /*
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index d2011fde24..7aff5fa664 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -515,7 +515,7 @@ int hap_enable(struct domain *d, u32 mode)
             d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
         }
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             rv = p2m_alloc_table(d->arch.altp2m_p2m[i]);
             if ( rv != 0 )
@@ -538,7 +538,7 @@ void hap_final_teardown(struct domain *d)
     unsigned int i;
 
     if ( hvm_altp2m_supported() )
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
             p2m_teardown(d->arch.altp2m_p2m[i], true, NULL);
 
     /* Destroy nestedp2m's first */
@@ -590,7 +590,7 @@ void hap_teardown(struct domain *d, bool *preempted)
         FREE_XENHEAP_PAGE(d->arch.altp2m_eptp);
         FREE_XENHEAP_PAGE(d->arch.altp2m_visible_eptp);
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             p2m_teardown(d->arch.altp2m_p2m[i], false, preempted);
             if ( preempted && *preempted )
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 60a0cce68a..1bf40cb746 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -347,12 +347,12 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
     /* altp2m view 0 is treated as the hostp2m */
     if ( altp2m_idx )
     {
-        if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+        if ( altp2m_idx >= min(d->max_altp2m, MAX_EPTP) ||
              d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
              mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->max_altp2m)];
     }
 
     if ( !xenmem_access_to_p2m_access(p2m, access, &a) )
@@ -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) ||
+        if ( altp2m_idx >= min(d->max_altp2m, MAX_EPTP) ||
              d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
              mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->max_altp2m)];
     }
 
     p2m_lock(p2m);
@@ -466,12 +466,12 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access,
     }
     else if ( altp2m_idx ) /* altp2m view 0 is treated as the hostp2m */
     {
-        if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+        if ( altp2m_idx >= min(d->max_altp2m, MAX_EPTP) ||
              d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
              mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        p2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->max_altp2m)];
     }
 
     return _p2m_get_mem_access(p2m, gfn, access);
@@ -486,7 +486,7 @@ void arch_p2m_set_access_required(struct domain *d, bool access_required)
     if ( altp2m_active(d) )
     {
         unsigned int i;
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
 
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index da28266ef0..3aaf1a3b8d 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -912,7 +912,7 @@ static int nominate_page(struct domain *d, gfn_t gfn,
 
         altp2m_list_lock(d);
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             ap2m = d->arch.altp2m_p2m[i];
             if ( !ap2m )
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f83610cb8c..6b75fafd49 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1293,7 +1293,7 @@ static void ept_set_ad_sync(struct domain *d, bool value)
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             struct p2m_domain *p2m;
 
@@ -1500,7 +1500,7 @@ void setup_ept_dump(void)
 
 void p2m_init_altp2m_ept(struct domain *d, unsigned int i)
 {
-    struct p2m_domain *p2m = array_access_nospec(d->arch.altp2m_p2m, i);
+    struct p2m_domain *p2m = d->arch.altp2m_p2m[array_index_nospec(i, d->max_altp2m)];
     struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
     struct ept_data *ept;
 
@@ -1519,7 +1519,7 @@ unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp)
 
     altp2m_list_lock(d);
 
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+    for ( i = 0; i < d->max_altp2m; i++ )
     {
         if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
             continue;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index eb7996170d..a7144fc8e1 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -105,7 +105,7 @@ void p2m_change_entry_type_global(struct domain *d,
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -140,7 +140,7 @@ void p2m_memory_type_changed(struct domain *d)
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -913,7 +913,7 @@ void p2m_change_type_range(struct domain *d,
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -986,7 +986,7 @@ int p2m_finish_type_change(struct domain *d,
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->max_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -1780,7 +1780,7 @@ bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
     struct domain *d = v->domain;
     bool rc = false;
 
-    if ( idx >= MAX_ALTP2M )
+    if ( idx >= d->max_altp2m )
         return rc;
 
     altp2m_list_lock(d);
@@ -1886,8 +1886,8 @@ static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
 {
     struct p2m_domain *p2m;
 
-    ASSERT(idx < MAX_ALTP2M);
-    p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+    ASSERT(idx < d->max_altp2m);
+    p2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->max_altp2m)];
 
     p2m_lock(p2m);
 
@@ -1912,7 +1912,7 @@ void p2m_flush_altp2m(struct domain *d)
 
     altp2m_list_lock(d);
 
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+    for ( i = 0; i < d->max_altp2m; i++ )
     {
         p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
         d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
@@ -1928,9 +1928,9 @@ static int p2m_activate_altp2m(struct domain *d, unsigned int idx,
     struct p2m_domain *hostp2m, *p2m;
     int rc;
 
-    ASSERT(idx < MAX_ALTP2M);
+    ASSERT(idx < d->max_altp2m);
 
-    p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+    p2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->max_altp2m)];
     hostp2m = p2m_get_hostp2m(d);
 
     p2m_lock(p2m);
@@ -1968,7 +1968,7 @@ int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
     int rc = -EINVAL;
     struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
 
-    if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
+    if ( idx >= min(d->max_altp2m, MAX_EPTP) )
         return rc;
 
     altp2m_list_lock(d);
@@ -1995,7 +1995,7 @@ int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
 
     altp2m_list_lock(d);
 
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+    for ( i = 0; i < d->max_altp2m; i++ )
     {
         if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             continue;
@@ -2017,7 +2017,7 @@ int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
     struct p2m_domain *p2m;
     int rc = -EBUSY;
 
-    if ( !idx || idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
+    if ( !idx || idx >= min(d->max_altp2m, MAX_EPTP) )
         return rc;
 
     rc = domain_pause_except_self(d);
@@ -2030,7 +2030,7 @@ int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
     if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
          mfn_x(INVALID_MFN) )
     {
-        p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+        p2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->max_altp2m)];
 
         if ( !_atomic_read(p2m->active_vcpus) )
         {
@@ -2055,7 +2055,7 @@ int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
     struct vcpu *v;
     int rc = -EINVAL;
 
-    if ( idx >= MAX_ALTP2M )
+    if ( idx >= d->max_altp2m )
         return rc;
 
     rc = domain_pause_except_self(d);
@@ -2090,13 +2090,13 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
     mfn_t mfn;
     int rc = -EINVAL;
 
-    if ( idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+    if ( idx >=  min(d->max_altp2m, MAX_EPTP) ||
          d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
          mfn_x(INVALID_MFN) )
         return rc;
 
     hp2m = p2m_get_hostp2m(d);
-    ap2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+    ap2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->max_altp2m)];
 
     p2m_lock(hp2m);
     p2m_lock(ap2m);
@@ -2152,7 +2152,7 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
 
     altp2m_list_lock(d);
 
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+    for ( i = 0; i < d->max_altp2m; i++ )
     {
         p2m_type_t t;
         p2m_access_t a;
@@ -2175,7 +2175,7 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
             else
             {
                 /* At least 2 altp2m's impacted, so reset everything */
-                for ( i = 0; i < MAX_ALTP2M; i++ )
+                for ( i = 0; i < d->max_altp2m; i++ )
                 {
                     if ( i == last_reset_idx ||
                          d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
@@ -2575,12 +2575,12 @@ int p2m_set_suppress_ve_multi(struct domain *d,
 
     if ( sve->view > 0 )
     {
-        if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+        if ( sve->view >= min(d->max_altp2m, MAX_EPTP) ||
              d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
              mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, sve->view);
+        p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(sve->view, d->max_altp2m)];
     }
 
     p2m_lock(host_p2m);
@@ -2643,12 +2643,12 @@ int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
 
     if ( altp2m_idx > 0 )
     {
-        if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+        if ( altp2m_idx >= min(d->max_altp2m, MAX_EPTP) ||
              d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
              mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->max_altp2m)];
     }
     else
         p2m = host_p2m;
@@ -2679,9 +2679,9 @@ int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
 
     /*
      * Eptp index is correlated with altp2m index and should not exceed
-     * min(MAX_ALTP2M, MAX_EPTP).
+     * min(d->max_altp2m, MAX_EPTP).
      */
-    if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+    if ( altp2m_idx >= min(d->max_altp2m, MAX_EPTP) ||
          d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
          mfn_x(INVALID_MFN) )
         rc = -EINVAL;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6773f7fb90..18785cc22a 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -568,6 +568,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         }
     }
 
+    if ( config->max_altp2m && !hvm_altp2m_supported() )
+    {
+        dprintk(XENLOG_INFO, "altp2m requested but not available\n");
+        return -EINVAL;
+    }
+
     if ( config->vmtrace_size && !vmtrace_available )
     {
         dprintk(XENLOG_INFO, "vmtrace requested but not available\n");
@@ -610,6 +616,7 @@ struct domain *domain_create(domid_t domid,
     if ( config )
     {
         d->options = config->flags;
+        d->max_altp2m = config->max_altp2m;
         d->vmtrace_size = config->vmtrace_size;
     }
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 132b841995..46436fcb0b 100644
--- 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. */
 
 #ifdef CONFIG_ARGO
-- 
2.34.1

[PATCH 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages

[Petr Beneš]

From: Petr Beneš <w1benny@gmail.com>

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š <w1benny@gmail.com>
---
 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

Re: [PATCH 3/7] tools/xl: Add max_altp2m parameter

[Jan Beulich]

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

Re: [PATCH 1/7] x86/p2m: Add braces for better code clarity

[Jan Beulich]

On 24.04.2024 22:41, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> No functional change.
> 
> Signed-off-by: Petr Beneš <w1benny@gmail.com>

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);

Re: [PATCH 2/7] x86/hap: Refactor boolean field assignments

[Jan Beulich]

On 24.04.2024 22:42, Petr Beneš wrote:
> From: Petr Beneš <w.benny@outlook.com>
> 
> No functional change.
> 
> Signed-off-by: Petr Beneš <w1benny@gmail.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 1/7] x86/p2m: Add braces for better code clarity

[Petr Beneš]

On Thu, Apr 25, 2024 at 8:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.04.2024 22:41, Petr Beneš wrote:
> > From: Petr Beneš <w1benny@gmail.com>
> >
> > No functional change.
> >
> > Signed-off-by: Petr Beneš <w1benny@gmail.com>
>
> 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.

Re: [PATCH 3/7] tools/xl: Add max_altp2m parameter

[Petr Beneš]

On Thu, Apr 25, 2024 at 8:19 AM Jan Beulich <jbeulich@suse.com> 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 4/7] tools/ocaml: Add max_altp2m parameter

[Christian Lindig]

> On 24 Apr 2024, at 21:42, Petr Beneš <w1benny@gmail.com> wrote:
> 
> From: Petr Beneš <w1benny@gmail.com>
> 
> Allow developers using the OCaml bindings to set the max_altp2m parameter.
> 
> Signed-off-by: Petr Beneš <w1benny@gmail.com>
> ---
> 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_FRAMES    Field(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_ARCH                Field(config, 11)
> +#define VAL_MAX_ALTP2M          Field(config, 6)
> +#define VAL_MAX_GRANT_FRAMES    Field(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_ARCH                Field(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
> 

This looks correct from an OCaml perspective.

Acked-by: Christian Lindig <christian.lindig@cloud.com>

Re: [PATCH 3/7] tools/xl: Add max_altp2m parameter

[Jan Beulich]

On 25.04.2024 10:51, Petr Beneš wrote:
> On Thu, Apr 25, 2024 at 8:19 AM Jan Beulich <jbeulich@suse.com> 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).
> 
> 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?

That's one way of approaching it, yes.

Jan

Re: [PATCH 1/7] x86/p2m: Add braces for better code clarity

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

On Wed, 24 Apr 2024, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> No functional change.
> 
> Signed-off-by: Petr Beneš <w1benny@gmail.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>