[dpdk-dev] [PATCH v2 0/7] virtio 1.0 enabling for virtio pmd driver

[Tetsuya Mukawa]

On 2016/01/14 15:41, Tetsuya Mukawa wrote:
>
>>> 2. Abstraction of read/write accesses.
>>>
>>> It may be difficult to cleanly rebase my patches on this patches,
>>> because virtio_read_caps() is not abstracted.
>>> Let me describe it more.
>>> So far, we need to handle below 3 virtio-net devices..
>>>   - physical virtio-net device.
>>>   - virtual virtio-net device in virtio-net PMD. (Jianfeng's patch)
>>>   - virtual virtio-net device in QEMU. (my patch)
>>>
>>> Almost all code of the virtio-net PMD can be shared between above
>>> different cases.
>>> Probably big difference is how to access to configuration space.
>>>
>>> Yuanhan's patch introduces an abstraction layer to hide configuration
>>> space layout and how to access it.
>>> Is it possible to separate?
>>> I guess "access method" will be nice to be abstracted separately from
>>> "configuration space layout".
>>> Probably access method will be defined by "eth_dev->dev_type" and the
>>> PMD name like "eth_cvio".
>>> And "configuration space layout" will be defined by capability list of
>>> PCI configuration layout.
>>>
>>> For example, if access method like below are abstracted separately and
>>> current "virtio_pci.c" is implemented on this abstraction, we can easily
>>> re-use virtio_read_caps().
>>>   - how to read/write virtio configuration space.
>>>   - how to mmap PCI configuration space.
>>>   - how to read/(write) PCI configuration space.
>>
>> I basically agree with you. We have two dimensions here:
>>
>> legacy             modern
>> physical virtio device:                     Use
>> virtio_read_caps_phys() to distinguish
>> virtual virtio device (Tetsuya):       Use virtio_read_caps_virt() to
>> distinguish
>> virtual virtio device (Jianfeng):    does not need a "configuration
>> space layout", no need to distinguish
>>
>> So in vtpci_init(), we needs to test "eth_dev->dev_type" firstly
>>
>> vtpci_init() {
>>     if (eth_dev->dev_type == RTE_ETH_DEV_PCI) {
>>         if (virtio_read_caps_phys()) {
>>             // modern
>>         } else {
>>             // legacy
>>         }
>>     } else {
>>         if (Tetsuya's way) {
>>             if (virtio_read_caps_virt()) {
>>                 // modern
>>             } else {
>>                 // legacy
>>             }
>>         } else {
>>             // Jianfeng's way
>>         }
>>     }
>> }
>>
>> And from Yuanhan's angle, I think he does not need to address this
>> problem. How do you think?
> Yes, I agree he doesn't need.
>
> Firstly, I have implemented like above, then I noticed that
> virtio_read_caps_phy() and virtio_read_caps_virt() are same except for
> access method.
> Anyway, I guess abstracting access method is not so difficult.
> If you are OK, I want to send RFC on Yuanhan's patch. Is it OK?

Hi Jianfeng,

I will submit patches to abstract pci access method.
The patches will be on Yuanhan's latest virtio-1.0 patches.

I guess your container patches also can be on the patches.
Could you please check it?

Thanks,
Tetsuya