Re: [patch 2/5] KVM: reintroduce guest mode bit and unify remote request code

Avi Kivity
2009-08-28T07:05:47Z

On 08/27/2009 05:07 PM, Marcelo Tosatti wrote:
> On Thu, Aug 27, 2009 at 04:24:58PM +0300, Avi Kivity wrote:
>
>> On 08/27/2009 03:45 PM, Marcelo Tosatti wrote:
>>
>>>> Why not bool guest_mode? Saves two atomics per exit.
>>>>
>>>>
>>> It must be atomic since GUEST_MODE / VCPU_KICKED bits are manipulated by
>>> multiple CPU's.
>>>
>>>
>> bools are atomic.
>>
> OK.
>
> - VCPU_KICKED requires test_and_set. GUEST_MODE/VCPU_KICKED accesses
> must not be reordered.
>

Why do we need both, btw? Set your vcpu->requests bit, if guest_mode is
true, clear it and IPI. So guest_mode=false means, we might be in guest
mode but if so we're due for a kick anyway.

> (OK, could have GUEST_MODE in a bool even so, but its easier to read
> by keeping them together, at least to me).
>
> - Its easier to cacheline align with longs rather than bools?
>

To cacheline align we need to pack everything important at the front of
the structure.

> - From testing it seems the LOCK prefix is not heavy, as long as its
> cpu local (probably due to 7.1.4 Effects of a LOCK Operation on
> Internal Processor Caches?).
>

Yes, in newer processors atomics are not nearly as expensive as they
used to be.

> BTW,
>
> 7.1.2.2 Software Controlled Bus Locking
>
> Software should access semaphores (shared memory used for signalling
> between multiple processors) using identical addresses and operand
> lengths. For example, if one processor accesses a semaphore using a word
> access, other processors should not access the semaphore using a byte
> access.
>
> The bit operations use 32-bit access, but the vcpu->requests check in
> vcpu_enter_guest uses 64-bit access.
>

That's true, and bitops sometimes even uses byte operations.

> Is that safe?
>

My guess yes, but not efficient.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.