All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable
@ 2025-07-01 19:54 Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 1/7] xen: Refactor altp2m options into a structured format Petr Beneš
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Anthony PERARD, Juergen Gross, Andrew Cooper,
	Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Christian Lindig, David Scott,
	Bertrand Marquis, Volodymyr Babchuk, Nick Rosbrook, George Dunlap,
	Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu

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

(This is continuation of the v6 series posted a year ago - apologies for the
delay.)

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 v6:
- Rebased on top of staging
- Added missing Acks/Reviewed-bys where appropriate.
- No changes in patches since v6, with the exception of 0004: xen: Make the
  maximum number of altp2m views configurable for x86... which was the only
  patch that was left unacked/not reviewed
  - In that patch, I made changes suggested by Jan - that is:
    - Create altp2m_is_eptp_valid function and use it in places where
      we don't control the index
    - Fixed a nit: "Number of altp2ms to allocate." -> "... to permit."
    - Cosmetic: moved altp2m_vcpu_idx() in altp2m.h up, so the order of
      functions matches with the order in the #else block

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š (7):
  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                |  7 +++
 tools/libs/light/libxl_create.c      | 19 +++++--
 tools/libs/light/libxl_internal.h    |  1 +
 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  | 21 ++++++--
 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/altp2m.h    | 28 +++++++++--
 xen/arch/x86/include/asm/domain.h    |  9 ++--
 xen/arch/x86/include/asm/p2m.h       |  4 +-
 xen/arch/x86/mm/altp2m.c             | 74 +++++++++++++++-------------
 xen/arch/x86/mm/hap/hap.c            |  6 +--
 xen/arch/x86/mm/mem_access.c         | 20 +++-----
 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 +
 27 files changed, 216 insertions(+), 88 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v7 1/7] xen: Refactor altp2m options into a structured format
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Anthony PERARD, Juergen Gross, Andrew Cooper,
	Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Christian Lindig, David Scott,
	Bertrand Marquis, Volodymyr Babchuk, Christian Lindig,
	Julien Grall

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

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š <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Julien Grall <jgrall@amazon.com> # arm
Reviewed-by: Jan Beulich <jbeulich@suse.com> # 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 9525d22312..8a85fba1cf 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -622,17 +622,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 863ab3c778..b51fd66788 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -225,7 +225,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 be58a23dd7..79a144e61b 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -693,7 +693,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 b67342797f..56c3816187 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -622,7 +622,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) )
@@ -709,7 +709,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 056360d5fe..56c7de3977 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -665,7 +665,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 be19ab5e26..a69dd96084 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -99,6 +99,7 @@ struct xen_domctl_createdomain {
 
     uint32_t grant_opts;
 
+    struct {
 /*
  * Enable altp2m mixed mode.
  *
@@ -115,7 +116,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_opts;
+        uint32_t opts;
+    } altp2m;
 
     /* Per-vCPU buffer size in bytes.  0 to disable. */
     uint32_t vmtrace_size;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 1/7] xen: Refactor altp2m options into a structured format Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-02 13:29   ` Nick Rosbrook
                     ` (2 more replies)
  2025-07-01 19:54 ` [PATCH v7 3/7] docs/man: Add altp2m_count parameter to the xl.cfg manual Petr Beneš
                   ` (5 subsequent siblings)
  7 siblings, 3 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Nick Rosbrook, George Dunlap, Anthony PERARD,
	Juergen Gross

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

Introduce a new altp2m_count parameter to control the maximum number of altp2m
views a domain can use. By default, if altp2m_count is unspecified and altp2m
is enabled, the value is set to 10, reflecting the legacy behavior.

This change is preparatory; it establishes the groundwork for the feature but
does not activate it.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
---
 tools/golang/xenlight/helpers.gen.go | 2 ++
 tools/golang/xenlight/types.gen.go   | 1 +
 tools/include/libxl.h                | 7 +++++++
 tools/libs/light/libxl_create.c      | 9 +++++++++
 tools/libs/light/libxl_internal.h    | 1 +
 tools/libs/light/libxl_types.idl     | 1 +
 tools/xl/xl_parse.c                  | 9 +++++++++
 7 files changed, 30 insertions(+)

diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 191be87297..515a6ef10a 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1167,6 +1167,7 @@ if err := x.ArchX86.MsrRelaxed.fromC(&xc.arch_x86.msr_relaxed);err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
 x.Altp2M = Altp2MMode(xc.altp2m)
+x.Altp2MCount = uint32(xc.altp2m_count)
 x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
 if err := x.Vpmu.fromC(&xc.vpmu);err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1695,6 +1696,7 @@ if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
 xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.altp2m_count = C.uint32_t(x.Altp2MCount)
 xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
 if err := x.Vpmu.toC(&xc.vpmu); err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 656933c6c9..1a4f076bd2 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -604,6 +604,7 @@ ArchX86 struct {
 MsrRelaxed Defbool
 }
 Altp2M Altp2MMode
+Altp2MCount uint32
 VmtraceBufKb int
 Vpmu Defbool
 TrapUnmappedAccesses Defbool
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index a8704e0268..d6b6e5d2dd 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1257,6 +1257,13 @@ typedef struct libxl__ctx libxl_ctx;
  */
 #define LIBXL_HAVE_ALTP2M 1
 
+/*
+ * LIBXL_HAVE_ALTP2M_COUNT
+ * If this is defined, then libxl supports setting the maximum number of
+ * alternate p2m tables.
+ */
+#define LIBXL_HAVE_ALTP2M_COUNT 1
+
 /*
  * LIBXL_HAVE_REMUS
  * If this is defined, then libxl supports remus.
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 8a85fba1cf..acf7fd9837 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         return -ERROR_INVAL;
     }
 
+    if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
+        if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
+            b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
+            /* Reflect the default legacy count */
+            b_info->altp2m_count = 10;
+        else
+            b_info->altp2m_count = 0;
+    }
+
     /* Assume that providing a bootloader user implies enabling restrict. */
     libxl_defbool_setdefault(&b_info->bootloader_restrict,
                              !!b_info->bootloader_user);
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 75bb0b94cf..2a5b233caa 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -111,6 +111,7 @@
 #define LIBXL_MIN_DOM0_MEM (128*1024)
 #define LIBXL_INVALID_GFN (~(uint64_t)0)
 #define LIBXL_VGA_HOLE_SIZE 0x20
