Re: [PATCH v2 00/18] x86: adventures in Address Space Isolation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2025 15:26, Roger Pau Monne wrote:
> Hello,
> 
> The aim of this series is to introduce the functionality required to
> create linear mappings visible to a single pCPU.
> 
> Doing so requires having a per-vCPU root page-table (L4), and hence
> requires shadowing the guest selected L4 on PV guests.  As follow ups
> (and partially to ensure the per-CPU mappings work fine) the CPU stacks
> are switched to use per-CPU mappings, so that remote stack contents are
> not by default mapped on all page-tables (note: for this to be true the
> directmap entries for the stack pages would need to be removed also).
> 
> There's one known shortcoming with the presented code: migration of PV
> guests using per-vCPU root page-tables is not working.  I need to
> introduce extra logic to deal with PV shadow mode when using unique root
> page-tables.  I don't think this should block the series however, such
> missing functionality can always be added as follow up work.
> paging_domctl() is adjusted to reflect this restriction.
> 
> The main differences compared to v1 are the usage of per-vCPU root page
> tables (as opposed to per-pCPU), and the usage of the existing perdomain
> family of functions to manage the mappings in the per-domain slot, that
> now becomes per-vCPU.
> 
> All patches until 17 are mostly preparatory, I think there's a nice
> cleanup and generalization of the creation and managing of per-domain
> mappings, by no longer storing references to L1 page-tables in the vCPU
> or domain struct.

Since you referred me to the cover letter, I've looked back here after
making some more progress with the series. Along with my earlier comment
towards the need or ultimate goal, ...

> Patch 13 introduces the command line option, and would need discussion
> and integration with the sparse direct map series.  IMO we should get
> consensus on how we want the command line to look ASAP, so that we can
> basic parsing logic in place to be used by both the work here and the
> direct map removal series.
> 
> As part of this series the map_domain_page() helpers are also switched
> to create per-vCPU mappings (see patch 15), which converts an existing
> interface into creating per-vCPU mappings.  Such interface can be used
> to hide (map per-vCPU) further data that we don't want to be part of the
> direct map, or even shared between vCPUs of the same domain.  Also all
> existing users of the interface will already create per-vCPU mappings
> without needing additional changes.
> 
> Note that none of the logic introduced in the series removes entries for
> the directmap, so even when creating the per-CPU mappings the underlying
> physical addresses are fully accessible when using it's direct map
> entries.
> 
> I also haven't done any benchmarking.  Doesn't seem to cripple
> performance up to the point that XenRT jobs would timeout before
> finishing, that the only objective reference I can provide at the
> moment.
> 
> The series has been extensively tested on XenRT, but that doesn't cover
> all possible use-cases, so it's likely to still have some rough edges,
> handle with care.
> 
> Thanks, Roger.
> 
> Roger Pau Monne (18):
>   x86/mm: purge unneeded destroy_perdomain_mapping()
>   x86/domain: limit window where curr_vcpu != current on context switch
>   x86/mm: introduce helper to detect per-domain L1 entries that need
>     freeing
>   x86/pv: introduce function to populate perdomain area and use it to
>     map Xen GDT
>   x86/mm: switch destroy_perdomain_mapping() parameter from domain to
>     vCPU
>   x86/pv: set/clear guest GDT mappings using
>     {populate,destroy}_perdomain_mapping()
>   x86/pv: update guest LDT mappings using the linear entries
>   x86/pv: remove stashing of GDT/LDT L1 page-tables
>   x86/mm: simplify create_perdomain_mapping() interface
>   x86/mm: switch {create,destroy}_perdomain_mapping() domain parameter
>     to vCPU
>   x86/pv: untie issuing FLUSH_ROOT_PGTBL from XPTI
>   x86/mm: move FLUSH_ROOT_PGTBL handling before TLB flush
>   x86/spec-ctrl: introduce Address Space Isolation command line option
>   x86/mm: introduce per-vCPU L3 page-table
>   x86/mm: introduce a per-vCPU mapcache when using ASI
>   x86/pv: allow using a unique per-pCPU root page table (L4)
>   x86/mm: switch to a per-CPU mapped stack when using ASI
>   x86/mm: zero stack on context switch
> 
>  docs/misc/xen-command-line.pandoc    |  24 +++
>  xen/arch/x86/cpu/mcheck/mce.c        |   4 +
>  xen/arch/x86/domain.c                | 157 +++++++++++----
>  xen/arch/x86/domain_page.c           | 105 ++++++----
>  xen/arch/x86/flushtlb.c              |  28 ++-
>  xen/arch/x86/hvm/hvm.c               |   6 -
>  xen/arch/x86/include/asm/config.h    |  16 +-
>  xen/arch/x86/include/asm/current.h   |  58 +++++-
>  xen/arch/x86/include/asm/desc.h      |   6 +-
>  xen/arch/x86/include/asm/domain.h    |  50 +++--
>  xen/arch/x86/include/asm/flushtlb.h  |   2 +-
>  xen/arch/x86/include/asm/mm.h        |  15 +-
>  xen/arch/x86/include/asm/processor.h |   5 +
>  xen/arch/x86/include/asm/pv/mm.h     |   5 +
>  xen/arch/x86/include/asm/smp.h       |  12 ++
>  xen/arch/x86/include/asm/spec_ctrl.h |   4 +
>  xen/arch/x86/mm.c                    | 291 +++++++++++++++++++++------
>  xen/arch/x86/mm/hap/hap.c            |   2 +-
>  xen/arch/x86/mm/paging.c             |   6 +
>  xen/arch/x86/mm/shadow/hvm.c         |   2 +-
>  xen/arch/x86/mm/shadow/multi.c       |   2 +-
>  xen/arch/x86/pv/descriptor-tables.c  |  47 ++---
>  xen/arch/x86/pv/dom0_build.c         |  12 +-
>  xen/arch/x86/pv/domain.c             |  57 ++++--
>  xen/arch/x86/pv/mm.c                 |  43 +++-
>  xen/arch/x86/setup.c                 |  32 ++-
>  xen/arch/x86/smp.c                   |  39 ++++
>  xen/arch/x86/smpboot.c               |  26 ++-
>  xen/arch/x86/spec_ctrl.c             | 205 ++++++++++++++++++-
>  xen/arch/x86/traps.c                 |  25 ++-
>  xen/arch/x86/x86_64/mm.c             |   7 +-
>  xen/common/smp.c                     |  10 +
>  xen/common/stop_machine.c            |  10 +
>  xen/include/xen/smp.h                |   8 +
>  34 files changed, 1052 insertions(+), 269 deletions(-)

... this diffstat (even after subtracting out the contribution of the last two
patches in the series) doesn't really look like a cleanup / simplification.
Things becoming slightly slower (because of the L1 no longer directly available
to modify) may, otoh, not be a significant issue, if we assume that GDT/LDT
manipulation isn't normally a very frequent operation.

IOW my earlier request stands: Can you please try to make more clear (in the
patch descriptions) what exactly the motivation for these changes is? Just
doing things differently with more code overall can't be it, I don't think.

Jan