Re: [PATCH v2] s390/vfio-ap: Driver feature advertisement

[Jason J. Herne]

On 9/11/24 2:57 AM, Heiko Carstens wrote:
> On Tue, Sep 10, 2024 at 07:34:40AM -0400, Jason J. Herne wrote:
> > Advertise features of the driver for the benefit of automated tooling
> > like Libvirt and mdevctl.
> >
> > Signed-off-by: Jason J. Herne <jjherne@xxxxxxxxxxxxx>
> > Reviewed-by: Anthony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> > ---
> >   Documentation/arch/s390/vfio-ap.rst | 34 +++++++++++++++++++++++++++++
> >   drivers/s390/crypto/vfio_ap_drv.c   | 13 +++++++++++
> >   2 files changed, 47 insertions(+)
>
> ...
>
> > +Driver Features
> > +===============
> > +The vfio_ap driver exposes a sysfs file containing supported features.
> > +This exists so third party tools (like Libvirt and mdevctl) can query the
> > +availability of specific features.
> > +
> > +The features list can be found here: /sys/bus/matrix/devices/matrix/features
> > +
> > +Entries are \n delimited. Each entry contains a key value pair. The key is made
> > +up of a combination of alphanumeric and underscore characters. The separator
> > +consists of a space, a colon and then another space. The value consists of
> > +alphanumeric, space, and underscore characters.
> > +
> > +Example:
> > +cat /sys/bus/matrix/devices/matrix/features
> > +flags : guest_matrix dyn ap_config
> > +
> > +Presently only a single field named flags is defined. It is meant to advertise a
> > +list of features the driver provides. The flags fields advertises the following
> > +features:
>
> I stumbled across this only now: sysfs files are not supposed to have
> several key value pairs. Actually the file(name) itself is supposed to
> be the key and its contents are the value. So I would expect:
>
> cat /sys/bus/matrix/devices/matrix/flags
> guest_matrix dyn ap_config
>
> Which is also easier to parse. Is there a good reason why this does
> not follow the general approach for sysfs files?


We talked about a few designs for this feature. This patch represents 
where we landed. I'm happy to make the change to the way you suggest but 
I'll give some time for Boris or Tony to speak up if they disagree.