Re: the P2M/VP patch merge plan (was Re: [Xen-ia64-devel] [PATCH][RFC][TAKE5] the P2M/VP patches)

[Tristan Gingold <Tristan.Gingold@xxxxxxxx>]

Le Mercredi 17 Mai 2006 04:36, Isaku Yamahata a écrit :
> On Tue, May 16, 2006 at 05:30:17PM +0200, Tristan Gingold wrote:
> > [...]
> >
> > > > > The following is what I notice now.
> > > > >
> > > > > - pgd_populate(), pud_populate(), pmd_populate()
> > > > >   What if two cpu try to populate same virtual address?
> > > > >   Given that page allocation on demand is now removed, it might be
> > > > > possible to all necessary pgd/pud/pmd/pte page is allocate at
> > > > > domain creation.
> >
> > This could be easily fixed.  However there is also no lock in the current
> > Xen, so I think the kernel never does that.
>
> Although current linux kernel doesn't,
> I will add such a code to avoid oom-kiler in dom0 when vt-i domain
> is created.
> Hmm, it doesn't seem that you have the necessity of protecting the p2m
> table. On the other hand I have.
> I'll work on adding protection the p2m table. Is it okay?
I still do not understand why we need to protect p2m table.
I don't have your deep understanding, but why two cpus can try to fill the 
same p2m entry. 

> Later we may want to evaluate its peformance impact though.

> > > > > - guest_physmap_add_page()
> > > > >     assign_domain_page_replace()
> > > > >       ptep_get_and_clear()
> > > > >                       <<<<<<<<<<<< what if another cpu does
> > > > > set_pte() here? set_pte()
> > > > >     set_gpfn_from_mfn()
> >
> > We should create a ptep_get_and_replace.
> > Is it enough ?
>
> Just for the integrity of the p2m table, it might be enough.
> But I'm not sure.
>
> > Since kernel is not supposed to access to pages being replaced, this
> > shouldn't happen, should it ?
>
> What do you mean by 'not supposed'?
> I think that nothing guarantees it.
> Xen shouldn't rely on the fact that xenLinux just doesn't but a guest
> domain surely can.
If a guest domain is still writing (or reading) a dying page, it miss-behaves.
For sure, we must protect Xen against misbehavior, but this is only a security 
issue (ie, if SMP-g doesn't work yet this is not due to this).

> > > > > - memory ordering
> > > > >   set_pte() doesn't have any memory relase semantics.
> > > > >   And readers (i.e. *(pte)) doesn't have acquire semantics.
> > > > >   I guess some memory barrier is required.
> > > > >   (spin lock means memory barrier)
> >
> > I have added mp().
>
> typo? mb()?
Yes.

Tristan.


_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel