[PATCH 0/1] Add virtio-input driver.
Hi, This patch adds a virtio driver for input devices. Specification: https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007 Qemu patches; https://lists.gnu.org/archive/html/qemu-devel/2015-03/threads.html#03973 Gerd Hoffmann (1): Add virtio-input driver. drivers/virtio/Kconfig| 10 ++ drivers/virtio/Makefile | 1 + drivers/virtio/virtio_input.c | 313 ++ include/uapi/linux/virtio_ids.h | 1 + include/uapi/linux/virtio_input.h | 65 5 files changed, 390 insertions(+) create mode 100644 drivers/virtio/virtio_input.c create mode 100644 include/uapi/linux/virtio_input.h -- 1.8.3.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/1] Add virtio-input driver.
virtio-input is basically evdev-events-over-virtio, so this driver isn't
much more than reading configuration from config space and forwarding
incoming events to the linux input layer.
Signed-off-by: Gerd Hoffmann
---
drivers/virtio/Kconfig| 10 ++
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio_input.c | 313 ++
include/uapi/linux/virtio_ids.h | 1 +
include/uapi/linux/virtio_input.h | 65
5 files changed, 390 insertions(+)
create mode 100644 drivers/virtio/virtio_input.c
create mode 100644 include/uapi/linux/virtio_input.h
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index b546da5..cab9f3f 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -48,6 +48,16 @@ config VIRTIO_BALLOON
If unsure, say M.
+config VIRTIO_INPUT
+ tristate "Virtio input driver"
+ depends on VIRTIO
+ depends on INPUT
+ ---help---
+This driver supports virtio input devices such as
+keyboards, mice and tablets.
+
+If unsure, say M.
+
config VIRTIO_MMIO
tristate "Platform bus driver for memory mapped virtio devices"
depends on HAS_IOMEM
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index d85565b..41e30e3 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
+obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
new file mode 100644
index 000..2d425cf
--- /dev/null
+++ b/drivers/virtio/virtio_input.c
@@ -0,0 +1,313 @@
+#include
+#include
+#include
+
+#include
+#include
+
+struct virtio_input {
+ struct virtio_device *vdev;
+ struct input_dev *idev;
+ char name[64];
+ char serial[64];
+ char phys[64];
+ struct virtqueue *evt, *sts;
+ struct virtio_input_event evts[64];
+};
+
+static ssize_t serial_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct input_dev *idev = to_input_dev(dev);
+ struct virtio_input *vi = input_get_drvdata(idev);
+ return sprintf(buf, "%s\n", vi->serial);
+}
+static DEVICE_ATTR_RO(serial);
+
+static struct attribute *dev_attrs[] = {
+ &dev_attr_serial.attr,
+ NULL
+};
+
+static umode_t dev_attrs_are_visible(struct kobject *kobj,
+struct attribute *a, int n)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct input_dev *idev = to_input_dev(dev);
+ struct virtio_input *vi = input_get_drvdata(idev);
+
+ if (a == &dev_attr_serial.attr && !strlen(vi->serial))
+ return 0;
+
+ return a->mode;
+}
+
+static struct attribute_group dev_attr_grp = {
+ .attrs =dev_attrs,
+ .is_visible = dev_attrs_are_visible,
+};
+
+static const struct attribute_group *dev_attr_groups[] = {
+ &dev_attr_grp,
+ NULL
+};
+
+static void virtinput_queue_evtbuf(struct virtio_input *vi,
+ struct virtio_input_event *evtbuf)
+{
+ struct scatterlist sg[1];
+
+ sg_init_one(sg, evtbuf, sizeof(*evtbuf));
+ virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
+}
+
+static void virtinput_recv_events(struct virtqueue *vq)
+{
+ struct virtio_input *vi = vq->vdev->priv;
+ struct virtio_input_event *event;
+ unsigned int len;
+
+ while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
+ input_event(vi->idev,
+ le16_to_cpu(event->type),
+ le16_to_cpu(event->code),
+ le32_to_cpu(event->value));
+ virtinput_queue_evtbuf(vi, event);
+ }
+ virtqueue_kick(vq);
+}
+
+static int virtinput_send_status(struct virtio_input *vi,
+u16 type, u16 code, s32 value)
+{
+ struct virtio_input_event *stsbuf;
+ struct scatterlist sg[1];
+
+ stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
+ if (!stsbuf)
+ return -ENOMEM;
+
+ stsbuf->type = cpu_to_le16(type);
+ stsbuf->code = cpu_to_le16(code);
+ stsbuf->value = cpu_to_le32(value);
+ sg_init_one(sg, stsbuf, sizeof(*stsbuf));
+ virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
+ virtqueue_kick(vi->sts);
+ return 0;
+}
+
+static void virtinput_recv_status(struct virtqueue *vq)
+{
+ struct virtio_input *vi = vq->vdev->priv;
+ struct virtio_input_event *stsbuf;
+ unsigned int len;
+
+ while ((stsbuf = virtqueue_get_buf(vi->sts,
Re: [PATCH 9/9] qspinlock,x86,kvm: Implement KVM support for paravirt qspinlock
On Wed, Mar 18, 2015 at 10:45:55PM -0400, Waiman Long wrote: > On 03/16/2015 09:16 AM, Peter Zijlstra wrote: > I do have some concern about this call site patching mechanism as the > modification is not atomic. The spin_unlock() calls are in many places in > the kernel. There is a possibility that a thread is calling a certain > spin_unlock call site while it is being patched by another one with the > alternative() function call. > > So far, I don't see any problem with bare metal where paravirt_patch_insns() > is used to patch it to the move instruction. However, in a virtual guest > enivornment where paravirt_patch_call() was used, there were situations > where the system panic because of page fault on some invalid memory in the > kthread. If you look at the paravirt_patch_call(), you will see: > > : > b->opcode = 0xe8; /* call */ > b->delta = delta; > > If another CPU reads the instruction at the call site at the right moment, > it will get the modified call instruction, but not the new delta value. It > will then jump to a random location. I believe that was causing the system > panic that I saw. > > So I think it is kind of risky to use it here unless we can guarantee that > call site patching is atomic wrt other CPUs. Just look at where the patching is done: init/main.c:start_kernel() check_bugs() alternative_instructions() apply_paravirt() We're UP and not holding any locks, disable IRQs (see text_poke_early()) and have NMIs 'disabled'. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 8/9] qspinlock: Generic paravirt support
On Wed, Mar 18, 2015 at 04:50:37PM -0400, Waiman Long wrote:
> >+this_cpu_write(__pv_lock_wait, lock);
>
> We may run into the same problem of needing to have 4 queue nodes per CPU.
> If an interrupt happens just after the write and before the actual wait and
> it goes through the same sequence, it will overwrite the __pv_lock_wait[]
> entry. So we may have lost wakeup. That is why the pvticket lock code did
> that just before the actual wait with interrupt disabled. We probably
> couldn't disable interrupt here. So we may need to move the actual write to
> the KVM and Xen code if we keep the current logic.
Hmm indeed. So I didn't actually mean to keep this code, but yes I
missed that.
> >+/*
> >+ * At this point the memory pointed at by lock can be freed/reused,
> >+ * however we can still use the pointer value to search in our cpu
> >+ * array.
> >+ *
> >+ * XXX: get rid of this loop
> >+ */
> >+for_each_possible_cpu(cpu) {
> >+if (per_cpu(__pv_lock_wait, cpu) == lock)
> >+pv_kick(cpu);
> >+}
> >+}
>
> I do want to get rid of this loop too. On average, we have to scan about
> half the number of CPUs available. So it isn't that different
> performance-wise compared with my original idea of following the list from
> tail to head. And how about your idea of propagating the current head down
> the linked list?
Yeah, so I was half done with that patch when I fell asleep on the
flight home. Didn't get around to 'fixing' it. It applies and builds but
doesn't actually work.
_However_ I'm not sure I actually like it much.
The problem I have with it are these wait loops, they can generate the
same problem we're trying to avoid.
So I was now thinking of hashing the lock pointer; let me go and quickly
put something together.
---
kernel/locking/qspinlock.c |8 +-
kernel/locking/qspinlock_paravirt.h | 124
2 files changed, 101 insertions(+), 31 deletions(-)
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -246,10 +246,10 @@ static __always_inline void set_locked(s
*/
static __always_inline void __pv_init_node(struct mcs_spinlock *node) { }
-static __always_inline void __pv_wait_node(struct mcs_spinlock *node) { }
+static __always_inline void __pv_wait_node(u32 old, struct mcs_spinlock *node)
{ }
static __always_inline void __pv_kick_node(struct mcs_spinlock *node) { }
-static __always_inline void __pv_wait_head(struct qspinlock *lock) { }
+static __always_inline void __pv_wait_head(struct qspinlock *lock, struct
mcs_spinlock *node) { }
#define pv_enabled() false
@@ -399,7 +399,7 @@ void queue_spin_lock_slowpath(struct qsp
prev = decode_tail(old);
WRITE_ONCE(prev->next, node);
- pv_wait_node(node);
+ pv_wait_node(old, node);
arch_mcs_spin_lock_contended(&node->locked);
}
@@ -414,7 +414,7 @@ void queue_spin_lock_slowpath(struct qsp
* sequentiality; this is because the set_locked() function below
* does not imply a full barrier.
*/
- pv_wait_head(lock);
+ pv_wait_head(lock, node);
while ((val = smp_load_acquire(&lock->val.counter)) &
_Q_LOCKED_PENDING_MASK)
cpu_relax();
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -29,8 +29,14 @@ struct pv_node {
int cpu;
u8 state;
+ struct pv_node *head;
};
+static inline struct pv_node *pv_decode_tail(u32 tail)
+{
+ return (struct pv_node *)decode_tail(tail);
+}
+
/*
* Initialize the PV part of the mcs_spinlock node.
*/
@@ -42,17 +48,49 @@ static void pv_init_node(struct mcs_spin
pn->cpu = smp_processor_id();
pn->state = vcpu_running;
+ pn->head = NULL;
+}
+
+static void pv_propagate_head(struct pv_node *pn, struct pv_node *ppn)
+{
+ /*
+* When we race against the first waiter or ourselves we have to wait
+* until the previous link updates its head pointer before we can
+* propagate it.
+*/
+ while (!READ_ONCE(ppn->head)) {
+ /*
+* queue_spin_lock_slowpath could have been waiting for the
+* node->next store above before setting node->locked.
+*/
+ if (ppn->mcs.locked)
+ return;
+
+ cpu_relax();
+ }
+ /*
+* If we interleave such that the store from pv_set_head() happens
+* between our load and store we could have over-written the new head
+* value.
+*
+* Therefore use cmpxchg to only propagate the old head value if our
+* head value is unmodified.
+*/
+ (void)cmpxchg(&pn->head, NULL, READ_ONCE(ppn->head));
}
/*
* Wait for node->locked to become true, halt the vcpu after
Re: [PATCH 8/9] qspinlock: Generic paravirt support
On Thu, Mar 19, 2015 at 11:12:42AM +0100, Peter Zijlstra wrote:
> So I was now thinking of hashing the lock pointer; let me go and quickly
> put something together.
A little something like so; ideally we'd allocate the hashtable since
NR_CPUS is kinda bloated, but it shows the idea I think.
And while this has loops in (the rehashing thing) their fwd progress
does not depend on other CPUs.
And I suspect that for the typical lock contention scenarios its
unlikely we ever really get into long rehashing chains.
---
include/linux/lfsr.h| 49
kernel/locking/qspinlock_paravirt.h | 143
2 files changed, 178 insertions(+), 14 deletions(-)
--- /dev/null
+++ b/include/linux/lfsr.h
@@ -0,0 +1,49 @@
+#ifndef _LINUX_LFSR_H
+#define _LINUX_LFSR_H
+
+/*
+ * Simple Binary Galois Linear Feedback Shift Register
+ *
+ * http://en.wikipedia.org/wiki/Linear_feedback_shift_register
+ *
+ */
+
+extern void __lfsr_needs_more_taps(void);
+
+static __always_inline u32 lfsr_taps(int bits)
+{
+ if (bits == 1) return 0x0001;
+ if (bits == 2) return 0x0001;
+ if (bits == 3) return 0x0003;
+ if (bits == 4) return 0x0009;
+ if (bits == 5) return 0x0012;
+ if (bits == 6) return 0x0021;
+ if (bits == 7) return 0x0041;
+ if (bits == 8) return 0x008E;
+ if (bits == 9) return 0x0108;
+ if (bits == 10) return 0x0204;
+ if (bits == 11) return 0x0402;
+ if (bits == 12) return 0x0829;
+ if (bits == 13) return 0x100D;
+ if (bits == 14) return 0x2015;
+
+ /*
+* For more taps see:
+* http://users.ece.cmu.edu/~koopman/lfsr/index.html
+*/
+ __lfsr_needs_more_taps();
+
+ return 0;
+}
+
+static inline u32 lfsr(u32 val, int bits)
+{
+ u32 bit = val & 1;
+
+ val >>= 1;
+ if (bit)
+ val ^= lfsr_taps(bits);
+ return val;
+}
+
+#endif /* _LINUX_LFSR_H */
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -2,6 +2,9 @@
#error "do not include this file"
#endif
+#include
+#include
+
/*
* Implement paravirt qspinlocks; the general idea is to halt the vcpus instead
* of spinning them.
@@ -107,7 +110,120 @@ static void pv_kick_node(struct mcs_spin
pv_kick(pn->cpu);
}
-static DEFINE_PER_CPU(struct qspinlock *, __pv_lock_wait);
+/*
+ * Hash table using open addressing with an LFSR probe sequence.
+ *
+ * Since we should not be holding locks from NMI context (very rare indeed) the
+ * max load factor is 0.75, which is around the point where open addressing
+ * breaks down.
+ *
+ * Instead of probing just the immediate bucket we probe all buckets in the
+ * same cacheline.
+ *
+ * http://en.wikipedia.org/wiki/Hash_table#Open_addressing
+ *
+ */
+
+#define HB_RESERVED((struct qspinlock *)1)
+
+struct pv_hash_bucket {
+ struct qspinlock *lock;
+ int cpu;
+};
+
+/*
+ * XXX dynamic allocate using nr_cpu_ids instead...
+ */
+#define PV_LOCK_HASH_BITS (2 + NR_CPUS_BITS)
+
+#if PV_LOCK_HASH_BITS < 6
+#undef PV_LOCK_HASH_BITS
+#define PB_LOCK_HASH_BITS 6
+#endif
+
+#define PV_LOCK_HASH_SIZE (1 << PV_LOCK_HASH_BITS)
+
+static struct pv_hash_bucket __pv_lock_hash[PV_LOCK_HASH_SIZE]
cacheline_aligned;
+
+#define PV_HB_PER_LINE (SMP_CACHE_BYTES / sizeof(struct
pv_hash_bucket))
+
+static inline u32 hash_align(u32 hash)
+{
+ return hash & ~(PV_HB_PER_LINE - 1);
+}
+
+static struct qspinlock **pv_hash(struct qspinlock *lock)
+{
+ u32 hash = hash_ptr(lock, PV_LOCK_HASH_BITS);
+ struct pv_hash_bucket *hb, *end;
+
+ if (!hash)
+ hash = 1;
+
+ hb = &__pv_lock_hash[hash_align(hash)];
+ for (;;) {
+ for (end = hb + PV_HB_PER_LINE; hb < end; hb++) {
+ if (cmpxchg(&hb->lock, NULL, HB_RESERVED)) {
+ WRITE_ONCE(hb->cpu, smp_processor_id());
+ /*
+* Since we must read lock first and cpu
+* second, we must write cpu first and lock
+* second, therefore use HB_RESERVE to mark an
+* entry in use before writing the values.
+*
+* This can cause hb_hash_find() to not find a
+* cpu even though _Q_SLOW_VAL, this is not a
+* problem since we re-check l->locked before
+* going to sleep and the unlock will have
+* cleared l->locked already.
+*/
+ smp_wmb(); /* matches rmb from pv_hash_find */
+ WRITE_ONCE(hb->lock, lock);
+ goto done;
+ }
+
Re: [PATCH 1/1] Add virtio-input driver.
On Thu, Mar 19, 2015 at 10:13:11AM +0100, Gerd Hoffmann wrote:
> virtio-input is basically evdev-events-over-virtio, so this driver isn't
> much more than reading configuration from config space and forwarding
> incoming events to the linux input layer.
>
> Signed-off-by: Gerd Hoffmann
What worries me is how well are these events specified.
Will we be able to write drivers for non-linux guests?
Not sure where to discuss this - this seems like an inappropriate
thread.
Comments on linux driver below.
> ---
> drivers/virtio/Kconfig| 10 ++
> drivers/virtio/Makefile | 1 +
> drivers/virtio/virtio_input.c | 313
> ++
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_input.h | 65
> 5 files changed, 390 insertions(+)
> create mode 100644 drivers/virtio/virtio_input.c
> create mode 100644 include/uapi/linux/virtio_input.h
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index b546da5..cab9f3f 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -48,6 +48,16 @@ config VIRTIO_BALLOON
>
>If unsure, say M.
>
> +config VIRTIO_INPUT
> + tristate "Virtio input driver"
> + depends on VIRTIO
> + depends on INPUT
> + ---help---
> + This driver supports virtio input devices such as
> + keyboards, mice and tablets.
> +
> + If unsure, say M.
> +
> config VIRTIO_MMIO
> tristate "Platform bus driver for memory mapped virtio devices"
> depends on HAS_IOMEM
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index d85565b..41e30e3 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> +obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> new file mode 100644
> index 000..2d425cf
> --- /dev/null
> +++ b/drivers/virtio/virtio_input.c
> @@ -0,0 +1,313 @@
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +struct virtio_input {
> + struct virtio_device *vdev;
> + struct input_dev *idev;
> + char name[64];
> + char serial[64];
> + char phys[64];
> + struct virtqueue *evt, *sts;
> + struct virtio_input_event evts[64];
> +};
> +
> +static ssize_t serial_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct input_dev *idev = to_input_dev(dev);
> + struct virtio_input *vi = input_get_drvdata(idev);
> + return sprintf(buf, "%s\n", vi->serial);
> +}
> +static DEVICE_ATTR_RO(serial);
> +
> +static struct attribute *dev_attrs[] = {
> + &dev_attr_serial.attr,
> + NULL
> +};
> +
> +static umode_t dev_attrs_are_visible(struct kobject *kobj,
> + struct attribute *a, int n)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct input_dev *idev = to_input_dev(dev);
> + struct virtio_input *vi = input_get_drvdata(idev);
> +
> + if (a == &dev_attr_serial.attr && !strlen(vi->serial))
> + return 0;
> +
> + return a->mode;
> +}
> +
> +static struct attribute_group dev_attr_grp = {
> + .attrs =dev_attrs,
> + .is_visible = dev_attrs_are_visible,
> +};
> +
> +static const struct attribute_group *dev_attr_groups[] = {
> + &dev_attr_grp,
> + NULL
> +};
> +
> +static void virtinput_queue_evtbuf(struct virtio_input *vi,
> +struct virtio_input_event *evtbuf)
> +{
> + struct scatterlist sg[1];
> +
> + sg_init_one(sg, evtbuf, sizeof(*evtbuf));
> + virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
> +}
> +
> +static void virtinput_recv_events(struct virtqueue *vq)
> +{
> + struct virtio_input *vi = vq->vdev->priv;
> + struct virtio_input_event *event;
> + unsigned int len;
> +
> + while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
> + input_event(vi->idev,
> + le16_to_cpu(event->type),
> + le16_to_cpu(event->code),
> + le32_to_cpu(event->value));
> + virtinput_queue_evtbuf(vi, event);
> + }
> + virtqueue_kick(vq);
> +}
> +
> +static int virtinput_send_status(struct virtio_input *vi,
> + u16 type, u16 code, s32 value)
> +{
> + struct virtio_input_event *stsbuf;
> + struct scatterlist sg[1];
> +
> + stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> + if (!stsbuf)
> + return -ENOMEM;
Does this return an error to userspace?
If so it's not a good idea I think, GFP_AT
Re: [PATCH 0/1] Add virtio-input driver.
On Thu, Mar 19, 2015 at 10:13:10AM +0100, Gerd Hoffmann wrote: > Hi, > > This patch adds a virtio driver for input devices. > > Specification: > https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input > > https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007 OK, I don't know which thread should I use for spec discussions. Referring to that: "See file:///usr/include/linux/input.h." Is likely not present on many systems, or might not include the info you refer to. "type, code and value are filled according to the linux input layer (evdev) interface" Which version? How will non-linux guests know what to implement? > > Qemu patches; > https://lists.gnu.org/archive/html/qemu-devel/2015-03/threads.html#03973 > > Gerd Hoffmann (1): > Add virtio-input driver. > > drivers/virtio/Kconfig| 10 ++ > drivers/virtio/Makefile | 1 + > drivers/virtio/virtio_input.c | 313 > ++ > include/uapi/linux/virtio_ids.h | 1 + > include/uapi/linux/virtio_input.h | 65 > 5 files changed, 390 insertions(+) > create mode 100644 drivers/virtio/virtio_input.c > create mode 100644 include/uapi/linux/virtio_input.h > > -- > 1.8.3.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 8/9] qspinlock: Generic paravirt support
On Thu, Mar 19, 2015 at 01:25:36PM +0100, Peter Zijlstra wrote:
> +static struct qspinlock **pv_hash(struct qspinlock *lock)
> +{
> + u32 hash = hash_ptr(lock, PV_LOCK_HASH_BITS);
> + struct pv_hash_bucket *hb, *end;
> +
> + if (!hash)
> + hash = 1;
> +
> + hb = &__pv_lock_hash[hash_align(hash)];
> + for (;;) {
> + for (end = hb + PV_HB_PER_LINE; hb < end; hb++) {
> + if (cmpxchg(&hb->lock, NULL, HB_RESERVED)) {
That should be: !cmpxchg(), bit disturbing that that booted.
> + WRITE_ONCE(hb->cpu, smp_processor_id());
> + /*
> + * Since we must read lock first and cpu
> + * second, we must write cpu first and lock
> + * second, therefore use HB_RESERVE to mark an
> + * entry in use before writing the values.
> + *
> + * This can cause hb_hash_find() to not find a
> + * cpu even though _Q_SLOW_VAL, this is not a
> + * problem since we re-check l->locked before
> + * going to sleep and the unlock will have
> + * cleared l->locked already.
> + */
> + smp_wmb(); /* matches rmb from pv_hash_find */
> + WRITE_ONCE(hb->lock, lock);
> + goto done;
> + }
> + }
> +
> + hash = lfsr(hash, PV_LOCK_HASH_BITS);
> + hb = &__pv_lock_hash[hash_align(hash)];
> + }
> +
> +done:
> + return &hb->lock;
> +}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/1] Add virtio-input driver.
On Do, 2015-03-19 at 14:35 +0100, Michael S. Tsirkin wrote: > On Thu, Mar 19, 2015 at 10:13:10AM +0100, Gerd Hoffmann wrote: > > Hi, > > > > This patch adds a virtio driver for input devices. > > > > Specification: > > https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input > > > > https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007 > > > OK, I don't know which thread should I use for spec discussions. > Referring to that: > > "See file:///usr/include/linux/input.h." > > Is likely not present on many systems, or might not include > the info you refer to. Dunno what the best way to deal with it is. Link to the version online @ kernel.org instead maybe? > "type, code and value are filled according to the linux input layer > (evdev) interface" > > Which version? Latest. As far I know there never ever have been incompatible changes to the interface, and given this is userspace/kernel abi I don't expect that to happen in the future. > How will non-linux guests know what to implement? There are some docs on the linux input layer and evdev events in Documentation/input/ in the kernel tree. cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/1] Add virtio-input driver.
Hi
(CC: Dmitry)
On Thu, Mar 19, 2015 at 10:13 AM, Gerd Hoffmann wrote:
> virtio-input is basically evdev-events-over-virtio, so this driver isn't
> much more than reading configuration from config space and forwarding
> incoming events to the linux input layer.
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/virtio/Kconfig| 10 ++
> drivers/virtio/Makefile | 1 +
> drivers/virtio/virtio_input.c | 313
> ++
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_input.h | 65
> 5 files changed, 390 insertions(+)
> create mode 100644 drivers/virtio/virtio_input.c
> create mode 100644 include/uapi/linux/virtio_input.h
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index b546da5..cab9f3f 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -48,6 +48,16 @@ config VIRTIO_BALLOON
>
> If unsure, say M.
>
> +config VIRTIO_INPUT
> + tristate "Virtio input driver"
> + depends on VIRTIO
> + depends on INPUT
> + ---help---
> +This driver supports virtio input devices such as
> +keyboards, mice and tablets.
> +
> +If unsure, say M.
> +
> config VIRTIO_MMIO
> tristate "Platform bus driver for memory mapped virtio devices"
> depends on HAS_IOMEM
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index d85565b..41e30e3 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> +obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> new file mode 100644
> index 000..2d425cf
> --- /dev/null
> +++ b/drivers/virtio/virtio_input.c
> @@ -0,0 +1,313 @@
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +struct virtio_input {
> + struct virtio_device *vdev;
> + struct input_dev *idev;
> + char name[64];
> + char serial[64];
> + char phys[64];
> + struct virtqueue *evt, *sts;
> + struct virtio_input_event evts[64];
> +};
> +
> +static ssize_t serial_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct input_dev *idev = to_input_dev(dev);
> + struct virtio_input *vi = input_get_drvdata(idev);
> + return sprintf(buf, "%s\n", vi->serial);
> +}
> +static DEVICE_ATTR_RO(serial);
> +
> +static struct attribute *dev_attrs[] = {
> + &dev_attr_serial.attr,
> + NULL
> +};
> +
> +static umode_t dev_attrs_are_visible(struct kobject *kobj,
> +struct attribute *a, int n)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct input_dev *idev = to_input_dev(dev);
> + struct virtio_input *vi = input_get_drvdata(idev);
> +
> + if (a == &dev_attr_serial.attr && !strlen(vi->serial))
> + return 0;
> +
> + return a->mode;
> +}
> +
> +static struct attribute_group dev_attr_grp = {
> + .attrs =dev_attrs,
> + .is_visible = dev_attrs_are_visible,
> +};
> +
> +static const struct attribute_group *dev_attr_groups[] = {
> + &dev_attr_grp,
> + NULL
> +};
> +
> +static void virtinput_queue_evtbuf(struct virtio_input *vi,
> + struct virtio_input_event *evtbuf)
> +{
> + struct scatterlist sg[1];
> +
> + sg_init_one(sg, evtbuf, sizeof(*evtbuf));
> + virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
> +}
> +
> +static void virtinput_recv_events(struct virtqueue *vq)
> +{
> + struct virtio_input *vi = vq->vdev->priv;
> + struct virtio_input_event *event;
> + unsigned int len;
> +
> + while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
> + input_event(vi->idev,
> + le16_to_cpu(event->type),
> + le16_to_cpu(event->code),
> + le32_to_cpu(event->value));
> + virtinput_queue_evtbuf(vi, event);
> + }
> + virtqueue_kick(vq);
> +}
> +
> +static int virtinput_send_status(struct virtio_input *vi,
> +u16 type, u16 code, s32 value)
> +{
> + struct virtio_input_event *stsbuf;
> + struct scatterlist sg[1];
> +
> + stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> + if (!stsbuf)
> + return -ENOMEM;
> +
> + stsbuf->type = cpu_to_le16(type);
> + stsbuf->code = cpu_to_le16(code);
> + stsbuf->value = cpu_to_le32(value);
> + sg_init_one(sg, stsbuf, sizeof(*stsbuf
Re: [PATCH 1/1] Add virtio-input driver.
On Thu, Mar 19, 2015 at 01:29:49PM +0100, David Herrmann wrote:
> Hi
>
> (CC: Dmitry)
>
> On Thu, Mar 19, 2015 at 10:13 AM, Gerd Hoffmann wrote:
> > virtio-input is basically evdev-events-over-virtio, so this driver isn't
> > much more than reading configuration from config space and forwarding
> > incoming events to the linux input layer.
> >
> > Signed-off-by: Gerd Hoffmann
> > ---
> > drivers/virtio/Kconfig| 10 ++
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/virtio_input.c | 313
> > ++
> > include/uapi/linux/virtio_ids.h | 1 +
> > include/uapi/linux/virtio_input.h | 65
> > 5 files changed, 390 insertions(+)
> > create mode 100644 drivers/virtio/virtio_input.c
> > create mode 100644 include/uapi/linux/virtio_input.h
> >
> > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> > index b546da5..cab9f3f 100644
> > --- a/drivers/virtio/Kconfig
> > +++ b/drivers/virtio/Kconfig
> > @@ -48,6 +48,16 @@ config VIRTIO_BALLOON
> >
> > If unsure, say M.
> >
> > +config VIRTIO_INPUT
> > + tristate "Virtio input driver"
> > + depends on VIRTIO
> > + depends on INPUT
> > + ---help---
> > +This driver supports virtio input devices such as
> > +keyboards, mice and tablets.
> > +
> > +If unsure, say M.
> > +
> > config VIRTIO_MMIO
> > tristate "Platform bus driver for memory mapped virtio devices"
> > depends on HAS_IOMEM
> > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> > index d85565b..41e30e3 100644
> > --- a/drivers/virtio/Makefile
> > +++ b/drivers/virtio/Makefile
> > @@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> > virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> > obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> > +obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> > diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> > new file mode 100644
> > index 000..2d425cf
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_input.c
> > @@ -0,0 +1,313 @@
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +struct virtio_input {
> > + struct virtio_device *vdev;
> > + struct input_dev *idev;
> > + char name[64];
> > + char serial[64];
> > + char phys[64];
> > + struct virtqueue *evt, *sts;
> > + struct virtio_input_event evts[64];
> > +};
> > +
> > +static ssize_t serial_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct input_dev *idev = to_input_dev(dev);
> > + struct virtio_input *vi = input_get_drvdata(idev);
> > + return sprintf(buf, "%s\n", vi->serial);
> > +}
> > +static DEVICE_ATTR_RO(serial);
What is serial? Serial number?
> > +
> > +static struct attribute *dev_attrs[] = {
> > + &dev_attr_serial.attr,
> > + NULL
> > +};
> > +
> > +static umode_t dev_attrs_are_visible(struct kobject *kobj,
> > +struct attribute *a, int n)
> > +{
> > + struct device *dev = container_of(kobj, struct device, kobj);
> > + struct input_dev *idev = to_input_dev(dev);
> > + struct virtio_input *vi = input_get_drvdata(idev);
> > +
> > + if (a == &dev_attr_serial.attr && !strlen(vi->serial))
> > + return 0;
> > +
> > + return a->mode;
> > +}
> > +
> > +static struct attribute_group dev_attr_grp = {
> > + .attrs =dev_attrs,
> > + .is_visible = dev_attrs_are_visible,
> > +};
> > +
> > +static const struct attribute_group *dev_attr_groups[] = {
> > + &dev_attr_grp,
> > + NULL
> > +};
> > +
> > +static void virtinput_queue_evtbuf(struct virtio_input *vi,
> > + struct virtio_input_event *evtbuf)
> > +{
> > + struct scatterlist sg[1];
> > +
> > + sg_init_one(sg, evtbuf, sizeof(*evtbuf));
> > + virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
> > +}
> > +
> > +static void virtinput_recv_events(struct virtqueue *vq)
> > +{
> > + struct virtio_input *vi = vq->vdev->priv;
> > + struct virtio_input_event *event;
> > + unsigned int len;
> > +
> > + while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
> > + input_event(vi->idev,
> > + le16_to_cpu(event->type),
> > + le16_to_cpu(event->code),
> > + le32_to_cpu(event->value));
> > + virtinput_queue_evtbuf(vi, event);
> > + }
> > + virtqueue_kick(vq);
> > +}
> > +
> > +static int virtinput_send_status(struct virtio_input *vi,
> > +u16 type, u16 code, s32 value)
> > +{
> > +
Re: [PATCH 0/1] Add virtio-input driver.
On Thu, Mar 19, 2015 at 03:46:44PM +0100, Gerd Hoffmann wrote: > On Do, 2015-03-19 at 14:35 +0100, Michael S. Tsirkin wrote: > > On Thu, Mar 19, 2015 at 10:13:10AM +0100, Gerd Hoffmann wrote: > > > Hi, > > > > > > This patch adds a virtio driver for input devices. > > > > > > Specification: > > > https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input > > > > > > https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007 > > > > > > OK, I don't know which thread should I use for spec discussions. > > Referring to that: > > > > "See file:///usr/include/linux/input.h." > > > > Is likely not present on many systems, or might not include > > the info you refer to. > > Dunno what the best way to deal with it is. Link to the version online > @ kernel.org instead maybe? > > > "type, code and value are filled according to the linux input layer > > (evdev) interface" > > > > Which version? > > Latest. As far I know there never ever have been incompatible changes > to the interface, and given this is userspace/kernel abi I don't expect > that to happen in the future. More events are added though, are they not? And distros backport rundom subsets. So I worry: what happens e.g. if you migrate between hosts which expose slightly different subsets of events? Might e.g. a button get stuck because button-press event was sent but button-release wasn't? > > How will non-linux guests know what to implement? > > There are some docs on the linux input layer and evdev events in > Documentation/input/ in the kernel tree. > > cheers, > Gerd -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/1] Add virtio-input driver.
On Thu, Mar 19, 2015 at 03:46:44PM +0100, Gerd Hoffmann wrote: > On Do, 2015-03-19 at 14:35 +0100, Michael S. Tsirkin wrote: > > On Thu, Mar 19, 2015 at 10:13:10AM +0100, Gerd Hoffmann wrote: > > > Hi, > > > > > > This patch adds a virtio driver for input devices. > > > > > > Specification: > > > https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input > > > > > > https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007 > > > > > > OK, I don't know which thread should I use for spec discussions. > > Referring to that: > > > > "See file:///usr/include/linux/input.h." > > > > Is likely not present on many systems, or might not include > > the info you refer to. > > Dunno what the best way to deal with it is. Link to the version online > @ kernel.org instead maybe? > > > "type, code and value are filled according to the linux input layer > > (evdev) interface" > > > > Which version? > > Latest. As far I know there never ever have been incompatible changes > to the interface, and given this is userspace/kernel abi I don't expect > that to happen in the future. > > > How will non-linux guests know what to implement? > > There are some docs on the linux input layer and evdev events in > Documentation/input/ in the kernel tree. > > cheers, > Gerd Also, the spec needs to be rewritten a bit more formally, with conformance clauses separated from freetext description, and linked to from appropriate section. " motion events are send from the device" send->sent -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/1] Add virtio-input driver.
On 19/03/2015 17:33, Michael S. Tsirkin wrote: > On Thu, Mar 19, 2015 at 03:46:44PM +0100, Gerd Hoffmann wrote: >> Latest. As far I know there never ever have been incompatible changes >> to the interface, and given this is userspace/kernel abi I don't expect >> that to happen in the future. > > More events are added though, are they not? And distros backport rundom > subsets. > So I worry: what happens e.g. if you migrate between hosts which expose > slightly different subsets of events? > Might e.g. a button get stuck because button-press event was > sent but button-release wasn't? I think this is the same as SCSI. You can migrate between hosts which expose slightly different command sets. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/1] Add virtio-input driver.
Hey
On Thu, Mar 19, 2015 at 5:27 PM, Dmitry Torokhov
wrote:
> On Thu, Mar 19, 2015 at 01:29:49PM +0100, David Herrmann wrote:
[...]
>> > +static int virtinput_probe(struct virtio_device *vdev)
>> > +{
>> > + struct virtio_input *vi;
>> > + size_t size;
>> > + int abs, err;
>> > +
>> > + vi = kzalloc(sizeof(*vi), GFP_KERNEL);
>> > + if (!vi) {
>> > + err = -ENOMEM;
>> > + goto out1;
>> > + }
>> > + vdev->priv = vi;
>> > + vi->vdev = vdev;
>> > +
>> > + err = virtinput_init_vqs(vi);
>> > + if (err)
>> > + goto out2;
>> > +
>> > + vi->idev = input_allocate_device();
>> > + if (!vi->idev) {
>> > + err = -ENOMEM;
>> > + goto out3;
>> > + }
>> > + input_set_drvdata(vi->idev, vi);
>> > +
>> > + size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_NAME, 0);
>> > + virtio_cread_bytes(vi->vdev, offsetof(struct virtio_input_config,
>> > u),
>> > + vi->name, min(size, sizeof(vi->name)));
>> > + size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_SERIAL, 0);
>> > + virtio_cread_bytes(vi->vdev, offsetof(struct virtio_input_config,
>> > u),
>> > + vi->serial, min(size, sizeof(vi->serial)));
>> > + snprintf(vi->phys, sizeof(vi->phys),
>> > +"virtio%d/input0", vdev->index);
>> > +
>> > + virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_PROP_BITS, 0,
>> > + vi->idev->propbit, INPUT_PROP_CNT);
>> > + size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_REP);
>> > + if (size)
>> > + set_bit(EV_REP, vi->idev->evbit);
>> > +
>> > + vi->idev->name = vi->name;
>> > + vi->idev->phys = vi->phys;
>>
>> Can you set vi->idev->uniq to the virtio-bus path?
>
> No, uniq can't be phys as phys is unique within the system while uniq is
> like serial number or UUID and should never repeat.
...sorry, my bad! We should still forward it from the host, imo. It's
really handy for applications to re-detect devices.
Thanks
David
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Xen-devel] [PATCH 0/9] qspinlock stuff -v15
On 16/03/15 13:16, Peter Zijlstra wrote:
>
> I feel that if someone were to do a Xen patch we can go ahead and merge this
> stuff (finally!).
This seems work for me, but I've not got time to give it a more thorough
testing.
You can fold this into your series.
There doesn't seem to be a way to disable QUEUE_SPINLOCKS when supported by
the arch, is this intentional? If so, the existing ticketlock code could go.
David
8<--
x86/xen: paravirt support for qspinlocks
Provide the wait and kick ops necessary for paravirt-aware queue
spinlocks.
Signed-off-by: David Vrabel
---
arch/x86/xen/spinlock.c | 40 +---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 956374c..b019b2a 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -95,17 +95,43 @@ static inline void spin_time_accum_blocked(u64 start)
}
#endif /* CONFIG_XEN_DEBUG_FS */
+static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
+static DEFINE_PER_CPU(char *, irq_name);
+static bool xen_pvspin = true;
+
+#ifdef CONFIG_QUEUE_SPINLOCK
+
+#include
+
+PV_CALLEE_SAVE_REGS_THUNK(__pv_queue_spin_unlock);
+
+static void xen_qlock_wait(u8 *ptr, u8 val)
+{
+ int irq = __this_cpu_read(lock_kicker_irq);
+
+ xen_clear_irq_pending(irq);
+
+ barrier();
+
+ if (READ_ONCE(*ptr) == val)
+ xen_poll_irq(irq);
+}
+
+static void xen_qlock_kick(int cpu)
+{
+ xen_send_IPI_one(cpu, XEN_SPIN_UNLOCK_VECTOR);
+}
+
+#else
+
struct xen_lock_waiting {
struct arch_spinlock *lock;
__ticket_t want;
};
-static DEFINE_PER_CPU(int, lock_kicker_irq) = -1;
-static DEFINE_PER_CPU(char *, irq_name);
static DEFINE_PER_CPU(struct xen_lock_waiting, lock_waiting);
static cpumask_t waiting_cpus;
-static bool xen_pvspin = true;
__visible void xen_lock_spinning(struct arch_spinlock *lock, __ticket_t want)
{
int irq = __this_cpu_read(lock_kicker_irq);
@@ -217,6 +243,7 @@ static void xen_unlock_kick(struct arch_spinlock *lock,
__ticket_t next)
}
}
}
+#endif /* !QUEUE_SPINLOCK */
static irqreturn_t dummy_handler(int irq, void *dev_id)
{
@@ -280,8 +307,15 @@ void __init xen_init_spinlocks(void)
return;
}
printk(KERN_DEBUG "xen: PV spinlocks enabled\n");
+#ifdef CONFIG_QUEUE_SPINLOCK
+ pv_lock_ops.queue_spin_lock_slowpath = __pv_queue_spin_lock_slowpath;
+ pv_lock_ops.queue_spin_unlock = PV_CALLEE_SAVE(__pv_queue_spin_unlock);
+ pv_lock_ops.wait = xen_qlock_wait;
+ pv_lock_ops.kick = xen_qlock_kick;
+#else
pv_lock_ops.lock_spinning = PV_CALLEE_SAVE(xen_lock_spinning);
pv_lock_ops.unlock_kick = xen_unlock_kick;
+#endif
}
/*
--
1.7.10.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Xen-devel] [PATCH 0/9] qspinlock stuff -v15
On Thu, Mar 19, 2015 at 06:01:34PM +, David Vrabel wrote: > This seems work for me, but I've not got time to give it a more thorough > testing. > > You can fold this into your series. Thanks! > There doesn't seem to be a way to disable QUEUE_SPINLOCKS when supported by > the arch, is this intentional? If so, the existing ticketlock code could go. Yeah, its left as a rudiment such that if we find issues with the qspinlock code we can 'revert' with a trivial patch. If no issues show up we can rip out all the old code in a subsequent release. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 4/4] virtio_pci: drop msi_off on probe
pci core now disables msi on probe automatically, drop this from device-specific code. Cc: Bjorn Helgaas Cc: linux-...@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index e894eb2..806bb2c 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -501,9 +501,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev, INIT_LIST_HEAD(&vp_dev->virtqueues); spin_lock_init(&vp_dev->lock); - /* Disable MSI/MSIX to bring device to a known good state. */ - pci_msi_off(pci_dev); - /* enable the device */ rc = pci_enable_device(pci_dev); if (rc) -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 9/9] qspinlock, x86, kvm: Implement KVM support for paravirt qspinlock
On 03/19/2015 06:01 AM, Peter Zijlstra wrote: On Wed, Mar 18, 2015 at 10:45:55PM -0400, Waiman Long wrote: On 03/16/2015 09:16 AM, Peter Zijlstra wrote: I do have some concern about this call site patching mechanism as the modification is not atomic. The spin_unlock() calls are in many places in the kernel. There is a possibility that a thread is calling a certain spin_unlock call site while it is being patched by another one with the alternative() function call. So far, I don't see any problem with bare metal where paravirt_patch_insns() is used to patch it to the move instruction. However, in a virtual guest enivornment where paravirt_patch_call() was used, there were situations where the system panic because of page fault on some invalid memory in the kthread. If you look at the paravirt_patch_call(), you will see: : b->opcode = 0xe8; /* call */ b->delta = delta; If another CPU reads the instruction at the call site at the right moment, it will get the modified call instruction, but not the new delta value. It will then jump to a random location. I believe that was causing the system panic that I saw. So I think it is kind of risky to use it here unless we can guarantee that call site patching is atomic wrt other CPUs. Just look at where the patching is done: init/main.c:start_kernel() check_bugs() alternative_instructions() apply_paravirt() We're UP and not holding any locks, disable IRQs (see text_poke_early()) and have NMIs 'disabled'. You are probably right. The initial apply_paravirt() was done before the SMP boot. Subsequent ones were at kernel module load time. I put a counter in the __native_queue_spin_unlock() and it registered 26949 unlock calls in a 16-cpu guest before it got patched out. The panic that I observed before might be due to some coding error of my own. -Longman ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 8/9] qspinlock: Generic paravirt support
On 03/19/2015 08:25 AM, Peter Zijlstra wrote:
On Thu, Mar 19, 2015 at 11:12:42AM +0100, Peter Zijlstra wrote:
So I was now thinking of hashing the lock pointer; let me go and quickly
put something together.
A little something like so; ideally we'd allocate the hashtable since
NR_CPUS is kinda bloated, but it shows the idea I think.
And while this has loops in (the rehashing thing) their fwd progress
does not depend on other CPUs.
And I suspect that for the typical lock contention scenarios its
unlikely we ever really get into long rehashing chains.
---
include/linux/lfsr.h| 49
kernel/locking/qspinlock_paravirt.h | 143
2 files changed, 178 insertions(+), 14 deletions(-)
This is a much better alternative.
--- /dev/null
+++ b/include/linux/lfsr.h
@@ -0,0 +1,49 @@
+#ifndef _LINUX_LFSR_H
+#define _LINUX_LFSR_H
+
+/*
+ * Simple Binary Galois Linear Feedback Shift Register
+ *
+ * http://en.wikipedia.org/wiki/Linear_feedback_shift_register
+ *
+ */
+
+extern void __lfsr_needs_more_taps(void);
+
+static __always_inline u32 lfsr_taps(int bits)
+{
+ if (bits == 1) return 0x0001;
+ if (bits == 2) return 0x0001;
+ if (bits == 3) return 0x0003;
+ if (bits == 4) return 0x0009;
+ if (bits == 5) return 0x0012;
+ if (bits == 6) return 0x0021;
+ if (bits == 7) return 0x0041;
+ if (bits == 8) return 0x008E;
+ if (bits == 9) return 0x0108;
+ if (bits == 10) return 0x0204;
+ if (bits == 11) return 0x0402;
+ if (bits == 12) return 0x0829;
+ if (bits == 13) return 0x100D;
+ if (bits == 14) return 0x2015;
+
+ /*
+* For more taps see:
+* http://users.ece.cmu.edu/~koopman/lfsr/index.html
+*/
+ __lfsr_needs_more_taps();
+
+ return 0;
+}
+
+static inline u32 lfsr(u32 val, int bits)
+{
+ u32 bit = val& 1;
+
+ val>>= 1;
+ if (bit)
+ val ^= lfsr_taps(bits);
+ return val;
+}
+
+#endif /* _LINUX_LFSR_H */
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -2,6 +2,9 @@
#error "do not include this file"
#endif
+#include
+#include
+
/*
* Implement paravirt qspinlocks; the general idea is to halt the vcpus
instead
* of spinning them.
@@ -107,7 +110,120 @@ static void pv_kick_node(struct mcs_spin
pv_kick(pn->cpu);
}
-static DEFINE_PER_CPU(struct qspinlock *, __pv_lock_wait);
+/*
+ * Hash table using open addressing with an LFSR probe sequence.
+ *
+ * Since we should not be holding locks from NMI context (very rare indeed) the
+ * max load factor is 0.75, which is around the point where open addressing
+ * breaks down.
+ *
+ * Instead of probing just the immediate bucket we probe all buckets in the
+ * same cacheline.
+ *
+ * http://en.wikipedia.org/wiki/Hash_table#Open_addressing
+ *
+ */
+
+#define HB_RESERVED((struct qspinlock *)1)
+
+struct pv_hash_bucket {
+ struct qspinlock *lock;
+ int cpu;
+};
+
+/*
+ * XXX dynamic allocate using nr_cpu_ids instead...
+ */
+#define PV_LOCK_HASH_BITS (2 + NR_CPUS_BITS)
+
As said here, we should make it dynamically allocated depending on
num_possible_cpus().
+#if PV_LOCK_HASH_BITS< 6
+#undef PV_LOCK_HASH_BITS
+#define PB_LOCK_HASH_BITS 6
+#endif
+
+#define PV_LOCK_HASH_SIZE (1<< PV_LOCK_HASH_BITS)
+
+static struct pv_hash_bucket __pv_lock_hash[PV_LOCK_HASH_SIZE]
cacheline_aligned;
+
+#define PV_HB_PER_LINE (SMP_CACHE_BYTES / sizeof(struct
pv_hash_bucket))
+
+static inline u32 hash_align(u32 hash)
+{
+ return hash& ~(PV_HB_PER_LINE - 1);
+}
+
+static struct qspinlock **pv_hash(struct qspinlock *lock)
+{
+ u32 hash = hash_ptr(lock, PV_LOCK_HASH_BITS);
+ struct pv_hash_bucket *hb, *end;
+
+ if (!hash)
+ hash = 1;
+
+ hb =&__pv_lock_hash[hash_align(hash)];
+ for (;;) {
+ for (end = hb + PV_HB_PER_LINE; hb< end; hb++) {
+ if (cmpxchg(&hb->lock, NULL, HB_RESERVED)) {
+ WRITE_ONCE(hb->cpu, smp_processor_id());
+ /*
+* Since we must read lock first and cpu
+* second, we must write cpu first and lock
+* second, therefore use HB_RESERVE to mark an
+* entry in use before writing the values.
+*
+* This can cause hb_hash_find() to not find a
+* cpu even though _Q_SLOW_VAL, this is not a
+* problem since we re-check l->locked before
+* going to sleep and the unlock will have
+* cleared l->locked already.
+*/
+ smp_wmb();
