Date:   Mon, 17 Feb 2020 14:08:03 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Jason Gunthorpe <jgg@...lanox.com>
Cc:     mst@...hat.com, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
        tiwei.bie@...el.com, maxime.coquelin@...hat.com,
        cunming.liang@...el.com, zhihong.wang@...el.com,
        rob.miller@...adcom.com, xiao.w.wang@...el.com,
        haotian.wang@...ive.com, lingshan.zhu@...el.com,
        eperezma@...hat.com, lulu@...hat.com, parav@...lanox.com,
        kevin.tian@...el.com, stefanha@...hat.com, rdunlap@...radead.org,
        hch@...radead.org, aadam@...hat.com, jiri@...lanox.com,
        shahafs@...lanox.com, hanand@...inx.com, mhabets@...arflare.com
Subject: Re: [PATCH V2 3/5] vDPA: introduce vDPA bus


On 2020/2/14 下午9:52, Jason Gunthorpe wrote:
> On Fri, Feb 14, 2020 at 11:23:27AM +0800, Jason Wang wrote:
>
>>>> Though all vDPA devices have the same programming interface, but the
>>>> semantic is different. So it looks to me that use bus complies what
>>>> class.rst said:
>>>>
>>>> "
>>>>
>>>> Each device class defines a set of semantics and a programming interface
>>>> that devices of that class adhere to. Device drivers are the
>>>> implementation of that programming interface for a particular device on
>>>> a particular bus.
>>>>
>>>> "
>>> Here we are talking about the /dev/XX node that provides the
>>> programming interface.
>>
>> I'm confused here, are you suggesting to use class to create char device in
>> vhost-vdpa? That's fine but the comment should go for vhost-vdpa patch.
> Certainly yes, something creating many char devs should have a
> class. That makes the sysfs work as expected
>
> I suppose this is vhost user?


Actually not.

Vhost-user is the vhost protocol that is used for userspace vhost 
backend (usually though a UNIX domain socket).

What's being done in the vhost-vpda is a new type of the vhost in kernel.


> I admit I don't really see how this
> vhost stuff works, all I see are global misc devices? Very unusual for
> a new subsystem to be using global misc devices..


Vhost is not a subsystem right now, e.g for it's net implementation, it 
was loosely coupled with a socket.

I thought you were copied in the patch [1], maybe we can move vhost 
related discussion there to avoid confusion.

[1] https://lwn.net/Articles/811210/


>
> I would have expected that a single VDPA device comes out as a single
> char dev linked to only that VDPA device.
>
>>> All the vdpa devices have the same basic
>>> chardev interface and discover any semantic variations 'in band'
>> That's not true, char interface is only used for vhost. Kernel virtio driver
>> does not need char dev but a device on the virtio bus.
> Okay, this is fine, but why do you need two busses to accomplish this?


The reasons are:

- vDPA ops is designed to be functional as a software assisted transport 
(control path) for virtio, so it's fit for a new transport driver but 
not directly into virtio bus. VOP use similar design.
- virtio bus is designed for kernel drivers but not userspace, and it 
can not be easily extended to support userspace driver but requires some 
major refactoring. E.g the virtio bus operations requires the virtqueue 
to be allocated by the transport driver.

So it's cheaper and simpler to introduce a new bus instead of 
refactoring a well known bus and API where brunches of drivers and 
devices had been implemented for years.


>
> Shouldn't the 'struct virito_device' be the plug in point for HW
> drivers I was talking about - and from there a vhost-user can connect
> to the struct virtio_device to give it a char dev or a kernel driver
> can connect to link it to another subsystem?


 From vhost point of view, it would only need to connect vDPA bus, no 
need to go for virtio bus. Vhost device talks to vDPA device through 
vDPA bus. Virito device talks to vDPA device through a new vDPA 
transport driver.


>
> It is easy to see something is going wrong with this design because
> the drivers/virtio/virtio_vdpa.c mainly contains a bunch of trampoline
> functions reflecting identical calls from one ops struct to a
> different ops struct.


That's pretty normal, since part of the virtio ops could be 1:1 mapped 
to some device function. If you see MMIO and PCI transport, you can see 
something similar. The only difference is that in the case of VDPA the 
function is assisted or emulated by hardware vDPA driver.


>   This suggests the 'vdpa' is some subclass of
> 'virtio' and it is possibly better to model it by extending 'struct
> virito_device' to include the vdpa specific stuff.


Going for such kind of modeling, virtio-pci and virtio-mmio could be 
also treated as a subclass of virtio as well, they were all implemented 
via a dedicated transport driver.


>
> Where does the vhost-user char dev get invovled in with the v2 series?
> Is that included?


We're working on the a new version, but for the bus/driver part it 
should be the same as version 1.

Thanks


>
>>> Every class of virtio traffic is going to need a special HW driver to
>>> enable VDPA, that special driver can create the correct vhost side
>>> class device.
>> Are you saying, e.g it's the charge of IFCVF driver to create vhost char dev
>> and other stuffs?
> No.
>
> Jason
>

Powered by blists - more mailing lists