* [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
When OS eject a vcpu (like: echo 1 > /sys/bus/acpi/devices/LNXCPUXX/eject),
it will call acpi EJ0 method, the firmware will write the new cpumap, QEMU
will know which vcpu need to be ejected.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/acpi/piix4.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index c885690..1aaa7a4 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -61,6 +61,7 @@ struct pci_status {
typedef struct CPUStatus {
uint8_t sts[PIIX4_PROC_LEN];
+ uint8_t old_sts[PIIX4_PROC_LEN];
} CPUStatus;
typedef struct PIIX4PMState {
@@ -610,6 +611,12 @@ static const MemoryRegionOps piix4_pci_ops = {
},
};
+static void acpi_piix_eject_vcpu(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);
+}
+
static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
{
PIIX4PMState *s = opaque;
@@ -622,7 +629,26 @@ static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
unsigned int size)
{
- /* TODO: implement VCPU removal on guest signal that CPU can be removed */
+ PIIX4PMState *s = opaque;
+ CPUStatus *cpus = &s->gpe_cpu;
+ uint8_t val;
+ int i;
+ int64_t cpuid = 0;
+
+ val = cpus->old_sts[addr] ^ data;
+
+ if (val == 0)
+ return;
+
+ for (i = 0; i < 8; i++) {
+ if (val & 1 << i) {
+ cpuid = 8 * addr + i;
+ }
+ }
+
+ if (cpuid != 0) {
+ acpi_piix_eject_vcpu(cpuid);
+ }
}
static const MemoryRegionOps cpu_hotplug_ops = {
@@ -647,11 +673,17 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
ACPIGPE *gpe = &s->ar.gpe;
CPUClass *k = CPU_GET_CLASS(cpu);
int64_t cpu_id;
+ int i;
assert(s != NULL);
*gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
cpu_id = k->get_arch_id(CPU(cpu));
+
+ for (i = 0; i < PIIX4_PROC_LEN; i++) {
+ g->old_sts[i] = g->sts[i];
+ }
+
if (action == PLUG) {
g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
} else {
@@ -675,6 +707,7 @@ static void piix4_init_cpu_status(CPUState *cpu, void *data)
g_assert((id / 8) < PIIX4_PROC_LEN);
g->sts[id / 8] |= (1 << (id % 8));
+ g->old_sts[id / 8] |= (1 << (id % 8));
}
static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 5:10 ` Andreas Färber
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
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@cn.fujitsu.com>
---
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;
+ }
+
+ if (kvm_destroy_vcpu(cpu) < 0) {
+ fprintf(stderr, "kvm_destroy_vcpu failed.\n");
+ exit(1);
+ }
+
+ qdev_free(DEVICE(X86_CPU(cpu)));
+}
+
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);
+ 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);
+
+/**
* 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;
+}
+
int kvm_init_vcpu(CPUState *cpu)
{
KVMState *s = kvm_state;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
@ 2013-08-29 5:10 ` Andreas Färber
2013-08-29 10:08 ` chenfan
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Färber @ 2013-08-29 5:10 UTC (permalink / raw)
To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, kvm@vger.kernel.org list
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@cn.fujitsu.com>
> ---
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
2013-08-29 5:10 ` Andreas Färber
@ 2013-08-29 10:08 ` chenfan
0 siblings, 0 replies; 11+ messages in thread
From: chenfan @ 2013-08-29 10:08 UTC (permalink / raw)
To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel, kvm@vger.kernel.org list
On Thu, 2013-08-29 at 07:10 +0200, Andreas Färber wrote:
> 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@cn.fujitsu.com>
> > ---
> > 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.
OK. Thanks.
>
> Also, why is this only in the KVM code path? Isn't this needed for TCG
> as well?
I will add work for TCG support later.
>
> > +
> > + 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.
I think, In kvm.h, there is not one '_IO' number reserved for removing a
vCPU. like KVM_REMOVE_VCPU or other. maybe the original idea for
releasing a vCPU does not need to use a new ioctl to free the allocated
vCPU.
Thanks for your review.
Chen
>
> Regards,
> Andreas
>
> > +
> > int kvm_init_vcpu(CPUState *cpu)
> > {
> > KVMState *s = kvm_state;
> >
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier'
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
Rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier', for
adding vcpu-remove notifier support.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/acpi/piix4.c | 10 +++++-----
hw/i386/pc.c | 2 +-
include/sysemu/sysemu.h | 2 +-
qom/cpu.c | 10 +++++-----
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 44bc809..0a58ff7 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -96,7 +96,7 @@ typedef struct PIIX4PMState {
uint8_t s4_val;
CPUStatus gpe_cpu;
- Notifier cpu_added_notifier;
+ Notifier cpu_hotplug_notifier;
} PIIX4PMState;
#define TYPE_PIIX4_PM "PIIX4_PM"
@@ -700,9 +700,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
pm_update_sci(s);
}
-static void piix4_cpu_added_req(Notifier *n, void *opaque)
+static void piix4_cpu_hotplug(Notifier *n, void *opaque)
{
- PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
+ PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
}
@@ -738,8 +738,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s,
"acpi-cpu-hotplug", PIIX4_PROC_LEN);
memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu);
- s->cpu_added_notifier.notify = piix4_cpu_added_req;
- qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
+ s->cpu_hotplug_notifier.notify = piix4_cpu_hotplug;
+ qemu_register_cpu_hotplug_notifier(&s->cpu_hotplug_notifier);
}
static void enable_device(PIIX4PMState *s, int slot)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8bc8ce..c0e7cbd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -408,7 +408,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
/* init CPU hotplug notifier */
cpu_hotplug_cb.rtc_state = s;
cpu_hotplug_cb.cpu_added_notifier.notify = rtc_notify_cpu_added;
- qemu_register_cpu_added_notifier(&cpu_hotplug_cb.cpu_added_notifier);
+ qemu_register_cpu_hotplug_notifier(&cpu_hotplug_cb.cpu_added_notifier);
if (set_boot_dev(s, boot_device)) {
exit(1);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d7a77b6..a7384c0 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -153,7 +153,7 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
void drive_hot_add(Monitor *mon, const QDict *qdict);
/* CPU hotplug */
-void qemu_register_cpu_added_notifier(Notifier *notifier);
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier);
/* pcie aer error injection */
void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
diff --git a/qom/cpu.c b/qom/cpu.c
index e71e57b..e3e75de 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -79,12 +79,12 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
}
/* CPU hot-plug notifiers */
-static NotifierList cpu_added_notifiers =
- NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
+static NotifierList cpu_hotplug_notifiers =
+ NOTIFIER_LIST_INITIALIZER(cpu_hotplug_notifiers);
-void qemu_register_cpu_added_notifier(Notifier *notifier)
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier)
{
- notifier_list_add(&cpu_added_notifiers, notifier);
+ notifier_list_add(&cpu_hotplug_notifiers, notifier);
}
void cpu_reset_interrupt(CPUState *cpu, int mask)
@@ -230,7 +230,7 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
- notifier_list_notify(&cpu_added_notifiers, dev);
+ notifier_list_notify(&cpu_hotplug_notifiers, dev);
cpu_resume(cpu);
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
` (2 preceding siblings ...)
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 12:28 ` Eric Blake
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/i386/pc.c | 5 +++++
hw/i386/pc_piix.c | 1 +
include/hw/boards.h | 2 ++
include/hw/i386/pc.h | 1 +
qapi-schema.json | 12 ++++++++++++
qmp-commands.hx | 23 +++++++++++++++++++++++
qmp.c | 9 +++++++++
7 files changed, 53 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c0e7cbd..75fc9bb 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -958,6 +958,11 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
}
+void pc_hot_del_cpu(const int64_t id, Error **errp)
+{
+ /* TODO: hot remove VCPU. */
+}
+
void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
{
int i;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6e1e654..d779b75 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -347,6 +347,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
.desc = "Standard PC (i440FX + PIIX, 1996)",
.init = pc_init_pci_1_6,
.hot_add_cpu = pc_hot_add_cpu,
+ .hot_del_cpu = pc_hot_del_cpu,
.max_cpus = 255,
.is_default = 1,
DEFAULT_MACHINE_OPTIONS,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..fea3737 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -23,6 +23,7 @@ typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
typedef void QEMUMachineResetFunc(void);
typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
+typedef void QEMUMachineHotDelCPUFunc(const int64_t id, Error **errp);
typedef struct QEMUMachine {
const char *name;
@@ -31,6 +32,7 @@ typedef struct QEMUMachine {
QEMUMachineInitFunc *init;
QEMUMachineResetFunc *reset;
QEMUMachineHotAddCPUFunc *hot_add_cpu;
+ QEMUMachineHotDelCPUFunc *hot_del_cpu;
BlockInterfaceType block_default_type;
int max_cpus;
unsigned int no_serial:1,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f79d478..b7e66f4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -96,6 +96,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
void pc_hot_add_cpu(const int64_t id, Error **errp);
+void pc_hot_del_cpu(const int64_t id, Error **errp);
void pc_acpi_init(const char *default_dsdt);
PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
diff --git a/qapi-schema.json b/qapi-schema.json
index a51f7d2..6d3bf5f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1432,6 +1432,18 @@
##
{ 'command': 'cpu-add', 'data': {'id': 'int'} }
+# @cpu-del
+
+# Deletes CPU with specified ID
+#
+# @id: ID of CPU to be deleted, valid values [0..max_cpus)
+#
+# Returns: Nothing on success
+#
+# Since 1.6
+##
+{ 'command': 'cpu-del', 'data': {'id': 'int'} }
+
##
# @memsave:
#
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cf47e3f..16b54fd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -411,6 +411,29 @@ Example:
EQMP
{
+ .name = "cpu-del",
+ .args_type = "id:i",
+ .mhandler.cmd_new = qmp_marshal_input_cpu_del,
+ },
+
+SQMP
+cpu-del
+-------
+
+Deletes virtual cpu
+
+Arguments:
+
+- "id": cpu id (json-int)
+
+Example:
+
+-> { "execute": "cpu-del", "arguments": { "id": 2 } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "memsave",
.args_type = "val:l,size:i,filename:s,cpu:i?",
.mhandler.cmd_new = qmp_marshal_input_memsave,
diff --git a/qmp.c b/qmp.c
index 4c149b3..84dc873 100644
--- a/qmp.c
+++ b/qmp.c
@@ -118,6 +118,15 @@ void qmp_cpu_add(int64_t id, Error **errp)
}
}
+void qmp_cpu_del(int64_t id, Error **errp)
+{
+ if (current_machine->hot_del_cpu) {
+ current_machine->hot_del_cpu(id, errp);
+ } else {
+ error_setg(errp, "Not supported");
+ }
+}
+
#ifndef CONFIG_VNC
/* If VNC support is enabled, the "true" query-vnc command is
defined in the VNC subsystem */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
@ 2013-08-29 12:28 ` Eric Blake
0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2013-08-29 12:28 UTC (permalink / raw)
To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]
On 08/28/2013 08:09 PM, Chen Fan wrote:
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
> hw/i386/pc.c | 5 +++++
> hw/i386/pc_piix.c | 1 +
> include/hw/boards.h | 2 ++
> include/hw/i386/pc.h | 1 +
> qapi-schema.json | 12 ++++++++++++
> qmp-commands.hx | 23 +++++++++++++++++++++++
> qmp.c | 9 +++++++++
> 7 files changed, 53 insertions(+)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c0e7cbd..75fc9bb 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -958,6 +958,11 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
> }
>
> +void pc_hot_del_cpu(const int64_t id, Error **errp)
> +{
> + /* TODO: hot remove VCPU. */
> +}
That seems fishy.
> +++ b/qapi-schema.json
> @@ -1432,6 +1432,18 @@
> ##
> { 'command': 'cpu-add', 'data': {'id': 'int'} }
>
> +# @cpu-del
> +
> +# Deletes CPU with specified ID
> +#
> +# @id: ID of CPU to be deleted, valid values [0..max_cpus)
> +#
> +# Returns: Nothing on success
> +#
> +# Since 1.6
1.6 is already out the door. This must be 1.7.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
` (3 preceding siblings ...)
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
Move struct HotplugEventType from file piix4.c to file qom/cpu.c,
and add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/acpi/piix4.c | 8 ++------
include/qom/cpu.h | 10 ++++++++++
qom/cpu.c | 6 +++++-
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0a58ff7..fa82768 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -669,11 +669,6 @@ static const MemoryRegionOps cpu_hotplug_ops = {
},
};
-typedef enum {
- PLUG,
- UNPLUG,
-} HotplugEventType;
-
static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
HotplugEventType action)
{
@@ -703,8 +698,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
static void piix4_cpu_hotplug(Notifier *n, void *opaque)
{
PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
+ CPUNotifier *notifier = opaque;
- piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
+ piix4_cpu_hotplug_req(s, CPU(notifier->dev), notifier->type);
}
static void piix4_init_cpu_status(CPUState *cpu, void *data)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index fa8ec8a..c6f612d 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -518,6 +518,16 @@ void qemu_init_vcpu(CPUState *cpu);
*/
void cpu_single_step(CPUState *cpu, int enabled);
+typedef enum {
+ PLUG,
+ UNPLUG,
+} HotplugEventType;
+
+typedef struct CPUNotifier {
+ DeviceState *dev;
+ HotplugEventType type;
+} CPUNotifier;
+
#ifdef CONFIG_SOFTMMU
extern const struct VMStateDescription vmstate_cpu_common;
#else
diff --git a/qom/cpu.c b/qom/cpu.c
index e3e75de..3439c5d 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -227,10 +227,14 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model)
static void cpu_common_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cpu = CPU(dev);
+ CPUNotifier notifier;
+
+ notifier.dev = dev;
+ notifier.type = PLUG;
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
- notifier_list_notify(&cpu_hotplug_notifiers, dev);
+ notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier);
cpu_resume(cpu);
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn'
2013-08-29 2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
` (4 preceding siblings ...)
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier Chen Fan
@ 2013-08-29 2:09 ` Chen Fan
2013-08-29 5:21 ` Andreas Färber
5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber
Implement cpu interface 'cpu_common_unrealizefn' for emiting vcpu-remove
notifier to ACPI, then ACPI could send sci interrupt to OS for hot-remove
vcpu.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/i386/pc.c | 19 ++++++++++++++++++-
qom/cpu.c | 13 +++++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 75fc9bb..9a87ac0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -960,7 +960,24 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
void pc_hot_del_cpu(const int64_t id, Error **errp)
{
- /* TODO: hot remove VCPU. */
+ CPUState *s = NULL;
+ X86CPU *cpu = NULL;
+ DeviceState *ds = NULL;
+ DeviceClass *dc = NULL;
+
+ s = qemu_get_cpu(id);
+ if (s == NULL) {
+ error_setg(errp, "Unable to find cpu-index: %" PRIi64
+ ",it non-exists or has been deleted.", id);
+ return;
+ }
+
+ cpu = X86_CPU(s);
+ ds = DEVICE(cpu);
+ dc = DEVICE_GET_CLASS(ds);
+ if (dc->unrealize) {
+ dc->unrealize(ds, errp);
+ }
}
void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
diff --git a/qom/cpu.c b/qom/cpu.c
index 3439c5d..d2b0c9e 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -239,6 +239,18 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
}
}
+static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
+{
+ CPUNotifier notifier;
+
+ notifier.dev = dev;
+ notifier.type = UNPLUG;
+
+ if (dev->hotplugged) {
+ notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier);
+ }
+}
+
static void cpu_common_initfn(Object *obj)
{
CPUState *cpu = CPU(obj);
@@ -269,6 +281,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
k->gdb_read_register = cpu_common_gdb_read_register;
k->gdb_write_register = cpu_common_gdb_write_register;
dc->realize = cpu_common_realizefn;
+ dc->unrealize = cpu_common_unrealizefn;
dc->no_user = 1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn'
2013-08-29 2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
@ 2013-08-29 5:21 ` Andreas Färber
0 siblings, 0 replies; 11+ messages in thread
From: Andreas Färber @ 2013-08-29 5:21 UTC (permalink / raw)
To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, Paolo Bonzini
Am 29.08.2013 04:09, schrieb Chen Fan:
> Implement cpu interface 'cpu_common_unrealizefn' for emiting vcpu-remove
> notifier to ACPI, then ACPI could send sci interrupt to OS for hot-remove
> vcpu.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
> hw/i386/pc.c | 19 ++++++++++++++++++-
> qom/cpu.c | 13 +++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 75fc9bb..9a87ac0 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -960,7 +960,24 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>
> void pc_hot_del_cpu(const int64_t id, Error **errp)
> {
> - /* TODO: hot remove VCPU. */
> + CPUState *s = NULL;
"cs" since this is in PC code, to free the "s" identifier.
> + X86CPU *cpu = NULL;
> + DeviceState *ds = NULL;
"dev" please for consistency.
> + DeviceClass *dc = NULL;
> +
> + s = qemu_get_cpu(id);
Same question as on 2/6, whether id == cpu_index here.
> + if (s == NULL) {
> + error_setg(errp, "Unable to find cpu-index: %" PRIi64
> + ",it non-exists or has been deleted.", id);
Space after comma please in English language and "doesn't exist".
> + return;
> + }
> +
> + cpu = X86_CPU(s);
> + ds = DEVICE(cpu);
> + dc = DEVICE_GET_CLASS(ds);
> + if (dc->unrealize) {
> + dc->unrealize(ds, errp);
> + }
Never call this directly, this is missing important device cleanups such
as VMState. What you want is:
object_property_set_bool(OBJECT(cs), false, "realized", errp);
> }
>
> void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 3439c5d..d2b0c9e 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -239,6 +239,18 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> }
> }
>
> +static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
> +{
> + CPUNotifier notifier;
> +
> + notifier.dev = dev;
> + notifier.type = UNPLUG;
> +
> + if (dev->hotplugged) {
> + notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier);
> + }
Apart from the misindentation this looks wrong. If we initialize a
machine with -smp 2 we should be allowed to hot-unplug CPU 1 IMO. Are
there any reasons not to allow that?
I would simply drop the dev->hotplugged check and always emit the
notification. When QEMU is shut down, the unrealizefn and finalizers are
not called, I believe.
Regards,
Andreas
> +}
> +
> static void cpu_common_initfn(Object *obj)
> {
> CPUState *cpu = CPU(obj);
> @@ -269,6 +281,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> k->gdb_read_register = cpu_common_gdb_read_register;
> k->gdb_write_register = cpu_common_gdb_write_register;
> dc->realize = cpu_common_realizefn;
> + dc->unrealize = cpu_common_unrealizefn;
> dc->no_user = 1;
> }
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 11+ messages in thread