Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
Hi, And is it possible to use offset within BAR and/or memory BARs? If yes I'd strongly prefer this. What is the point? Do you want place virtio regions and vga framebuffer in the same pci bar? Why? virtio is mmio and traps into qemu on access, whereas the vga framebuffer is memory-backed (with dirty tracking turned on). Don't think this is a good idea, even though the memory api would probably allow to do this. cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4] Add virtio-input driver.
On Tue, Mar 24, 2015 at 01:08:46PM +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 kra...@redhat.com Looks good overallm though I think I still see a couple of minor issues. --- MAINTAINERS | 6 + drivers/virtio/Kconfig| 10 ++ drivers/virtio/Makefile | 1 + drivers/virtio/virtio_input.c | 363 ++ include/uapi/linux/Kbuild | 1 + include/uapi/linux/virtio_ids.h | 1 + include/uapi/linux/virtio_input.h | 76 7 files changed, 458 insertions(+) create mode 100644 drivers/virtio/virtio_input.c create mode 100644 include/uapi/linux/virtio_input.h diff --git a/MAINTAINERS b/MAINTAINERS index 358eb01..6f233dd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10442,6 +10442,12 @@ S: Maintained F: drivers/vhost/ F: include/uapi/linux/vhost.h +VIRTIO INPUT DRIVER +M: Gerd Hoffmann kra...@redhat.com +S: Maintained +F: drivers/virtio/virtio_input.c +F: include/uapi/linux/virtio_input.h + VIA RHINE NETWORK DRIVER M: Roger Luethi r...@hellgate.ch S: Maintained 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..fc99a15 --- /dev/null +++ b/drivers/virtio/virtio_input.c @@ -0,0 +1,363 @@ +#include linux/module.h +#include linux/virtio.h +#include linux/input.h + +#include uapi/linux/virtio_ids.h +#include uapi/linux/virtio_input.h + +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]; + spinlock_t lock; + bool ready; +}; + +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 long flags; + unsigned int len; + + spin_lock_irqsave(vi-lock, flags); + if (vi-ready) { + 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); + } + spin_unlock_irqrestore(vi-lock, flags); +} + +static int virtinput_send_status(struct virtio_input *vi, + u16 type, u16 code, s32 value) +{ + struct virtio_input_event *stsbuf; + struct scatterlist sg[1]; + unsigned long flags; + int rc; + + 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)); + + spin_lock_irqsave(vi-lock, flags); + if (vi-ready) { + rc = virtqueue_add_outbuf(vi-sts, sg, 1, stsbuf, GFP_ATOMIC); + virtqueue_kick(vi-sts); + } else { + rc = -ENODEV; + } + spin_unlock_irqrestore(vi-lock, flags); + + if (rc
Re: [PATCH] Add virtio gpu driver.
On Thu, Mar 26, 2015 at 08:12:39AM +0100, Gerd Hoffmann wrote: On Mi, 2015-03-25 at 18:09 +0100, Michael S. Tsirkin wrote: On Wed, Mar 25, 2015 at 04:37:16PM +0100, Gerd Hoffmann wrote: Hi, BTW can we teach virtio-gpu to look for framebuffer using virtio pci caps? The virtio-gpu driver doesn't matter much here, it doesn't use it anyway. Or are there limitations such as only using IO port BARs, or compatibility with BIOS code etc that limit us to specific BARs anyway? Yes, vgabios code needs to know. Currently it has bar #2 for the vga framebuffer bar hardcoded. It's 16bit code. I don't feel like making the probing more complicated ... cheers, Gerd OK - you are saying all VGA cards use bar #2 for this functionality, so we are just following established practice here? vgabios checks pci ids to figure. qxl+stdvga use bar #0, vmware-vga bar #1, virtio-vga bar #2. cheers, Gerd And is it possible to use offset within BAR and/or memory BARs? If yes I'd strongly prefer this. As for writing 16 bit code, I need to do this for virtio scsi/blk anyway, so we'll be able to share code. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
On Thu, Mar 26, 2015 at 09:42:47AM +0100, Gerd Hoffmann wrote: Hi, And is it possible to use offset within BAR and/or memory BARs? If yes I'd strongly prefer this. What is the point? Do you want place virtio regions and vga framebuffer in the same pci bar? Why? virtio is mmio and traps into qemu on access, whereas the vga framebuffer is memory-backed (with dirty tracking turned on). Don't think this is a good idea, even though the memory api would probably allow to do this. cheers, Gerd Absolutely, it's pretty common to mix regions in a BAR. For example, we have virtio kick (ioeventfd backed, handled in kernel) in same BAR as common and device specific configuration. We did the same thing you are now doing with the virtio BAR, and now we have to maintain two code bases, virtio pci config was designed to be future proof so why not use it? This is mostly just making sure we don't paint ourselves into a corner. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 06/10] virtio_pci: drop msi_off on probe
pci core now disables msi on probe automatically, drop this from device-specific code. Cc: Bjorn Helgaas bhelg...@google.com Cc: linux-...@vger.kernel.org Signed-off-by: Michael S. Tsirkin m...@redhat.com --- 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
[PATCH v5] 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 kra...@redhat.com --- MAINTAINERS | 6 + drivers/virtio/Kconfig| 10 + drivers/virtio/Makefile | 1 + drivers/virtio/virtio_input.c | 384 ++ include/uapi/linux/Kbuild | 1 + include/uapi/linux/virtio_ids.h | 1 + include/uapi/linux/virtio_input.h | 76 7 files changed, 479 insertions(+) create mode 100644 drivers/virtio/virtio_input.c create mode 100644 include/uapi/linux/virtio_input.h diff --git a/MAINTAINERS b/MAINTAINERS index 358eb01..6f233dd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10442,6 +10442,12 @@ S: Maintained F: drivers/vhost/ F: include/uapi/linux/vhost.h +VIRTIO INPUT DRIVER +M: Gerd Hoffmann kra...@redhat.com +S: Maintained +F: drivers/virtio/virtio_input.c +F: include/uapi/linux/virtio_input.h + VIA RHINE NETWORK DRIVER M: Roger Luethi r...@hellgate.ch S: Maintained 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..60e2a16 --- /dev/null +++ b/drivers/virtio/virtio_input.c @@ -0,0 +1,384 @@ +#include linux/module.h +#include linux/virtio.h +#include linux/virtio_config.h +#include linux/input.h + +#include uapi/linux/virtio_ids.h +#include uapi/linux/virtio_input.h + +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]; + spinlock_t lock; + bool ready; +}; + +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 long flags; + unsigned int len; + + spin_lock_irqsave(vi-lock, flags); + if (vi-ready) { + while ((event = virtqueue_get_buf(vi-evt, len)) != NULL) { + spin_unlock_irqrestore(vi-lock, flags); + input_event(vi-idev, + le16_to_cpu(event-type), + le16_to_cpu(event-code), + le32_to_cpu(event-value)); + spin_lock_irqsave(vi-lock, flags); + virtinput_queue_evtbuf(vi, event); + } + virtqueue_kick(vq); + } + spin_unlock_irqrestore(vi-lock, flags); +} + +/* + * On error we are losing the status update, which isn't critical as + * this is typically used for stuff like keyboard leds. + */ +static int virtinput_send_status(struct virtio_input *vi, +u16 type, u16 code, s32 value) +{ + struct virtio_input_event *stsbuf; + struct scatterlist sg[1]; + unsigned long flags; + int rc; + + 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)); + + spin_lock_irqsave(vi-lock, flags); + if (vi-ready) { + rc = virtqueue_add_outbuf(vi-sts, sg, 1, stsbuf, GFP_ATOMIC); +
Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
On Thu, Mar 26, 2015 at 12:38:43PM +0100, Gerd Hoffmann wrote: Hi, Absolutely, it's pretty common to mix regions in a BAR. For example, we have virtio kick (ioeventfd backed, handled in kernel) in same BAR as common and device specific configuration. We did the same thing you are now doing with the virtio BAR, and now we have to maintain two code bases, virtio pci config was designed to be future proof so why not use it? It's not about virtio at all. It's about vga compatibility, so we have a simple framebuffer as boot display. Only used when virtio is *not* enabled. I don't know. This seems exactly like the kind of thing we had in mind when we added the virtio pci capability. For example, we have text in spec that requires drivers to skip unknown capabilities. And yes, if bios pokes at a specific bar then we do need to list this info in the virtio spec so this makes it an issue that is virtio related. This is mostly just making sure we don't paint ourselves into a corner. It's a simple memory bar. vga cards have that since pci was invented (standalone ones, chipset graphics aside), and there havn't been fundamental changes ... cheers, Gerd Yes, it's not about what we put there now. It's about being able to move things about in the future without breaking guests. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5] Add virtio-input driver.
On Thu, Mar 26, 2015 at 11:49:25AM +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 kra...@redhat.com Still a bit worried about using input.h as host/guest interface (can't we use some formal standard, e.g. USB HID?), but I'll let Rusty decide that. Otherwise mostly looks good. One nit below. --- Could you pls include changelog in the future? You are sending multiple versions per day and it's hard to keep up. +static unsigned int features[] = { + /* none */ +}; An empty line wouldn't hurt here about variable definition. +static struct virtio_device_id id_table[] = { + { VIRTIO_ID_INPUT, VIRTIO_DEV_ANY_ID }, + { 0 }, +}; ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
Hi, Absolutely, it's pretty common to mix regions in a BAR. For example, we have virtio kick (ioeventfd backed, handled in kernel) in same BAR as common and device specific configuration. We did the same thing you are now doing with the virtio BAR, and now we have to maintain two code bases, virtio pci config was designed to be future proof so why not use it? It's not about virtio at all. It's about vga compatibility, so we have a simple framebuffer as boot display. Only used when virtio is *not* enabled. This is mostly just making sure we don't paint ourselves into a corner. It's a simple memory bar. vga cards have that since pci was invented (standalone ones, chipset graphics aside), and there havn't been fundamental changes ... cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] Add virtio gpu driver.
On Wed, Mar 25, 2015 at 03:53:09PM +0100, Gerd Hoffmann wrote: Signed-off-by: Dave Airlie airl...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com Standard request from my side for new drm drivers (especially if they're this simple): Can you please update the drivers to latest drm internal interfaces, i.e. using universal planes and atomic? Have a docs / sample code pointer for me? Picking any of the recently converted drivers or recently merged atomic drivers should be fairly informative. Overall conversion procedure is detailed in http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html http://blog.ffwll.ch/2015/01/update-for-atomic-display-updates.html And ofc there's the kerneldoc stuff in the drm docbook. If you have questions probably best to ask them in #dri-devel irc, most of the people who've done atomic conversions hang out there and can help out. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5] Add virtio-input driver.
Michael S. Tsirkin m...@redhat.com writes: On Thu, Mar 26, 2015 at 11:49:25AM +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 kra...@redhat.com Still a bit worried about using input.h as host/guest interface (can't we use some formal standard, e.g. USB HID?), but I'll let Rusty decide that. I like the simplicity, and this API is pretty well proven. Since this is under drivers/virtio rather than drivers/input, I've applied it to my tree. I have a dream where Denys Vlasenko adds this and the virtio gl device to lguest, and we have X running under lguest :) Applied! Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
On Thu, Mar 26, 2015 at 04:07:16PM +0100, Gerd Hoffmann wrote: Hi, I don't know. This seems exactly like the kind of thing we had in mind when we added the virtio pci capability. For example, we have text in spec that requires drivers to skip unknown capabilities. And yes, if bios pokes at a specific bar then we do need to list this info in the virtio spec so this makes it an issue that is virtio related. Hmm, virtio-vga is a two-in-one device basically. When virtio is enabled it behaves like virtio-gpu-pci, otherwise it behaves very simliar to stdvga. So you need to know nothing about virtio to handle the vga side, and I want keep it that way. When no vga compatibility is needed there always is the option to just use virtio-gpu-pci instead. Yes, it's not about what we put there now. It's about being able to move things about in the future without breaking guests. We don't have that today for stdvga, and I still fail to see what this buys us. Completely different thing crossing my mind: I think we can make virtio-vga fully compatible with stdvga. stdvga has two bars, memory (#0) and mmio (#2). We can make the mmio bar larger and place all the virtio regions there. Full compatibility with some standard sounds like a better motivation, yes. I think in any case I'll go split off the vga compatibility bits to a different patch (and possible a separate patch series). cheers, Gerd Will you still need me to change core to claim specific memory only? -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/9] qspinlock stuff -v15
On Wed, Mar 25, 2015 at 03:47:39PM -0400, Konrad Rzeszutek Wilk wrote: Ah nice. That could be spun out as a seperate patch to optimize the existing ticket locks I presume. Yes I suppose we can do something similar for the ticket and patch in the right increment. We'd need to restructure the code a bit, but its not fundamentally impossible. We could equally apply the head hashing to the current ticket implementation and avoid the current bitmap iteration. Now with the old pv ticketlock code an vCPU would only go to sleep once and be woken up when it was its turn. With this new code it is woken up twice (and twice it goes to sleep). With an overcommit scenario this would imply that we will have at least twice as many VMEXIT as with the previous code. An astute observation, I had not considered that. I presume when you did benchmarking this did not even register? Thought I wonder if it would if you ran the benchmark for a week or so. You presume I benchmarked :-) I managed to boot something virt and run hackbench in it. I wouldn't know a representative virt setup if I ran into it. The thing is, we want this qspinlock for real hardware because its faster and I really want to avoid having to carry two spinlock implementations -- although I suppose that if we really really have to we could. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] Add virtio gpu driver.
On Mi, 2015-03-25 at 18:09 +0100, Michael S. Tsirkin wrote: On Wed, Mar 25, 2015 at 04:37:16PM +0100, Gerd Hoffmann wrote: Hi, BTW can we teach virtio-gpu to look for framebuffer using virtio pci caps? The virtio-gpu driver doesn't matter much here, it doesn't use it anyway. Or are there limitations such as only using IO port BARs, or compatibility with BIOS code etc that limit us to specific BARs anyway? Yes, vgabios code needs to know. Currently it has bar #2 for the vga framebuffer bar hardcoded. It's 16bit code. I don't feel like making the probing more complicated ... cheers, Gerd OK - you are saying all VGA cards use bar #2 for this functionality, so we are just following established practice here? vgabios checks pci ids to figure. qxl+stdvga use bar #0, vmware-vga bar #1, virtio-vga bar #2. cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.
Hi, I don't know. This seems exactly like the kind of thing we had in mind when we added the virtio pci capability. For example, we have text in spec that requires drivers to skip unknown capabilities. And yes, if bios pokes at a specific bar then we do need to list this info in the virtio spec so this makes it an issue that is virtio related. Hmm, virtio-vga is a two-in-one device basically. When virtio is enabled it behaves like virtio-gpu-pci, otherwise it behaves very simliar to stdvga. So you need to know nothing about virtio to handle the vga side, and I want keep it that way. When no vga compatibility is needed there always is the option to just use virtio-gpu-pci instead. Yes, it's not about what we put there now. It's about being able to move things about in the future without breaking guests. We don't have that today for stdvga, and I still fail to see what this buys us. Completely different thing crossing my mind: I think we can make virtio-vga fully compatible with stdvga. stdvga has two bars, memory (#0) and mmio (#2). We can make the mmio bar larger and place all the virtio regions there. I think in any case I'll go split off the vga compatibility bits to a different patch (and possible a separate patch series). cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization