qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: "cjia@nvidia.com" <cjia@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"aik@ozlabs.ru" <aik@ozlabs.ru>,
	"Zhengxiao.zx@alibaba-inc.com" <Zhengxiao.zx@alibaba-inc.com>,
	"shuangtai.tst@alibaba-inc.com" <shuangtai.tst@alibaba-inc.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"eauger@redhat.com" <eauger@redhat.com>,
	"Liu, Yi L" <yi.l.liu@intel.com>,
	"eskultet@redhat.com" <eskultet@redhat.com>,
	"Yang, Ziye" <ziye.yang@intel.com>,
	"mlevitsk@redhat.com" <mlevitsk@redhat.com>,
	"pasic@linux.ibm.com" <pasic@linux.ibm.com>,
	"libvir-list@redhat.com" <libvir-list@redhat.com>,
	"arei.gonglei@huawei.com" <arei.gonglei@huawei.com>,
	"felipe@nutanix.com" <felipe@nutanix.com>,
	"Ken.Xue@amd.com" <Ken.Xue@amd.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"dgilbert@redhat.com" <dgilbert@redhat.com>,
	"zhenyuw@linux.intel.com" <zhenyuw@linux.intel.com>,
	"jonathan.davies@nutanix.com" <jonathan.davies@nutanix.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"intel-gvt-dev@lists.freedesktop.org"
	<intel-gvt-dev@lists.freedesktop.org>,
	"Liu, Changpeng" <changpeng.liu@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Wang, Zhi A" <zhi.a.wang@intel.com>,
	"dinechin@redhat.com" <dinechin@redhat.com>,
	"He, Shaopeng" <shaopeng.he@intel.com>
Subject: Re: [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
Date: Thu, 9 May 2019 17:24:49 +0200	[thread overview]
Message-ID: <20190509172449.723a048b.cohuck@redhat.com> (raw)
In-Reply-To: <20190508115704.GB24397@joy-OptiPlex-7040>

On Wed, 8 May 2019 07:57:05 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:

> On Tue, May 07, 2019 at 05:19:54PM +0800, Cornelia Huck wrote:
> > On Sun,  5 May 2019 21:49:04 -0400
> > Yan Zhao <yan.y.zhao@intel.com> wrote:
> >   
> > > version attribute is used to check two mdev devices' compatibility.
> > > 
> > > The key point of this version attribute is that it's rw.
> > > User space has no need to understand internal of device version and no
> > > need to compare versions by itself.
> > > Compared to reading version strings from both two mdev devices being
> > > checked, user space only reads from one mdev device's version attribute.
> > > After getting its version string, user space writes this string into the
> > > other mdev device's version attribute. Vendor driver of mdev device
> > > whose version attribute being written will check device compatibility of
> > > the two mdev devices for user space and return success for compatibility
> > > or errno for incompatibility.  
> > 
> > I'm still missing a bit _what_ is actually supposed to be
> > compatible/incompatible. I'd assume some internal state descriptions
> > (even if this is not actually limited to migration).
> >  
> right.
> originally, I thought this attribute should only contain a device's hardware
> compatibility info. But seems also including vendor specific software migration
> version is more reasonable, because general VFIO migration code cannot know
> version of vendor specific software migration code until migration data is
> transferring to the target vm. Then renaming it to migration_version is more
> appropriate.
> :)

Nod.

(...)

> > > @@ -246,6 +249,143 @@ Directories and files under the sysfs for Each Physical Device
> > >    This attribute should show the number of devices of type <type-id> that can be
> > >    created.
> > >  
> > > +* version
> > > +
> > > +  This attribute is rw, and is optional.
> > > +  It is used to check device compatibility between two mdev devices and is
> > > +  accessed in pairs between the two mdev devices being checked.
> > > +  The intent of this attribute is to make an mdev device's version opaque to
> > > +  user space, so instead of reading two mdev devices' version strings and
> > > +  comparing in userspace, user space should only read one mdev device's version
> > > +  attribute, and writes this version string into the other mdev device's version
> > > +  attribute. Then vendor driver of mdev device whose version attribute being
> > > +  written would check the incoming version string and tell user space whether
> > > +  the two mdev devices are compatible via return value. That's why this
> > > +  attribute is writable.  
> > 
> > I would reword this a bit:
> > 
> > "This attribute provides a way to check device compatibility between
> > two mdev devices from userspace. The intended usage is for userspace to
> > read the version attribute from one mdev device and then writing that
> > value to the version attribute of the other mdev device. The second
> > mdev device indicates compatibility via the return code of the write
> > operation. This makes compatibility between mdev devices completely
> > vendor-defined and opaque to userspace."
> > 
> > We still should explain _what_ compatibility we're talking about here,
> > though.
> >   
> Thanks. It's much better than mine:) 
> Then I'll change compatibility --> migration compatibility.

Ok, with that it should be clear enough.

