On Wed, May 08, 2024 at 01:23:23PM +0200, Roger Pau Monne wrote:
> Iterate over the p2m up to the maximum recorded gfn and remove any foreign
> mappings, in order to drop the underlying page references and thus don't keep
> extra page references if a domain is destroyed while still having foreign
> mappings on it's p2m.
>
> The logic is similar to the one used on Arm.
>
> Note that foreign mappings cannot be created by guests that have altp2m or
> nested HVM enabled, as p2ms different than the host one are not currently
> scrubbed when destroyed in order to drop references to any foreign maps.
>
> It's unclear whether the right solution is to take an extra reference when
> foreign maps are added to p2ms different than the host one, or just rely on
> the
> host p2m already having a reference. The mapping being removed from the host
> p2m should cause it to be dropped on all domain p2ms.
>
> Signed-off-by: Roger Pau Monné <[email protected]>
> ---
> Changes since v1:
> - Use existing p2m max_mapped_pfn field.
> - Prevent creating foreign mappings by guests that have altp2m or nestedhvm
> enabled.
> ---
> CHANGELOG.md | 1 +
> xen/arch/x86/domain.c | 8 +++-
> xen/arch/x86/include/asm/p2m.h | 26 +++++++------
> xen/arch/x86/mm/p2m-basic.c | 17 +++++++++
> xen/arch/x86/mm/p2m.c | 68 ++++++++++++++++++++++++++++++++--
> 5 files changed, 103 insertions(+), 17 deletions(-)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 8041cfb7d243..09bdb9b97578 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -14,6 +14,7 @@ The format is based on [Keep a
> Changelog](https://keepachangelog.com/en/1.0.0/)
> - HVM PIRQs are disabled by default.
> - Reduce IOMMU setup time for hardware domain.
> - xl/libxl configures vkb=[] for HVM domains with priority over vkb_device.
> + - Allow HVM/PVH domains to map foreign pages.
>
> ### Added
> - On x86:
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index dff790060605..1cb3ccddab00 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -718,7 +718,7 @@ int arch_sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> - if ( altp2m && (altp2m & (altp2m - 1)) )
> + if ( altp2m & (altp2m - 1) )
> {
> dprintk(XENLOG_INFO, "Multiple altp2m options selected in flags:
> %#x\n",
> config->flags);
> @@ -2387,6 +2387,7 @@ int domain_relinquish_resources(struct domain *d)
> enum {
> PROG_iommu_pagetables = 1,
> PROG_shared,
> + PROG_mappings,
> PROG_paging,
> PROG_vcpu_pagetables,
> PROG_xen,
> @@ -2435,6 +2436,11 @@ int domain_relinquish_resources(struct domain *d)
> }
> #endif
>
> + PROGRESS(mappings):
> + ret = relinquish_p2m_mapping(d);
> + if ( ret )
> + return ret;
> +
> PROGRESS(paging):
>
> /* Tear down paging-assistance stuff. */
> diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
> index 107b9f260848..c1478ffc3647 100644
> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -383,6 +383,8 @@ struct p2m_domain {
>
> /* Number of foreign mappings. */
> unsigned long nr_foreign;
> + /* Cursor for iterating over the p2m on teardown. */
> + unsigned long teardown_gfn;
> #endif /* CONFIG_HVM */
> };
>
> @@ -395,16 +397,7 @@ struct p2m_domain {
> #endif
> #include <xen/p2m-common.h>
>
> -static inline bool arch_acquire_resource_check(struct domain *d)
> -{
> - /*
> - * FIXME: Until foreign pages inserted into the P2M are properly
> - * reference counted, it is unsafe to allow mapping of
> - * resource pages unless the caller is the hardware domain
> - * (see set_foreign_p2m_entry()).
> - */
> - return !paging_mode_translate(d) || is_hardware_domain(d);
This must be:
return is_pv_domain(d) ||
(d->arch.hvm.params[HVM_PARAM_ALTP2M] == XEN_ALTP2M_disabled &&
!nestedhvm_enabled(d));
Sorry.
