KVM Archive on lore.kernel.org
 help / color / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jason Wang <jasowang@redhat.com>, Jason Gunthorpe <jgg@mellanox.com>
Cc: "mst@redhat.com" <mst@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"virtualization@lists.linux-foundation.org" 
	<virtualization@lists.linux-foundation.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Bie, Tiwei" <tiwei.bie@intel.com>,
	"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
	"Liang, Cunming" <cunming.liang@intel.com>,
	"Wang, Zhihong" <zhihong.wang@intel.com>,
	"rob.miller@broadcom.com" <rob.miller@broadcom.com>,
	"Wang, Xiao W" <xiao.w.wang@intel.com>,
	"haotian.wang@sifive.com" <haotian.wang@sifive.com>,
	"Zhu, Lingshan" <lingshan.zhu@intel.com>,
	"eperezma@redhat.com" <eperezma@redhat.com>,
	"lulu@redhat.com" <lulu@redhat.com>,
	Parav Pandit <parav@mellanox.com>,
	"stefanha@redhat.com" <stefanha@redhat.com>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"aadam@redhat.com" <aadam@redhat.com>,
	"jakub.kicinski@netronome.com" <jakub.kicinski@netronome.com>,
	Jiri Pirko <jiri@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	"hanand@xilinx.com" <hanand@xilinx.com>,
	"mhabets@solarflare.com" <mhabets@solarflare.com>
Subject: RE: [PATCH 3/5] vDPA: introduce vDPA bus
Date: Tue, 21 Jan 2020 08:40:07 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D73EBA4@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <03cfbcc2-fef0-c9d8-0b08-798b2a293b8c@redhat.com>

> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, January 17, 2020 11:03 AM
> 
> 
> On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
> > On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
> >> vDPA device is a device that uses a datapath which complies with the
> >> virtio specifications with vendor specific control path. vDPA devices
> >> can be both physically located on the hardware or emulated by
> >> software. vDPA hardware devices are usually implemented through PCIE
> >> with the following types:
> >>
> >> - PF (Physical Function) - A single Physical Function
> >> - VF (Virtual Function) - Device that supports single root I/O
> >>    virtualization (SR-IOV). Its Virtual Function (VF) represents a
> >>    virtualized instance of the device that can be assigned to different
> >>    partitions
> >> - VDEV (Virtual Device) - With technologies such as Intel Scalable
> >>    IOV, a virtual device composed by host OS utilizing one or more
> >>    ADIs.

the concept of VDEV includes both software bits and ADIs. If you
only take about hardware types, using ADI is more accurate.

> >> - SF (Sub function) - Vendor specific interface to slice the Physical
> >>    Function to multiple sub functions that can be assigned to different
> >>    partitions as virtual devices.
> > I really hope we don't end up with two different ways to spell this
> > same thing.
> 
> 
> I think you meant ADI vs SF. It looks to me that ADI is limited to the
> scope of scalable IOV but SF not.

ADI is just a term for minimally assignable resource in Scalable IOV. 
'assignable' implies several things, e.g. the resource can be independently 
mapped to/accessed by user space or guest, DMAs between two
ADIs are isolated, operating one ADI doesn't affecting another ADI,
etc.  I'm not clear about  other vendor specific interfaces, but supposing
they need match the similar requirements. Then do we really want to
differentiate ADI vs. SF? What about merging them with ADI as just
one example of finer-grained slicing?

