On Mon, Sep 13, 2021 at 09:38:30PM +0200, Thomas Gleixner wrote:> On Mon, Sep 13 2021 at 15:07, Jason Wang wrote: > > On Mon, Sep 13, 2021 at 2:50 PM Michael S. Tsirkin <mst at redhat.com> wrote: > >> > But doen't "irq is disabled" basically mean "we told the hypervisor > >> > to disable the irq"? What extractly prevents hypervisor from > >> > sending the irq even if guest thinks it disabled it? > >> > >> More generally, can't we for example blow away the > >> indir_desc array that we use to keep the ctx pointers? > >> Won't that be enough? > > > > I'm not sure how it is related to the indirect descriptor but an > > example is that all the current driver will assume: > > > > 1) the interrupt won't be raised before virtio_device_ready() > > 2) the interrupt won't be raised after reset() > > If that assumption exists, then you better keep the interrupt line > disabled until virtio_device_ready() has completedstarted not completed. device is allowed to send config interrupts right after DRIVER_OK status is set by virtio_device_ready.> and disable it again > before reset() is invoked. That's a question of general robustness and > not really a question of trusted hypervisors and encrypted guests.We can do this for some MSIX interrupts, sure. Not for shared interrupts though.> >> > > > > > > +void vp_disable_vectors(struct virtio_device *vdev) > >> > > > > > > { > >> > > > > > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > >> > > > > > > int i; > >> > > > > > > @@ -34,7 +34,20 @@ void vp_synchronize_vectors(struct virtio_device *vdev) > >> > > > > > > synchronize_irq(vp_dev->pci_dev->irq); > > Don't you want the same change for non-MSI interrupts?We can't disable them - these are shared.> Thanks, > > tglx
On Mon, Sep 13 2021 at 16:54, Michael S. Tsirkin wrote:> On Mon, Sep 13, 2021 at 09:38:30PM +0200, Thomas Gleixner wrote: >> On Mon, Sep 13 2021 at 15:07, Jason Wang wrote: >> > On Mon, Sep 13, 2021 at 2:50 PM Michael S. Tsirkin <mst at redhat.com> wrote: >> >> > But doen't "irq is disabled" basically mean "we told the hypervisor >> >> > to disable the irq"? What extractly prevents hypervisor from >> >> > sending the irq even if guest thinks it disabled it? >> >> >> >> More generally, can't we for example blow away the >> >> indir_desc array that we use to keep the ctx pointers? >> >> Won't that be enough? >> > >> > I'm not sure how it is related to the indirect descriptor but an >> > example is that all the current driver will assume: >> > >> > 1) the interrupt won't be raised before virtio_device_ready() >> > 2) the interrupt won't be raised after reset() >> >> If that assumption exists, then you better keep the interrupt line >> disabled until virtio_device_ready() has completed > > started not completed. device is allowed to send > config interrupts right after DRIVER_OK status is set by > virtio_device_ready.Whatever: * Define the exact point from which on the driver is able to handle the interrupt and put the enable after that point * Define the exact point from which on the driver is unable to handle the interrupt and put the disable before that point The above is blury.>> and disable it again >> before reset() is invoked. That's a question of general robustness and >> not really a question of trusted hypervisors and encrypted guests. > > We can do this for some MSIX interrupts, sure. Not for shared interrupts though.See my reply to the next patch. The problem is the same: * Define the exact point from which on the driver is able to handle the interrupt and allow the handler to proceed after that point * Define the exact point from which on the driver is unable to handle the interrupt and ensure that the handler denies to proceed before that point Same story just a different mechanism. Thanks, tglx
On Mon, Sep 13 2021 at 16:54, Michael S. Tsirkin wrote:> On Mon, Sep 13, 2021 at 09:38:30PM +0200, Thomas Gleixner wrote: >> and disable it again >> before reset() is invoked. That's a question of general robustness and >> not really a question of trusted hypervisors and encrypted guests. > > We can do this for some MSIX interrupts, sure. Not for shared interrupts though.But you have to make sure that the handler does not run before and after the defined points. And that's even more important for shared because with shared interrupts the interrupt can be raised at any point in time via the other devices which share the line. Thanks, tglx