+#define LIBXL_ALTP2M_COUNT_DEFAULT (~(uint32_t)0)
 /* use 0 as the domid of the toolstack domain for now */
 #define LIBXL_TOOLSTACK_DOMID 0
 #define QEMU_SIGNATURE "DeviceModelRecord0002"
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index a3a79d12b2..fe251649f3 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -731,6 +731,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # Alternate p2m is not bound to any architecture or guest type, as it is
     # supported by x86 HVM and ARM support is planned.
     ("altp2m", libxl_altp2m_mode),
+    ("altp2m_count", uint32, {'init_val': 'LIBXL_ALTP2M_COUNT_DEFAULT'}),
 
     # Size of preallocated vmtrace trace buffers (in KBYTES).
     # Use zero value to disable this feature.
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 7e11c62ba0..28cdbf07c2 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2099,6 +2099,15 @@ void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_long(config, "altp2m_count", &l, 1)) {
+        if (l != (uint16_t)l) {
+            fprintf(stderr, "ERROR: invalid value %ld for \"altp2m_count\"\n", l);
+            exit (1);
+        }
+
+        b_info->altp2m_count = l;
+    }
+
     if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", &l, 1) && l) {
         b_info->vmtrace_buf_kb = l;
     }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 3/7] docs/man: Add altp2m_count parameter to the xl.cfg manual
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 1/7] xen: Refactor altp2m options into a structured format Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86 Petr Beneš
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel; +Cc: Petr Beneš, Anthony PERARD

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

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

Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
---
 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 1d122982c6..5362fb0e9a 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2023,6 +2023,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<altp2m_count=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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
                   ` (2 preceding siblings ...)
  2025-07-01 19:54 ` [PATCH v7 3/7] docs/man: Add altp2m_count parameter to the xl.cfg manual Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-02 14:11   ` Jan Beulich
  2025-07-01 19:54 ` [PATCH v7 5/7] tools/libxl: Activate the altp2m_count feature Petr Beneš
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
	Anthony PERARD, Jan Beulich, Roger Pau Monné,
	Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu

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

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š <w1benny@gmail.com>
---
 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 | 28 ++++++++++--
 xen/arch/x86/include/asm/domain.h |  9 ++--
 xen/arch/x86/include/asm/p2m.h    |  4 +-
 xen/arch/x86/mm/altp2m.c          | 74 +++++++++++++++++--------------
 xen/arch/x86/mm/hap/hap.c         |  6 +--
 xen/arch/x86/mm/mem_access.c      | 20 +++------
 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, 140 insertions(+), 78 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 79a144e61b..9a4f72c62b 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -693,7 +693,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 56c3816187..155ea1f79f 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -716,16 +716,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 ( !hvm_altp2m_supported() )
