Re: [PATCH 02/16] virtio: unify config_changed handling

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:06:52 +0300 Michael S. Tsirkin m...@redhat.com wrote: Replace duplicated code in all transports with a single wrapper in virtio.c. The only functional change is in virtio_mmio.c: if a buggy device sends us an interrupt before driver is set, we previously returned

Re: [PATCH v5 2/3] virtio_pci: Use the DMA API for virtqueues when possible

2014-10-06 Thread Christian Borntraeger
Am 29.09.2014 20:55, schrieb Andy Lutomirski: On Wed, Sep 17, 2014 at 7:16 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 17, 2014 at 08:02:31AM -0400, Benjamin Herrenschmidt wrote: On Tue, 2014-09-16 at 22:22 -0700, Andy Lutomirski wrote: On non-PPC systems, virtio_pci should use

Re: [PATCH 03/16] virtio: refactor to use drv_to_virtio

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:06:59 +0300 Michael S. Tsirkin m...@redhat.com wrote: Use drv_to_virtio instead of open-coding it. Fix whitespace damage introduced by virtio: unify config_changed handling Would it make sense to merge this into the previous patch? Signed-off-by: Michael S.

Re: [PATCH v5 2/3] virtio_pci: Use the DMA API for virtqueues when possible

2014-10-06 Thread Benjamin Herrenschmidt
On Mon, 2014-10-06 at 11:59 +0200, Christian Borntraeger wrote: Just as a comment: On s390 we always considered the memory access as access to real memory (not device memory) for virtio accesses. I prefer to not touch the DMA API on s390 as it is quite s390-PCI specific but it is somewhat

Re: [PATCH 04/16] virtio-pci: move freeze/restore to virtio core

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:02 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is in preparation to extending config changed event handling in core. Wrapping these in an API also seems to make for a cleaner code. Signed-off-by: Michael S. Tsirkin m...@redhat.com ---

Re: [PATCH 05/16] virtio: defer config changed notifications

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:05 +0300 Michael S. Tsirkin m...@redhat.com wrote: Defer config changed notifications that arrive during probe/scan/freeze/restore. This will allow drivers to set DRIVER_OK earlier, without worrying about racing with config change interrupts. This change will

Re: [PATCH 06/16] virtio_blk: drop config_enable

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:07 +0300 Michael S. Tsirkin m...@redhat.com wrote: Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio blk. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Sergei Shtylyov
Hello. On 10/5/2014 8:07 PM, Michael S. Tsirkin wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all workqueues non-reentrant

Re: [PATCH 07/16] virtio-blk: drop config_mutex

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:10 +0300 Michael S. Tsirkin m...@redhat.com wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 03:46:15PM +0400, Sergei Shtylyov wrote: Hello. On 10/5/2014 8:07 PM, Michael S. Tsirkin wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:16 +0300 Michael S. Tsirkin m...@redhat.com wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all

Re: [PATCH 05/16] virtio: defer config changed notifications

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 01:33:38PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:05 +0300 Michael S. Tsirkin m...@redhat.com wrote: Defer config changed notifications that arrive during probe/scan/freeze/restore. This will allow drivers to set DRIVER_OK earlier, without

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 01:56:10PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:16 +0300 Michael S. Tsirkin m...@redhat.com wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit

Re: [PATCH 08/16] virtio_net: drop config_enable

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 01:50:36PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:13 +0300 Michael S. Tsirkin m...@redhat.com wrote: Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio net. On removal, flush is now sufficient

Re: [PATCH 11/16] virtio_net: minor cleanup

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:21 +0300 Michael S. Tsirkin m...@redhat.com wrote: goto done; done: return; is ugly, it was put there to make diff review easier. replace by open-coded return. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- drivers/net/virtio_net.c | 6 ++

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Sergei Shtylyov
On 10/6/2014 3:56 PM, Michael S. Tsirkin wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all workqueues non-reentrant all

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 04:07:32PM +0400, Sergei Shtylyov wrote: On 10/6/2014 3:56 PM, Michael S. Tsirkin wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit

Re: [PATCH 06/16] virtio_blk: drop config_enable

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 15:09:53 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Oct 06, 2014 at 01:42:29PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:07 +0300 Michael S. Tsirkin m...@redhat.com wrote: Now that virtio core ensures config changes don't arrive during

Re: [PATCH 06/16] virtio_blk: drop config_enable

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 02:20:38PM +0200, Cornelia Huck wrote: On Mon, 6 Oct 2014 15:09:53 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Oct 06, 2014 at 01:42:29PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:07 +0300 Michael S. Tsirkin m...@redhat.com wrote:

Re: [PATCH 09/16] virtio-net: drop config_mutex

2014-10-06 Thread Sergei Shtylyov
On 10/6/2014 4:22 PM, Michael S. Tsirkin wrote: config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all workqueues non-reentrant all

