On Fri, Jan 31, 2025 at 10:53:15AM +0100, Eric Auger wrote:
> Hi Kirill, Michael
>
> On 8/8/24 9:51 AM, Kirill A. Shutemov wrote:
> > Hongyu reported a hang on kexec in a VM. QEMU reported invalid memory
> > accesses during the hang.
> >
> > Invalid read at addr 0x102877002, size 2, region '(null)', reason:
> > rejected
> > Invalid write at addr 0x102877A44, size 2, region '(null)', reason:
> > rejected
> > ...
> >
> > It was traced down to virtio-console. Kexec works fine if virtio-console
> > is not in use.
> >
> > Looks like virtio-console continues to write to the MMIO even after
> > underlying virtio-pci device is removed.
> >
> > The problem can be mitigated by removing all virtio devices on virtio
> > bus shutdown.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shute...@linux.intel.com>
> > Reported-by: Hongyu Ning <hongyu.n...@linux.intel.com>
>
> Gentle ping on that patch that seems to have fallen though the cracks.
>
> I think this fix is really needed. I have another test case with a
> rebooting guest exposed with virtio-net (backed by vhost-net) and
> viommu. Since there is currently no shutdown for the virtio-net, on
> reboot, the IOMMU is disabled through the native_machine_shutdown()/
> x86_platform.iommu_shutdown() while the virtio-net is still alive.
>
> Normally device_shutdown() should call virtio-net shutdown before the
> IOMMU tear down and we wouldn't see any spurious transactions after
> iommu shutdown.
>
> With that fix, the above test case is fixed and I do not see spurious
> vhost IOTLB miss spurious requests.
>
> For more details, see qemu thread ([PATCH] hw/virtio/vhost: Disable
> IOTLB callbacks when IOMMU gets disabled,
> https://lore.kernel.org/all/20250120173339.865681-1-eric.au...@redhat.com/)
>
>
> Reviewed-by: Eric Auger <eric.au...@redhat.com>
> Tested-by: Eric Auger <eric.au...@redhat.com>
>
> Thanks
>
> Eric
>
> > ---
> > drivers/virtio/virtio.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > index a9b93e99c23a..6c2f908eb22c 100644
> > --- a/drivers/virtio/virtio.c
> > +++ b/drivers/virtio/virtio.c
> > @@ -356,6 +356,15 @@ static void virtio_dev_remove(struct device *_d)
> > of_node_put(dev->dev.of_node);
> > }
> >
> > +static void virtio_dev_shutdown(struct device *_d)
> > +{
> > + struct virtio_device *dev = dev_to_virtio(_d);
> > + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> > +
> > + if (drv && drv->remove)
> > + drv->remove(dev);
I am concerned that full remove is a heavyweight operation.
Do not want to slow down reboots even more.
How about just doing a reset, instead?
> > +}
> > +
> > static const struct bus_type virtio_bus = {
> > .name = "virtio",
> > .match = virtio_dev_match,
> > @@ -363,6 +372,7 @@ static const struct bus_type virtio_bus = {
> > .uevent = virtio_uevent,
> > .probe = virtio_dev_probe,
> > .remove = virtio_dev_remove,
> > + .shutdown = virtio_dev_shutdown,
> > };
> >
> > int __register_virtio_driver(struct virtio_driver *driver, struct module
> > *owner)
