[PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Alexander Graf]

KVM has an ioctl to define which signal mask should be used while running
inside VCPU_RUN. At least for big endian systems, this mask is different
on 32-bit and 64-bit systems (though the size is identical).

Add a compat wrapper that converts the mask to whatever the kernel accepts,
allowing 32-bit kvm user space to set signal masks.

This patch fixes qemu with --enable-io-thread on ppc64 hosts when running
32-bit user land.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 kernel/compat.c     |    1 +
 virt/kvm/kvm_main.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/kernel/compat.c b/kernel/compat.c
index 9214dcd..506e176 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -882,6 +882,7 @@ sigset_from_compat (sigset_t *set, compat_sigset_t *compat)
 	case 1: set->sig[0] = compat->sig[0] | (((long)compat->sig[1]) << 32 );
 	}
 }
+EXPORT_SYMBOL_GPL(sigset_from_compat);
 
 asmlinkage long
 compat_sys_rt_sigtimedwait (compat_sigset_t __user *uthese,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f78ddb8..f03db82 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -84,6 +84,8 @@ struct dentry *kvm_debugfs_dir;
 
 static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
 			   unsigned long arg);
+static long kvm_vcpu_compat_ioctl(struct file *file, unsigned int ioctl,
+				  unsigned long arg);
 static int hardware_enable_all(void);
 static void hardware_disable_all(void);
 
@@ -1585,7 +1587,9 @@ static int kvm_vcpu_release(struct inode *inode, struct file *filp)
 static struct file_operations kvm_vcpu_fops = {
 	.release        = kvm_vcpu_release,
 	.unlocked_ioctl = kvm_vcpu_ioctl,
-	.compat_ioctl   = kvm_vcpu_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl   = kvm_vcpu_compat_ioctl,
+#endif
 	.mmap           = kvm_vcpu_mmap,
 	.llseek		= noop_llseek,
 };
@@ -1874,6 +1878,50 @@ out:
 	return r;
 }
 
+#ifdef CONFIG_COMPAT
+static long kvm_vcpu_compat_ioctl(struct file *filp,
+				  unsigned int ioctl, unsigned long arg)
+{
+	struct kvm_vcpu *vcpu = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	int r;
+
+	if (vcpu->kvm->mm != current->mm)
+		return -EIO;
+
+	switch (ioctl) {
+	case KVM_SET_SIGNAL_MASK: {
+		struct kvm_signal_mask __user *sigmask_arg = argp;
+		struct kvm_signal_mask kvm_sigmask;
+		compat_sigset_t csigset;
+		sigset_t sigset;
+
+		if (argp) {
+			r = -EFAULT;
+			if (copy_from_user(&kvm_sigmask, argp,
+					   sizeof kvm_sigmask))
+				goto out;
+			r = -EINVAL;
+			if (kvm_sigmask.len != sizeof csigset)
+				goto out;
+			r = -EFAULT;
+			if (copy_from_user(&csigset, sigmask_arg->sigset,
+					   sizeof csigset))
+				goto out;
+		}
+		sigset_from_compat(&sigset, &csigset);
+		r = kvm_vcpu_ioctl_set_sigmask(vcpu, &sigset);
+		break;
+	}
+	default:
+		r = kvm_vcpu_ioctl(filp, ioctl, arg);
+	}
+
+out:
+	return r;
+}
+#endif
+
 static long kvm_vm_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
-- 
1.6.0.2

Re: [PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Arnd Bergmann]

On Tuesday 07 June 2011 22:25:15 Alexander Graf wrote:
> +static long kvm_vcpu_compat_ioctl(struct file *filp,
> +                                 unsigned int ioctl, unsigned long arg)
> +{
> +       struct kvm_vcpu *vcpu = filp->private_data;
> +       void __user *argp = (void __user *)arg;

Converting a compat user argument into a pointer should use the
compat_ptr() function to do the right thing on s390. Otherwise
your patch looks good.

	Arnd

[PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Alexander Graf]

KVM has an ioctl to define which signal mask should be used while running
inside VCPU_RUN. At least for big endian systems, this mask is different
on 32-bit and 64-bit systems (though the size is identical).

Add a compat wrapper that converts the mask to whatever the kernel accepts,
allowing 32-bit kvm user space to set signal masks.

This patch fixes qemu with --enable-io-thread on ppc64 hosts when running
32-bit user land.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - use compat_ptr
  - only declare compat call with CONFIG_COMPAT
---
 kernel/compat.c     |    1 +
 virt/kvm/kvm_main.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/kernel/compat.c b/kernel/compat.c
index 9214dcd..506e176 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -882,6 +882,7 @@ sigset_from_compat (sigset_t *set, compat_sigset_t *compat)
 	case 1: set->sig[0] = compat->sig[0] | (((long)compat->sig[1]) << 32 );
 	}
 }
+EXPORT_SYMBOL_GPL(sigset_from_compat);
 
 asmlinkage long
 compat_sys_rt_sigtimedwait (compat_sigset_t __user *uthese,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f78ddb8..04dfce9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -84,6 +84,10 @@ struct dentry *kvm_debugfs_dir;
 
 static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
 			   unsigned long arg);
+#ifdef CONFIG_COMPAT
+static long kvm_vcpu_compat_ioctl(struct file *file, unsigned int ioctl,
+				  unsigned long arg);
+#endif
 static int hardware_enable_all(void);
 static void hardware_disable_all(void);
 
