| Message ID | 20240716094619.1713905-3-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VIRTIO-IOMMU/VFIO: Revert IOMMUDevice clear and fix hotunplug | expand |
On 7/16/24 11:45, Eric Auger wrote: > Now we have switched to PCIIOMMUOps to convey host IOMMU information, > the host reserved regions are transmitted when the PCIe topology is > built. This happens way before the virtio-iommu driver calls the probe > request. So let's remove the probe_done flag that allowed to check > the probe was not done before the IOMMU MR got enabled. Besides this > probe_done flag had a flaw wrt migration since it was not saved/restored. > > The only case at risk is if 2 devices were plugged to a > PCIe to PCI bridge and thus aliased. First of all we > discovered in the past this case was not properly supported for > neither SMMU nor virtio-iommu on guest kernel side: see > > [RFC] virtio-iommu: Take into account possible aliasing in virtio_iommu_mr() > https://lore.kernel.org/all/20230116124709.793084-1-eric.auger@redhat.com/ > > If this were supported by the guest kernel, it is unclear what the call > sequence would be from a virtio-iommu driver point of view. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > include/hw/virtio/virtio-iommu.h | 1 - > hw/virtio/virtio-iommu.c | 3 --- > 2 files changed, 4 deletions(-) > > diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h > index bdb3da72d0..7db4210b16 100644 > --- a/include/hw/virtio/virtio-iommu.h > +++ b/include/hw/virtio/virtio-iommu.h > @@ -43,7 +43,6 @@ typedef struct IOMMUDevice { > MemoryRegion bypass_mr; /* The alias of shared memory MR */ > GList *resv_regions; > GList *host_resv_ranges; > - bool probe_done; > } IOMMUDevice; > > typedef struct IOMMUPciBus { > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > index 4e34dacd6e..2c54c0d976 100644 > --- a/hw/virtio/virtio-iommu.c > +++ b/hw/virtio/virtio-iommu.c > @@ -555,8 +555,6 @@ static int virtio_iommu_set_host_iova_ranges(VirtIOIOMMU *s, PCIBus *bus, > > current_ranges = sdev->host_resv_ranges; > > - g_assert(!sdev->probe_done); > - > /* check that each new resv region is included in an existing one */ > if (sdev->host_resv_ranges) { > range_inverse_array(iova_ranges, > @@ -956,7 +954,6 @@ static int virtio_iommu_probe(VirtIOIOMMU *s, > } > buf += count; > free -= count; > - sdev->probe_done = true; > > return VIRTIO_IOMMU_S_OK; > }
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h index bdb3da72d0..7db4210b16 100644 --- a/include/hw/virtio/virtio-iommu.h +++ b/include/hw/virtio/virtio-iommu.h @@ -43,7 +43,6 @@ typedef struct IOMMUDevice { MemoryRegion bypass_mr; /* The alias of shared memory MR */ GList *resv_regions; GList *host_resv_ranges; - bool probe_done; } IOMMUDevice; typedef struct IOMMUPciBus { diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 4e34dacd6e..2c54c0d976 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -555,8 +555,6 @@ static int virtio_iommu_set_host_iova_ranges(VirtIOIOMMU *s, PCIBus *bus, current_ranges = sdev->host_resv_ranges; - g_assert(!sdev->probe_done); - /* check that each new resv region is included in an existing one */ if (sdev->host_resv_ranges) { range_inverse_array(iova_ranges, @@ -956,7 +954,6 @@ static int virtio_iommu_probe(VirtIOIOMMU *s, } buf += count; free -= count; - sdev->probe_done = true; return VIRTIO_IOMMU_S_OK; }
Now we have switched to PCIIOMMUOps to convey host IOMMU information, the host reserved regions are transmitted when the PCIe topology is built. This happens way before the virtio-iommu driver calls the probe request. So let's remove the probe_done flag that allowed to check the probe was not done before the IOMMU MR got enabled. Besides this probe_done flag had a flaw wrt migration since it was not saved/restored. The only case at risk is if 2 devices were plugged to a PCIe to PCI bridge and thus aliased. First of all we discovered in the past this case was not properly supported for neither SMMU nor virtio-iommu on guest kernel side: see [RFC] virtio-iommu: Take into account possible aliasing in virtio_iommu_mr() https://lore.kernel.org/all/20230116124709.793084-1-eric.auger@redhat.com/ If this were supported by the guest kernel, it is unclear what the call sequence would be from a virtio-iommu driver point of view. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- include/hw/virtio/virtio-iommu.h | 1 - hw/virtio/virtio-iommu.c | 3 --- 2 files changed, 4 deletions(-)