Re: [Minios-devel] [UNIKRAFT PATCH v3] plat/virtio: Update off-by-one device id

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Cristian Banu,

Thank you the update.

This patch looks fine.

Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>

Thanks & Regards
Sharan

On 12/07/2018 06:19 AM, Cristian Banu wrote:
> Extract the virtio_device_id for legacy devices from the PCI Subsystem
> Device ID. The previous behaviour resulted in some devices having the
> wrong ID attributed to them. For example, a virtio-9p device has a PCI
> Device ID of 0x1009 (which would have resulted in virtio_device_id of
> 10), but a PCI Subsystem Device ID of 9 (which is the correct constant
> under virtio_ids.h).
>
> Removes now unused function virtio_device_id_add and misleading macro
> VIRTIO_PCI_LEGACY_DEVICEID_START.
>
> Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
> ---
>   plat/drivers/virtio/virtio_pci.c | 14 +-------------
>   1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/plat/drivers/virtio/virtio_pci.c b/plat/drivers/virtio/virtio_pci.c
> index 8503b534d054..e398bfc650ce 100644
> --- a/plat/drivers/virtio/virtio_pci.c
> +++ b/plat/drivers/virtio/virtio_pci.c
> @@ -46,7 +46,6 @@
>   #include <virtio/virtio_pci.h>
>   
>   #define VENDOR_QUMRANET_VIRTIO           (0x1AF4)
> -#define VIRTIO_PCI_LEGACY_DEVICEID_START (0x0FFF)
>   #define VIRTIO_PCI_MODERN_DEVICEID_START (0x1040)
>   
>   static struct uk_alloc *a;
> @@ -93,9 +92,6 @@ static struct virtqueue *vpci_legacy_vq_setup(struct 
> virtio_dev *vdev,
>                                               __u16 num_desc,
>                                               virtqueue_callback_t callback,
>                                               struct uk_alloc *a);
> -static inline void virtio_device_id_add(struct virtio_dev *vdev,
> -                                       __u16 pci_dev_id,
> -                                       __u16 vpci_dev_id_start);
>   static int virtio_pci_handle(void *arg);
>   static int vpci_legacy_notify(struct virtio_dev *vdev, __u16 queue_id);
>   static int virtio_pci_legacy_add_dev(struct pci_device *pci_dev,
> @@ -338,13 +334,6 @@ static void vpci_legacy_pci_features_set(struct virtio_dev 
> *vdev,
>                         VIRTIO_PCI_GUEST_FEATURES, (__u32)features);
>   }
>   
> -static inline void virtio_device_id_add(struct virtio_dev *vdev,
> -                                       __u16 pci_dev_id,
> -                                       __u16 pci_dev_id_start)
> -{
> -       vdev->id.virtio_device_id = (pci_dev_id - pci_dev_id_start);
> -}
> -
>   static int virtio_pci_legacy_add_dev(struct pci_device *pci_dev,
>                                      struct virtio_pci_dev *vpci_dev)
>   {
> @@ -364,8 +353,7 @@ static int virtio_pci_legacy_add_dev(struct pci_device 
> *pci_dev,
>                    pci_dev->id.device_id);
>   
>   	/* Mapping the virtio device identifier */
> -       virtio_device_id_add(&vpci_dev->vdev, pci_dev->id.device_id,
> -                            VIRTIO_PCI_LEGACY_DEVICEID_START);
> +       vpci_dev->vdev.id.virtio_device_id = pci_dev->id.subsystem_device_id;
>         return 0;
>   }
>   
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel