Re: [RFC] virtio-net: support modern-transtional devices

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 5/28/2023 7:28 PM, Michael S. Tsirkin wrote:
> On Sat, May 27, 2023 at 02:15:42AM +0800, Zhu Lingshan wrote:
>> Current virtio-net only probes a device with VIRITO_ID_NET == 1.
>>
>> For a modern-transtional virtio-net device which has a transtional
>> device id 0x1000 and acts as a modern device, current virtio-pci
>> modern driver will assign the sub-device-id to its mdev->id.device,
>> which may not be 0x1, this sub-device-id is up to the vendor.
>>
>> That means virtio-net driver doesn't probe a modern-transitonal
>> virtio-net with a sub-device-id other than 0x1, which is a bug.
> No, the bug is in the device. Legacy linux drivers always looked at
> sub device id (other OSes might differ). So it makes no sense
> for a transitional device to have sub-device-id other than 0x1.
> Don't have time to look at spec but I think you will find it there.
That is true for a software emulated transitional device,
because there is only "generation" of instance in the hypervisor,
that allowing it to ensure its sub-device-id always be 0x01,
and it fits VIRTIO_ID_NET.

However, a vendor may produce multiple generations of transitional
hardware. The sub-device-id is up to the vendor, and it is the
only way to for a driver to identify a device, other IDs are all
fixed as 0x1af4, 0x1000 and 0x8086 for Intel.

So the sub-device-id has to be unique and differ from others, can not 
always be 0x01.

I propose this fix, all changes are for modern-transitional devices in 
modern
code path, not for legacy nor legacy-transitional.

Thanks

>
>
>> Other types of devices also have similar issues, like virito-blk.
>>
>> I propose to fix this problem of modern-transitonal device
>> whith this solution, all in the modern code path:
>> 1) assign the device id to mdev->id.device
>> 2) add transitional device ids in the virtio-net(and others) probe table.
>>
>> Comments are welcome!
>>
>> Thanks!
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   drivers/net/virtio_net.c               | 1 +
>>   drivers/virtio/virtio_pci_modern_dev.c | 2 +-
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 56ca1d270304..6b45d8602a6b 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -4250,6 +4250,7 @@ static __maybe_unused int virtnet_restore(struct virtio_device *vdev)
>>   
>>   static struct virtio_device_id id_table[] = {
>>   	{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
>> +	{ VIRTIO_TRANS_ID_NET, VIRTIO_DEV_ANY_ID },
>>   	{ 0 },
>>   };
>>   
>> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
>> index 869cb46bef96..80846e1195ce 100644
>> --- a/drivers/virtio/virtio_pci_modern_dev.c
>> +++ b/drivers/virtio/virtio_pci_modern_dev.c
>> @@ -229,7 +229,7 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
>>   		/* Transitional devices: use the PCI subsystem device id as
>>   		 * virtio device id, same as legacy driver always did.
>>   		 */
>> -		mdev->id.device = pci_dev->subsystem_device;
>> +		mdev->id.device = pci_dev->device;
>>   	} else {
>>   		/* Modern devices: simply use PCI device id, but start from 0x1040. */
>>   		mdev->id.device = pci_dev->device - 0x1040;
>> -- 
>> 2.39.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization