From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH RFC v2 3/4] Break dependency between vcpu index in
	vcpus array and vcpu_id.
Date: Tue, 26 May 2009 11:55:03 +0300
Message-ID: <20090526085503.GP3948@redhat.com>
References: <1243266636-16914-1-git-send-email-gleb@redhat.com> <1243266636-16914-4-git-send-email-gleb@redhat.com> <4A1BA979.4010808@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:34846 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753331AbZEZIzD (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 May 2009 04:55:03 -0400
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n4Q8t5Z4005928
	for <kvm@vger.kernel.org>; Tue, 26 May 2009 04:55:05 -0400
Content-Disposition: inline
In-Reply-To: <4A1BA979.4010808@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.