Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH V2 3/5] vDPA: introduce vDPA bus
To:     Jason Gunthorpe <jgg@mellanox.com>
Cc:     mst@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
        tiwei.bie@intel.com, maxime.coquelin@redhat.com,
        cunming.liang@intel.com, zhihong.wang@intel.com,
        rob.miller@broadcom.com, xiao.w.wang@intel.com,
        haotian.wang@sifive.com, lingshan.zhu@intel.com,
        eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
        kevin.tian@intel.com, stefanha@redhat.com, rdunlap@infradead.org,
        hch@infradead.org, aadam@redhat.com, jiri@mellanox.com,
        shahafs@mellanox.com, hanand@xilinx.com, mhabets@solarflare.com
References: <20200210035608.10002-1-jasowang@redhat.com>
 <20200210035608.10002-4-jasowang@redhat.com>
 <20200211134746.GI4271@mellanox.com>
 <cf7abcc9-f8ef-1fe2-248e-9b9028788ade@redhat.com>
 <20200212125108.GS4271@mellanox.com>
 <12775659-1589-39e4-e344-b7a2c792b0f3@redhat.com>
 <20200213134128.GV4271@mellanox.com>
 <ebaea825-5432-65e2-2ab3-720a8c4030e7@redhat.com>
 <20200213150542.GW4271@mellanox.com>
 <8b3e6a9c-8bfd-fb3c-12a8-2d6a3879f1ae@redhat.com>
 <20200214135232.GB4271@mellanox.com>
From:   Jason Wang <jasowang@redhat.com>
Message-ID: <01c86ebb-cf4b-691f-e682-2d6f93ddbcf7@redhat.com>
Date:   Mon, 17 Feb 2020 14:08:03 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20200214135232.GB4271@mellanox.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020/2/14 =E4=B8=8B=E5=8D=889: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 inter=
face
>>>> 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 devi=
ce 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=20
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=20
was loosely coupled with a socket.

I thought you were copied in the patch [1], maybe we can move vhost=20
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=20
(control path) for virtio, so it's fit for a new transport driver but=20
not directly into virtio bus. VOP use similar design.
- virtio bus is designed for kernel drivers but not userspace, and it=20
can not be easily extended to support userspace driver but requires some=20
major refactoring. E.g the virtio bus operations requires the virtqueue=20
to be allocated by the transport driver.

So it's cheaper and simpler to introduce a new bus instead of=20
refactoring a well known bus and API where brunches of drivers and=20
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=20
need to go for virtio bus. Vhost device talks to vDPA device through=20
vDPA bus. Virito device talks to vDPA device through a new vDPA=20
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=20
to some device function. If you see MMIO and PCI transport, you can see=20
something similar. The only difference is that in the case of VDPA the=20
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=20
also treated as a subclass of virtio as well, they were all implemented=20
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=20
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 ch=
ar dev
>> and other stuffs?
> No.
>
> Jason
>