> 
> 
> >
> >> @@ -0,0 +1,2 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +obj-$(CONFIG_VDPA) += vdpa.o
> >> diff --git a/drivers/virtio/vdpa/vdpa.c b/drivers/virtio/vdpa/vdpa.c
> >> new file mode 100644
> >> index 000000000000..2b0e4a9f105d
> >> +++ b/drivers/virtio/vdpa/vdpa.c
> >> @@ -0,0 +1,141 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * vDPA bus.
> >> + *
> >> + * Copyright (c) 2019, Red Hat. All rights reserved.
> >> + *     Author: Jason Wang <jasowang@redhat.com>
> > 2020 tests days
> 
> 
> Will fix.
> 
> 
> >
> >> + *
> >> + */
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/idr.h>
> >> +#include <linux/vdpa.h>
> >> +
> >> +#define MOD_VERSION  "0.1"
> > I think module versions are discouraged these days
> 
> 
> Will remove.
> 
> 
> >
> >> +#define MOD_DESC     "vDPA bus"
> >> +#define MOD_AUTHOR   "Jason Wang <jasowang@redhat.com>"
> >> +#define MOD_LICENSE  "GPL v2"
> >> +
> >> +static DEFINE_IDA(vdpa_index_ida);
> >> +
> >> +struct device *vdpa_get_parent(struct vdpa_device *vdpa)
> >> +{
> >> +	return vdpa->dev.parent;
> >> +}
> >> +EXPORT_SYMBOL(vdpa_get_parent);
> >> +
> >> +void vdpa_set_parent(struct vdpa_device *vdpa, struct device *parent)
> >> +{
> >> +	vdpa->dev.parent = parent;
> >> +}
> >> +EXPORT_SYMBOL(vdpa_set_parent);
> >> +
> >> +struct vdpa_device *dev_to_vdpa(struct device *_dev)
> >> +{
> >> +	return container_of(_dev, struct vdpa_device, dev);
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_to_vdpa);
> >> +
> >> +struct device *vdpa_to_dev(struct vdpa_device *vdpa)
> >> +{
> >> +	return &vdpa->dev;
> >> +}
> >> +EXPORT_SYMBOL_GPL(vdpa_to_dev);
> > Why these trivial assessors? Seems unnecessary, or should at least be
> > static inlines in a header
> 
> 
> Will fix.
> 
> 
> >
> >> +int register_vdpa_device(struct vdpa_device *vdpa)
> >> +{
> > Usually we want to see symbols consistently prefixed with vdpa_*, is
> > there a reason why register/unregister are swapped?
> 
> 
> I follow the name from virtio. I will switch to vdpa_*.
> 
> 
> >
> >> +	int err;
> >> +
> >> +	if (!vdpa_get_parent(vdpa))
> >> +		return -EINVAL;
> >> +
> >> +	if (!vdpa->config)
> >> +		return -EINVAL;
> >> +
> >> +	err = ida_simple_get(&vdpa_index_ida, 0, 0, GFP_KERNEL);
> >> +	if (err < 0)
> >> +		return -EFAULT;
> >> +
> >> +	vdpa->dev.bus = &vdpa_bus;
> >> +	device_initialize(&vdpa->dev);
> > IMHO device_initialize should not be called inside something called
> > register, toooften we find out that the caller drivers need the device
> > to be initialized earlier, ie to use the kref, or something.
> >
> > I find the best flow is to have some init function that does the
> > device_initialize and sets the device_name that the driver can call
> > early.
> 
> 
> Ok, will do.
> 
> 
> >
> > Shouldn't there be a device/driver matching process of some kind?
> 
> 
> The question is what do we want do match here.
> 
> 1) "virtio" vs "vhost", I implemented matching method for this in mdev
> series, but it looks unnecessary for vDPA device driver to know about
> this. Anyway we can use sysfs driver bind/unbind to switch drivers
> 2) virtio device id and vendor id. I'm not sure we need this consider
> the two drivers so far (virtio/vhost) are all bus drivers.
> 
> Thanks
> 
> 
> >
> > Jason
> >


  parent reply index

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16 12:42 [PATCH 0/5] vDPA support Jason Wang
2020-01-16 12:42 ` [PATCH 1/5] vhost: factor out IOTLB Jason Wang
2020-01-17  4:14   ` Randy Dunlap
2020-01-17  9:34     ` Jason Wang
2020-01-18  0:01   ` kbuild test robot
2020-01-18  0:40   ` kbuild test robot
2020-01-16 12:42 ` [PATCH 2/5] vringh: IOTLB support Jason Wang
2020-01-17 21:54   ` kbuild test robot
2020-01-17 22:33   ` kbuild test robot
2020-01-16 12:42 ` [PATCH 3/5] vDPA: introduce vDPA bus Jason Wang
2020-01-16 15:22   ` Jason Gunthorpe
2020-01-17  3:03     ` Jason Wang
2020-01-17 13:54       ` Jason Gunthorpe
2020-01-20  7:50         ` Jason Wang
2020-01-20 12:17         ` Michael S. Tsirkin
2020-01-20 17:50           ` Jason Gunthorpe
2020-01-20 21:56             ` Michael S. Tsirkin
2020-01-21 14:12               ` Jason Gunthorpe
2020-01-21 14:15                 ` Michael S. Tsirkin
2020-01-21 14:16                   ` Jason Gunthorpe
2020-01-21  8:40       ` Tian, Kevin [this message]
2020-01-21  9:41         ` Jason Wang
2020-01-17  4:16   ` Randy Dunlap
2020-01-17  9:34     ` Jason Wang
2020-01-17 12:13   ` Michael S. Tsirkin
2020-01-17 13:52     ` Jason Wang
     [not found]       ` <CAJPjb1+fG9L3=iKbV4Vn13VwaeDZZdcfBPvarogF_Nzhk+FnKg@mail.gmail.com>
