Updated documentation for optional read only sysfs attribute.
Signed-off-by: Parav Pandit <[email protected]>
---
Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..0ab03d3f5629 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev Device
|--- remove
|--- mdev_type {link to its type}
|--- vendor-specific-attributes [optional]
+ |--- alias [optional]
* remove (write only)
@@ -281,6 +282,10 @@ Example::
# echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
+* alias (read only)
+Whenever a parent requested to generate an alias, each mdev is assigned a unique
+alias by the mdev core. This file shows the alias of the mdev device.
+
Mediated device Hot plug
------------------------
--
2.19.2
On Thu, 29 Aug 2019 06:19:03 -0500
Parav Pandit <[email protected]> wrote:
> Updated documentation for optional read only sysfs attribute.
I'd probably merge this into the patch introducing the attribute.
>
> Signed-off-by: Parav Pandit <[email protected]>
> ---
> Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..0ab03d3f5629 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev Device
> |--- remove
> |--- mdev_type {link to its type}
> |--- vendor-specific-attributes [optional]
> + |--- alias [optional]
"optional" implies "not always present" to me, not "might return a read
error if not available". Don't know if there's a better way to tag
this? Or make it really optional? :)
>
> * remove (write only)
>
> @@ -281,6 +282,10 @@ Example::
>
> # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
>
> +* alias (read only)
> +Whenever a parent requested to generate an alias, each mdev is assigned a unique
> +alias by the mdev core. This file shows the alias of the mdev device.
It's not really the parent, but the vendor driver requesting this,
right? Also, "each mdev" is a bit ambiguous, as this is only true for
the subset of mdevs created via that driver. Lastly, if we stick with
the "returns an error if not implemented" approach, that should also be
mentioned here.
> +
> Mediated device Hot plug
> ------------------------
>
> -----Original Message-----
> From: Cornelia Huck <[email protected]>
> Sent: Friday, August 30, 2019 6:19 PM
> To: Parav Pandit <[email protected]>
> Cc: [email protected]; Jiri Pirko <[email protected]>;
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
>
> On Thu, 29 Aug 2019 06:19:03 -0500
> Parav Pandit <[email protected]> wrote:
>
> > Updated documentation for optional read only sysfs attribute.
>
> I'd probably merge this into the patch introducing the attribute.
>
Ok. I will spin v3.
> >
> > Signed-off-by: Parav Pandit <[email protected]>
> > ---
> > Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > b/Documentation/driver-api/vfio-mediated-device.rst
> > index 25eb7d5b834b..0ab03d3f5629 100644
> > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev
> Device
> > |--- remove
> > |--- mdev_type {link to its type}
> > |--- vendor-specific-attributes [optional]
> > + |--- alias [optional]
>
> "optional" implies "not always present" to me, not "might return a read error if
> not available". Don't know if there's a better way to tag this? Or make it really
> optional? :)
May be write it as,
alias [ optional when requested by parent ]
>
> >
> > * remove (write only)
> >
> > @@ -281,6 +282,10 @@ Example::
> >
> > # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> >
> > +* alias (read only)
> > +Whenever a parent requested to generate an alias, each mdev is
> > +assigned a unique alias by the mdev core. This file shows the alias of the
> mdev device.
>
> It's not really the parent, but the vendor driver requesting this, right? Also,
At mdev level, it only knows parent->ops structure, whether parent is registered by vendor driver or something else.
> "each mdev" is a bit ambiguous,
It is in context of the parent. Sentence is not starting with "each mdev".
But may be more verbosely written as,
Whenever a parent requested to generate an alias, Each mdev device of such parent is assigned
unique alias by the mdev core. This file shows the alias of the mdev device.
> created via that driver. Lastly, if we stick with the "returns an error if not
> implemented" approach, that should also be mentioned here.
Ok. Will spin v3 to describe it.
>
> > +
> > Mediated device Hot plug
> > ------------------------
> >
On Fri, 30 Aug 2019 13:10:17 +0000
Parav Pandit <[email protected]> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <[email protected]>
> > Sent: Friday, August 30, 2019 6:19 PM
> > To: Parav Pandit <[email protected]>
> > Cc: [email protected]; Jiri Pirko <[email protected]>;
> > [email protected]; [email protected]; [email protected]; linux-
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
> >
> > On Thu, 29 Aug 2019 06:19:03 -0500
> > Parav Pandit <[email protected]> wrote:
> >
> > > Updated documentation for optional read only sysfs attribute.
> >
> > I'd probably merge this into the patch introducing the attribute.
> >
> Ok. I will spin v3.
>
> > >
> > > Signed-off-by: Parav Pandit <[email protected]>
> > > ---
> > > Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > > b/Documentation/driver-api/vfio-mediated-device.rst
> > > index 25eb7d5b834b..0ab03d3f5629 100644
> > > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > > @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev
> > Device
> > > |--- remove
> > > |--- mdev_type {link to its type}
> > > |--- vendor-specific-attributes [optional]
> > > + |--- alias [optional]
> >
> > "optional" implies "not always present" to me, not "might return a read error if
> > not available". Don't know if there's a better way to tag this? Or make it really
> > optional? :)
>
> May be write it as,
>
> alias [ optional when requested by parent ]
I'm not sure what 'optional when requested' is supposed to mean...
maybe something like 'content optional' or so?
>
> >
> > >
> > > * remove (write only)
> > >
> > > @@ -281,6 +282,10 @@ Example::
> > >
> > > # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> > >
> > > +* alias (read only)
> > > +Whenever a parent requested to generate an alias, each mdev is
> > > +assigned a unique alias by the mdev core. This file shows the alias of the
> > mdev device.
> >
> > It's not really the parent, but the vendor driver requesting this, right? Also,
> At mdev level, it only knows parent->ops structure, whether parent is registered by vendor driver or something else.
Who else is supposed to create the mdev device?
>
> > "each mdev" is a bit ambiguous,
> It is in context of the parent. Sentence is not starting with "each mdev".
> But may be more verbosely written as,
>
> Whenever a parent requested to generate an alias, Each mdev device of such parent is assigned
> unique alias by the mdev core. This file shows the alias of the mdev device.
I'd really leave the parent out of this: this seems more like an
implementation detail. It's more that alias may either contain an
alias, or return a read error if no alias has been generated. Who
requested the alias to be generated is probably not really of interest
to the userspace reader.
>
> > created via that driver. Lastly, if we stick with the "returns an error if not
> > implemented" approach, that should also be mentioned here.
> Ok. Will spin v3 to describe it.
>
> >
> > > +
> > > Mediated device Hot plug
> > > ------------------------
> > >
>
> -----Original Message-----
> From: Cornelia Huck <[email protected]>
> Sent: Monday, September 2, 2019 8:07 PM
> To: Parav Pandit <[email protected]>
> Cc: [email protected]; Jiri Pirko <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
>
> On Fri, 30 Aug 2019 13:10:17 +0000
> Parav Pandit <[email protected]> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <[email protected]>
> > > Sent: Friday, August 30, 2019 6:19 PM
> > > To: Parav Pandit <[email protected]>
> > > Cc: [email protected]; Jiri Pirko <[email protected]>;
> > > [email protected]; [email protected]; [email protected];
> > > linux- [email protected]; [email protected]
> > > Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
> > >
> > > On Thu, 29 Aug 2019 06:19:03 -0500
> > > Parav Pandit <[email protected]> wrote:
> > >
> > > > Updated documentation for optional read only sysfs attribute.
> > >
> > > I'd probably merge this into the patch introducing the attribute.
> > >
> > Ok. I will spin v3.
> >
> > > >
> > > > Signed-off-by: Parav Pandit <[email protected]>
> > > > ---
> > > > Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > > > b/Documentation/driver-api/vfio-mediated-device.rst
> > > > index 25eb7d5b834b..0ab03d3f5629 100644
> > > > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > > > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > > > @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each
> > > > mdev
> > > Device
> > > > |--- remove
> > > > |--- mdev_type {link to its type}
> > > > |--- vendor-specific-attributes [optional]
> > > > + |--- alias [optional]
> > >
> > > "optional" implies "not always present" to me, not "might return a
> > > read error if not available". Don't know if there's a better way to
> > > tag this? Or make it really optional? :)
> >
> > May be write it as,
> >
> > alias [ optional when requested by parent ]
>
> I'm not sure what 'optional when requested' is supposed to mean...
> maybe something like 'content optional' or so?
>
> >
> > >
> > > >
> > > > * remove (write only)
> > > >
> > > > @@ -281,6 +282,10 @@ Example::
> > > >
> > > > # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> > > >
> > > > +* alias (read only)
> > > > +Whenever a parent requested to generate an alias, each mdev is
> > > > +assigned a unique alias by the mdev core. This file shows the
> > > > +alias of the
> > > mdev device.
> > >
> > > It's not really the parent, but the vendor driver requesting this,
> > > right? Also,
> > At mdev level, it only knows parent->ops structure, whether parent is
> registered by vendor driver or something else.
>
> Who else is supposed to create the mdev device?
If you nitpick the language what is the vendor id for sample mttty driver?
Mtty is not a 'vendor driver' per say.
>
> >
> > > "each mdev" is a bit ambiguous,
> > It is in context of the parent. Sentence is not starting with "each mdev".
> > But may be more verbosely written as,
> >
> > Whenever a parent requested to generate an alias, Each mdev device of
> > such parent is assigned unique alias by the mdev core. This file shows the
> alias of the mdev device.
>
> I'd really leave the parent out of this: this seems more like an
> implementation detail. It's more that alias may either contain an alias, or
> return a read error if no alias has been generated. Who requested the alias
> to be generated is probably not really of interest to the userspace reader.
>
The documentation is for user and developer both.
It is not the right claim that 'only user care' for this.
Otherwise all the .ko diagrams and API description etc doesn't make any sense to the user.
For user it doesn't matter whether alias length is provided by 'vendor driver' or 'registered parent'.
This note on who should specify the alias length is mainly for the developers.
> >
> > > created via that driver. Lastly, if we stick with the "returns an
> > > error if not implemented" approach, that should also be mentioned
> here.
> > Ok. Will spin v3 to describe it.
> >
> > >
> > > > +
> > > > Mediated device Hot plug
> > > > ------------------------
> > > >
> >