On Thu, Nov 06, 2008 at 01:57:52PM +0100, Alexander Graf wrote:
>
> On 05.11.2008, at 21:58, Eduardo Habkost wrote:
>
>> On Wed, Nov 05, 2008 at 11:41:04AM +0100, Alexander Graf wrote:
>>> X86 CPUs need to have some magic happening to enable the
>>> virtualization
>>> extensions on them. This magic can result in unpleasant results for
>>> users, like blocking other VMMs from working (vmx) or using invalid
>>> TLB
>>> entries (svm).
>>>
>>> Currently KVM activates virtualization when the respective kernel
>>> module
>>> is loaded. This blocks us from autoloading KVM modules without
>>> breaking
>>> other VMMs.
>>>
>>> To circumvent this problem at least a bit, this patch introduces on
>>> demand activation of virtualization. This means, that instead
>>> virtualization is enabled on creation of the first virtual machine
>>> and disabled on removal of the last one.
>>>
>>> So using this, KVM can be easily autoloaded, while keeping other
>>> hypervisors usable.
>>>
>>> v2 adds returns to non-x86 hardware_enables and adds IA64 change
>>> v3 changes:
>>> - use spin_lock instead of atomics
>>> - put locking to new functions hardware_{en,dis}able_all that get
>>> called
>>> on VM creation/destruction
>>> - remove usage counter checks where not necessary
>>> - return -EINVAL for IA64 slot < 0 case
>>>
>>> Signed-off-by: Alexander Graf <[EMAIL PROTECTED]>
>>> ---
>> <snip>
>>>
>>> -static void hardware_enable(void *junk)
>>> +static void hardware_enable(void *_r)
>>> {
>>> int cpu = raw_smp_processor_id();
>>> + int r;
>>> +
>>> + /* If enabling a previous CPU failed already, let's not continue */
>>> + if (_r && *((int*)_r))
>>> + return;
>>>
>>> if (cpu_isset(cpu, cpus_hardware_enabled))
>>> return;
>>> + r = kvm_arch_hardware_enable(NULL);
>>> + if (_r)
>>> + *((int*)_r) = r;
>>> + if (r) {
>>> + printk(KERN_INFO "kvm: enabling virtualization on "
>>> + "CPU%d failed\n", cpu);
>>> + return;
>>> + }
>>> +
>>> cpu_set(cpu, cpus_hardware_enabled);
>>> - kvm_arch_hardware_enable(NULL);
>>> +}
>>
>> Doesn't on_each_cpu() run the function in parallel on all CPUs? If so,
>> there is a race between checking *_r and setting *_r.
>
> Good question - it doesn't really hurt to write the value though, if we
> only write it on error.
> So I guess we could just remove the first check and check on if( r &&
> _r) later on.
I think the first check doesn't hurt, and using if(_r && r) on the second
check should work. I am not sure if there are no pitfalls here due to
memory ordering or delayed write on some arches, however.
--
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