+        {
+            dprintk(XENLOG_INFO, "altp2m is not supported\n");
+            return -EINVAL;
+        }
+
+        if ( !hap )
+        {
+            dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
+            return -EINVAL;
+        }
+
+        if ( nested_virt )
+        {
+            dprintk(XENLOG_INFO,
+                    "Nested virt and altp2m are not supported together\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 56c7de3977..f4bf0c8374 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4674,6 +4674,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;
 
@@ -5266,7 +5272,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 218cb2c1af..3b63ac8b54 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4967,7 +4967,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/altp2m.h b/xen/arch/x86/include/asm/altp2m.h
index c57a8c5588..2882b3e7f0 100644
--- a/xen/arch/x86/include/asm/altp2m.h
+++ b/xen/arch/x86/include/asm/altp2m.h
@@ -13,12 +13,32 @@
 #include <xen/sched.h>         /* for struct vcpu, struct domain */
 #include <asm/hvm/vcpu.h>      /* for vcpu_altp2m */
 
+static inline bool altp2m_is_eptp_valid(const struct domain *d,
+                                        unsigned int idx)
+{
+    ASSERT(d->nr_altp2m <= MAX_EPTP);
+
+    /*
+     * EPTP index is correlated with altp2m index and should not exceed
+     * d->nr_altp2m.
+     */
+
+    return idx < d->nr_altp2m &&
+        d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
+        mfn_x(INVALID_MFN);
+}
+
 /* Alternate p2m HVM on/off per domain */
 static inline bool altp2m_active(const struct domain *d)
 {
     return d->arch.altp2m_active;
 }
 
+static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
+{
+    return vcpu_altp2m(v).p2midx;
+}
+
 /* Alternate p2m VCPU */
 void altp2m_vcpu_initialise(struct vcpu *v);
 void altp2m_vcpu_destroy(struct vcpu *v);
@@ -26,11 +46,13 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
 void altp2m_vcpu_disable_ve(struct vcpu *v);
 
-static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
+#else
+
+static inline bool altp2m_is_eptp_valid(const struct domain *d,
+                                        unsigned int idx)
 {
-    return vcpu_altp2m(v).p2midx;
+    return false;
 }
-#else
 
 static inline bool altp2m_active(const struct domain *d)
 {
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 8c0dea12a5..deb247480e 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,12 @@ struct paging_vcpu {
     struct shadow_vcpu shadow;
 };
 
-#define MAX_NESTEDP2M 10
+#define MAX_EPTP        (PAGE_SIZE / sizeof(uint64_t))
+#define MAX_NR_ALTP2M   MAX_EPTP
+#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 +354,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 4358cc15a2..a2d7483602 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -901,7 +901,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];
 }
@@ -911,7 +911,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/arch/x86/mm/altp2m.c
index 6fe1e9ed6b..2b1a05fc0d 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -15,6 +15,9 @@
 void
 altp2m_vcpu_initialise(struct vcpu *v)
 {
+    if ( v->domain->nr_altp2m == 0 )
+        return;
+
     if ( v != current )
         vcpu_pause(v);
 
@@ -32,6 +35,9 @@ altp2m_vcpu_destroy(struct vcpu *v)
 {
     struct p2m_domain *p2m;
 
+    if ( v->domain->nr_altp2m == 0 )
+        return;
+
     if ( v != current )
         vcpu_pause(v);
 
@@ -122,7 +128,12 @@ 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++ )
+    d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *, d->nr_altp2m);
+
+    if ( !d->arch.altp2m_p2m )
+        return -ENOMEM;
+
+    for ( i = 0; i < d->nr_altp2m; i++ )
     {
         d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
         if ( p2m == NULL )
@@ -143,7 +154,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 +165,8 @@ void p2m_teardown_altp2m(struct domain *d)
         d->arch.altp2m_p2m[i] = NULL;
         p2m_free_one(p2m);
     }
+
+    XFREE(d->arch.altp2m_p2m);
 }
 
 int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
@@ -200,7 +216,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->nr_altp2m )
         return rc;
 
     altp2m_list_lock(d);
@@ -306,8 +322,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->nr_altp2m);
+    p2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->nr_altp2m)];
 
     p2m_lock(p2m);
 
@@ -332,7 +348,7 @@ void p2m_flush_altp2m(struct domain *d)
 
     altp2m_list_lock(d);
 
-    for ( i = 0; i < MAX_ALTP2M; i++ )
+    for ( i = 0; i < d->nr_altp2m; i++ )
     {
         p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
         d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
@@ -348,9 +364,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->nr_altp2m);
 
