Hi All:
This is a rework on the IRQ hardening for virtio which is done
previously by the following commits are reverted:
9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
080cd7c3ac87 ("virtio-pci: harden INTX interrupts")
The reason is that it depends on the IRQF_NO_AUTOEN which may conflict
with the assumption of the affinity managed IRQ that is used by some
virtio drivers. And what's more, it is only done for virtio-pci but
not other transports.
In this rework, I try to implement a general virtio solution which
borrows the idea of the INTX hardening by re-using per virtqueue
boolean vq->broken and toggle it in virtio_device_ready() and
virtio_reset_device(). Then we can simply reuse the existing checks in
the vring_interrupt() and return early if the driver is not ready.
Note that, I only did compile test on ccw and MMIO transport.
Please review.
Changes since V4:
- use spin_lock_irq()/spin_unlock_irq() to synchronize with
vring_interrupt() for ccw
- use spin_lock()/spin_unlock() to protect vring_interrupt() for non
airq
- add comment to explain the ordering implications of set_status() for
PCI, ccw and MMIO
- various tweaks on the comments and changelogs
Changes since V3:
- Rename synchornize_vqs() to synchronize_cbs()
- tweak the comment for synchronize_cbs()
- switch to use a dedicated helper __virtio_unbreak_device() and
document it should be only used for probing
- switch to use rwlock to synchornize the non airq for ccw
Changes since V2:
- add ccw and MMIO support
- rename synchronize_vqs() to synchronize_cbs()
- switch to re-use vq->broken instead of introducing new device
attributes for the future virtqueue reset support
- remove unnecssary READ_ONCE()/WRITE_ONCE()
- a new patch to remove device triggerable BUG_ON()
- more tweaks on the comments
Changes since v1:
- Use transport specific irq synchronization method when possible
- Drop the module parameter and enable the hardening unconditonally
- Tweak the barrier/ordering facilities used in the code
- Reanme irq_soft_enabled to driver_ready
- Avoid unnecssary IRQ synchornization (e.g during boot)
Jason Wang (8):
virtio: use virtio_reset_device() when possible
virtio: introduce config op to synchronize vring callbacks
virtio-pci: implement synchronize_cbs()
virtio-mmio: implement synchronize_cbs()
virtio-ccw: implement synchronize_cbs()
virtio: allow to unbreak virtqueue
virtio: harden vring IRQ
virtio: use WARN_ON() to warning illegal status value
Stefano Garzarella (1):
virtio: use virtio_device_ready() in virtio_device_restore()
drivers/s390/virtio/virtio_ccw.c | 31 +++++++++++++++++
drivers/virtio/virtio.c | 24 +++++++++----
drivers/virtio/virtio_mmio.c | 14 ++++++++
drivers/virtio/virtio_pci_legacy.c | 1 +
drivers/virtio/virtio_pci_modern.c | 2 ++
drivers/virtio/virtio_pci_modern_dev.c | 5 +++
drivers/virtio/virtio_ring.c | 32 +++++++++++++++---
include/linux/virtio.h | 1 +
include/linux/virtio_config.h | 47 +++++++++++++++++++++++++-
9 files changed, 145 insertions(+), 12 deletions(-)
--
2.25.1
From: Stefano Garzarella <[email protected]>
It will allow us to do extension on virtio_device_ready() without
duplicating code.
Cc: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Halil Pasic <[email protected]>
Cc: Cornelia Huck <[email protected]>
Cc: Vineeth Vijayan <[email protected]>
Cc: Peter Oberparleiter <[email protected]>
Cc: [email protected]
Reviewed-by: Cornelia Huck <[email protected]>
Signed-off-by: Stefano Garzarella <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
---
drivers/virtio/virtio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 22f15f444f75..75c8d560bbd3 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -526,8 +526,9 @@ int virtio_device_restore(struct virtio_device *dev)
goto err;
}
- /* Finally, tell the device we're all set */
- virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+ /* If restore didn't do it, mark device DRIVER_OK ourselves. */
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ virtio_device_ready(dev);
virtio_config_enable(dev);
--
2.25.1
On Wed, 18 May 2022 11:59:42 +0800
Jason Wang <[email protected]> wrote:
> Hi All:
Sorry for being slow on this one. I'm pretty much under water. Will try
to get some regression-testing done till tomorrow end of day.
Regards,
Halil
On Wed, May 25, 2022 at 12:28 AM Halil Pasic <[email protected]> wrote:
>
> On Mon, 23 May 2022 10:53:23 +0200
> Halil Pasic <[email protected]> wrote:
>
> > On Wed, 18 May 2022 11:59:42 +0800
> > Jason Wang <[email protected]> wrote:
> >
> > > Hi All:
> >
> > Sorry for being slow on this one. I'm pretty much under water. Will try
> > to get some regression-testing done till tomorrow end of day.
> >
>
> Did some testing with the two stage indicators disabled. Didn't see any
> significant difference in performance, and with that also no performance
> regression. IMHO we are good to go ahead!
Great!
>
> Sorry it took so long.
No worries and thanks a lot for the help.
I will repost a version with some comments tweaked that is suggested
by Cornelia Huck.
Thanks
>
> Regards,
> Halil
>
On Mon, 23 May 2022 10:53:23 +0200
Halil Pasic <[email protected]> wrote:
> On Wed, 18 May 2022 11:59:42 +0800
> Jason Wang <[email protected]> wrote:
>
> > Hi All:
>
> Sorry for being slow on this one. I'm pretty much under water. Will try
> to get some regression-testing done till tomorrow end of day.
>
Did some testing with the two stage indicators disabled. Didn't see any
significant difference in performance, and with that also no performance
regression. IMHO we are good to go ahead!
Sorry it took so long.
Regards,
Halil