2020-01-19  9:07         ` Shahaf Shuler
2020-01-19  9:59           ` Michael S. Tsirkin
2020-01-20  8:44             ` Jason Wang
2020-01-20 12:09               ` Michael S. Tsirkin
2020-01-21  3:32                 ` Jason Wang
2020-01-20  8:43           ` Jason Wang
2020-01-20 17:49             ` Jason Gunthorpe
2020-01-20 20:51               ` Shahaf Shuler
2020-01-20 21:25                 ` Michael S. Tsirkin
2020-01-20 21:47                   ` Shahaf Shuler
2020-01-20 21:59                     ` Michael S. Tsirkin
2020-01-21  6:01                       ` Shahaf Shuler
2020-01-21  7:57                         ` Jason Wang
2020-01-21 14:07                   ` Jason Gunthorpe
2020-01-21 14:16                     ` Michael S. Tsirkin
2020-01-20 21:48               ` Michael S. Tsirkin
2020-01-21  4:00               ` Jason Wang
2020-01-21  5:47                 ` Michael S. Tsirkin
2020-01-21  8:00                   ` Jason Wang
2020-01-21  8:15                     ` Michael S. Tsirkin
2020-01-21  8:35                       ` Jason Wang
2020-01-21 11:09                         ` Shahaf Shuler
2020-01-22  6:36                           ` Jason Wang
2020-01-21 14:05                       ` Jason Gunthorpe
2020-01-21 14:17                         ` Michael S. Tsirkin
2020-01-22  6:18                           ` Jason Wang
2020-01-20  8:19         ` Jason Wang
2020-01-16 12:42 ` [PATCH 4/5] virtio: introduce a vDPA based transport Jason Wang
2020-01-16 15:38   ` Jason Gunthorpe
2020-01-17  9:32     ` Jason Wang
2020-01-17 14:00       ` Jason Gunthorpe
2020-01-20  7:52         ` Jason Wang
2020-01-17  4:10   ` Randy Dunlap
2020-01-16 12:42 ` [PATCH 5/5] vdpasim: vDPA device simulator Jason Wang
2020-01-16 15:47   ` Jason Gunthorpe
2020-01-17  9:32     ` Jason Wang
2020-01-17 14:10       ` Jason Gunthorpe
2020-01-20  8:01         ` Jason Wang
2020-02-04  4:19         ` Jason Wang
2020-01-17  4:12   ` Randy Dunlap
2020-01-17  9:35     ` Jason Wang
2020-01-18 18:18   ` kbuild test robot
2020-01-28  3:32   ` Dan Carpenter
2020-02-04  4:07     ` Jason Wang
2020-02-04  8:21   ` Zhu Lingshan
2020-02-04  8:28     ` Jason Wang
2020-02-04 12:52       ` Jason Gunthorpe
2020-02-05  3:14         ` Jason Wang
2020-01-21  8:44 ` [PATCH 0/5] vDPA support Tian, Kevin
2020-01-21  9:39   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AADFC41AFE54684AB9EE6CBC0274A5D19D73EBA4@SHSMSX104.ccr.corp.intel.com \
    --to=kevin.tian@intel.com \
    --cc=aadam@redhat.com \
    --cc=cunming.liang@intel.com \
    --cc=eperezma@redhat.com \
    --cc=hanand@xilinx.com \
    --cc=haotian.wang@sifive.com \
    --cc=hch@infradead.org \
    --cc=jakub.kicinski@netronome.com \
    --cc=jasowang@redhat.com \
    --cc=jgg@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=kvm@vger.kernel.org \
    --cc=lingshan.zhu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lulu@redhat.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mhabets@solarflare.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=rdunlap@infradead.org \
    --cc=rob.miller@broadcom.com \
    --cc=shahafs@mellanox.com \
    --cc=stefanha@redhat.com \
    --cc=tiwei.bie@intel.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xiao.w.wang@intel.com \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git