* [PATCH] virtio: unify config_changed handling
@ 2014-09-22 7:20 Michael S. Tsirkin
2014-09-22 11:45 ` Christian Borntraeger
0 siblings, 1 reply; 2+ messages in thread
From: Michael S. Tsirkin @ 2014-09-22 7:20 UTC (permalink / raw)
To: linux-kernel
Cc: Christian Borntraeger, Cornelia Huck, linux390,
Martin Schwidefsky, Heiko Carstens, Rusty Russell, Ashutosh Dixit,
Greg Kroah-Hartman, Sudeep Dutt, Nikhil Rao,
Dasaratharaman Chandramouli, Wolfram Sang, Siva Yerramreddy,
Heinz Graalfs, linux-s390, virtualization
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 not look like a big deal.
See also commit 3fff0179e33cd7d0a688dab65700c46ad089e934
virtio-pci: do not oops on config change if driver not loaded.
for the original motivation behind the driver check.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio.h | 2 ++
drivers/misc/mic/card/mic_virtio.c | 6 +-----
drivers/s390/kvm/kvm_virtio.c | 9 +--------
drivers/s390/kvm/virtio_ccw.c | 6 +-----
drivers/virtio/virtio.c | 10 ++++++++++
drivers/virtio/virtio_mmio.c | 7 ++-----
drivers/virtio/virtio_pci.c | 6 +-----
7 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b46671e..3c19bd3 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -108,6 +108,8 @@ void unregister_virtio_device(struct virtio_device *dev);
void virtio_break_device(struct virtio_device *dev);
+void virtio_config_changed(struct virtio_device *dev);
+
/**
* virtio_driver - operations for a virtio I/O driver
* @driver: underlying device driver (populate name and owner).
diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c
index f14b600..e647947 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -462,16 +462,12 @@ static void mic_handle_config_change(struct mic_device_desc __iomem *d,
struct mic_device_ctrl __iomem *dc
= (void __iomem *)d + mic_aligned_desc_size(d);
struct mic_vdev *mvdev = (struct mic_vdev *)ioread64(&dc->vdev);
- struct virtio_driver *drv;
if (ioread8(&dc->config_change) != MIC_VIRTIO_PARAM_CONFIG_CHANGED)
return;
dev_dbg(mdrv->dev, "%s %d\n", __func__, __LINE__);
- drv = container_of(mvdev->vdev.dev.driver,
- struct virtio_driver, driver);
- if (drv->config_changed)
- drv->config_changed(&mvdev->vdev);
+ virtio_config_changed(&mvdev->vdev);
iowrite8(1, &dc->guest_ack);
}
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index a134965..6431290 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -406,15 +406,8 @@ static void kvm_extint_handler(struct ext_code ext_code,
switch (param) {
case VIRTIO_PARAM_CONFIG_CHANGED:
- {
- struct virtio_driver *drv;
- drv = container_of(vq->vdev->dev.driver,
- struct virtio_driver, driver);
- if (drv->config_changed)
- drv->config_changed(vq->vdev);
-
+ virtio_config_changed(vq->vdev);
break;
- }
case VIRTIO_PARAM_DEV_ADD:
schedule_work(&hotplug_work);
break;
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index d2c0b44..6cbe6ef 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -940,11 +940,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
vring_interrupt(0, vq);
}
if (test_bit(0, &vcdev->indicators2)) {
- drv = container_of(vcdev->vdev.dev.driver,
- struct virtio_driver, driver);
-
- if (drv && drv->config_changed)
- drv->config_changed(&vcdev->vdev);
+ virtio_config_changed(&vcdev->vdev);
clear_bit(0, &vcdev->indicators2);
}
}
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index fed0ce1..470b74f 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -239,6 +239,16 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
+void virtio_config_changed(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = container_of(dev->dev.driver,
+ struct virtio_driver, driver);
+
+ if (drv && drv->config_changed)
+ drv->config_changed(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_config_changed);
+
static int virtio_init(void)
{
if (bus_register(&virtio_bus) != 0)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index c600ccf..ef9a165 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -234,8 +234,6 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
{
struct virtio_mmio_device *vm_dev = opaque;
struct virtio_mmio_vq_info *info;
- struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
- struct virtio_driver, driver);
unsigned long status;
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
@@ -244,9 +242,8 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
- if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
- && vdrv && vdrv->config_changed) {
- vdrv->config_changed(&vm_dev->vdev);
+ if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
+ virtio_config_changed(&vm_dev->vdev);
ret = IRQ_HANDLED;
}
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..58f7e45 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -211,12 +211,8 @@ static bool vp_notify(struct virtqueue *vq)
static irqreturn_t vp_config_changed(int irq, void *opaque)
{
struct virtio_pci_device *vp_dev = opaque;
- struct virtio_driver *drv;
- drv = container_of(vp_dev->vdev.dev.driver,
- struct virtio_driver, driver);
- if (drv && drv->config_changed)
- drv->config_changed(&vp_dev->vdev);
+ virtio_config_changed(&vp_dev->vdev);
return IRQ_HANDLED;
}
--
MST
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] virtio: unify config_changed handling
2014-09-22 7:20 [PATCH] virtio: unify config_changed handling Michael S. Tsirkin
@ 2014-09-22 11:45 ` Christian Borntraeger
0 siblings, 0 replies; 2+ messages in thread
From: Christian Borntraeger @ 2014-09-22 11:45 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Cornelia Huck, linux390, Martin Schwidefsky, Heiko Carstens,
Rusty Russell, Ashutosh Dixit, Greg Kroah-Hartman, Sudeep Dutt,
Nikhil Rao, Dasaratharaman Chandramouli, Wolfram Sang,
Siva Yerramreddy, Heinz Graalfs, linux-s390, virtualization
On 09/22/2014 09:20 AM, Michael S. Tsirkin 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 IRQ_NONE,
> now we return IRQ_HANDLED.
>
> As this must not happen in practice, this does not look like a big deal.
>
> See also commit 3fff0179e33cd7d0a688dab65700c46ad089e934
> virtio-pci: do not oops on config change if driver not loaded.
> for the original motivation behind the driver check.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
please fold in the whitespace fix,
Reviewed-by: Christian Borntraeger <borntraeger@d.ibm.com> # s390 parts
> ---
> include/linux/virtio.h | 2 ++
> drivers/misc/mic/card/mic_virtio.c | 6 +-----
> drivers/s390/kvm/kvm_virtio.c | 9 +--------
> drivers/s390/kvm/virtio_ccw.c | 6 +-----
> drivers/virtio/virtio.c | 10 ++++++++++
> drivers/virtio/virtio_mmio.c | 7 ++-----
> drivers/virtio/virtio_pci.c | 6 +-----
> 7 files changed, 18 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index b46671e..3c19bd3 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -108,6 +108,8 @@ void unregister_virtio_device(struct virtio_device *dev);
>
> void virtio_break_device(struct virtio_device *dev);
>
> +void virtio_config_changed(struct virtio_device *dev);
> +
> /**
> * virtio_driver - operations for a virtio I/O driver
> * @driver: underlying device driver (populate name and owner).
> diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c
> index f14b600..e647947 100644
> --- a/drivers/misc/mic/card/mic_virtio.c
> +++ b/drivers/misc/mic/card/mic_virtio.c
> @@ -462,16 +462,12 @@ static void mic_handle_config_change(struct mic_device_desc __iomem *d,
> struct mic_device_ctrl __iomem *dc
> = (void __iomem *)d + mic_aligned_desc_size(d);
> struct mic_vdev *mvdev = (struct mic_vdev *)ioread64(&dc->vdev);
> - struct virtio_driver *drv;
>
> if (ioread8(&dc->config_change) != MIC_VIRTIO_PARAM_CONFIG_CHANGED)
> return;
>
> dev_dbg(mdrv->dev, "%s %d\n", __func__, __LINE__);
> - drv = container_of(mvdev->vdev.dev.driver,
> - struct virtio_driver, driver);
> - if (drv->config_changed)
> - drv->config_changed(&mvdev->vdev);
> + virtio_config_changed(&mvdev->vdev);
> iowrite8(1, &dc->guest_ack);
> }
>
> diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
> index a134965..6431290 100644
> --- a/drivers/s390/kvm/kvm_virtio.c
> +++ b/drivers/s390/kvm/kvm_virtio.c
> @@ -406,15 +406,8 @@ static void kvm_extint_handler(struct ext_code ext_code,
>
> switch (param) {
> case VIRTIO_PARAM_CONFIG_CHANGED:
> - {
> - struct virtio_driver *drv;
> - drv = container_of(vq->vdev->dev.driver,
> - struct virtio_driver, driver);
> - if (drv->config_changed)
> - drv->config_changed(vq->vdev);
> -
> + virtio_config_changed(vq->vdev);
> break;
> - }
> case VIRTIO_PARAM_DEV_ADD:
> schedule_work(&hotplug_work);
> break;
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index d2c0b44..6cbe6ef 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -940,11 +940,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
> vring_interrupt(0, vq);
> }
> if (test_bit(0, &vcdev->indicators2)) {
> - drv = container_of(vcdev->vdev.dev.driver,
> - struct virtio_driver, driver);
> -
> - if (drv && drv->config_changed)
> - drv->config_changed(&vcdev->vdev);
> + virtio_config_changed(&vcdev->vdev);
> clear_bit(0, &vcdev->indicators2);
> }
> }
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index fed0ce1..470b74f 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -239,6 +239,16 @@ void unregister_virtio_device(struct virtio_device *dev)
> }
> EXPORT_SYMBOL_GPL(unregister_virtio_device);
>
> +void virtio_config_changed(struct virtio_device *dev)
> +{
> + struct virtio_driver *drv = container_of(dev->dev.driver,
> + struct virtio_driver, driver);
> +
> + if (drv && drv->config_changed)
> + drv->config_changed(dev);
> +}
> +EXPORT_SYMBOL_GPL(virtio_config_changed);
> +
> static int virtio_init(void)
> {
> if (bus_register(&virtio_bus) != 0)
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index c600ccf..ef9a165 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -234,8 +234,6 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
> {
> struct virtio_mmio_device *vm_dev = opaque;
> struct virtio_mmio_vq_info *info;
> - struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
> - struct virtio_driver, driver);
> unsigned long status;
> unsigned long flags;
> irqreturn_t ret = IRQ_NONE;
> @@ -244,9 +242,8 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
> status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
> writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
>
> - if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
> - && vdrv && vdrv->config_changed) {
> - vdrv->config_changed(&vm_dev->vdev);
> + if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
> + virtio_config_changed(&vm_dev->vdev);
> ret = IRQ_HANDLED;
> }
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index 3d1463c..58f7e45 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -211,12 +211,8 @@ static bool vp_notify(struct virtqueue *vq)
> static irqreturn_t vp_config_changed(int irq, void *opaque)
> {
> struct virtio_pci_device *vp_dev = opaque;
> - struct virtio_driver *drv;
> - drv = container_of(vp_dev->vdev.dev.driver,
> - struct virtio_driver, driver);
>
> - if (drv && drv->config_changed)
> - drv->config_changed(&vp_dev->vdev);
> + virtio_config_changed(&vp_dev->vdev);
> return IRQ_HANDLED;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-09-22 11:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-22 7:20 [PATCH] virtio: unify config_changed handling Michael S. Tsirkin
2014-09-22 11:45 ` Christian Borntraeger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).