On Thu, May 21, 2009 at 03:56:46PM +0300, Gleb Natapov wrote: > Move decision on how to store vcpus into arch code. X86 finds first free > slot in vcpus array and uses id that userspace pass as vcpu's apic id. > Other arches keep using it as array index for now. > > Signed-off-by: Gleb Natapov <[email protected]> > --- > arch/ia64/kvm/kvm-ia64.c | 25 ++++++++++++++++++--- > arch/powerpc/kvm/booke.c | 20 ++++++++++++++++- > arch/powerpc/kvm/powerpc.c | 7 +++++- > arch/s390/kvm/kvm-s390.c | 24 ++++++++++++++++++-- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/lapic.c | 4 +- > arch/x86/kvm/x86.c | 45 > ++++++++++++++++++++++++++++++++++++++- > include/linux/kvm_host.h | 7 +++++- > virt/kvm/kvm_main.c | 27 +--------------------- > 9 files changed, 122 insertions(+), 38 deletions(-) > > diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c > index 8a5911c..575765c 100644 > --- a/arch/ia64/kvm/kvm-ia64.c > +++ b/arch/ia64/kvm/kvm-ia64.c > @@ -1200,7 +1200,7 @@ out: > > #define PALE_RESET_ENTRY 0x80000000ffffffb0UL > > -int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > +int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu, u32 id) > { > struct kvm_vcpu *v; > int r; > @@ -1212,6 +1212,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > union context *p_ctx = &vcpu->arch.guest; > struct kvm_vcpu *vmm_vcpu = to_guest(vcpu->kvm, vcpu); > > + vcpu->vcpu_id = id; > + > /*Init vcpu context for first run.*/ > if (IS_ERR(vmm_vcpu)) > return PTR_ERR(vmm_vcpu); > @@ -1321,8 +1323,7 @@ fail: > return r; > } > > -struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, > - unsigned int id) > +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id) > { > struct kvm_vcpu *vcpu; > unsigned long vm_base = kvm->arch.vm_base; > @@ -1332,6 +1333,9 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, > BUG_ON(sizeof(struct kvm_vcpu) > VCPU_STRUCT_SIZE/2); > > r = -EINVAL; > + if (!valid_vcpu_idx(id)) > + goto fail; > + > if (id >= KVM_MAX_VCPUS) { > printk(KERN_ERR"kvm: Can't configure vcpus > %ld", > KVM_MAX_VCPUS); > @@ -1365,6 +1369,19 @@ fail: > > int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) > { > + struct kvm *kvm = vcpu->kvm; > + > + mutex_lock(&kvm->lock); > + if (kvm->vcpus[vcpu->vcpu_id]) { > + mutex_unlock(&kvm->lock); > + kvm_arch_vcpu_destroy(vcpu); > + return -EEXIST; > + } > + /* first one created is BSP */ > + if (!kvm->bsp_vcpu) > + kvm->bsp_vcpu = vcpu; > + kvm->vcpus[vcpu->vcpu_id] = vcpu; > + mutex_unlock(&kvm->lock);
Why don't make a convention that vcpu_id 0 is the BSP, by default, instead of the first vcpu created? This way if userspace creates vcpu 3 first, there are no problems. That brings the question, why is the apic_id passed as the argument to vcpu_create? It seems that the argument to vcpu_create should be the kVM's internal vcpu_id, and the apic_id should come from somewhere else, apic_mmio_write maybe? AFAICS there is no linkage between apic_id and BSP (MP SPEC says BSP is determined by hardware and BIOS), but in KVM's BIOS case the BSP is conventioned to be vcpu with vcpu_id == 0, no? Another thing, it would be better if the linking of the vcpu in the array could be in arch independent code as it is today? -- 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