-    p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+    p2m = d->arch.altp2m_p2m[array_index_nospec(idx, d->nr_altp2m)];
     hostp2m = p2m_get_hostp2m(d);
 
     p2m_lock(p2m);
@@ -388,7 +404,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 >= d->nr_altp2m )
         return rc;
 
     altp2m_list_lock(d);
@@ -415,7 +431,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->nr_altp2m; i++ )
     {
         if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             continue;
@@ -437,7 +453,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 >= d->nr_altp2m )
         return rc;
 
     rc = domain_pause_except_self(d);
@@ -450,7 +466,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->nr_altp2m)];
 
         if ( !_atomic_read(p2m->active_vcpus) )
         {
@@ -475,7 +491,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->nr_altp2m )
         return rc;
 
     rc = domain_pause_except_self(d);
@@ -510,13 +526,11 @@ 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) ||
-         d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
-         mfn_x(INVALID_MFN) )
+    if ( !altp2m_is_eptp_valid(d, idx) )
         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->nr_altp2m)];
 
     p2m_lock(hp2m);
     p2m_lock(ap2m);
@@ -572,7 +586,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->nr_altp2m; i++ )
     {
         p2m_type_t t;
         p2m_access_t a;
@@ -595,7 +609,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->nr_altp2m; i++ )
                 {
                     if ( i == last_reset_idx ||
                          d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
@@ -659,12 +673,11 @@ 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) ||
-             d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
-             mfn_x(INVALID_MFN) )
+        if ( !altp2m_is_eptp_valid(d, sve->view) )
             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->nr_altp2m)];
     }
 
     p2m_lock(host_p2m);
@@ -727,12 +740,11 @@ 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) ||
-             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
-             mfn_x(INVALID_MFN) )
+        if ( !altp2m_is_eptp_valid(d, altp2m_idx) )
             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->nr_altp2m)];
     }
     else
         p2m = host_p2m;
@@ -761,13 +773,7 @@ int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
 
     altp2m_list_lock(d);
 
-    /*
-     * Eptp index is correlated with altp2m index and should not exceed
-     * min(MAX_ALTP2M, MAX_EPTP).
-     */
-    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_is_eptp_valid(d, altp2m_idx) )
         rc = -EINVAL;
     else if ( visible )
         d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index ec5043a8aa..dcc15d6f1a 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->nr_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->nr_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->nr_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 21b5b7ecda..3208b34925 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -350,12 +350,10 @@ 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) ||
-             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
-             mfn_x(INVALID_MFN) )
+        if ( !altp2m_is_eptp_valid(d, altp2m_idx) )
             return -EINVAL;
 
-        ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->nr_altp2m)];
     }
 
     if ( !xenmem_access_to_p2m_access(p2m, access, &a) )
@@ -406,12 +404,10 @@ 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_is_eptp_valid(d, altp2m_idx) )
             return -EINVAL;
 
-        ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->nr_altp2m)];
     }
 
     p2m_lock(p2m);
@@ -469,12 +465,10 @@ 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) ||
-             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
-             mfn_x(INVALID_MFN) )
+        if ( !altp2m_is_eptp_valid(d, altp2m_idx) )
             return -EINVAL;
 
-        p2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+        p2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, d->nr_altp2m)];
     }
 
     return _p2m_get_mem_access(p2m, gfn, access);
@@ -489,7 +483,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->nr_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..83bb9dd5df 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->nr_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 0cf6818c13..305404f392 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1301,7 +1301,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->nr_altp2m; i++ )
         {
             struct p2m_domain *p2m;
 
@@ -1508,7 +1508,8 @@ void __init 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->nr_altp2m)];
     struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
     struct ept_data *ept;
 
