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

[Cristian Banu <cristb@xxxxxxxxx>]

Hi Sharan,

Thanks for the detailed explanation and references!
I re-submitted the patch with the subsystem_device_id solution.

Regards,
Cristian

On Wed, Dec 5, 2018 at 7:08 PM Sharan Santhanam
<sharan.santhanam@xxxxxxxxx> wrote:
>
> Hey Cristian Banu,
>
> The original idea of subtracting the 0x0FFF is flawed as there is
> mismatch in following the device(s) [1], [2].
> * network device (legacy)
> * block device (legacy)
> * SCSI host bus adapter device
> * entropy generator device
>
> Instead of this solution what we could do is:
>
> @@ -364,8 +363,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;
>
> In qemu they set this value as per patch series [3]
>
>
> [1] https://lists.oasis-open.org/archives/virtio/201406/msg00007.html
> [2]
> https://virtio-dev.oasis-open.narkive.com/ZJpUKtPi/patch-pci-switch-from-subsystem-id-to-device-id
> [3] https://patchwork.kernel.org/patch/10038753/
>
> Thanks & Regards
> Sharan
>
> On 12/05/2018 04:15 PM, Cristian Banu wrote:
> > The virtio_device_id is computed as the difference between pci_dev_id
> > and VIRTIO_PCI_LEGACY_DEVICEID_START. Considering the values for device
> > ids given in virtio_ids.h, the previous value of 0x0FFF would have
> > created an off-by-one error in virtio_device_id.
> >
> > For example, a 9P device has pci_dev_id = 0x1009, thus virtio_device_id
> > would've been 0xA instead of 0x9, as required by virtio_ids.h.
> >
> > Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
> > ---
> >   plat/drivers/virtio/virtio_pci.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/plat/drivers/virtio/virtio_pci.c 
> > b/plat/drivers/virtio/virtio_pci.c
> > index 8503b534d054..44b1fc8e7f12 100644
> > --- a/plat/drivers/virtio/virtio_pci.c
> > +++ b/plat/drivers/virtio/virtio_pci.c
> > @@ -46,7 +46,7 @@
> >   #include <virtio/virtio_pci.h>
> >
> >   #define VENDOR_QUMRANET_VIRTIO           (0x1AF4)
> > -#define VIRTIO_PCI_LEGACY_DEVICEID_START (0x0FFF)
> > +#define VIRTIO_PCI_LEGACY_DEVICEID_START (0x1000)
> >   #define VIRTIO_PCI_MODERN_DEVICEID_START (0x1040)
> >
> >   static struct uk_alloc *a;
> >

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel