| Message ID | 20200326054136.2543-2-zhenyuw@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VFIO mdev aggregated resources handling | expand |
> 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'? > + > +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? > + > +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. > Mediated device Hot plug > ------------------------ > > -- > 2.25.1
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. 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. > > + > > +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. Thanks > > > Mediated device Hot plug > > ------------------------ > > > > -- > > 2.25.1 >
> 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.
On 2020.03.27 06:16:11 +0000, Tian, Kevin wrote: > > 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. > yep, I ignored mdev_type link there. The original purpose is to have a general agreed directory to put generic parameters on mdev. > 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 e.g vfio-ccw or future SIOV driver is free to have custom attributes for mdev resource definition. If you choose that way, fine it's just defined by vendor. But if utilize common mdev parameter attributes, you get common defined resource config method instead. > > > > > > > + > > > > +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. > Sure. Thanks > > > > Mediated device Hot plug > > > > ------------------------ > > > > > > > > -- > > > > 2.25.1 > > > > > > > -- > > Open Source Technology Center, Intel ltd. > > > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
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) + +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. + +2) max_aggregation (read only) + +Show maxium instances for aggregation. + Mediated device Hot plug ------------------------
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(+)