Re: [PATCH 06/16] virtio_blk: drop config_enable

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 15:31:10 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Oct 06, 2014 at 02:20:38PM +0200, Cornelia Huck wrote: On Mon, 6 Oct 2014 15:09:53 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Oct 06, 2014 at 01:42:29PM +0200, Cornelia Huck wrote: On

Re: [PATCH 10/16] virtio: add API to enable VQs early

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:19 +0300 Michael S. Tsirkin m...@redhat.com wrote: virtio spec requires DRIVER_OK to be set before VQs are used, but some drivers use VQs before probe function returns. Since DRIVER_OK is set after probe, this violates the spec. Even though transitional devices

Re: [PATCH 16/16] virtio_net: fix use after free on allocation failure

2014-10-06 Thread Cornelia Huck
On Sun, 5 Oct 2014 19:07:38 +0300 Michael S. Tsirkin m...@redhat.com wrote: In the extremely unlikely event that driver initialization fails after RX buffers are added, virtio net frees RX buffers while VQs are still active, potentially causing device to use a freed buffer. To fix, reset

Re: [PATCH 10/16] virtio: add API to enable VQs early

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 04:08:16PM +0200, Cornelia Huck wrote: On Sun, 5 Oct 2014 19:07:19 +0300 Michael S. Tsirkin m...@redhat.com wrote: virtio spec requires DRIVER_OK to be set before VQs are used, but some drivers use VQs before probe function returns. Since DRIVER_OK is set after

[PATCH v2 00/15] virtio: fix spec compliance issues

2014-10-06 Thread Michael S. Tsirkin
Rusty, I have a mind to include this patchset for this merge window. Any input on this? This fixes the following virtio spec compliance issues: 1. on restore, drivers use device before setting ACKNOWLEDGE and DRIVER bits 2. on probe, drivers aren't prepared to handle config interrupts arriving

[PATCH v2 01/15] virtio_pci: fix virtio spec compliance on restore

2014-10-06 Thread Michael S. Tsirkin
On restore, virtio pci does the following: + set features + init vqs etc - device can be used at this point! + set ACKNOWLEDGE,DRIVER and DRIVER_OK status bits This is in violation of the virtio spec, which requires the following order: - ACKNOWLEDGE - DRIVER - init vqs - DRIVER_OK This

[PATCH v2 03/15] virtio-pci: move freeze/restore to virtio core

2014-10-06 Thread Michael S. Tsirkin
This is in preparation to extending config changed event handling in core. Wrapping these in an API also seems to make for a cleaner code. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/linux/virtio.h | 6 + drivers/virtio/virtio.c | 53

[PATCH v2 04/15] virtio: defer config changed notifications

2014-10-06 Thread Michael S. Tsirkin
Defer config changed notifications that arrive during probe/scan/freeze/restore. This will allow drivers to set DRIVER_OK earlier, without worrying about racing with config change interrupts. This change will also benefit old hypervisors (before 2009) that send interrupts without checking

[PATCH v2 06/15] virtio-blk: drop config_mutex

2014-10-06 Thread Michael S. Tsirkin
config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all workqueues non-reentrant all workqueues are non-reentrant, and config_enable is now

[PATCH v2 05/15] virtio_blk: drop config_enable

2014-10-06 Thread Michael S. Tsirkin
Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio blk. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify the driver, and will allow setting DRIVER_OK earlier without losing config change

[PATCH v2 07/15] virtio_net: drop config_enable

2014-10-06 Thread Michael S. Tsirkin
Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio net. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify the driver, and will allow setting DRIVER_OK earlier without losing config change

[PATCH v2 02/15] virtio: unify config_changed handling

2014-10-06 Thread Michael S. Tsirkin
Replace duplicated code in all transports with a single wrapper in virtio.c. The only functional change is in virtio_mmio.c: if a buggy device sends us an interrupt before driver is set, we previously returned IRQ_NONE, now we return IRQ_HANDLED. As this must not happen in practice, this does

[PATCH v2 09/15] virtio_net: minor cleanup

2014-10-06 Thread Michael S. Tsirkin
goto done; done: return; is ugly, it was put there to make diff review easier. replace by open-coded return. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Cornelia Huck cornelia.h...@de.ibm.com --- drivers/net/virtio_net.c | 6 ++ 1 file changed, 2

[PATCH v2 11/15] virtio_net: enable VQs early

2014-10-06 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio net violated this rule by using receive VQs within probe. To fix, call virtio_enable_vqs_early before using VQs. Signed-off-by: Michael S. Tsirkin m...@redhat.com ---

[PATCH v2 12/15] virtio_blk: enable VQs early

2014-10-06 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio block violated this rule by calling add_disk, which causes the VQ to be used directly within probe. To fix, call virtio_enable_vqs_early before using VQs. Signed-off-by: Michael

[PATCH v2 10/15] virtio: add API to enable VQs early

