Re: [Xen-devel] [PATCH V2 3/3] x86/altp2m: fix display frozen when switching to a new view early

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.10.18 at 15:19, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>  xen/arch/x86/mm/p2m-ept.c |  8 +++++
>  xen/arch/x86/mm/p2m.c     | 83 
> +++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 78 insertions(+), 13 deletions(-)

What about p2m-pt.c?

> @@ -287,24 +286,47 @@ int p2m_is_logdirty_range(struct p2m_domain *p2m, 
> unsigned long start,
>      return 0;
>  }
>  
> +static void _p2m_change_entry_type_global(struct p2m_domain *p2m,

Instead of prefixing another underscore, why don't you
drop p2m_ from this static helper?

> +                                          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);
> +
> +    _p2m_change_entry_type_global(p2m_get_hostp2m(d), ot, nt);

Use hostp2m here too?

> +
> +#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 *p2m = d->arch.altp2m_p2m[i];
> +
> +                p2m_lock(p2m);
> +                _p2m_change_entry_type_global(p2m, ot, nt);
> +                p2m_unlock(p2m);
> +            }
> +    }
> +#endif
> +
> +    p2m_unlock(hostp2m);
>  }
>  
> -void p2m_memory_type_changed(struct domain *d)
> +static void _p2m_memory_type_changed(struct p2m_domain *p2m)

Same as above.

> @@ -313,6 +335,22 @@ void p2m_memory_type_changed(struct domain *d)
>      }
>  }
>  
> +void p2m_memory_type_changed(struct domain *d)
> +{
> +#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) )
> +                _p2m_memory_type_changed(d->arch.altp2m_p2m[i]);
> +    }
> +#endif
> +
> +    _p2m_memory_type_changed(p2m_get_hostp2m(d));
> +}

Is there a particular reason this one doesn't follow the host-then-
alt pattern used above and elsewhere? If so, a comment should
be attached. If not, using that same model would seem better.
(Same then below for the range function.)

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel