On Wed, Mar 30, 2022 at 10:38:06AM +0800, Jason Wang wrote: > On Wed, Mar 30, 2022 at 6:04 AM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > > > On Tue, Mar 29, 2022 at 08:13:57PM +0200, Thomas Gleixner wrote: > > > On Tue, Mar 29 2022 at 10:37, Michael S. Tsirkin wrote: > > > > On Tue, Mar 29, 2022 at 10:35:21AM +0200, Thomas Gleixner wrote: > > > > We are trying to fix the driver since at the moment it does not > > > > have the dev->ok flag at all. > > > > > > > > And I suspect virtio is not alone in that. > > > > So it would have been nice if there was a standard flag > > > > replacing the driver-specific dev->ok above, and ideally > > > > would also handle the case of an interrupt triggering > > > > too early by deferring the interrupt until the flag is set. > > > > > > > > And in fact, it does kind of exist: IRQF_NO_AUTOEN, and you would call > > > > enable_irq instead of dev->ok = true, except > > > > - it doesn't work with affinity managed IRQs > > > > - it does not work with shared IRQs > > > > > > > > So using dev->ok as you propose above seems better at this point. > > > > > > Unless there is a big enough amount of drivers which could make use of a > > > generic mechanism for that. > > > > > > >> If any driver does this in the wrong order, then the driver is > > > >> broken. > > > > > > > > I agree, however: > > > > $ git grep synchronize_irq `git grep -l request_irq drivers/net/`|wc -l > > > > 113 > > > > $ git grep -l request_irq drivers/net/|wc -l > > > > 397 > > > > > > > > I suspect there are more drivers which in theory need the > > > > synchronize_irq dance but in practice do not execute it. > > > > > > That really depends on when the driver requests the interrupt, when > > > it actually enables the interrupt in the device itself > > > > This last point does not matter since we are talking about protecting > > against buggy/malicious devices. They can inject the interrupt anyway > > even if driver did not configure it. > > > > > and how the > > > interrupt service routine works. > > > > > > So just doing that grep dance does not tell much. You really have to do > > > a case by case analysis. > > > > > > Thanks, > > > > > > tglx > > > > > > I agree. In fact, at least for network the standard approach is to > > request interrupts in the open call, virtio net is unusual > > in doing it in probe. We should consider changing that. > > Jason? > > This probably works only for virtio-net and it looks like not trivial > since we don't have a specific core API to request interrupts. > > Thanks We'll need a new API, for sure. E.g. find vqs with no callback on probe, and then virtio_request_vq_callbacks separately. The existing API that specifies callbacks during find vqs can be used by other drivers. > > > > -- > > MST > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
