> From: Razvan Cojocaru [mailto:[email protected]]
> Sent: Sunday, November 11, 2018 10:07 PM
>
> When an new altp2m view is created very early on guest boot, the
> display will freeze (although the guest will run normally). This
> may also happen on resizing the display. The reason is the way
> Xen currently (mis)handles logdirty VGA: it intentionally
> misconfigures VGA pages so that they will fault.
>
> The problem is that it only does this in the host p2m. Once we
> switch to a new altp2m, the misconfigured entries will no longer
> fault, so the display will not be updated.
>
> This patch:
> * updates ept_handle_misconfig() to use the active altp2m instead
> of the hostp2m;
> * modifies p2m_change_entry_type_global(), p2m_memory_type_changed
> and p2m_change_type_range() to propagate their changes to all
> valid altp2ms.
>
> Signed-off-by: Razvan Cojocaru <[email protected]>
> Suggested-by: George Dunlap <[email protected]>
>
> ---
> CC: Jun Nakajima <[email protected]>
> CC: Kevin Tian <[email protected]>
> CC: George Dunlap <[email protected]>
> CC: Jan Beulich <[email protected]>
> CC: Andrew Cooper <[email protected]>
> CC: Wei Liu <[email protected]>
>
> ---
> Changes since V4:
> - Now ASSERT()ing that altp2m is _not_ active in
> p2m_pt_handle_deferred_changes(), with added comment.
> - p2m_memory_type_changed() and p2m_change_type_range() now
> process altp2ms with the hostp2m lock taken.
> ---
> xen/arch/x86/mm/p2m-ept.c | 8 ++++
> xen/arch/x86/mm/p2m-pt.c | 8 ++++
> xen/arch/x86/mm/p2m.c | 115
> ++++++++++++++++++++++++++++++++++++++--------
> xen/include/asm-x86/p2m.h | 6 +--
> 4 files changed, 114 insertions(+), 23 deletions(-)
>
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index fabcd06..e6fa85f 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -657,6 +657,9 @@ bool_t ept_handle_misconfig(uint64_t gpa)
> bool_t spurious;
> int rc;
>
> + if ( altp2m_active(curr->domain) )
> + p2m = p2m_get_altp2m(curr);
> +
> p2m_lock(p2m);
>
> spurious = curr->arch.hvm.vmx.ept_spurious_misconfig;
> @@ -1440,6 +1443,11 @@ void p2m_init_altp2m_ept(struct domain *d,
> unsigned int i)
> struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> struct ept_data *ept;
>
> + p2m->max_mapped_pfn = hostp2m->max_mapped_pfn;
> + p2m->default_access = hostp2m->default_access;
> + p2m->domain = hostp2m->domain;
> +
> + p2m->global_logdirty = hostp2m->global_logdirty;
> p2m->ept.ad = hostp2m->ept.ad;
> p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> p2m->max_remapped_gfn = 0;
> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
> index 55df185..3828088 100644
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -29,6 +29,7 @@
> #include <xen/event.h>
> #include <xen/trace.h>
> #include <public/vm_event.h>
> +#include <asm/altp2m.h>
> #include <asm/domain.h>
> #include <asm/page.h>
> #include <asm/paging.h>
> @@ -464,6 +465,13 @@ int p2m_pt_handle_deferred_changes(uint64_t
> gpa)
> struct p2m_domain *p2m = p2m_get_hostp2m(current->domain);
> int rc;
>
> + /*
> + * Should altp2m ever be enabled for NPT / shadow use, this code
> + * should be updated to make use of the active altp2m, like
> + * ept_handle_misconfig().
> + */
> + ASSERT(!altp2m_active(current->domain));
> +
> p2m_lock(p2m);
> rc = do_recalc(p2m, PFN_DOWN(gpa));
> p2m_unlock(p2m);
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 69536c1..c8561ba 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -277,7 +277,6 @@ int p2m_init(struct domain *d)
> int p2m_is_logdirty_range(struct p2m_domain *p2m, unsigned long start,
> unsigned long end)
> {
> - ASSERT(p2m_is_hostp2m(p2m));
> if ( p2m->global_logdirty ||
> rangeset_contains_range(p2m->logdirty_ranges, start, end) )
> return 1;
> @@ -286,31 +285,79 @@ int p2m_is_logdirty_range(struct p2m_domain
> *p2m, unsigned long start,
> return 0;
> }
>
> +static void change_entry_type_global(struct p2m_domain *p2m,
> + p2m_type_t ot, p2m_type_t nt)
> +{
> + p2m->change_entry_type_global(p2m, ot, nt);
> + p2m->global_logdirty = (nt == p2m_ram_logdirty);
> +}
> +
> void p2m_change_entry_type_global(struct domain *d,
> p2m_type_t ot, p2m_type_t nt)
> {
> - struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
>
> ASSERT(ot != nt);
> ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
>
> - p2m_lock(p2m);
> - p2m->change_entry_type_global(p2m, ot, nt);
> - p2m->global_logdirty = (nt == p2m_ram_logdirty);
> - p2m_unlock(p2m);
> + p2m_lock(hostp2m);
> +
> + change_entry_type_global(hostp2m, ot, nt);
> +
> +#ifdef CONFIG_HVM
> + if ( unlikely(altp2m_active(d)) )
> + {
> + unsigned int i;
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> + {
> + struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> +
> + p2m_lock(altp2m);
> + change_entry_type_global(altp2m, ot, nt);
> + p2m_unlock(altp2m);
> + }
> + }
> +#endif
> +
> + p2m_unlock(hostp2m);
> +}
> +
> +#ifdef CONFIG_HVM
> +/* There's already a memory_type_changed() in asm/mtrr.h. */
> +static void _memory_type_changed(struct p2m_domain *p2m)
> +{
> + if ( p2m->memory_type_changed )
> + p2m->memory_type_changed(p2m);
> }
>
> void p2m_memory_type_changed(struct domain *d)
why making whole p2m_memory_type_changed under CONFIG_HVM?
> {
> - struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
>
> - if ( p2m->memory_type_changed )
> + p2m_lock(hostp2m);
> +
> + _memory_type_changed(hostp2m);
> +
> + if ( unlikely(altp2m_active(d)) )
> {
> - p2m_lock(p2m);
> - p2m->memory_type_changed(p2m);
> - p2m_unlock(p2m);
> + unsigned int i;
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> + {
> + struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> +
> + p2m_lock(altp2m);
> + _memory_type_changed(altp2m);
> + p2m_unlock(altp2m);
> + }
> }
> +
> + p2m_unlock(hostp2m);
> }
> +#endif
>
> int p2m_set_ioreq_server(struct domain *d,
> unsigned int flags,
> @@ -992,18 +1039,14 @@ int p2m_change_type_one(struct domain *d,
> unsigned long gfn_l,
> }
>
> /* Modify the p2m type of a range of gfns from ot to nt. */
> -void p2m_change_type_range(struct domain *d,
> - unsigned long start, unsigned long end,
> - p2m_type_t ot, p2m_type_t nt)
> +static void change_type_range(struct p2m_domain *p2m,
> + unsigned long start, unsigned long end,
> + p2m_type_t ot, p2m_type_t nt)
> {
> unsigned long gfn = start;
> - struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + struct domain *d = p2m->domain;
> int rc = 0;
>
> - ASSERT(ot != nt);
> - ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
> -
> - p2m_lock(p2m);
> p2m->defer_nested_flush = 1;
>
> if ( unlikely(end > p2m->max_mapped_pfn) )
> @@ -1047,7 +1090,39 @@ void p2m_change_type_range(struct domain *d,
> p2m->defer_nested_flush = 0;
> if ( nestedhvm_enabled(d) )
> p2m_flush_nestedp2m(d);
> - p2m_unlock(p2m);
> +}
> +
> +void p2m_change_type_range(struct domain *d,
> + unsigned long start, unsigned long end,
> + p2m_type_t ot, p2m_type_t nt)
> +{
> + struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> +
> + ASSERT(ot != nt);
> + ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
> +
> + p2m_lock(hostp2m);
> +
> + change_type_range(hostp2m, start, end, ot, nt);
> +
> +#ifdef CONFIG_HVM
> + if ( unlikely(altp2m_active(d)) )
> + {
> + unsigned int i;
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> + {
> + struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> +
> + p2m_lock(altp2m);
> + change_type_range(altp2m, start, end, ot, nt);
> + p2m_unlock(altp2m);
> + }
> + }
> +#endif
> +
> + p2m_unlock(hostp2m);
> }
>
> /*
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index c7f5710..be5b7a2 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -630,9 +630,6 @@ int p2m_finish_type_change(struct domain *d,
> gfn_t first_gfn,
> unsigned long max_nr);
>
> -/* Report a change affecting memory types. */
> -void p2m_memory_type_changed(struct domain *d);
> -
> int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
> unsigned long end);
>
> @@ -663,6 +660,9 @@ void p2m_pod_dump_data(struct domain *d);
>
> #ifdef CONFIG_HVM
>
> +/* Report a change affecting memory types. */
> +void p2m_memory_type_changed(struct domain *d);
> +
> /* Called by p2m code when demand-populating a PoD page */
> bool
> p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn,
> unsigned int order);
> --
> 2.7.4
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