@@ -1527,7 +1528,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->nr_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 b9a7c2dc53..b4dd12433e 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->nr_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -149,7 +149,7 @@ bool p2m_memory_type_changed(struct domain *d)
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->nr_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -934,7 +934,7 @@ void p2m_change_type_range(struct domain *d,
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->nr_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
@@ -1007,7 +1007,7 @@ int p2m_finish_type_change(struct domain *d,
     {
         unsigned int i;
 
-        for ( i = 0; i < MAX_ALTP2M; i++ )
+        for ( i = 0; i < d->nr_altp2m; i++ )
         {
             if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
             {
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 303c338ef2..77c3486065 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -811,6 +811,7 @@ struct domain *domain_create(domid_t domid,
     if ( config )
     {
         d->options = config->flags;
+        d->nr_altp2m = config->altp2m.nr;
         d->vmtrace_size = config->vmtrace_size;
     }
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a69dd96084..573d34f239 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -116,7 +116,10 @@ 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 opts;
+        uint16_t opts;
+
+        /* Number of altp2ms to permit. */
+        uint16_t nr;
     } altp2m;
 
     /* Per-vCPU buffer size in bytes.  0 to disable. */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index fe53d4fab7..f8be4ea1d9 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -619,6 +619,8 @@ struct domain
         unsigned int guest_request_sync          : 1;
     } monitor;
 
+    unsigned int nr_altp2m;    /* Number of altp2m tables. */
+
     unsigned int vmtrace_size; /* Buffer size in bytes, or 0 to disable. */
 
 #ifdef CONFIG_ARGO
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 5/7] tools/libxl: Activate the altp2m_count feature
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
                   ` (3 preceding siblings ...)
  2025-07-01 19:54 ` [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86 Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 6/7] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0 Petr Beneš
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel; +Cc: Petr Beneš, Anthony PERARD, Juergen Gross

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

This commit activates the previously introduced altp2m_count parameter,
establishing the connection between libxl and Xen.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
---
 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 acf7fd9837..75f0d47f97 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -595,6 +595,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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 6/7] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
                   ` (4 preceding siblings ...)
  2025-07-01 19:54 ` [PATCH v7 5/7] tools/libxl: Activate the altp2m_count feature Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-01 19:54 ` [PATCH v7 7/7] tools/ocaml: Add altp2m_count parameter Petr Beneš
  2025-07-02  7:31 ` [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Jan Beulich
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Jan Beulich, Andrew Cooper, Roger Pau Monné

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

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š <w1benny@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 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 155ea1f79f..872e97999c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -739,8 +739,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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v7 7/7] tools/ocaml: Add altp2m_count parameter
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
                   ` (5 preceding siblings ...)
  2025-07-01 19:54 ` [PATCH v7 6/7] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0 Petr Beneš
@ 2025-07-01 19:54 ` Petr Beneš
  2025-07-02  7:31 ` [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Jan Beulich
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Beneš @ 2025-07-01 19:54 UTC (permalink / raw)
  To: xen-devel
  Cc: Petr Beneš, Christian Lindig, David Scott, Anthony PERARD,
	Christian Lindig

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

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

Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
---
 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 7e1aabad6c..97108b9d86 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -87,6 +87,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 f44dba61ae..9fccb2c2c2 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -79,6 +79,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 b51fd66788..ac2a7537d6 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -205,13 +205,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_ARCH                Field(config, 12)
+#define VAL_ALTP2M_COUNT        Field(config, 10)
+#define VAL_VMTRACE_BUF_KB      Field(config, 11)
+#define VAL_CPUPOOL_ID          Field(config, 12)
+#define VAL_ARCH                Field(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");
@@ -226,7 +235,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),
@@ -286,6 +296,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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable
  2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
                   ` (6 preceding siblings ...)
  2025-07-01 19:54 ` [PATCH v7 7/7] tools/ocaml: Add altp2m_count parameter Petr Beneš
@ 2025-07-02  7:31 ` Jan Beulich
  7 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2025-07-02  7:31 UTC (permalink / raw)
  To: Petr Beneš
  Cc: Anthony PERARD, Juergen Gross, Andrew Cooper, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini,
	Christian Lindig, David Scott, Bertrand Marquis,
	Volodymyr Babchuk, Nick Rosbrook, George Dunlap, Tamas K Lengyel,
	Alexandru Isaila, Petre Pircalabu, xen-devel

On 01.07.2025 21:54, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> (This is continuation of the v6 series posted a year ago - apologies for the
> delay.)
> 
> 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 v6:
> - Rebased on top of staging
> - Added missing Acks/Reviewed-bys where appropriate.
> - No changes in patches since v6, with the exception of 0004: xen: Make the
>   maximum number of altp2m views configurable for x86... which was the only
>   patch that was left unacked/not reviewed

Not exactly: "unacked", unless further qualified, generally means "some
necessary ack(s) missing", not "there are no acks at all". Patch 2, for
example, is still lacking a Go ack. (I didn't go any further than that,
yet, so this may well be the only missing one.)

Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
@ 2025-07-02 13:29   ` Nick Rosbrook
  2025-07-07 12:43   ` Jan Beulich
  2025-07-07 13:35   ` Anthony PERARD
  2 siblings, 0 replies; 17+ messages in thread
From: Nick Rosbrook @ 2025-07-02 13:29 UTC (permalink / raw)
  To: Petr Beneš
  Cc: xen-devel, Nick Rosbrook, George Dunlap, Anthony PERARD,
	Juergen Gross

On Tue, Jul 1, 2025 at 3:54 PM Petr Beneš <w1benny@gmail.com> wrote:
>
> From: Petr Beneš <w1benny@gmail.com>
>
> Introduce a new altp2m_count parameter to control the maximum number of altp2m
> views a domain can use. By default, if altp2m_count is unspecified and altp2m
> is enabled, the value is set to 10, reflecting the legacy behavior.
>
> This change is preparatory; it establishes the groundwork for the feature but
> does not activate it.
>
> Signed-off-by: Petr Beneš <w1benny@gmail.com>
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

For the tools/golang changes,

Acked-by: Nick Rosbrook <enr0n@ubuntu.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86
  2025-07-01 19:54 ` [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86 Petr Beneš
@ 2025-07-02 14:11   ` Jan Beulich
  2025-07-05  0:21     ` Petr Beneš
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2025-07-02 14:11 UTC (permalink / raw)
  To: Petr Beneš
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
	Roger Pau Monné, Tamas K Lengyel, Alexandru Isaila,
	Petre Pircalabu, xen-devel

On 01.07.2025 21:54, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> 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.

I can't spot such an introduction; instead I can spot such a stub already in
the tree.

> --- a/xen/arch/x86/include/asm/altp2m.h
> +++ b/xen/arch/x86/include/asm/altp2m.h
> @@ -13,12 +13,32 @@
>  #include <xen/sched.h>         /* for struct vcpu, struct domain */
>  #include <asm/hvm/vcpu.h>      /* for vcpu_altp2m */
>  
> +static inline bool altp2m_is_eptp_valid(const struct domain *d,
> +                                        unsigned int idx)
> +{
> +    ASSERT(d->nr_altp2m <= MAX_EPTP);
> +
> +    /*
> +     * EPTP index is correlated with altp2m index and should not exceed
> +     * d->nr_altp2m.
> +     */

If the two were merely correlated, that wouldn't be enough for ...

> +    return idx < d->nr_altp2m &&
> +        d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=

... this to be safe. The two match, don't they? And to better make the
connection here, maybe also add BUILD_BUG_ON(MAX_EPTP < MAX_NR_ALTP2M)?
Perhaps along with making the ASSERT() above use MAX_NR_ALTP2M.

> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -258,11 +258,12 @@ struct paging_vcpu {
>      struct shadow_vcpu shadow;
>  };
>  
> -#define MAX_NESTEDP2M 10
> +#define MAX_EPTP        (PAGE_SIZE / sizeof(uint64_t))
> +#define MAX_NR_ALTP2M   MAX_EPTP
> +#define MAX_NESTEDP2M   10
>  
> -#define MAX_ALTP2M      10 /* arbitrary */
>  #define INVALID_ALTP2M  0xffff
> -#define MAX_EPTP        (PAGE_SIZE / sizeof(uint64_t))

You touch many of the uses of MAX_EPTP anyway. I wonder if this wasn't a good
opportunity to correct its name (to e.g. NR_EPTP or NUM_EPTP). But maybe that
would better be a follow-on change ...

> @@ -122,7 +128,12 @@ 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++ )
> +    d->arch.altp2m_p2m = xzalloc_array(struct p2m_domain *, d->nr_altp2m);

Note that new code is now expected to use the xv*alloc*() family of functions.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -619,6 +619,8 @@ struct domain
>          unsigned int guest_request_sync          : 1;
>      } monitor;
>  
> +    unsigned int nr_altp2m;    /* Number of altp2m tables. */

We may have had that discussion earlier on, but it has been too long ago now:
Why is this not an x86-only field (i.e. in struct arch_domain)? Or one
dependent upon CONFIG_ALTP2M?

Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86
  2025-07-02 14:11   ` Jan Beulich
@ 2025-07-05  0:21     ` Petr Beneš
  2025-07-07  6:53       ` Jan Beulich
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Beneš @ 2025-07-05  0:21 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
	Roger Pau Monné, Tamas K Lengyel, Alexandru Isaila,
	Petre Pircalabu, xen-devel

On Wed, Jul 2, 2025 at 4:11 PM Jan Beulich <jbeulich@suse.com> wrote:
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -619,6 +619,8 @@ struct domain
> >          unsigned int guest_request_sync          : 1;
> >      } monitor;
> >
> > +    unsigned int nr_altp2m;    /* Number of altp2m tables. */
>
> We may have had that discussion earlier on, but it has been too long ago now:
> Why is this not an x86-only field (i.e. in struct arch_domain)? Or one
> dependent upon CONFIG_ALTP2M?

altp2m is not architecture specific. Though having it guarded with
CONFIG_ALTP2M is a good idea. Of course, each of its ~40 usages (that
is outside of altp2m.c) would have to be guarded, too.

P.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86
  2025-07-05  0:21     ` Petr Beneš
@ 2025-07-07  6:53       ` Jan Beulich
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2025-07-07  6:53 UTC (permalink / raw)
  To: Petr Beneš
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD,
	Roger Pau Monné, Tamas K Lengyel, Alexandru Isaila,
	Petre Pircalabu, xen-devel

On 05.07.2025 02:21, Petr Beneš wrote:
> On Wed, Jul 2, 2025 at 4:11 PM Jan Beulich <jbeulich@suse.com> wrote:
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -619,6 +619,8 @@ struct domain
>>>          unsigned int guest_request_sync          : 1;
>>>      } monitor;
>>>
>>> +    unsigned int nr_altp2m;    /* Number of altp2m tables. */
>>
>> We may have had that discussion earlier on, but it has been too long ago now:
>> Why is this not an x86-only field (i.e. in struct arch_domain)? Or one
>> dependent upon CONFIG_ALTP2M?
> 
> altp2m is not architecture specific. Though having it guarded with
> CONFIG_ALTP2M is a good idea. Of course, each of its ~40 usages (that
> is outside of altp2m.c) would have to be guarded, too.