> 
> > > +
> > > +  when reading this attribute, it should show device version string of
> > > +  the device of type <type-id>.
> > > +
> > > +  This string is private to vendor driver itself. Vendor driver is able to
> > > +  freely define format and length of device version string.
> > > +  e.g. It can use a combination of pciid of parent device + mdev type.
> > > +
> > > +  When writing a string to this attribute, vendor driver should analyze this
> > > +  string and check whether the mdev device being identified by this string is
> > > +  compatible with the mdev device for this attribute. vendor driver should then
> > > +  return written string's length if it regards the two mdev devices are
> > > +  compatible; vendor driver should return negative errno if it regards the two
> > > +  mdev devices are not compatible.
> > > +
> > > +  User space should treat ANY of below conditions as two mdev devices not
> > > +  compatible:
> > > +  (1) any one of the two mdev devices does not have a version attribute
> > > +  (2) error when read from one mdev device's version attribute  
> > 
> > s/read/reading/
> >   
> > > +  (3) error when write one mdev device's version string to the other mdev  
> > 
> > s/write/writing/
> >   
> > > +  device's version attribute
> > > +
> > > +  User space should regard two mdev devices compatible when ALL of below
> > > +  conditions are met:
> > > +  (1) success when read from one mdev device's version attribute.  
> > 
> > s/read/reading/
> >   
> > > +  (2) success when write one mdev device's version string to the other mdev  
> > 
> > s/write/writing/  
> got it. thanks for pointing them out:)
> >   
> > > +  device's version attribute
> > > +
> > > +  Errno:
> > > +  If vendor driver wants to claim a mdev device incompatible to all other mdev  
> > 
> > "If the vendor driver wants to designate a mdev device..."
> >   
> ok. thanks:)
> > > +  devices, it should not register version attribute for this mdev device. But if
> > > +  a vendor driver has already registered version attribute and it wants to claim
> > > +  a mdev device incompatible to all other mdev devices, it needs to return
> > > +  -ENODEV on access to this mdev device's version attribute.
> > > +  If a mdev device is only incompatible to certain mdev devices, write of
> > > +  incompatible mdev devices's version strings to its version attribute should
> > > +  return -EINVAL;  
> > 
> > 
> > Maybe put the defined return code into a bulleted list instead? But
> > this looks reasonable as well.
> >   
> as user space have no idea of those errno and only gets 0/1 as return code from
> read/write. maybe I can move this description of errno to patch 2/2 as an
> example?

Confused. They should get -EINVAL/-ENODEV/... all right, shouldn't they?

> 
> > > +
> > > +  This attribute can be taken advantage of by live migration.
> > > +  If user space detects two mdev devices are compatible through version
> > > +  attribute, it can start migration between the two mdev devices, otherwise it
> > > +  should abort its migration attempts between the two mdev devices.  
> > 
> > (...)
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev  



  reply	other threads:[~2019-05-09 15:26 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06  1:45 [Qemu-devel] [PATCH v2 0/2] introduction of version attribute for VFIO live migration Yan Zhao
2019-05-06  1:49 ` [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device Yan Zhao
2019-05-07  9:19   ` Cornelia Huck
2019-05-08 11:57     ` Yan Zhao
2019-05-09 15:24       ` Cornelia Huck [this message]
2019-05-10  2:43         ` Yan Zhao
2019-05-07 21:18   ` Alex Williamson
2019-05-08 11:27     ` Yan Zhao
2019-05-08 21:22       ` Alex Williamson
2019-05-08 15:27         ` [Qemu-devel] [libvirt] " Boris Fiuczynski
2019-05-09  6:55           ` Yan Zhao
2019-05-14 15:31           ` Alex Williamson
2019-05-28 20:57             ` Boris Fiuczynski
2019-05-29 14:08               ` Alex Williamson
2019-05-09  3:10         ` [Qemu-devel] " Yan Zhao
2019-05-09  3:38           ` Alex Williamson
2019-05-09  5:48             ` Yan Zhao
2019-05-09 15:38     ` Cornelia Huck
2019-05-09 15:48       ` Dr. David Alan Gilbert
2019-05-09 15:54         ` Cornelia Huck
2019-05-09 16:48           ` Dr. David Alan Gilbert
2019-05-10  9:08             ` Cornelia Huck
2019-05-10  9:36               ` Dr. David Alan Gilbert
2019-05-10  9:48                 ` Cornelia Huck
2019-05-13  1:16                   ` Yan Zhao
2019-05-13 13:28                   ` Erik Skultety
2019-05-14  6:12                     ` Yan Zhao
2019-05-14  7:03                       ` Cornelia Huck
2019-05-14  7:20                       ` Erik Skultety
2019-05-14  7:32                         ` Yan Zhao
2019-05-14  7:43                           ` Erik Skultety
2019-05-14  7:47                             ` Yan Zhao
2019-05-14  9:51                               ` Cornelia Huck
2019-05-14 10:57                                 ` Erik Skultety
2019-05-14 11:01                                 ` Dr. David Alan Gilbert
2019-05-14 11:30                                   ` Cornelia Huck
2019-05-14 15:01                             ` Alex Williamson
2019-05-16  1:00                               ` Yan Zhao
2019-05-06  1:51 ` [Qemu-devel] [PATCH v2 2/2] drm/i915/gvt: export mdev device version to sysfs for Intel vGPU Yan Zhao
2019-05-06  3:20   ` Zhenyu Wang
2019-05-06  7:41     ` Zhenyu Wang
2019-05-07  5:43       ` Yan Zhao
2019-05-07  9:27   ` Cornelia Huck
2019-05-08 12:02     ` Yan Zhao
2019-05-08 10:50   ` Dr. David Alan Gilbert
2019-05-08 12:10     ` Yan Zhao

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=20190509172449.723a048b.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=Ken.Xue@amd.com \
    --cc=Zhengxiao.zx@alibaba-inc.com \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=arei.gonglei@huawei.com \
    --cc=changpeng.liu@intel.com \
    --cc=cjia@nvidia.com \
    --cc=dgilbert@redhat.com \
    --cc=dinechin@redhat.com \
    --cc=eauger@redhat.com \
    --cc=eskultet@redhat.com \
    --cc=felipe@nutanix.com \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=jonathan.davies@nutanix.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=libvir-list@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=shaopeng.he@intel.com \
    --cc=shuangtai.tst@alibaba-inc.com \
    --cc=yan.y.zhao@intel.com \
    --cc=yi.l.liu@intel.com \
    --cc=zhenyuw@linux.intel.com \
    --cc=zhi.a.wang@intel.com \
    --cc=ziye.yang@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).