Am 29.08.2013 04:09, schrieb Chen Fan: > After ACPI get a signal to eject a vcpu, then it will notify > the vcpu thread of needing to exit, before the vcpu exiting, > will release the vcpu related objects. > > Signed-off-by: Chen Fan <chen.fan.fnst@xxxxxxxxxxxxxx> > --- > cpus.c | 36 ++++++++++++++++++++++++++++++++++++ > hw/acpi/piix4.c | 16 ++++++++++++---- > include/qom/cpu.h | 9 +++++++++ > include/sysemu/kvm.h | 1 + > kvm-all.c | 26 ++++++++++++++++++++++++++ > 5 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 70cc617..6b793cb 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -697,6 +697,30 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) > qemu_cpu_kick(cpu); > } > > +static void qemu_kvm_destroy_vcpu(CPUState *cpu) > +{ > + CPUState *pcpu, *pcpu1; > + > + pcpu = first_cpu; > + pcpu1 = NULL; > + > + while (pcpu) { > + if (pcpu == cpu && pcpu1) { > + pcpu1->next_cpu = cpu->next_cpu; > + break; > + } > + pcpu1 = pcpu; > + pcpu = pcpu->next_cpu; > + } No, no, no. :) I specifically posted the "QOM CPUState, part 12" series early to avoid exactly such code appearing! Give me a few minutes to apply that to qom-cpu and then please rebase your work on git://github.com/afaerber/qemu-cpu.git qom-cpu using QTAILQ macro and --subject-prefix="RFC qom-cpu v2" for the next version of the series. Also, why is this only in the KVM code path? Isn't this needed for TCG as well? > + > + if (kvm_destroy_vcpu(cpu) < 0) { > + fprintf(stderr, "kvm_destroy_vcpu failed.\n"); > + exit(1); > + } > + > + qdev_free(DEVICE(X86_CPU(cpu))); DEVICE(cpu) should be sufficient. > +} > + > static void flush_queued_work(CPUState *cpu) > { > struct qemu_work_item *wi; > @@ -788,6 +812,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) > } > } > qemu_kvm_wait_io_event(cpu); > + if (cpu->exit && !cpu_can_run(cpu)) { > + qemu_kvm_destroy_vcpu(cpu); > + qemu_mutex_unlock(&qemu_global_mutex); > + return NULL; > + } > } > > return NULL; > @@ -1080,6 +1109,13 @@ static void qemu_dummy_start_vcpu(CPUState *cpu) > } > } > > +void qemu_down_vcpu(CPUState *cpu) > +{ > + cpu->stop = true; > + cpu->exit = true; > + qemu_cpu_kick(cpu); > +} > + > void qemu_init_vcpu(CPUState *cpu) > { > cpu->nr_cores = smp_cores; > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 1aaa7a4..44bc809 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -611,10 +611,18 @@ static const MemoryRegionOps piix4_pci_ops = { > }, > }; > > -static void acpi_piix_eject_vcpu(int64_t cpuid) > +static void acpi_piix_eject_vcpu(PIIX4PMState *s, int64_t cpuid) > { > - /* TODO: eject a vcpu, release allocated vcpu and exit the vcpu pthread. */ > - PIIX4_DPRINTF("vcpu: %" PRIu64 " need to be ejected.\n", cpuid); > + CPUStatus *cpus = &s->gpe_cpu; > + CPUState *cs = NULL; > + > + cs = qemu_get_cpu(cpuid); Are you sure this is correct as 0-based index? Igor? > + if (cs == NULL) { > + return; > + } > + > + cpus->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8)); > + qemu_down_vcpu(cs); > } > > static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size) > @@ -647,7 +655,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data, > } > > if (cpuid != 0) { > - acpi_piix_eject_vcpu(cpuid); > + acpi_piix_eject_vcpu(s, cpuid); > } > } > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 3e49936..fa8ec8a 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -180,6 +180,7 @@ struct CPUState { > bool created; > bool stop; > bool stopped; > + bool exit; > volatile sig_atomic_t exit_request; > volatile sig_atomic_t tcg_exit_req; > uint32_t interrupt_request; > @@ -489,6 +490,14 @@ void cpu_exit(CPUState *cpu); > void cpu_resume(CPUState *cpu); > > /** > + * qemu_down_vcpu: > + * @cpu: The vCPU will to down. > + * > + * Down a vCPU. > + */ > +void qemu_down_vcpu(CPUState *cpu); The naming and documentation sounds wrong language-wise to me, but I am not a native speaker either. Maybe "tear down" instead of "down"? Or simply qemu_request_vcpu_removal() or something like that? > + > +/** > * qemu_init_vcpu: > * @cpu: The vCPU to initialize. > * > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index de74411..fd85605 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -158,6 +158,7 @@ int kvm_has_intx_set_mask(void); > > int kvm_init_vcpu(CPUState *cpu); > int kvm_cpu_exec(CPUState *cpu); > +int kvm_destroy_vcpu(CPUState *cpu); > > #ifdef NEED_CPU_H > > diff --git a/kvm-all.c b/kvm-all.c > index 716860f..fda3601 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -225,6 +225,32 @@ static void kvm_reset_vcpu(void *opaque) > kvm_arch_reset_vcpu(cpu); > } > > +int kvm_destroy_vcpu(CPUState *cpu) > +{ > + KVMState *s = kvm_state; > + long mmap_size; > + int ret = 0; > + > + DPRINTF("kvm_destroy_vcpu\n"); > + > + mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0); > + if (mmap_size < 0) { > + ret = mmap_size; > + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n"); > + goto err; > + } > + > + ret = munmap(cpu->kvm_run, mmap_size); > + if (ret < 0) { > + goto err; > + } > + > + close(cpu->kvm_fd); > + > +err: > + return ret; > +} I always assumed we'd need a new ioctl to inform the kernel about our intent to remove a vCPU before doing such cleanups? CC'ing kvm list. Regards, Andreas > + > int kvm_init_vcpu(CPUState *cpu) > { > KVMState *s = kvm_state; > -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- 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