On 16.07.2025 22:15, Petr Beneš wrote:
> From: Petr Beneš <w1be...@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.
>
> 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š <w1be...@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 | 72 ++++++++++++++++---------------
> xen/arch/x86/mm/hap/hap.c | 6 +--
> xen/arch/x86/mm/mem_access.c | 22 ++++------
> xen/arch/x86/mm/mem_sharing.c | 4 +-
> xen/arch/x86/mm/p2m-ept.c | 7 +--
> xen/arch/x86/mm/p2m-pt.c | 2 +
> xen/arch/x86/mm/p2m.c | 8 ++--
> xen/common/domain.c | 6 +++
> xen/include/public/domctl.h | 5 ++-
> xen/include/xen/sched.h | 4 ++
> 17 files changed, 151 insertions(+), 78 deletions(-)
I understand the cover letter has some revlog, but may I please ask to have
such (also) per-patch?
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4675,6 +4675,12 @@ static int do_altp2m_op(
> goto out;
> }
>
> + if ( d->nr_altp2m == 0 )
> + {
> + rc = -EINVAL;
> + goto out;
> + }
Besides disliking EINVAL here, I also question this general rejection. For
e.g. HVMOP_altp2m_get_domain_state it may well make sense to still get back
"disabled". Yet I understand this affects a few of the pre-existing checks
as well.
With the error code changed to EOPNOTSUPP (as every other similar path uses,
with the sole exception of the "if ( XEN_ALTP2M_disabled == mode )" one):
Acked-by: Jan Beulich <jbeul...@suse.com>
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -81,6 +81,7 @@ bool p2m_mem_access_emulate_check(struct vcpu *v,
>
> if ( altp2m_active(d) )
> p2m = p2m_get_altp2m(v);
> +
> if ( !p2m )
> p2m = p2m_get_hostp2m(d);
>
> @@ -145,6 +146,7 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>
> if ( altp2m_active(d) )
> p2m = p2m_get_altp2m(v);
> +
> if ( !p2m )
> p2m = p2m_get_hostp2m(d);
I think such stray changes would better be omitted, especially from an already
large patch.
Jan
