[PATCH v8 0/7] x86: Make MAX_ALTP2M configurable

Petr Beneš posted 7 patches 3 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/cover.1752531797.git.w1benny@gmail.com
There is a newer version of this series
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      | 20 ++++++--
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               | 22 +++++++-
xen/arch/x86/hvm/monitor.c           |  2 +
xen/arch/x86/hvm/vmx/vmx.c           | 12 ++++-
xen/arch/x86/include/asm/altp2m.h    | 28 +++++++++--
xen/arch/x86/include/asm/domain.h    | 11 ++--
xen/arch/x86/include/asm/hvm/vcpu.h  |  4 ++
xen/arch/x86/include/asm/p2m.h       | 26 +++++++---
xen/arch/x86/mm/altp2m.c             | 75 +++++++++++++++-------------
xen/arch/x86/mm/hap/hap.c            | 12 +++--
xen/arch/x86/mm/mem_access.c         | 34 ++++++++-----
xen/arch/x86/mm/mem_sharing.c        |  4 +-
xen/arch/x86/mm/p2m-ept.c            | 15 ++++--
xen/arch/x86/mm/p2m-pt.c             |  2 +
xen/arch/x86/mm/p2m.c                | 17 +++++--
xen/common/domain.c                  |  3 ++
xen/include/public/domctl.h          |  7 ++-
xen/include/xen/sched.h              |  4 ++
30 files changed, 309 insertions(+), 94 deletions(-)
[PATCH v8 0/7] x86: Make MAX_ALTP2M configurable
Posted by Petr Beneš 3 months, 2 weeks ago
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.

This enhancement is particularly relevant for users leveraging Xen's features
for virtual machine introspection.

Changes since v7:
- Fix condition in libxl_create.c that caused assertions in CI.
- Removed incorrect mention of introduction of hvm_altp2m_supported() in commit
  message of patch 0004.
- Adjust comments in altp2m_is_eptp_valid().
- Guard ALTP2M features with CONFIG_ALTP2M where appropriate. I made my best to
  ensure that the following configurations compile correctly:
  * CONFIG_HVM=n (CONFIG_ALTP2M=n implied)
  * CONFIG_HVM=y CONFIG_ALTP2M=n
  * CONFIG_HVM=y CONFIG_ALTP2M=y
  * Note: Besides taking inspiration from CONFIG_MEM_SHARING &
    CONFIG_MEM_PAGING, the reason for guards instead of wrapper methods is that
    all ALTP2M features and fields became guarded by CONFIG_ALTP2M - therefore,
    lots of code inside ALTP2M blocks would be touching inexisting fields.

    This could be solved by introducing wrapped functions for each ALTP2M
    field, but I believe that would be overkill.

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      | 20 ++++++--
 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               | 22 +++++++-
 xen/arch/x86/hvm/monitor.c           |  2 +
 xen/arch/x86/hvm/vmx/vmx.c           | 12 ++++-
 xen/arch/x86/include/asm/altp2m.h    | 28 +++++++++--
 xen/arch/x86/include/asm/domain.h    | 11 ++--
 xen/arch/x86/include/asm/hvm/vcpu.h  |  4 ++
 xen/arch/x86/include/asm/p2m.h       | 26 +++++++---
 xen/arch/x86/mm/altp2m.c             | 75 +++++++++++++++-------------
 xen/arch/x86/mm/hap/hap.c            | 12 +++--
 xen/arch/x86/mm/mem_access.c         | 34 ++++++++-----
 xen/arch/x86/mm/mem_sharing.c        |  4 +-
 xen/arch/x86/mm/p2m-ept.c            | 15 ++++--
 xen/arch/x86/mm/p2m-pt.c             |  2 +
 xen/arch/x86/mm/p2m.c                | 17 +++++--
 xen/common/domain.c                  |  3 ++
 xen/include/public/domctl.h          |  7 ++-
 xen/include/xen/sched.h              |  4 ++
 30 files changed, 309 insertions(+), 94 deletions(-)

-- 
2.34.1


Re: [PATCH v8 0/7] x86: Make MAX_ALTP2M configurable
Posted by Jan Beulich 3 months, 2 weeks ago
On 15.07.2025 00:48, Petr Beneš wrote:
> 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

Please can you post new versions as applicable to present staging? The first
three of these patches were committed a week ago.

Jan

Re: [PATCH v8 0/7] x86: Make MAX_ALTP2M configurable
Posted by Petr Beneš 3 months, 2 weeks ago
On Tue, Jul 15, 2025 at 9:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 15.07.2025 00:48, Petr Beneš wrote:
> > 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
>
> Please can you post new versions as applicable to present staging? The first
> three of these patches were committed a week ago.

Not sure I understand. They were reverted. What does it mean to "post
new versions"? I've rebased my branch to current staging before
submitting this patch series.

P.
Re: [PATCH v8 0/7] x86: Make MAX_ALTP2M configurable
Posted by Jan Beulich 3 months, 2 weeks ago
On 15.07.2025 10:06, Petr Beneš wrote:
> On Tue, Jul 15, 2025 at 9:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 15.07.2025 00:48, Petr Beneš wrote:
>>> 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
>>
>> Please can you post new versions as applicable to present staging? The first
>> three of these patches were committed a week ago.
> 
> Not sure I understand. They were reverted. What does it mean to "post
> new versions"? I've rebased my branch to current staging before
> submitting this patch series.

Hmm, sorry, two of them were indeed reverted. I simply didn't remember, and
checked only for patches authored by you. Patch 1 wasn't reverted, though?

And then, as to patch 2 (which was the problematic one): Why did you retain
Anthony's R-b? In the v7 discussion I see no indication of him to keep the
tag with the change in place. Which (generally) is taken to mean the person
wants to take another look before possible re-offering the tag. With a bug
fixed, all R-b covering that area become invalid (unless explicitly
indicated otherwise).

Jan

Re: [PATCH v8 0/7] x86: Make MAX_ALTP2M configurable
Posted by Petr Beneš 3 months, 2 weeks ago
On Tue, Jul 15, 2025 at 10:15 AM Jan Beulich <jbeulich@suse.com> wrote:
> Hmm, sorry, two of them were indeed reverted. I simply didn't remember, and
> checked only for patches authored by you. Patch 1 wasn't reverted, though?

Huh, you're right, I missed that.

> And then, as to patch 2 (which was the problematic one): Why did you retain
> Anthony's R-b? In the v7 discussion I see no indication of him to keep the
> tag with the change in place. Which (generally) is taken to mean the person
> wants to take another look before possible re-offering the tag. With a bug
> fixed, all R-b covering that area become invalid (unless explicitly
> indicated otherwise).

Okay, that makes sense. I'm sorry, I'm still getting familiar with the
contribution process...

I'll resubmit.

P.