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
