Re: [SeaBIOS] [PATCH 3/3] acpi_piix4: Call KVM_SETSTATE_VCPU ioctl on cpu ejection

[Jan Kiszka]

On 2012-01-13 12:11, Vasilis Liaskovitis wrote:
> Signed-off-by: Vasilis Liaskovitis
>
> hw/acpi_piix4.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 8bf30dd..12eef55 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -502,6 +502,27 @@ static uint32_t cpuej_read(void *opaque, uint32_t addr)
>
> static void cpuej_write(void *opaque, uint32_t addr, uint32_t val)
>  {
> struct kvm_vcpu_state state;
> CPUState *env;
> int cpu;
> int ret;
> cpu = ffs(val);
> /* zero means no bit was set, i.e. no CPU ejection happened */
> if (!cpu)
>   return;
> cpu--;
> env = cpu_phyid_to_cpu((uint64_t)cpu);
> if (env != NULL) {
>    if (env->state == CPU_STATE_ZAPREQ) {
>        state.vcpu_id = env->cpu_index;
>        state.state = 1;
>        ret = kvm_vm_ioctl(env->kvm_state, KVM_SETSTATE_VCPU, &state);

That breaks in the absence of KVM or if it is not enabled.

Also, where was this IOCTL introduced? Where are the linux header changes?
>        if (ret)
>            fprintf(stderr, "KVM_SETSTATE_VCPU failed: %s\n",
>                    strerror(ret));
>    }
> }
> PIIX4_DPRINTF("cpuej write %x <== %d\n", addr, val);
>
> }

Jan

[PATCH 3/3] acpi_piix4: Call KVM_SETSTATE_VCPU ioctl on cpu ejection

[Vasilis Liaskovitis]

On Fri, Jan 13, 2012 at 12:58:53PM +0100, Jan Kiszka wrote:
> On 2012-01-13 12:11, Vasilis Liaskovitis wrote:
> > Signed-off-by: Vasilis Liaskovitis
> >
> > hw/acpi_piix4.c |   21 +++++++++++++++++++++
> >  1 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > index 8bf30dd..12eef55 100644
> > --- a/hw/acpi_piix4.c
> > +++ b/hw/acpi_piix4.c
> > @@ -502,6 +502,27 @@ static uint32_t cpuej_read(void *opaque, uint32_t addr)
> >
> > static void cpuej_write(void *opaque, uint32_t addr, uint32_t val)
> >  {
> > struct kvm_vcpu_state state;
> > CPUState *env;
> > int cpu;
> > int ret;
> > cpu = ffs(val);
> > /* zero means no bit was set, i.e. no CPU ejection happened */
> > if (!cpu)
> >   return;
> > cpu--;
> > env = cpu_phyid_to_cpu((uint64_t)cpu);
> > if (env != NULL) {
> >    if (env->state == CPU_STATE_ZAPREQ) {
> >        state.vcpu_id = env->cpu_index;
> >        state.state = 1;
> >        ret = kvm_vm_ioctl(env->kvm_state, KVM_SETSTATE_VCPU, &state);
>
> That breaks in the absence of KVM or if it is not enabled.

Right, I will rework.

Do we expect icc-bus related changes on a CPU unplug? This patch does not
handle this yet.
> Also, where was this IOCTL introduced? Where are the linux header changes?

The headers are here:
http://patchwork.ozlabs.org/patch/127834/

And the ioctl is introduced here:
http://patchwork.ozlabs.org/patch/127828/

Though the actual ioctl code seems to have dropped through the cracks in the
above patch. A sample implementation against 3.1.0 is below, but I have not
included it in the patch series. I expect the ioctl implementation to be part
of Liu 's kernel kvm-related series. In any case, this third patch depends on
the cpu zap/lifecycle patchseries and perhaps should be reviewed separately
from the first 2.

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6d3a724..8dd9ebd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2095,6 +2095,22 @@ static long kvm_vm_ioctl(struct file *filp,
    	r = kvm_ioeventfd(kvm, &data);
    	break;
    }
+	case KVM_SETSTATE_VCPU: {
+		struct kvm_vcpu_state vcpu_state;
+		struct kvm_vcpu *vcpu;
+		int idx;
+		r = -EFAULT;
+		if (copy_from_user(&vcpu_state, argp,
+					sizeof(struct kvm_vcpu_state)))
+			goto out;
+		idx = srcu_read_lock(&kvm->srcu);
+		kvm_for_each_vcpu(vcpu, kvm)
+		if (vcpu_state.vcpu_id == vcpu->vcpu_id)
+			vcpu->state = vcpu_state.state;
+		srcu_read_unlock(&kvm->srcu, idx);
+		r = 0;
+		break;
+	}
 #ifdef CONFIG_KVM_APIC_ARCHITECTURE
    case KVM_SET_BOOT_CPU_ID:
    	r = 0;