Or you make a suitable wrapper, evaluating to compile-time-constant 0 when
ALTP2M=n.

Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
  2025-07-02 13:29   ` Nick Rosbrook
@ 2025-07-07 12:43   ` Jan Beulich
  2025-07-07 13:35   ` Anthony PERARD
  2 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2025-07-07 12:43 UTC (permalink / raw)
  To: Petr Beneš
  Cc: Nick Rosbrook, George Dunlap, Anthony PERARD, Juergen Gross,
	xen-devel

On 01.07.2025 21:54, Petr Beneš wrote:
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          return -ERROR_INVAL;
>      }
>  
> +    if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
> +        if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||

Is it possible that this is what causes

Assertion failed: !libxl_defbool_is_default(db) (libxl.c: libxl_defbool_val: 337)

observable in CI? Unless we can get a fix pretty quickly, I expect at least two
of the three patches that were committed from this series will need reverting.

Jan

> +            b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
> +            /* Reflect the default legacy count */
> +            b_info->altp2m_count = 10;
> +        else
> +            b_info->altp2m_count = 0;
> +    }
> +
>      /* Assume that providing a bootloader user implies enabling restrict. */
>      libxl_defbool_setdefault(&b_info->bootloader_restrict,
>                               !!b_info->bootloader_user);


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
  2025-07-02 13:29   ` Nick Rosbrook
  2025-07-07 12:43   ` Jan Beulich
@ 2025-07-07 13:35   ` Anthony PERARD
  2025-07-07 14:52     ` Petr Beneš
  2 siblings, 1 reply; 17+ messages in thread
From: Anthony PERARD @ 2025-07-07 13:35 UTC (permalink / raw)
  To: Petr Beneš
  Cc: xen-devel, Nick Rosbrook, George Dunlap, Anthony PERARD,
	Juergen Gross

On Tue, Jul 01, 2025 at 07:54:24PM +0000, Petr Beneš wrote:
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 8a85fba1cf..acf7fd9837 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -421,6 +421,15 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          return -ERROR_INVAL;
>      }
>  
> +    if (b_info->altp2m_count == LIBXL_ALTP2M_COUNT_DEFAULT) {
> +        if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||

This access turned out to be an issue. "hvm.altp2m" is only set to a
default value for hvm guests on x86, in
libxl__arch_domain_build_info_setdefault() in ibxl_x86.c. So trying to
create a PV guest will fail here.

It seems that altp2m_count is going to be used for the creation of all
guest, right? That is in addition to HVM, it will be also used for PV
guest and on Arm, and any other architectures that could be added. What
should be the value of altp2m_count in all this case, if altp2m is only
set on x86 HVM guest?


> +            b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
> +            /* Reflect the default legacy count */
> +            b_info->altp2m_count = 10;
> +        else
> +            b_info->altp2m_count = 0;

Cheers,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-07 13:35   ` Anthony PERARD
@ 2025-07-07 14:52     ` Petr Beneš
  2025-07-09 15:30       ` Anthony PERARD
  0 siblings, 1 reply; 17+ messages in thread
