Re: [Xen-devel] [PATCH 04/18] arm/altp2m: Add altp2m init/teardown routines.

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

On 07/04/2016 11:19 PM, Sergej Proskurin wrote:
> 
> On 07/04/2016 07:44 PM, Julien Grall wrote:
>>
>>
>> On 04/07/16 17:56, Sergej Proskurin wrote:
>>> Hi Andrew,
>>>
>>> On 07/04/2016 06:43 PM, Andrew Cooper wrote:
>>>> On 04/07/16 17:40, Sergej Proskurin wrote:
>>>>>
>>>>>>>    {
>>>>>>> -    struct p2m_domain *p2m = &d->arch.p2m;
>>>>>>> +    int ret = 0;
>>>>>>> +
>>>>>>> +    spin_lock_init(&p2m->lock);
>>>>>>> +    INIT_PAGE_LIST_HEAD(&p2m->pages);
>>>>>>> +
>>>>>>> +    spin_lock(&p2m->lock);
>>>>>>> +
>>>>>>> +    p2m->domain = d;
>>>>>>> +    p2m->access_required = false;
>>>>>>> +    p2m->mem_access_enabled = false;
>>>>>>> +    p2m->default_access = p2m_access_rwx;
>>>>>>> +    p2m->p2m_class = p2m_host;
>>>>>>> +    p2m->root = NULL;
>>>>>>> +
>>>>>>> +    /* Adopt VMID of the associated domain */
>>>>>>> +    p2m->vmid = d->arch.p2m.vmid;
>>>>>> It looks like to me that re-using the same VMID will require more TLB
>>>>>> flush (such as when a VCPU is migrated to another physical CPU). So
>>>>>> could you explain why you decided to re-use the same VMID?
>>>>>>
>>>>> Please correct me if I am wrong, but I associate a VMID with an entire
>>>>> domain. Since, the altp2m view still belongs to the same domain
>>>>> (p2m_init_one is called only from p2m_init_altp2m), the code re-uses
>>>>> the
>>>>> old VMID.
>>>>
>>>> (I am not an ARM expert but) looking into VMIDs from the last time, they
>>>> are the TLB tag for the address space in use.
>>>>
>>>> Does ARM have shared TLBs between multiple cores?  If so, you must a
>>>> separate VMID, otherwise an ALT2PM used by one vcpu could cause a
>>>> separate vcpu with a different ALTP2M to reuse the wrong translation.
>>>>
>>>> ~Andrew
>>>>
>>>
>>> You're absolutely correct. However, on every VMENTRY Xen explicitly
>>> flushes the TLBs of the currently active domain (and with it, of the
>>> currently active (a)p2m table) and hence it should not result in an
>>> issue.
>>
>> VMENTRY is x86 not ARM. So are you sure you looked at the correct code?
>>
>> Regards,
>>
> 
> This is true. I just use the term VMENTER for describing transitions to
> guests on both, x86 and ARM. In ./xen/arch/arm/domain.c the function
> ctxt_switch_to calls p2m_restore_state on every context switch, wich in
> turn loads the VTTBR associated to the domain and flushes the TLBs.
> 

My bad: after loading the VTTBR, the TLBs are not explicitly flushed, as
I claimed before. That is, using a dedicated VMID per altp2m view is
definitely required. I will fix this. Thank you very much.

Best regards,
Sergej

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel