| Message ID | 20220512062103.1875-3-longpeng2@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add generic vDPA device support | expand |
On Thu, May 12, 2022 at 02:21:01PM +0800, Longpeng(Mike) wrote: > From: Longpeng <longpeng2@huawei.com> > > Add helpers to get the "Transitional PCI Device ID" and "class_id" > of the device specified by the "Virtio Device ID". > > These helpers will be used to build the generic vDPA device later. > > Signed-off-by: Longpeng <longpeng2@huawei.com> Except, the IDs in the current header a broken. I just fixed them and they will be hopefully OK in the next version. > --- > hw/virtio/virtio-pci.c | 77 ++++++++++++++++++++++++++++++++++++++++++ > hw/virtio/virtio-pci.h | 5 +++ > 2 files changed, 82 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 7cf1231c1c..fdfa205cee 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -19,6 +19,7 @@ > > #include "exec/memop.h" > #include "standard-headers/linux/virtio_pci.h" > +#include "standard-headers/linux/virtio_ids.h" > #include "hw/boards.h" > #include "hw/virtio/virtio.h" > #include "migration/qemu-file-types.h" > @@ -212,6 +213,79 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f) > return 0; > } > > +typedef struct VirtIOPCIIDInfo { > + /* virtio id */ > + uint16_t vdev_id; > + /* pci device id for the transitional device */ > + uint16_t trans_devid; > + uint16_t class_id; > +} VirtIOPCIIDInfo; > + > +#define VIRTIO_TRANS_DEV_ID_INFO(name, class) \ > + { \ > + .vdev_id = VIRTIO_ID_##name, \ > + .trans_devid = PCI_DEVICE_ID_VIRTIO_##name, \ > + .class_id = class, \ > + } > + > +#define VIRTIO_MODERN_DEV_ID_NFO(name, class) \ > + { \ > + .vdev_id = VIRTIO_ID_##name, \ > + .class_id = class, \ > + } > + No, I think I liked the original approach in the RFC better, even though it duplicates a tiny bit of code. This trick does not save a lot of typing and obscures the ID use in case it's wrong (as was the case just recently). > +static const VirtIOPCIIDInfo virtio_pci_id_info[] = { > + /* Non-transitional devices */ > + VIRTIO_MODERN_DEV_ID_NFO(CRYPTO, PCI_CLASS_OTHERS), > + VIRTIO_MODERN_DEV_ID_NFO(FS, PCI_CLASS_STORAGE_OTHER), > + /* Transitional devices */ > + VIRTIO_TRANS_DEV_ID_INFO(NET, PCI_CLASS_NETWORK_ETHERNET), > + VIRTIO_TRANS_DEV_ID_INFO(BLOCK, PCI_CLASS_STORAGE_SCSI), > + VIRTIO_TRANS_DEV_ID_INFO(CONSOLE, PCI_CLASS_COMMUNICATION_OTHER), > + VIRTIO_TRANS_DEV_ID_INFO(SCSI, PCI_CLASS_STORAGE_SCSI), > + VIRTIO_TRANS_DEV_ID_INFO(9P, PCI_BASE_CLASS_NETWORK), > + VIRTIO_TRANS_DEV_ID_INFO(BALLOON, PCI_CLASS_OTHERS), > + VIRTIO_TRANS_DEV_ID_INFO(RNG, PCI_CLASS_OTHERS), > +}; > + > +static const VirtIOPCIIDInfo *virtio_pci_get_id_info(uint16_t vdev_id) > +{ > + const VirtIOPCIIDInfo *info = NULL; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(virtio_pci_id_info); i++) { > + if (virtio_pci_id_info[i].vdev_id == vdev_id) { > + info = &virtio_pci_id_info[i]; > + break; > + } > + } > + > + if (!info) { > + /* The device id is invalid or not added to the id_info yet. */ > + error_report("Invalid virtio device(id %u)", vdev_id); > + abort(); > + } > + > + return info; > +} > + > +/* > + * Get the Transitional Device ID for the specific device, return > + * zero if the device is non-transitional. > + */ > +uint16_t virtio_pci_get_trans_devid(uint16_t device_id) > +{ > + return virtio_pci_get_id_info(device_id)->trans_devid; > +} > + > +/* > + * Get the Class ID for the specific device. > + */ > +uint16_t virtio_pci_get_class_id(uint16_t device_id) > +{ > + return virtio_pci_get_id_info(device_id)->class_id; > +} > + > static bool virtio_pci_ioeventfd_enabled(DeviceState *d) > { > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); > @@ -1675,6 +1749,9 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > * is set to PCI_SUBVENDOR_ID_REDHAT_QUMRANET by default. > */ > pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus)); > + if (proxy->trans_devid) { > + pci_config_set_device_id(config, proxy->trans_devid); > + } > } else { > /* pure virtio-1.0 */ > pci_set_word(config + PCI_VENDOR_ID, > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index 2446dcd9ae..f08665cd1b 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -146,6 +146,8 @@ struct VirtIOPCIProxy { > bool disable_modern; > bool ignore_backend_features; > OnOffAuto disable_legacy; > + /* Transitional device id */ > + uint16_t trans_devid; > uint32_t class_code; > uint32_t nvectors; > uint32_t dfselect; > @@ -158,6 +160,9 @@ struct VirtIOPCIProxy { > VirtioBusState bus; > }; > > +uint16_t virtio_pci_get_trans_devid(uint16_t device_id); > +uint16_t virtio_pci_get_class_id(uint16_t device_id); > + > static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) > { > return !proxy->disable_modern; > -- > 2.23.0
在 2022/5/12 14:56, Michael S. Tsirkin 写道: > On Thu, May 12, 2022 at 02:21:01PM +0800, Longpeng(Mike) wrote: >> From: Longpeng <longpeng2@huawei.com> >> >> Add helpers to get the "Transitional PCI Device ID" and "class_id" >> of the device specified by the "Virtio Device ID". >> >> These helpers will be used to build the generic vDPA device later. >> >> Signed-off-by: Longpeng <longpeng2@huawei.com> > > > Except, the IDs in the current header a broken. I just fixed them > and they will be hopefully OK in the next version. > Do you mean this patch ? ``` virtio: fix virtio transitional ids This commit fixes the transitional PCI device ID. Fixes: d61914ea6ada ("virtio: update virtio id table, add transitional ids") Signed-off-by: Shunsuke Mie <mie@igel.co.jp> Link: https://lore.kernel.org/r/20220510102723.87666-1-mie@igel.co.jp Signed-off-by: Michael S. Tsirkin <mst@redhat.com> ``` Luckily, we use PCI_DEVICE_ID_VIRTIO_xxx instead of VIRTIO_TRANS_ID_xxx here. >> --- >> hw/virtio/virtio-pci.c | 77 ++++++++++++++++++++++++++++++++++++++++++ >> hw/virtio/virtio-pci.h | 5 +++ >> 2 files changed, 82 insertions(+) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 7cf1231c1c..fdfa205cee 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -19,6 +19,7 @@ >> >> #include "exec/memop.h" >> #include "standard-headers/linux/virtio_pci.h" >> +#include "standard-headers/linux/virtio_ids.h" >> #include "hw/boards.h" >> #include "hw/virtio/virtio.h" >> #include "migration/qemu-file-types.h" >> @@ -212,6 +213,79 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f) >> return 0; >> } >> >> +typedef struct VirtIOPCIIDInfo { >> + /* virtio id */ >> + uint16_t vdev_id; >> + /* pci device id for the transitional device */ >> + uint16_t trans_devid; >> + uint16_t class_id; >> +} VirtIOPCIIDInfo; >> + >> +#define VIRTIO_TRANS_DEV_ID_INFO(name, class) \ >> + { \ >> + .vdev_id = VIRTIO_ID_##name, \ >> + .trans_devid = PCI_DEVICE_ID_VIRTIO_##name, \ >> + .class_id = class, \ >> + } >> + >> +#define VIRTIO_MODERN_DEV_ID_NFO(name, class) \ >> + { \ >> + .vdev_id = VIRTIO_ID_##name, \ >> + .class_id = class, \ >> + } >> + > > No, I think I liked the original approach in the RFC better, even though > it duplicates a tiny bit of code. > This trick does not save a lot of typing and obscures the ID > use in case it's wrong (as was the case just recently). > OK, will do in v6. Thanks. > >> +static const VirtIOPCIIDInfo virtio_pci_id_info[] = { >> + /* Non-transitional devices */ >> + VIRTIO_MODERN_DEV_ID_NFO(CRYPTO, PCI_CLASS_OTHERS), >> + VIRTIO_MODERN_DEV_ID_NFO(FS, PCI_CLASS_STORAGE_OTHER), >> + /* Transitional devices */ >> + VIRTIO_TRANS_DEV_ID_INFO(NET, PCI_CLASS_NETWORK_ETHERNET), >> + VIRTIO_TRANS_DEV_ID_INFO(BLOCK, PCI_CLASS_STORAGE_SCSI), >> + VIRTIO_TRANS_DEV_ID_INFO(CONSOLE, PCI_CLASS_COMMUNICATION_OTHER), >> + VIRTIO_TRANS_DEV_ID_INFO(SCSI, PCI_CLASS_STORAGE_SCSI), >> + VIRTIO_TRANS_DEV_ID_INFO(9P, PCI_BASE_CLASS_NETWORK), >> + VIRTIO_TRANS_DEV_ID_INFO(BALLOON, PCI_CLASS_OTHERS), >> + VIRTIO_TRANS_DEV_ID_INFO(RNG, PCI_CLASS_OTHERS), >> +}; >> + >> +static const VirtIOPCIIDInfo *virtio_pci_get_id_info(uint16_t vdev_id) >> +{ >> + const VirtIOPCIIDInfo *info = NULL; >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(virtio_pci_id_info); i++) { >> + if (virtio_pci_id_info[i].vdev_id == vdev_id) { >> + info = &virtio_pci_id_info[i]; >> + break; >> + } >> + } >> + >> + if (!info) { >> + /* The device id is invalid or not added to the id_info yet. */ >> + error_report("Invalid virtio device(id %u)", vdev_id); >> + abort(); >> + } >> + >> + return info; >> +} >> + >> +/* >> + * Get the Transitional Device ID for the specific device, return >> + * zero if the device is non-transitional. >> + */ >> +uint16_t virtio_pci_get_trans_devid(uint16_t device_id) >> +{ >> + return virtio_pci_get_id_info(device_id)->trans_devid; >> +} >> + >> +/* >> + * Get the Class ID for the specific device. >> + */ >> +uint16_t virtio_pci_get_class_id(uint16_t device_id) >> +{ >> + return virtio_pci_get_id_info(device_id)->class_id; >> +} >> + >> static bool virtio_pci_ioeventfd_enabled(DeviceState *d) >> { >> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); >> @@ -1675,6 +1749,9 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> * is set to PCI_SUBVENDOR_ID_REDHAT_QUMRANET by default. >> */ >> pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus)); >> + if (proxy->trans_devid) { >> + pci_config_set_device_id(config, proxy->trans_devid); >> + } >> } else { >> /* pure virtio-1.0 */ >> pci_set_word(config + PCI_VENDOR_ID, >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index 2446dcd9ae..f08665cd1b 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -146,6 +146,8 @@ struct VirtIOPCIProxy { >> bool disable_modern; >> bool ignore_backend_features; >> OnOffAuto disable_legacy; >> + /* Transitional device id */ >> + uint16_t trans_devid; >> uint32_t class_code; >> uint32_t nvectors; >> uint32_t dfselect; >> @@ -158,6 +160,9 @@ struct VirtIOPCIProxy { >> VirtioBusState bus; >> }; >> >> +uint16_t virtio_pci_get_trans_devid(uint16_t device_id); >> +uint16_t virtio_pci_get_class_id(uint16_t device_id); >> + >> static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) >> { >> return !proxy->disable_modern; >> -- >> 2.23.0 > > > .
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 7cf1231c1c..fdfa205cee 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -19,6 +19,7 @@ #include "exec/memop.h" #include "standard-headers/linux/virtio_pci.h" +#include "standard-headers/linux/virtio_ids.h" #include "hw/boards.h" #include "hw/virtio/virtio.h" #include "migration/qemu-file-types.h" @@ -212,6 +213,79 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f) return 0; } +typedef struct VirtIOPCIIDInfo { + /* virtio id */ + uint16_t vdev_id; + /* pci device id for the transitional device */ + uint16_t trans_devid; + uint16_t class_id; +} VirtIOPCIIDInfo; + +#define VIRTIO_TRANS_DEV_ID_INFO(name, class) \ + { \ + .vdev_id = VIRTIO_ID_##name, \ + .trans_devid = PCI_DEVICE_ID_VIRTIO_##name, \ + .class_id = class, \ + } + +#define VIRTIO_MODERN_DEV_ID_NFO(name, class) \ + { \ + .vdev_id = VIRTIO_ID_##name, \ + .class_id = class, \ + } + +static const VirtIOPCIIDInfo virtio_pci_id_info[] = { + /* Non-transitional devices */ + VIRTIO_MODERN_DEV_ID_NFO(CRYPTO, PCI_CLASS_OTHERS), + VIRTIO_MODERN_DEV_ID_NFO(FS, PCI_CLASS_STORAGE_OTHER), + /* Transitional devices */ + VIRTIO_TRANS_DEV_ID_INFO(NET, PCI_CLASS_NETWORK_ETHERNET), + VIRTIO_TRANS_DEV_ID_INFO(BLOCK, PCI_CLASS_STORAGE_SCSI), + VIRTIO_TRANS_DEV_ID_INFO(CONSOLE, PCI_CLASS_COMMUNICATION_OTHER), + VIRTIO_TRANS_DEV_ID_INFO(SCSI, PCI_CLASS_STORAGE_SCSI), + VIRTIO_TRANS_DEV_ID_INFO(9P, PCI_BASE_CLASS_NETWORK), + VIRTIO_TRANS_DEV_ID_INFO(BALLOON, PCI_CLASS_OTHERS), + VIRTIO_TRANS_DEV_ID_INFO(RNG, PCI_CLASS_OTHERS), +}; + +static const VirtIOPCIIDInfo *virtio_pci_get_id_info(uint16_t vdev_id) +{ + const VirtIOPCIIDInfo *info = NULL; + int i; + + for (i = 0; i < ARRAY_SIZE(virtio_pci_id_info); i++) { + if (virtio_pci_id_info[i].vdev_id == vdev_id) { + info = &virtio_pci_id_info[i]; + break; + } + } + + if (!info) { + /* The device id is invalid or not added to the id_info yet. */ + error_report("Invalid virtio device(id %u)", vdev_id); + abort(); + } + + return info; +} + +/* + * Get the Transitional Device ID for the specific device, return + * zero if the device is non-transitional. + */ +uint16_t virtio_pci_get_trans_devid(uint16_t device_id) +{ + return virtio_pci_get_id_info(device_id)->trans_devid; +} + +/* + * Get the Class ID for the specific device. + */ +uint16_t virtio_pci_get_class_id(uint16_t device_id) +{ + return virtio_pci_get_id_info(device_id)->class_id; +} + static bool virtio_pci_ioeventfd_enabled(DeviceState *d) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); @@ -1675,6 +1749,9 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) * is set to PCI_SUBVENDOR_ID_REDHAT_QUMRANET by default. */ pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus)); + if (proxy->trans_devid) { + pci_config_set_device_id(config, proxy->trans_devid); + } } else { /* pure virtio-1.0 */ pci_set_word(config + PCI_VENDOR_ID, diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 2446dcd9ae..f08665cd1b 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -146,6 +146,8 @@ struct VirtIOPCIProxy { bool disable_modern; bool ignore_backend_features; OnOffAuto disable_legacy; + /* Transitional device id */ + uint16_t trans_devid; uint32_t class_code; uint32_t nvectors; uint32_t dfselect; @@ -158,6 +160,9 @@ struct VirtIOPCIProxy { VirtioBusState bus; }; +uint16_t virtio_pci_get_trans_devid(uint16_t device_id); +uint16_t virtio_pci_get_class_id(uint16_t device_id); + static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) { return !proxy->disable_modern;