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 <gleb@xxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
