On 8/27/20 6:32 AM, Cornelia Huck wrote:
On Wed, 26 Aug 2020 10:49:47 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
On 8/25/20 6:04 AM, Cornelia Huck wrote:
On Fri, 21 Aug 2020 15:56:01 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
Let's set a version for the vfio_ap module so that automated regression
tests can determine whether dynamic configuration tests can be run or
not.
Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
---
drivers/s390/crypto/vfio_ap_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
index be2520cc010b..f4ceb380dd61 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -17,10 +17,12 @@
#define VFIO_AP_ROOT_NAME "vfio_ap"
#define VFIO_AP_DEV_NAME "matrix"
+#define VFIO_AP_MODULE_VERSION "1.2.0"
MODULE_AUTHOR("IBM Corporation");
MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2018");
MODULE_LICENSE("GPL v2");
+MODULE_VERSION(VFIO_AP_MODULE_VERSION);
static struct ap_driver vfio_ap_drv;
Setting a version manually has some drawbacks:
- tools wanting to check for capabilities need to keep track which
versions support which features
- you need to remember to actually bump the version when adding a new,
visible feature
(- selective downstream backports may get into a pickle, but that's
arguably not your problem)
Is there no way for a tool to figure out whether this is supported?
E.g., via existence of a sysfs file, or via a known error that will
occur. If not, it's maybe better to expose known capabilities via a
generic interface.
This patch series introduces a new mediated device sysfs attribute,
guest_matrix, so the automated tests could check for the existence
of that interface. The problem I have with that is it will work for
this version of the vfio_ap device driver - which may be all that is
ever needed - but does not account for future enhancements
which may need to be detected by tooling or automated tests.
It seems to me that regardless of how a tool detects whether
a feature is supported or not, it will have to keep track of that
somehow.
Which enhancements? If you change the interface in an incompatible way,
you have a different problem anyway. If someone trying to use the
enhanced version of the interface gets an error on a kernel providing
an older version of the interface, that's a reasonable way to discover
support.
I think "discover device driver capabilities by probing" is less
burdensome and error prone than trying to match up capabilities with a
version number. If you expose a version number, a tool would still have
to probe that version number, and then consult with a list of features
per version, which can easily go out of sync.
Can you provide more details about this generic interface of
which you speak?
If that is really needed, I'd probably do a driver sysfs attribute that
exposes a list of documented capabilities (as integer values, or as a
bit.) But since tools can simply check for guest_matrix to find out
about support for this feature here, it seems like overkill to me --
unless you have a multitude of features waiting in queue that need to
be made discoverable.
Currently there are two tools that probably need to be aware of
the changes to these assignment interfaces:
* The hades test framework has tests that will fail if run against
these patches that should be skipped if over-provisioning is
allowed. There are also tests under development to test the
function introduced by these patches that will fail if run against
an older version of the driver. These tests should be skipped in
that case.
* There is a tool under development for configuring AP matrix
mediated devices that probably need to be aware of the change
introduced by this series.
Since a tool would have to first determine whether a new sysfs
interface documenting facilities is available and it would only
expose one facility at this point, it seems reasonable for these tools
to check for the sysfs guest_matrix attribute to discern whether
over-provisioning is available or not. I'll go ahead and remove this
patch from the series.