2014-10-06 Thread Michael S. Tsirkin
virtio spec 0.9.X requires DRIVER_OK to be set before VQs are used, but some drivers use VQs before probe function returns. Since DRIVER_OK is set after probe, this violates the spec. Even though under virtio 1.0 transitional devices support this behaviour, we want to make it possible for those

[PATCH v2 15/15] virtio_net: fix use after free on allocation failure

2014-10-06 Thread Michael S. Tsirkin
In the extremely unlikely event that driver initialization fails after RX buffers are added, virtio net frees RX buffers while VQs are still active, potentially causing device to use a freed buffer. To fix, reset device first - same as we do on device removal. Signed-off-by: Michael S. Tsirkin

[PATCH v2 13/15] virtio_console: enable VQs early

2014-10-06 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio console violated this rule by adding inbufs, which causes the VQ to be used directly within probe. To fix, call virtio_enable_vqs_early before using VQs. Signed-off-by: Michael

[PATCH v2 08/15] virtio-net: drop config_mutex

2014-10-06 Thread Michael S. Tsirkin
config_mutex served two purposes: prevent multiple concurrent config change handlers, and synchronize access to config_enable flag. Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1 workqueue: make all workqueues non-reentrant all workqueues are non-reentrant, and config_enable is now

Re: [PATCH v2 02/15] virtio: unify config_changed handling

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:10:44 +0300 Michael S. Tsirkin m...@redhat.com wrote: Replace duplicated code in all transports with a single wrapper in virtio.c. The only functional change is in virtio_mmio.c: if a buggy device sends us an interrupt before driver is set, we previously returned

Re: [PATCH v2 03/15] virtio-pci: move freeze/restore to virtio core

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:10:51 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is in preparation to extending config changed event handling in core. Wrapping these in an API also seems to make for a cleaner code. Signed-off-by: Michael S. Tsirkin m...@redhat.com ---

[PATCH v2 14/15] 9p/trans_virtio: enable VQs early

2014-10-06 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, but virtio 9p device adds self to channel list within probe, at which point VQ can be used in violation of the spec. To fix, call virtio_enable_vqs_early before using VQs.

Re: [PATCH v2 03/15] virtio-pci: move freeze/restore to virtio core

2014-10-06 Thread Michael S. Tsirkin
On Mon, Oct 06, 2014 at 05:20:55PM +0200, Cornelia Huck wrote: On Mon, 6 Oct 2014 18:10:51 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is in preparation to extending config changed event handling in core. Wrapping these in an API also seems to make for a cleaner code.

Re: [PATCH v2 04/15] virtio: defer config changed notifications

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:10:55 +0300 Michael S. Tsirkin m...@redhat.com wrote: Defer config changed notifications that arrive during probe/scan/freeze/restore. This will allow drivers to set DRIVER_OK earlier, without worrying about racing with config change interrupts. This change will

Re: [PATCH v2 05/15] virtio_blk: drop config_enable

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:10:58 +0300 Michael S. Tsirkin m...@redhat.com wrote: Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio blk. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify

Re: [PATCH v2 07/15] virtio_net: drop config_enable

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:11:05 +0300 Michael S. Tsirkin m...@redhat.com wrote: Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio net. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify the

Re: [PATCH v2 10/15] virtio: add API to enable VQs early

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:11:16 +0300 Michael S. Tsirkin m...@redhat.com wrote: virtio spec 0.9.X requires DRIVER_OK to be set before VQs are used, but some drivers use VQs before probe function returns. Since DRIVER_OK is set after probe, this violates the spec. Even though under virtio 1.0

Re: [PATCH v2 03/15] virtio-pci: move freeze/restore to virtio core

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:26:55 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Oct 06, 2014 at 05:20:55PM +0200, Cornelia Huck wrote: On Mon, 6 Oct 2014 18:10:51 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is in preparation to extending config changed event handling in

Re: [PATCH v2 11/15] virtio_net: enable VQs early

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:11:19 +0300 Michael S. Tsirkin m...@redhat.com wrote: virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio net violated this rule by using receive VQs within probe. To fix, call virtio_enable_vqs_early

Re: [PATCH v2 12/15] virtio_blk: enable VQs early

2014-10-06 Thread Cornelia Huck
On Mon, 6 Oct 2014 18:11:22 +0300 Michael S. Tsirkin m...@redhat.com wrote: virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio block violated this rule by calling add_disk, which causes the VQ to be used directly within probe.

Re: [PATCH 08/16] virtio_net: drop config_enable

2014-10-06 Thread David Miller
From: Michael S. Tsirkin m...@redhat.com Date: Sun, 5 Oct 2014 19:07:13 +0300 Now that virtio core ensures config changes don't arrive during probing, drop config_enable flag in virtio net. On removal, flush is now sufficient to guarantee that no change work is queued. This help simplify