| Message ID | 20220930135810.1892149-5-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pci-ids: virtio cleanup | expand |
Hi Gerd, On Fri, Sep 30, 2022 at 03:58:09PM +0200, Gerd Hoffmann wrote: >Not needed for a virtio 1.0 device. > >Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >--- > include/hw/pci/pci.h | 1 - > hw/virtio/vhost-user-vsock-pci.c | 2 -- > hw/virtio/vhost-vsock-pci.c | 2 -- > 3 files changed, 5 deletions(-) > >diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >index 42c83cb5ed00..fe103f35d9d6 100644 >--- a/include/hw/pci/pci.h >+++ b/include/hw/pci/pci.h >@@ -83,7 +83,6 @@ extern bool pci_available; > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 >-#define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012 > > #define PCI_VENDOR_ID_REDHAT 0x1b36 > #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001 >diff --git a/hw/virtio/vhost-user-vsock-pci.c b/hw/virtio/vhost-user-vsock-pci.c >index e5a86e801362..8499b6871f50 100644 >--- a/hw/virtio/vhost-user-vsock-pci.c >+++ b/hw/virtio/vhost-user-vsock-pci.c >@@ -55,8 +55,6 @@ static void vhost_user_vsock_pci_class_init(ObjectClass *klass, void *data) > k->realize = vhost_user_vsock_pci_realize; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > device_class_set_props(dc, vhost_user_vsock_pci_properties); >- pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; >- pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; > pcidev_k->revision = 0x00; > pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; > } >diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c >index 9f34414d3814..170a806b6765 100644 >--- a/hw/virtio/vhost-vsock-pci.c >+++ b/hw/virtio/vhost-vsock-pci.c >@@ -65,8 +65,6 @@ static void vhost_vsock_pci_class_init(ObjectClass *klass, void *data) > k->realize = vhost_vsock_pci_realize; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > device_class_set_props(dc, vhost_vsock_pci_properties); >- pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; >- pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; > pcidev_k->revision = 0x00; > pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; > } Could we have migration issues with this change? This reminded me that we've had issues already with vsock being incorrectly exported as legacy, that we discovered when we added commit 9b3a35ec82 ("virtio: verify that legacy support is not accidentally on"). Then we needed commit d55f518248 ("virtio: skip legacy support check on machine types less than 5.1") to avoid migration issues. And we merged the following commits to force 1.0 in virtio-vsock devices for machine types >= 5.1 : - 6209070503 ("vhost-vsock-pci: force virtio version 1") - 27eda699f5 ("vhost-user-vsock-pci: force virtio version 1") Thanks, Stefano
> > diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c > > index 9f34414d3814..170a806b6765 100644 > > --- a/hw/virtio/vhost-vsock-pci.c > > +++ b/hw/virtio/vhost-vsock-pci.c > > @@ -65,8 +65,6 @@ static void vhost_vsock_pci_class_init(ObjectClass *klass, void *data) > > k->realize = vhost_vsock_pci_realize; > > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > > device_class_set_props(dc, vhost_vsock_pci_properties); > > - pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; > > - pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; > > pcidev_k->revision = 0x00; > > pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; > > } > > Could we have migration issues with this change? > > This reminded me that we've had issues already with vsock being incorrectly > exported as legacy, that we discovered when we added commit 9b3a35ec82 > ("virtio: verify that legacy support is not accidentally on"). > > Then we needed commit d55f518248 ("virtio: skip legacy support check on > machine types less than 5.1") to avoid migration issues. > > And we merged the following commits to force 1.0 in virtio-vsock devices for > machine types >= 5.1 : > - 6209070503 ("vhost-vsock-pci: force virtio version 1") > - 27eda699f5 ("vhost-user-vsock-pci: force virtio version 1") Oh, the virtio_pci_force_virtio_1() call is conditional. Hmm. The change will break vsock devices in legacy/transitional mode. So, if that is allowed for old machine types for backward compatibility reasons I guess I should better drop this patch. take care, Gerd
On Tue, Oct 04 2022, Gerd Hoffmann <kraxel@redhat.com> wrote: >> > diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c >> > index 9f34414d3814..170a806b6765 100644 >> > --- a/hw/virtio/vhost-vsock-pci.c >> > +++ b/hw/virtio/vhost-vsock-pci.c >> > @@ -65,8 +65,6 @@ static void vhost_vsock_pci_class_init(ObjectClass *klass, void *data) >> > k->realize = vhost_vsock_pci_realize; >> > set_bit(DEVICE_CATEGORY_MISC, dc->categories); >> > device_class_set_props(dc, vhost_vsock_pci_properties); >> > - pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; >> > - pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; >> > pcidev_k->revision = 0x00; >> > pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; >> > } >> >> Could we have migration issues with this change? >> >> This reminded me that we've had issues already with vsock being incorrectly >> exported as legacy, that we discovered when we added commit 9b3a35ec82 >> ("virtio: verify that legacy support is not accidentally on"). >> >> Then we needed commit d55f518248 ("virtio: skip legacy support check on >> machine types less than 5.1") to avoid migration issues. >> >> And we merged the following commits to force 1.0 in virtio-vsock devices for >> machine types >= 5.1 : >> - 6209070503 ("vhost-vsock-pci: force virtio version 1") >> - 27eda699f5 ("vhost-user-vsock-pci: force virtio version 1") > > Oh, the virtio_pci_force_virtio_1() call is conditional. Hmm. > > The change will break vsock devices in legacy/transitional mode. So, if > that is allowed for old machine types for backward compatibility reasons > I guess I should better drop this patch. Maybe add a comment to prevent others from falling into the same trap?
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 42c83cb5ed00..fe103f35d9d6 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -83,7 +83,6 @@ extern bool pci_available; #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 -#define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012 #define PCI_VENDOR_ID_REDHAT 0x1b36 #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001 diff --git a/hw/virtio/vhost-user-vsock-pci.c b/hw/virtio/vhost-user-vsock-pci.c index e5a86e801362..8499b6871f50 100644 --- a/hw/virtio/vhost-user-vsock-pci.c +++ b/hw/virtio/vhost-user-vsock-pci.c @@ -55,8 +55,6 @@ static void vhost_user_vsock_pci_class_init(ObjectClass *klass, void *data) k->realize = vhost_user_vsock_pci_realize; set_bit(DEVICE_CATEGORY_MISC, dc->categories); device_class_set_props(dc, vhost_user_vsock_pci_properties); - pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; - pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; pcidev_k->revision = 0x00; pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; } diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c index 9f34414d3814..170a806b6765 100644 --- a/hw/virtio/vhost-vsock-pci.c +++ b/hw/virtio/vhost-vsock-pci.c @@ -65,8 +65,6 @@ static void vhost_vsock_pci_class_init(ObjectClass *klass, void *data) k->realize = vhost_vsock_pci_realize; set_bit(DEVICE_CATEGORY_MISC, dc->categories); device_class_set_props(dc, vhost_vsock_pci_properties); - pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; - pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK; pcidev_k->revision = 0x00; pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER; }
Not needed for a virtio 1.0 device. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/hw/pci/pci.h | 1 - hw/virtio/vhost-user-vsock-pci.c | 2 -- hw/virtio/vhost-vsock-pci.c | 2 -- 3 files changed, 5 deletions(-)