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

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Wed, May 17, 2006 at 09:34:23AM +0200, Tristan Gingold wrote:
> 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. 

Your approach seems just band-aiding found races.
I don't believe that such a approach is good and
that we can achieve stability.
The essence is that tlb insert/purge virtulalization by xen
isn't serialized.

A big lock should be introduced as a first step.
It is O.K. that its performance may be poor at first.
Then the big lock should be relaxed later making sure that no race exists.


> > > > > > - 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.

Yes.


> 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).

It maybe true.
However fixing this requires a kind of exclusion, it will make a big impact
on code complexity and performance.
It would be very difficult to introduce a exclusion after performance
tuning. And tuning must be done again.
-- 
yamahata

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