Re: [Xen-devel] lock in vhpet

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] On Behalf Of Keir Fraser
> 
> > Hi keir
> >
> > I noticed that the changeset 15289 introuduced locking to platform
> > timers. And you mentioned that it only handy for correctness. Are
> > there some potential issues which is fixed by this patch? If not, I wonder 
> > why
> we need those locks?
> 
> Yes, issues were fixed by the patch. That's why I bothered to implement it.
> However I think the observed issues were with protecting the mechanisms in
> vpt.c, and the other locking at least partially may be overly cautious.
> 
> > I think it should be OS's responsibly to guarantee the access
> > sequentially, not hypervisor. Am I right?
> 
> It depends. Where an access is an apparently-atomic memory-mapped access,
> but implemented as a sequence of operations in the hypervisor, the hypervisor
> might need to maintain atomicity through locking.

But if there already has lock inside guest for those access, and there have no 
other code patch(like timer callback function) in hypervisor to access the 
shared data, then we don't need to use lock in hypersivor.

> > I don't know whether all those locks are necessary, but at least the
> > lock for vhpet, especially the reading lock, is not required.
> 
> This is definitely not true, for example locking is required around calls to
> create_periodic_time(), to serialise them. So in general the locking I added,
> even in vhpet.c is required. If you have a specific hot path you are looking 
> to
> optimise, and especially if you have numbers to back that up, then we can
> consider specific localised optimisations to avoid locking where we can reason
> it is not needed.
As I mentioned, if the guest can ensure there only one CPU to access the hpet 
at same time, this means the access itself already is serialized. 
Yes.Win8 is booting very slowly w/ more than 16 VCPUs. And this is due to the 
big lock in reading hpet. Also, the xentrace data shows lots of vmexit which 
mainly from PAUSE instruction from other cpus. So I think if guest already uses 
lock to protect the hpet access, why hypervisor do the same thing too?

yang

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