Re: lock in vhpet

[Keir Fraser <keir.xen@gmail.com>]

On 18/04/2012 01:52, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:

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

If there is a memory-mapped register access which is atomic on bare metal,
the guest may not bother with locking. We have to maintain that apparent
atomicity ourselves by implementing serialisation in the hypervisor.

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

If the HPET accesses are atomic on bare metal, we have to maintain that,
even if some guests have extra locking themselves. Also, in some cases Xen
needs locking to correctly maintain its own internal state regardless of
what an (untrusted) guest might do. So we cannot just get rid of the vhpet
lock everywhere. It's definitely not correct to do so. Optimising the hpet
read path however, sounds okay. I agree the lock may not be needed on that
specific path.

 -- Keir