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

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

On 04/07/2016 22:19, Sergej Proskurin wrote:
> On 07/04/2016 07:44 PM, Julien Grall wrote:
> > On 04/07/16 17:56, Sergej Proskurin wrote:
> > > 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.
Really? I have this patch series applied on top of staging and there is 
no TLB flush instruction in p2m_restore_state nor p2m_load*.
Regards,

--
Julien Grall

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