[PATCH 0/2] crypto: qat - fix KPT disable and clarify ABI

[PATCH 0/2] crypto: qat - fix KPT disable and clarify ABI

[nitesh.venkatesh]

From: Nitesh Venkatesh <nitesh.venkatesh@intel.com>

This small series fixes a recoverability issue in the QAT KPT (Key
Protection Technology) sysfs interface.

Patch 1 fixes the KPT enable sysfs attribute so that '0' writes are
always accepted when the device is down. The asym-service check
previously rejected disable writes when the service was not 'asym',
leaving KPT reported as enabled after switching services.

Patch 2 documents that the device must be in the down state before
the KPT enable attribute can be changed.

Nitesh Venkatesh (2):
  crypto: qat - allow KPT disable when service is not asym
  Documentation: qat: clarify KPT enable device-state requirement

 .../ABI/testing/sysfs-driver-qat_kpt          |  3 ++-
 .../intel/qat/qat_common/adf_sysfs_kpt.c      | 24 ++++++++++++-------
 2 files changed, 18 insertions(+), 9 deletions(-)


base-commit: 6f440283743bd408bfe95a3d1f77aac559130e39
-- 
2.52.0

[PATCH 1/2] crypto: qat - allow KPT disable when service is not asym

[nitesh.venkatesh]

From: Nitesh Venkatesh <nitesh.venkatesh@intel.com>

The KPT (Key Protection Technology) enable sysfs attribute rejects any
write when the currently enabled service is not 'asym', including writes
that disable KPT. This blocks a legitimate recovery flow:

  1. Configure the 'asym' service and enable KPT.
  2. Bring the device up, then down.
  3. Reconfigure the service to 'sym' and bring the device up.
  4. KPT is still reported as enabled, but is not active because the
     asymmetric service is no longer configured.
  5. Bring the device back down so the KPT enable attribute becomes
     writable.
  6. Writing '0' to the KPT enable attribute to clear the stale flag
     fails with:
       "KPT can only be enabled when the asymmetric service is enabled"

The asym-service check is only meaningful when enabling KPT; disabling
must always be permitted so a stale enable flag can be cleared after
the service configuration has changed.

Parse the user-supplied boolean before the service check and gate the
check on the enable direction. Disable requests are now accepted
regardless of the currently configured service, provided the device is
in the down state (which the attribute already requires for any write).

Fixes: fb98254a5eb9 ("crypto: qat - add KPT support for GEN6 devices")
Signed-off-by: Nitesh Venkatesh <nitesh.venkatesh@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Ahsan Atta <ahsan.atta@intel.com>
---
 .../intel/qat/qat_common/adf_sysfs_kpt.c      | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/intel/qat/qat_common/adf_sysfs_kpt.c b/drivers/crypto/intel/qat/qat_common/adf_sysfs_kpt.c
index 1f733ae80bdf..a0db98f33f1f 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_sysfs_kpt.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_sysfs_kpt.c
@@ -31,6 +31,7 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
 	struct adf_hw_device_data *hw_data;
 	struct adf_accel_dev *accel_dev;
 	bool enable;
+	int svc;
 	int ret;
 
 	accel_dev = adf_devmgr_pci_to_accel_dev(to_pci_dev(dev));
@@ -38,14 +39,25 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 
 	if (adf_dev_started(accel_dev)) {
-		dev_info(dev, "Device qat_dev%d must be down before enabling KPT\n",
+		dev_info(dev, "Device qat_dev%d must be down before changing KPT state\n",
 			 accel_dev->accel_id);
 		return -EINVAL;
 	}
 
-	if (adf_get_service_enabled(accel_dev) != SVC_ASYM) {
-		dev_info(dev, "KPT can only be enabled when the asymmetric service is enabled\n");
-		return -EINVAL;
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		svc = adf_get_service_enabled(accel_dev);
+		if (svc < 0)
+			return svc;
+
+		if (svc != SVC_ASYM) {
+			dev_info(dev,
+				 "KPT can only be enabled when the asymmetric crypto service is enabled\n");
+			return -EINVAL;
+		}
 	}
 
 	hw_data = GET_HW_DATA(accel_dev);
@@ -59,10 +71,6 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
 	if (!hw_data->accel_capabilities_mask)
 		return -EINVAL;
 
-	ret = kstrtobool(buf, &enable);
-	if (ret)
-		return ret;
-
 	user_data = GET_KPT_USER_DATA(accel_dev);
 	user_data->enable = enable;
 
-- 
2.52.0

[PATCH 2/2] Documentation: qat: clarify KPT enable device-state requirement

[nitesh.venkatesh]

From: Nitesh Venkatesh <nitesh.venkatesh@intel.com>

The ABI description for /sys/bus/pci/devices/<BDF>/qat_kpt/enable did
not mention that the device must be in the down state before the KPT
enable attribute can be written. This requirement is enforced by the
driver for both enabling and disabling KPT, but was not reflected in
the ABI documentation, leaving users to discover it via -EINVAL.

Update the description to note that the device must be in the down
state before changing the KPT state.

Signed-off-by: Nitesh Venkatesh <nitesh.venkatesh@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Ahsan Atta <ahsan.atta@intel.com>
---
 Documentation/ABI/testing/sysfs-driver-qat_kpt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-qat_kpt b/Documentation/ABI/testing/sysfs-driver-qat_kpt
index c6480ea1fa4f..80831ce3d2ca 100644
--- a/Documentation/ABI/testing/sysfs-driver-qat_kpt
+++ b/Documentation/ABI/testing/sysfs-driver-qat_kpt
@@ -14,7 +14,8 @@ Contact:	qat-linux@intel.com
 Description:
 		(RW) Enables or disables Key Protection Technology (KPT).
 
-		Write 1 to enable KPT, or 0 to disable it.
+		Write 1 to enable KPT, or 0 to disable it. The device must be
+		in the down state before changing the KPT state.
 
 		Example usage::
 
-- 
2.52.0