KVM Archive on lore.kernel.org
 help / color / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: "Jiang, Dave" <dave.jiang@intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"intel-gvt-dev@lists.freedesktop.org" 
	<intel-gvt-dev@lists.freedesktop.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH v2 1/2] Documentation/driver-api/vfio-mediated-device.rst: update for aggregation support
Date: Fri, 27 Mar 2020 06:16:11 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D7ECF0E@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20200326082142.GC8880@zhen-hp.sh.intel.com>

> From: Zhenyu Wang
> Sent: Thursday, March 26, 2020 4:22 PM
> 
> On 2020.03.26 08:17:20 +0000, Tian, Kevin wrote:
> > > From: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Sent: Thursday, March 26, 2020 1:42 PM
> > >
> > > Update doc for mdev aggregation support. Describe mdev generic
> > > parameter directory under mdev device directory.
> > >
> > > Cc: Kevin Tian <kevin.tian@intel.com>
> > > Cc: "Jiang, Dave" <dave.jiang@intel.com>
> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > ---
> > >  .../driver-api/vfio-mediated-device.rst       | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > > b/Documentation/driver-api/vfio-mediated-device.rst
> > > index 25eb7d5b834b..29c29432a847 100644
> > > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > > @@ -269,6 +269,9 @@ Directories and Files Under the sysfs for Each
> mdev
> > > Device
> > >    |--- [$MDEV_UUID]
> > >           |--- remove
> > >           |--- mdev_type {link to its type}
> > > +         |--- mdev [optional]
> > > +	     |--- aggregated_instances [optional]
> > > +	     |--- max_aggregation [optional]
> > >           |--- vendor-specific-attributes [optional]
> > >
> > >  * remove (write only)
> > > @@ -281,6 +284,22 @@ Example::
> > >
> > >  	# echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> > >
> > > +* mdev directory (optional)
> >
> > It sounds confusing to me when seeing a 'mdev' directory under a
> > mdev instance. How could one tell which attribute should put inside
> > or outside of 'mdev'?
> >
> 
> After mdev create you get uuid directory under normal device path, so
> from that point a 'mdev' directory can just tell this is a mdev
> device. And it's proposed by Alex before.

I didn't quite get. Isn't $MDEV_UUID plus mdev_type already tell this is
a mdev device? If it is insufficient, then we're broken already since there
is no such 'mdev' sub-directory before.

Alex?

> 
> Currently only mdev core could create attribute e.g 'remove' under
> device dir, vendor specific attrs need to be in attrs group. So 'mdev'
> directory here tries to be optional generic interface.

I'm a bit confused. Then why cannot the new nodes exposed through
vendor specific attributes? I may overlook previous discussion why using
attrs group doesn't work here. 😊

> 
> > > +
> > > +Vendor driver could create mdev directory to specify extra generic
> > > parameters
> > > +on mdev device by its type. Currently aggregation parameters are
> defined.
> > > +Vendor driver should provide both items to support.
> > > +
> > > +1) aggregated_instances (read/write)
> > > +
> > > +Set target aggregated instances for device. Reading will show current
> > > +count of aggregated instances. Writing value larger than
> max_aggregation
> > > +would fail and return error.
> >
> > Can one write a value multiple-times and at any time?
> >
> 
> yeah, of coz multiple times, but normally won't succeed after open.
> 
> > > +
> > > +2) max_aggregation (read only)
> > > +
> > > +Show maxium instances for aggregation.
> > > +
> >
> > "show maximum-allowed instances which can be aggregated for this
> device". is
> > this value static or dynamic? if dynamic then the user is expected to read
> this
> > field before every write. worthy of some clarification here.
> 
> yeah, user needs to read this before setting actual number, either static or
> dynamic
> depends on vendor resource type.

Then adding above information might make the description clearer.

Thanks
Kevin

> 
> Thanks
> 
> >
> > >  Mediated device Hot plug
> > >  ------------------------
> > >
> > > --
> > > 2.25.1
> >
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26  5:41 [PATCH v2 0/2] VFIO mdev aggregated resources handling Zhenyu Wang
2020-03-26  5:41 ` [PATCH v2 1/2] Documentation/driver-api/vfio-mediated-device.rst: update for aggregation support Zhenyu Wang
2020-03-26  8:17   ` Tian, Kevin
2020-03-26  8:21     ` Zhenyu Wang
2020-03-27  6:16       ` Tian, Kevin [this message]
2020-03-27  6:21         ` Zhenyu Wang
2020-03-26  5:41 ` [PATCH v2 2/2] drm/i915/gvt: mdev aggregation type Zhenyu Wang
2020-03-27  7:48   ` Tian, Kevin
2020-03-27  8:12     ` Zhenyu Wang
2020-03-27  8:44       ` Tian, Kevin
2020-03-27  8:58         ` Zhenyu Wang
2020-03-27  9:31           ` Tian, Kevin

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=AADFC41AFE54684AB9EE6CBC0274A5D19D7ECF0E@SHSMSX104.ccr.corp.intel.com \
    --to=kevin.tian@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=dave.jiang@intel.com \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=kvm@vger.kernel.org \
    --cc=zhenyuw@linux.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