Hi Cam,

Thanks for investigating this

On Thu, Nov 05, 2026 at 03:54:20PM -0500, Cam Miller wrote:
> Hi,
> 
> 
> Testing on x86, I noticed that userspace supplies a mechanism for 
> hot-removing a virtio-iommu device.
> 
>   echo 1 > /sys/bus/pci/devices/<virtio_iommu_dev>/remove
> 
> Hot-removing virtio-iommu should NOT be supported, right? The intention is 
> clear in the QEMU source,
> which fences off hot(un)plugging virtio-iommu devices.

We would also have to prevent:

* unbinding the driver. For that we can use .suppress_bind_attrs

* unbinding the transport driver: virtio-pci, virtio-mmio

* possibly other weird cases like PCI bridges upstream of the IOMMU
  getting removed

Module removal is fine because the IOMMU core holds a module ref for
each endpoint.

> If I am correct, then shall we fence off hot-removal (perhaps by simply 
> omitting this remove file
> from sysfs)?

That would be a PCI core change, I guess?

I'd much prefer handling forced removal as gently as possible in the
viommu driver, but as you've noticed, we're not there.

> Else, keep reading...
> 
> ...I believe I have found two bugs in viommu_remove() path that I would like 
> to bring attention to.
> The first bug is that the echo above causes the following kernel panic.

Yes please send a patch

[...]
> After applying the fix shown above, a second problem is exposed. 
> Unregistering the IOMMU while
> endpoints remain active is like pulling out the rug from underneath those 
> endpoints. The following
> warning catches this case.
> 
>   WARNING: drivers/iommu/iommu.c:743 at 
> __iommu_group_free_device.constprop.0+0xb9/0xe0, CPU#1:
> bash/1250
>   --- snip ---
>   Call Trace:
>    <TASK>
>    __iommu_group_remove_device+0x95/0xe0
>    ? __pfx_remove_iommu_group+0x10/0x10
>    remove_iommu_group+0x39/0x60
>    bus_for_each_dev+0x85/0xd0
>    iommu_device_unregister+0x3d/0xb0
>    viommu_remove+0x1d/0x60
>   --- snip ---

Device links would likely prevent this sort of thing. I worked on it a
while ago but at the time I was hoping the new fw_devlink would
automatically create those links. Unfortunately there was (is?) at least
a problem with having the IOMMU on the same bus as the endpoints. So we
should add explicit device_link_add() in the driver, to force endpoint
unbind before viommu unbind.

> ...Point being that hot-removing virtio-iommu is currently broken.
> 
> In summary:
> - Should hot-removal be supported/allowed?

Reluctantly allowed, I think

> - If no, then how shall we fence it off? Via sysfs?
> - Else, there are some bugs that need to be fixed (and I would be happy to 
> work on that if that is
> not already evident).

That would be really welcome!

Thanks,
Jean

Reply via email to