Re: [PATCH RFC v2 3/4] Break dependency between vcpu index in vcpus array and vcpu_id.

[Gleb Natapov <gleb@redhat.com>]

On Tue, May 26, 2009 at 11:34:01AM +0300, Avi Kivity wrote:
> Gleb Natapov wrote:
>> Archs are free to use vcpu_id as they see fit. For x86 it is used as
>> vcpu's apic id.
>>
>>   
>
> You need a KVM_CAP to inform userspace that the vcpu id has changed meaning.
>
Will add.

>>  inline int kvm_is_mmio_pfn(pfn_t pfn)
>>  {
>>  	if (pfn_valid(pfn)) {
>> @@ -1713,15 +1708,12 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu)
>>  /*
>>   * Creates some virtual cpus.  Good luck creating more than one.
>>   */
>> -static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n)
>> +static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>>  {
>>  	int r;
>>  	struct kvm_vcpu *vcpu;
>>  -	if (!valid_vcpu(n))
>> -		return -EINVAL;
>> -
>> -	vcpu = kvm_arch_vcpu_create(kvm, n);
>> +	vcpu = kvm_arch_vcpu_create(kvm, id);
>>  	if (IS_ERR(vcpu))
>>  		return PTR_ERR(vcpu);
>>  @@ -1732,25 +1724,36 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm 
>> *kvm, int n)
>>  		return r;
>>   	mutex_lock(&kvm->lock);
>> -	if (kvm->vcpus[n]) {
>> -		r = -EEXIST;
>> +	if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) {
>> +		r = -EINVAL;
>>  		goto vcpu_destroy;
>>  	}
>> -	kvm->vcpus[n] = vcpu;
>> -	if (n == 0)
>> -		kvm->bsp_vcpu = vcpu;
>> -	mutex_unlock(&kvm->lock);
>> +
>> +	for (r = 0; r < atomic_read(&kvm->online_vcpus); r++)
>> +		if (kvm->vcpus[r]->vcpu_id == id) {
>> +			r = -EEXIST;
>> +			goto vcpu_destroy;
>> +		}
>> +
>> +	BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]);
>>   	/* Now it's all set up, let userspace reach it */
>>  	kvm_get_kvm(kvm);
>>  	r = create_vcpu_fd(vcpu);
>> -	if (r < 0)
>> -		goto unlink;
>> +	if (r < 0) {
>> +		kvm_put_kvm(kvm);
>> +		goto vcpu_destroy;
>> +	}
>> +
>> +	kvm->vcpus[atomic_read(&kvm->online_vcpus)] = vcpu;
>> +	smp_wmb();
>> +	atomic_inc(&kvm->online_vcpus);
>> +
>> +	if (id == 0)
>> +		kvm->bsp_vcpu = vcpu;
>> +	mutex_unlock(&kvm->lock);
>>  	return r;
>>  -unlink:
>> -	mutex_lock(&kvm->lock);
>> -	kvm->vcpus[n] = NULL;
>>  vcpu_destroy:
>>  	mutex_unlock(&kvm->lock);
>>  	kvm_arch_vcpu_destroy(vcpu);
>>   
>
> Don't the vcpu ioctls need to be updated?  They get the vcpu id as a  
> parameter.
>
Are you sure they do? vcpu ioctls are issued on vcpu fd, no need to pass
vcpu id as a parameter.

--
			Gleb.