Re: [Xen-devel] [PATCH] x86/altp2m: propagate ept.ad changes to all active altp2ms

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 9/28/18 6:55 PM, Jan Beulich wrote:
>>>> On 28.09.18 at 17:25, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 9/28/18 5:52 PM, Jan Beulich wrote:
>>>>>> On 28.09.18 at 13:55, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> @@ -1218,34 +1219,67 @@ static void ept_tlb_flush(struct p2m_domain *p2m)
>>>>      ept_sync_domain_mask(p2m, p2m->domain->dirty_cpumask);
>>>>  }
>>>>  
>>>> +static void ept_set_ad_sync(struct p2m_domain *p2m, int value)
>>>> +{
>>>> +    struct domain *d = p2m->domain;
>>>> +    unsigned int i;
>>>> +
>>>> +    if ( likely(!altp2m_active(d)) )
>>>> +    {
>>>> +        p2m_lock(p2m);
>>>> +        p2m->ept.ad = value;
>>>> +        p2m_unlock(p2m);
>>>> +
>>>> +        return;
>>>> +    }
>>>
>>> Why would you want to skip updating the host p2m's flag when
>>> altp2m is active?
>>
>> It's not really skipped if I understand the altp2m code correctly: in
>> that case the hostp2m is d->arch.altp2m_p2m[0], which is take care of in
>> the loop below the code you've quoted.
> 
> p2m_init_altp2m() (and other code in p2m.c) suggests otherwise to me.

That's interesting, p2m_set_mem_access() is treating altp2m index 0 as
the hostp2m:

360 /*
361  * Set access type for a region of gfns.
362  * If gfn == INVALID_GFN, sets the default access type.
363  */
364 long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
365                         uint32_t start, uint32_t mask,
xenmem_access_t access,
366                         unsigned int altp2m_idx)
367 {
368     struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
369     p2m_access_t a;
370     unsigned long gfn_l;
371     long rc = 0;
372
373     /* altp2m view 0 is treated as the hostp2m */
374 #ifdef CONFIG_HVM
375     if ( altp2m_idx )
376     {
377         if ( altp2m_idx >= MAX_ALTP2M ||
378              d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
379             return -EINVAL;
380
381         ap2m = d->arch.altp2m_p2m[altp2m_idx];
382     }
383 #else
384     ASSERT(!altp2m_idx);
385 #endif

which would seem to imply that either we should be able to treat
d->arch.altp2m_p2m[0] and hostp2m interchangeably, or we are currently
wasting an altp2m array slot.


Thanks,
Razvan

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