| Message ID | 20200908193309.20569-3-eric.auger@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio-iommu-pci: Advertise the device as modern-only | expand |
On Tue, 8 Sep 2020 21:33:09 +0200 Eric Auger <eric.auger@redhat.com> wrote: > Commit 9b3a35ec82 ("virtio: verify that legacy support is not > accidentally on") added a safety check that requires to set > 'disable-legacy=on' on virtio-iommu-pci: > > qemu-system-aarch64: -device virtio-iommu-pci: device is modern-only, > use disable-legacy=on > > virtio-iommu was introduced after the release of VIRTIO 1.0 > specifications, so it should be 'modern-only'. > > This patch forces virtio version 1 and removes the 'transitional_name' > property removing the need to specify 'disable-legacy=on' on > virtio-iommu-pci device. Not sure whether this patch has been queued already, and how much we care about migration compatibility for virtio-iommu, but would it make sense to force modern on 5.1+ compat machines only? (see https://lore.kernel.org/qemu-devel/20200918074710.27810-1-sgarzare@redhat.com/) > > Cc: qemu-stable@nongnu.org > Signed-off-by: Eric Auger <eric.auger@redhat.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > --- > v1 -> v2: > - Added Connie's R-b > --- > hw/virtio/virtio-iommu-pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c > index ba62d60a0a..3b6f7a11c6 100644 > --- a/hw/virtio/virtio-iommu-pci.c > +++ b/hw/virtio/virtio-iommu-pci.c > @@ -68,6 +68,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > object_property_set_link(OBJECT(dev), "primary-bus", > OBJECT(pci_get_bus(&vpci_dev->pci_dev)), > &error_abort); > + virtio_pci_force_virtio_1(vpci_dev); > qdev_realize(vdev, BUS(&vpci_dev->bus), errp); > } > > @@ -97,7 +98,6 @@ static void virtio_iommu_pci_instance_init(Object *obj) > static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = { > .base_name = TYPE_VIRTIO_IOMMU_PCI, > .generic_name = "virtio-iommu-pci", > - .transitional_name = "virtio-iommu-pci-transitional", > .non_transitional_name = "virtio-iommu-pci-non-transitional", > .instance_size = sizeof(VirtIOIOMMUPCI), > .instance_init = virtio_iommu_pci_instance_init,
Hi Connie, On 9/18/20 11:29 AM, Cornelia Huck wrote: > On Tue, 8 Sep 2020 21:33:09 +0200 > Eric Auger <eric.auger@redhat.com> wrote: > >> Commit 9b3a35ec82 ("virtio: verify that legacy support is not >> accidentally on") added a safety check that requires to set >> 'disable-legacy=on' on virtio-iommu-pci: >> >> qemu-system-aarch64: -device virtio-iommu-pci: device is modern-only, >> use disable-legacy=on >> >> virtio-iommu was introduced after the release of VIRTIO 1.0 >> specifications, so it should be 'modern-only'. >> >> This patch forces virtio version 1 and removes the 'transitional_name' >> property removing the need to specify 'disable-legacy=on' on >> virtio-iommu-pci device. > > Not sure whether this patch has been queued already, and how much we > care about migration compatibility for virtio-iommu, but would it make > sense to force modern on 5.1+ compat machines only? (see > https://lore.kernel.org/qemu-devel/20200918074710.27810-1-sgarzare@redhat.com/) I don't think it was pulled yet. > >> >> Cc: qemu-stable@nongnu.org The virtio-iommu-pci device only is usable on ARM in dt mode so I don't think it has production users at the moment. Thanks Eric >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> >> --- >> v1 -> v2: >> - Added Connie's R-b >> --- >> hw/virtio/virtio-iommu-pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c >> index ba62d60a0a..3b6f7a11c6 100644 >> --- a/hw/virtio/virtio-iommu-pci.c >> +++ b/hw/virtio/virtio-iommu-pci.c >> @@ -68,6 +68,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) >> object_property_set_link(OBJECT(dev), "primary-bus", >> OBJECT(pci_get_bus(&vpci_dev->pci_dev)), >> &error_abort); >> + virtio_pci_force_virtio_1(vpci_dev); >> qdev_realize(vdev, BUS(&vpci_dev->bus), errp); >> } >> >> @@ -97,7 +98,6 @@ static void virtio_iommu_pci_instance_init(Object *obj) >> static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = { >> .base_name = TYPE_VIRTIO_IOMMU_PCI, >> .generic_name = "virtio-iommu-pci", >> - .transitional_name = "virtio-iommu-pci-transitional", >> .non_transitional_name = "virtio-iommu-pci-non-transitional", >> .instance_size = sizeof(VirtIOIOMMUPCI), >> .instance_init = virtio_iommu_pci_instance_init, > >
On Fri, 18 Sep 2020 12:24:02 +0200 Auger Eric <eric.auger@redhat.com> wrote: > Hi Connie, > > On 9/18/20 11:29 AM, Cornelia Huck wrote: > > On Tue, 8 Sep 2020 21:33:09 +0200 > > Eric Auger <eric.auger@redhat.com> wrote: > > > >> Commit 9b3a35ec82 ("virtio: verify that legacy support is not > >> accidentally on") added a safety check that requires to set > >> 'disable-legacy=on' on virtio-iommu-pci: > >> > >> qemu-system-aarch64: -device virtio-iommu-pci: device is modern-only, > >> use disable-legacy=on > >> > >> virtio-iommu was introduced after the release of VIRTIO 1.0 > >> specifications, so it should be 'modern-only'. > >> > >> This patch forces virtio version 1 and removes the 'transitional_name' > >> property removing the need to specify 'disable-legacy=on' on > >> virtio-iommu-pci device. > > > > Not sure whether this patch has been queued already, and how much we > > care about migration compatibility for virtio-iommu, but would it make > > sense to force modern on 5.1+ compat machines only? (see > > https://lore.kernel.org/qemu-devel/20200918074710.27810-1-sgarzare@redhat.com/) > > I don't think it was pulled yet. > > > >> > >> Cc: qemu-stable@nongnu.org > > The virtio-iommu-pci device only is usable on ARM in dt mode so I don't > think it has production users at the moment. OK, then we can keep this patch here, I guess.
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c index ba62d60a0a..3b6f7a11c6 100644 --- a/hw/virtio/virtio-iommu-pci.c +++ b/hw/virtio/virtio-iommu-pci.c @@ -68,6 +68,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(pci_get_bus(&vpci_dev->pci_dev)), &error_abort); + virtio_pci_force_virtio_1(vpci_dev); qdev_realize(vdev, BUS(&vpci_dev->bus), errp); } @@ -97,7 +98,6 @@ static void virtio_iommu_pci_instance_init(Object *obj) static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = { .base_name = TYPE_VIRTIO_IOMMU_PCI, .generic_name = "virtio-iommu-pci", - .transitional_name = "virtio-iommu-pci-transitional", .non_transitional_name = "virtio-iommu-pci-non-transitional", .instance_size = sizeof(VirtIOIOMMUPCI), .instance_init = virtio_iommu_pci_instance_init,
Commit 9b3a35ec82 ("virtio: verify that legacy support is not accidentally on") added a safety check that requires to set 'disable-legacy=on' on virtio-iommu-pci: qemu-system-aarch64: -device virtio-iommu-pci: device is modern-only, use disable-legacy=on virtio-iommu was introduced after the release of VIRTIO 1.0 specifications, so it should be 'modern-only'. This patch forces virtio version 1 and removes the 'transitional_name' property removing the need to specify 'disable-legacy=on' on virtio-iommu-pci device. Cc: qemu-stable@nongnu.org Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> --- v1 -> v2: - Added Connie's R-b --- hw/virtio/virtio-iommu-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)