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

[Avi Kivity <avi@redhat.com>]

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.