@@ -1585,7 +1589,9 @@ static int kvm_vcpu_release(struct inode *inode, struct file *filp)
 static struct file_operations kvm_vcpu_fops = {
 	.release        = kvm_vcpu_release,
 	.unlocked_ioctl = kvm_vcpu_ioctl,
-	.compat_ioctl   = kvm_vcpu_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl   = kvm_vcpu_compat_ioctl,
+#endif
 	.mmap           = kvm_vcpu_mmap,
 	.llseek		= noop_llseek,
 };
@@ -1874,6 +1880,50 @@ out:
 	return r;
 }
 
+#ifdef CONFIG_COMPAT
+static long kvm_vcpu_compat_ioctl(struct file *filp,
+				  unsigned int ioctl, unsigned long arg)
+{
+	struct kvm_vcpu *vcpu = filp->private_data;
+	void __user *argp = compat_ptr(arg);
+	int r;
+
+	if (vcpu->kvm->mm != current->mm)
+		return -EIO;
+
+	switch (ioctl) {
+	case KVM_SET_SIGNAL_MASK: {
+		struct kvm_signal_mask __user *sigmask_arg = argp;
+		struct kvm_signal_mask kvm_sigmask;
+		compat_sigset_t csigset;
+		sigset_t sigset;
+
+		if (argp) {
+			r = -EFAULT;
+			if (copy_from_user(&kvm_sigmask, argp,
+					   sizeof kvm_sigmask))
+				goto out;
+			r = -EINVAL;
+			if (kvm_sigmask.len != sizeof csigset)
+				goto out;
+			r = -EFAULT;
+			if (copy_from_user(&csigset, sigmask_arg->sigset,
+					   sizeof csigset))
+				goto out;
+		}
+		sigset_from_compat(&sigset, &csigset);
+		r = kvm_vcpu_ioctl_set_sigmask(vcpu, &sigset);
+		break;
+	}
+	default:
+		r = kvm_vcpu_ioctl(filp, ioctl, arg);
+	}
+
+out:
+	return r;
+}
+#endif
+
 static long kvm_vm_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
-- 
1.6.0.2

Re: [PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Avi Kivity]

On 06/08/2011 03:45 AM, Alexander Graf wrote:
> KVM has an ioctl to define which signal mask should be used while running
> inside VCPU_RUN. At least for big endian systems, this mask is different
> on 32-bit and 64-bit systems (though the size is identical).
>
> Add a compat wrapper that converts the mask to whatever the kernel accepts,
> allowing 32-bit kvm user space to set signal masks.
>
> This patch fixes qemu with --enable-io-thread on ppc64 hosts when running
> 32-bit user land.

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Paolo Bonzini]

On 06/09/2011 02:24 PM, Avi Kivity wrote:
> On 06/08/2011 03:45 AM, Alexander Graf wrote:
>> KVM has an ioctl to define which signal mask should be used while running
>> inside VCPU_RUN. At least for big endian systems, this mask is different
>> on 32-bit and 64-bit systems (though the size is identical).
>>
>> Add a compat wrapper that converts the mask to whatever the kernel
>> accepts,
>> allowing 32-bit kvm user space to set signal masks.
>>
>> This patch fixes qemu with --enable-io-thread on ppc64 hosts when running
>> 32-bit user land.
>
> Applied, thanks.

And here's the matching documentation change.  Kunz-kunz, Alex. :)

-------------------------- 8< -----------------------------
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu, 9 Jun 2011 17:48:50 +0200
Subject: [PATCH] KVM: fix documentation for KVM_SET_SIGNAL_MASK

The signal mask passed to KVM_SET_SIGNAL_MASK needs to be ABI-compatible
with sigprocmask, so that functions operating on sigset_t can be used
with it.  Referring to the signal mask as an array of bytes is incorrect
on big endian systems, fix it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
  Documentation/kvm/api.txt |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 9bef4e4..ec15972 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -464,8 +464,8 @@ signal mask.

  /* for KVM_SET_SIGNAL_MASK */
  struct kvm_signal_mask {
-	__u32 len;
-	__u8  sigset[0];
+	__u32          len;
+	unsigned long  sigset[0];
  };

  4.22 KVM_GET_FPU
-- 
1.7.4.4

Re: [PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

[Avi Kivity]

On 06/09/2011 06:57 PM, Paolo Bonzini wrote:
>
> -------------------------- 8< -----------------------------
> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> From: Paolo Bonzini <pbonzini@redhat.com>
> Date: Thu, 9 Jun 2011 17:48:50 +0200
> Subject: [PATCH] KVM: fix documentation for KVM_SET_SIGNAL_MASK
>
> The signal mask passed to KVM_SET_SIGNAL_MASK needs to be ABI-compatible
> with sigprocmask, so that functions operating on sigset_t can be used
> with it.  Referring to the signal mask as an array of bytes is incorrect
> on big endian systems, fix it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  Documentation/kvm/api.txt |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> index 9bef4e4..ec15972 100644
> --- a/Documentation/kvm/api.txt
> +++ b/Documentation/kvm/api.txt
> @@ -464,8 +464,8 @@ signal mask.
>
>  /* for KVM_SET_SIGNAL_MASK */
>  struct kvm_signal_mask {
> -    __u32 len;
> -    __u8  sigset[0];
> +    __u32          len;
> +    unsigned long  sigset[0];
>  };

But that doesn't match the definition in kvm.h.  If someone sets 
kvm_signal_mask::sigset[3], they'll get different answers if they look 
at the documentation or the code.

So it needs to be documented in English, not C.

(and not whitespace-damaged, either)

-- 
error compiling committee.c: too many arguments to function