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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.10.18 at 16:56, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> The biggest problem here is p2m->logdirty_ranges. This patch will
> (justly) not work, because struct rangeset is only forward-declared in
> xen/rangeset.h, so an incomplete type here:
> 
> -void p2m_init_altp2m_ept(struct domain *d, unsigned int i)
> +int p2m_init_altp2m_ept(struct domain *d, unsigned int i)
>  {
>      struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
>      struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
>      struct ept_data *ept;
> 
> +    if ( !p2m->logdirty_ranges )
> +        p2m->logdirty_ranges = rangeset_new(d, "log-dirty",
> +                                            RANGESETF_prettyprint_hex);
> +    if ( !p2m->logdirty_ranges )
> +        return -ENOMEM;
> +
> +    *p2m->logdirty_ranges = *hostp2m->logdirty_ranges;
> +
>      p2m->ept.ad = hostp2m->ept.ad;
> +    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->min_remapped_gfn = gfn_x(INVALID_GFN);
>      p2m->max_remapped_gfn = 0;
>      ept = &p2m->ept;
>      ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m));
>      d->arch.altp2m_eptp[i] = ept->eptp;
> +
> +    return 0;
> +}
> 
> But that's not even the biggest problem: even if that would compile, it
> would still be wrong, because logdirty_pages has pointers of its own,
> which means that two bitwise-copied distinct rangesets can still point
> to the same data and thus be vulnerable to race conditions and wanting
> synchronization.
> 
> Furthermore there's no rangeset_copy() function in sight in rangeset.h
> (though there is a rangeset_swap()).
> 
> Would you like me to add a rangeset_copy() function (presumably another
> intermediary patch) and proceed in that manner?

Roger recently has posted a patch adding rangeset_merge(), which I think
is more general than your rangeset_copy(). That said, I'm in no way
convinced copying (and then keeping in sync) the range sets across the
altp2m-s is the best approach. It may well be that the optimal solution is
somewhere in the middle between sharing everything and copying
everything.

Jan



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