From: Petr Beneš @ 2025-07-07 14:52 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: xen-devel, Nick Rosbrook, George Dunlap, Anthony PERARD,
	Juergen Gross

On Mon, Jul 7, 2025 at 3:35 PM Anthony PERARD <anthony@xenproject.org> wrote:
>
> It seems that altp2m_count is going to be used for the creation of all
> guest, right? That is in addition to HVM, it will be also used for PV
> guest and on Arm, and any other architectures that could be added.
>
> Anthony PERARD

I'm suggesting to use what is used elsewhere in the libxl_create.c:

(https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1233
and https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1241)

        ((d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
          libxl_defbool_val(d_config->b_info.u.hvm.altp2m))

In other words, add check for the LIBXL_DOMAIN_TYPE_HVM type before
calling that offending libxl_defbool_val(). Would that be okay?

>  What should be the value of altp2m_count in all this case, if altp2m is only
> set on x86 HVM guest?

0. Similarly to other fields that are implemented only for x86, but
are available (and technically implementable) elsewhere - like altp2m
field, vmtrace_buf_kb, etc.

P.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v7 2/7] tools/xl: Add altp2m_count parameter
  2025-07-07 14:52     ` Petr Beneš
@ 2025-07-09 15:30       ` Anthony PERARD
  0 siblings, 0 replies; 17+ messages in thread
From: Anthony PERARD @ 2025-07-09 15:30 UTC (permalink / raw)
  To: Petr Beneš
  Cc: Anthony PERARD, xen-devel, Nick Rosbrook, George Dunlap,
	Anthony PERARD, Juergen Gross

On Mon, Jul 07, 2025 at 04:52:04PM +0200, Petr Beneš wrote:
> On Mon, Jul 7, 2025 at 3:35 PM Anthony PERARD <anthony@xenproject.org> wrote:
> >
> > It seems that altp2m_count is going to be used for the creation of all
> > guest, right? That is in addition to HVM, it will be also used for PV
> > guest and on Arm, and any other architectures that could be added.
> 
> I'm suggesting to use what is used elsewhere in the libxl_create.c:
> 
> (https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1233
> and https://github.com/xen-project/xen/blob/9b0f0f6e235618c2764e925b58c4bfe412730ced/tools/libs/light/libxl_create.c#L1241)
> 
>         ((d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
>           libxl_defbool_val(d_config->b_info.u.hvm.altp2m))
> 
> In other words, add check for the LIBXL_DOMAIN_TYPE_HVM type before
> calling that offending libxl_defbool_val(). Would that be okay?

Yes, that would be fine.

> >  What should be the value of altp2m_count in all this case, if altp2m is only
> > set on x86 HVM guest?
> 
> 0. Similarly to other fields that are implemented only for x86, but
> are available (and technically implementable) elsewhere - like altp2m
> field, vmtrace_buf_kb, etc.

Ok.

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-07-09 15:30 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-01 19:54 [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Petr Beneš
2025-07-01 19:54 ` [PATCH v7 1/7] xen: Refactor altp2m options into a structured format Petr Beneš
2025-07-01 19:54 ` [PATCH v7 2/7] tools/xl: Add altp2m_count parameter Petr Beneš
2025-07-02 13:29   ` Nick Rosbrook
2025-07-07 12:43   ` Jan Beulich
2025-07-07 13:35   ` Anthony PERARD
2025-07-07 14:52     ` Petr Beneš
2025-07-09 15:30       ` Anthony PERARD
2025-07-01 19:54 ` [PATCH v7 3/7] docs/man: Add altp2m_count parameter to the xl.cfg manual Petr Beneš
2025-07-01 19:54 ` [PATCH v7 4/7] xen: Make the maximum number of altp2m views configurable for x86 Petr Beneš
2025-07-02 14:11   ` Jan Beulich
2025-07-05  0:21     ` Petr Beneš
2025-07-07  6:53       ` Jan Beulich
2025-07-01 19:54 ` [PATCH v7 5/7] tools/libxl: Activate the altp2m_count feature Petr Beneš
2025-07-01 19:54 ` [PATCH v7 6/7] xen/x86: Disallow creating domains with altp2m enabled and altp2m.nr == 0 Petr Beneš
2025-07-01 19:54 ` [PATCH v7 7/7] tools/ocaml: Add altp2m_count parameter Petr Beneš
2025-07-02  7:31 ` [PATCH v7 0/7] x86: Make MAX_ALTP2M configurable Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.