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

2011-06-19 Thread Avi Kivity 

On 06/09/2011 06:57 PM, Paolo Bonzini wrote:


-- 8 -
From  Mon Sep 17 00:00:00 2001
From: Paolo Bonzini pbonz...@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 pbonz...@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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

2011-06-07 Thread 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 ag...@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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-06-07 Thread 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
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] KVM: Add compat ioctl for KVM_SET_SIGNAL_MASK

2011-06-07 Thread 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 ag...@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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html