[PATCH RESEND 0/2] crypto: qat - enable three-service mode for QAT GEN5

[PATCH RESEND 0/2] crypto: qat - enable three-service mode for QAT GEN5

[Ahsan Atta]

Resending and we can ignore the feedback from Sashiko review because we
do not have QAT GEN5 SKUs that have some of the AEs fused off.

QAT GEN5 devices (qat_420xx) have enough accelerator engines to run
three services concurrently, but the driver currently only allows a
maximum of two to be configured at a time, matching the GEN4 limit.

This series lifts that restriction so that symmetric crypto, asymmetric
crypto and compression can be enabled together on GEN5, as is already
possible on GEN6 (qat_6xxx).

In summary:
Patch #1: Define the firmware object layout, ring-to-service mapping,
thread-to-arbiter mapping and service-to-accelerator-engine accounting
required for the sym;asym;dc configuration on GEN5, and register the
new service combination in adf_cfg_services so that it can be selected
through the cfg_services sysfs attribute.

Patch #2: Update the cfg_services ABI documentation to state that GEN5
devices (qat_420xx) support a maximum of three combined services,
alongside the existing GEN6 entry. Documentation only, no functional
change.

Bairavi Alagappan (2):
  crypto: qat - enable three-service mode for QAT GEN5 devices
  Documentation: qat: document three-service mode for QAT GEN5 devices

 Documentation/ABI/testing/sysfs-driver-qat    |   4 +-
 .../intel/qat/qat_420xx/adf_420xx_hw_data.c   | 104 +++++++++++++++++-
 .../intel/qat/qat_420xx/adf_420xx_hw_data.h   |   6 +
 .../intel/qat/qat_common/adf_cfg_services.c   |   3 +
 .../intel/qat/qat_common/adf_cfg_services.h   |   1 +
 5 files changed, 113 insertions(+), 5 deletions(-)


base-commit: 9c5d66e1ea78912b8a43ce16a1c14db1357e0ae0
-- 
2.50.1

[PATCH RESEND 1/2] crypto: qat - enable three-service mode for QAT GEN5 devices

[Ahsan Atta]

From: Bairavi Alagappan <bairavix.alagappan@intel.com>

QAT GEN5 devices have enough accelerator engines to support three
concurrent services. However, the qat_420xx module does not currently
allow configuring symmetric crypto, asymmetric crypto, and compression
services together.

Add support for enabling these three services concurrently on QAT GEN5
devices.

Define the corresponding firmware object layout, ring-to-service
mapping, thread-to-arbiter mapping, and service-to-accelerator-engine
accounting required for three-service mode.

Signed-off-by: Bairavi Alagappan <bairavix.alagappan@intel.com>
Signed-off-by: Ahsan Atta <ahsan.atta@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 .../intel/qat/qat_420xx/adf_420xx_hw_data.c   | 104 +++++++++++++++++-
 .../intel/qat/qat_420xx/adf_420xx_hw_data.h   |   6 +
 .../intel/qat/qat_common/adf_cfg_services.c   |   3 +
 .../intel/qat/qat_common/adf_cfg_services.h   |   1 +
 4 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
index d6f1f626cf5a..3f994ae6fd89 100644
--- a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
+++ b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
@@ -89,6 +89,14 @@ static const struct adf_fw_config adf_fw_dcc_config[] = {
 	{ADF_AE_GROUP_4, ADF_FW_ADMIN_OBJ},
 };
 
+static const struct adf_fw_config adf_fw_asym_sym_dc_config[] = {
+	{ADF_AE_GROUP_3, ADF_FW_DC_OBJ},
+	{ADF_AE_GROUP_2, ADF_FW_DC_OBJ},
+	{ADF_AE_GROUP_1, ADF_FW_SYM_OBJ},
+	{ADF_AE_GROUP_0, ADF_FW_ASYM_OBJ},
+	{ADF_AE_GROUP_4, ADF_FW_ADMIN_OBJ},
+};
+
 
 static struct adf_hw_device_class adf_420xx_class = {
 	.name = ADF_420XX_DEVICE_NAME,
@@ -135,6 +143,8 @@ static u32 uof_get_num_objs(struct adf_accel_dev *accel_dev)
 		return ARRAY_SIZE(adf_fw_asym_dc_config);
 	case SVC_SYM_DC:
 		return ARRAY_SIZE(adf_fw_sym_dc_config);
+	case SVC_SYM_ASYM_DC:
+		return ARRAY_SIZE(adf_fw_asym_sym_dc_config);
 	default:
 		return 0;
 	}
@@ -157,6 +167,8 @@ static const struct adf_fw_config *get_fw_config(struct adf_accel_dev *accel_dev
 		return adf_fw_asym_dc_config;
 	case SVC_SYM_DC:
 		return adf_fw_sym_dc_config;
+	case SVC_SYM_ASYM_DC:
+		return adf_fw_asym_sym_dc_config;
 	default:
 		return NULL;
 	}
@@ -288,13 +300,26 @@ static u32 get_accel_cap(struct adf_accel_dev *accel_dev)
 		return capabilities_asym | capabilities_dc;
 	case SVC_SYM_DC:
 		return capabilities_sym | capabilities_dc;
+	case SVC_SYM_ASYM_DC:
+		return capabilities_sym | capabilities_asym | capabilities_dc;
 	default:
 		return 0;
 	}
 }
 
+static const u32 thrd_to_arb_map_asym_sym_dc[ADF_420XX_MAX_ACCELENGINES] = {
+	0x00000011, 0x00000011, 0x00000011, 0x00000011,
+	0x00002222, 0x00002222, 0x00002222, 0x00002222,
+	0x000000cc, 0x000000cc, 0x000000cc, 0x000000cc,
+	0x000000cc, 0x000000cc, 0x000000cc, 0x000000cc,
+	0x0
+};
+
 static const u32 *adf_get_arbiter_mapping(struct adf_accel_dev *accel_dev)
 {
+	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
+		return thrd_to_arb_map_asym_sym_dc;
+
 	if (adf_gen4_init_thd2arb_map(accel_dev))
 		dev_warn(&GET_DEV(accel_dev),
 			 "Failed to generate thread to arbiter mapping");
@@ -425,6 +450,79 @@ static void adf_gen4_set_err_mask(struct adf_dev_err_mask *dev_err_mask)
 	dev_err_mask->ssmfeatren_mask = ADF_420XX_SSMFEATREN_MASK;
 }
 
+static u16 adf_420xx_get_ring_to_svc_map(struct adf_accel_dev *accel_dev)
+{
+	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
+		return ADF_GEN4_SYM_ASYM_DC_RING_TO_SRV_MAP;
+
+	return adf_gen4_get_ring_to_svc_map(accel_dev);
+}
+
+static bool adf_420xx_services_supported(unsigned long mask)
+{
+	int num_svc;
+
+	if (mask >= BIT(SVC_COUNT))
+		return false;
+
+	if (test_bit(SVC_DECOMP, &mask))
+		return false;
+
+	num_svc = hweight_long(mask);
+	switch (num_svc) {
+	case ADF_ONE_SERVICE:
+		return true;
+	case ADF_TWO_SERVICES:
+	case ADF_THREE_SERVICES:
+		return !test_bit(SVC_DCC, &mask);
+	default:
+		return false;
+	}
+}
+
+static void adf_update_rl_data(struct adf_rl_hw_data *rl_data)
+{
+	unsigned int num_groups;
+	unsigned int i;
+
+	num_groups = ARRAY_SIZE(adf_fw_asym_sym_dc_config) - ADF_GEN4_ADMIN_ACCELENGINES;
+
+	memset(rl_data->svc_ae_mask, 0, sizeof(rl_data->svc_ae_mask));
+
+	for (i = 0; i < num_groups; i++) {
+		switch (adf_fw_asym_sym_dc_config[i].obj) {
+		case ADF_FW_ASYM_OBJ:
+			rl_data->svc_ae_mask[SVC_ASYM] +=
+				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
+			break;
+		case ADF_FW_SYM_OBJ:
+			rl_data->svc_ae_mask[SVC_SYM] +=
+				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
+			break;
+		case ADF_FW_DC_OBJ:
+			rl_data->svc_ae_mask[SVC_DC] +=
+				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+static int adf_420xx_dev_config(struct adf_accel_dev *accel_dev)
+{
+	int ret;
+
+	ret = adf_gen4_dev_config(accel_dev);
+	if (ret)
+		return ret;
+
+	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
+		adf_update_rl_data(&GET_HW_DATA(accel_dev)->rl_data);
+
+	return 0;
+}
+
 void adf_init_hw_data_420xx(struct adf_hw_device_data *hw_data, u32 dev_id)
 {
 	hw_data->dev_class = &adf_420xx_class;
@@ -472,20 +570,20 @@ void adf_init_hw_data_420xx(struct adf_hw_device_data *hw_data, u32 dev_id)
 	hw_data->get_ena_thd_mask = get_ena_thd_mask;
 	hw_data->set_msix_rttable = adf_gen4_set_msix_default_rttable;
 	hw_data->set_ssm_wdtimer = adf_gen4_set_ssm_wdtimer;
-	hw_data->get_ring_to_svc_map = adf_gen4_get_ring_to_svc_map;
+	hw_data->get_ring_to_svc_map = adf_420xx_get_ring_to_svc_map;
 	hw_data->disable_iov = adf_disable_sriov;
 	hw_data->ring_pair_reset = adf_gen4_ring_pair_reset;
 	hw_data->bank_state_save = adf_bank_state_save;
 	hw_data->bank_state_restore = adf_bank_state_restore;
 	hw_data->enable_pm = adf_gen4_enable_pm;
 	hw_data->handle_pm_interrupt = adf_gen4_handle_pm_interrupt;
-	hw_data->dev_config = adf_gen4_dev_config;
+	hw_data->dev_config = adf_420xx_dev_config;
 	hw_data->start_timer = adf_timer_start;
 	hw_data->stop_timer = adf_timer_stop;
 	hw_data->get_hb_clock = adf_gen4_get_heartbeat_clock;
 	hw_data->num_hb_ctrs = ADF_NUM_HB_CNT_PER_AE;
 	hw_data->clock_frequency = ADF_420XX_AE_FREQ;
-	hw_data->services_supported = adf_gen4_services_supported;
+	hw_data->services_supported = adf_420xx_services_supported;
 	hw_data->get_svc_slice_cnt = adf_gen4_get_svc_slice_cnt;
 	hw_data->accel_capabilities_ext_mask = ADF_ACCEL_CAPABILITIES_EXT_ZSTD_LZ4S;
 
diff --git a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.h b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.h
index 99abbfc14820..870222bf855e 100644
--- a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.h
+++ b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.h
@@ -17,6 +17,12 @@
 #define ADF_420XX_PARITYERRORMASK_PKE_MASK	(0xFFF)
 #define ADF_420XX_PARITYERRORMASK_WAT_WCP_MASK	(0x3FF03FF)
 
+#define ADF_GEN4_SYM_ASYM_DC_RING_TO_SRV_MAP \
+	(ASYM << ADF_CFG_SERV_RING_PAIR_0_SHIFT | \
+	  SYM << ADF_CFG_SERV_RING_PAIR_1_SHIFT | \
+	 COMP << ADF_CFG_SERV_RING_PAIR_2_SHIFT | \
+	 COMP << ADF_CFG_SERV_RING_PAIR_3_SHIFT)
+
 /*
  * SSMFEATREN bit mask
  * BIT(4) - enables parity detection on CPP
diff --git a/drivers/crypto/intel/qat/qat_common/adf_cfg_services.c b/drivers/crypto/intel/qat/qat_common/adf_cfg_services.c
index f7237c7ef2b1..ef245c2e80c1 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_cfg_services.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_cfg_services.c
@@ -148,6 +148,9 @@ int adf_get_service_enabled(struct adf_accel_dev *accel_dev)
 	if (ret)
 		return ret;
 
+	if (test_bit(SVC_SYM, &mask) && test_bit(SVC_ASYM, &mask) && test_bit(SVC_DC, &mask))
+		return SVC_SYM_ASYM_DC;
+
 	if (test_bit(SVC_SYM, &mask) && test_bit(SVC_ASYM, &mask))
 		return SVC_SYM_ASYM;
 
diff --git a/drivers/crypto/intel/qat/qat_common/adf_cfg_services.h b/drivers/crypto/intel/qat/qat_common/adf_cfg_services.h
index 89be2f2c7233..d8b45645d2a9 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_cfg_services.h
+++ b/drivers/crypto/intel/qat/qat_common/adf_cfg_services.h
@@ -24,6 +24,7 @@ enum adf_composed_services {
 	SVC_SYM_ASYM = SVC_COUNT,
 	SVC_SYM_DC,
 	SVC_ASYM_DC,
+	SVC_SYM_ASYM_DC,
 };
 
 enum {
-- 
2.50.1

[PATCH RESEND 2/2] Documentation: qat: document three-service mode for QAT GEN5 devices

[Ahsan Atta]

From: Bairavi Alagappan <bairavix.alagappan@intel.com>

Update the cfg_services sysfs attribute documentation to describe
support for concurrent symmetric crypto, asymmetric crypto, and
compression services on QAT GEN5 devices.

Signed-off-by: Bairavi Alagappan <bairavix.alagappan@intel.com>
Signed-off-by: Ahsan Atta <ahsan.atta@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 Documentation/ABI/testing/sysfs-driver-qat | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-qat b/Documentation/ABI/testing/sysfs-driver-qat
index b0561b9fc4eb..48e17de3e0fd 100644
--- a/Documentation/ABI/testing/sysfs-driver-qat
+++ b/Documentation/ABI/testing/sysfs-driver-qat
@@ -41,8 +41,8 @@ Description:	(RW) Reports the current configuration of the QAT device.
 
 		Service combinations are permitted for all services except dcc.
 		On QAT GEN4 devices (qat_4xxx driver) a maximum of two services can be
-		combined and on QAT GEN6 devices (qat_6xxx driver ) a maximum of three
-		services can be combined.
+		combined. On QAT GEN5 devices (qat_420xx driver) and GEN6 devices
+		(qat_6xxx driver) a maximum of three services can be combined.
 		The order of services is not significant. For instance, sym;asym is
 		functionally equivalent to asym;sym.
 
-- 
2.50.1

Re: [PATCH RESEND 2/2] Documentation: qat: document three-service mode for QAT GEN5 devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] crypto: qat - stale rl_data.svc_ae_mask in adf_420xx_dev_config()
--

commit 0c0ee10ffff8629021fbc1b8499eb78f471d4264
Author: Bairavi Alagappan <bairavix.alagappan@intel.com>

Documentation: qat: document three-service mode for QAT GEN5 devices

This commit updates the sysfs ABI documentation for cfg_services to note
that QAT GEN5 devices (qat_420xx driver) support combining up to three
concurrent services.

> diff --git a/Documentation/ABI/testing/sysfs-driver-qat b/Documentation/ABI/testing/sysfs-driver-qat
> index b0561b9fc4eb9..48e17de3e0fdb 100644
> --- a/Documentation/ABI/testing/sysfs-driver-qat
> +++ b/Documentation/ABI/testing/sysfs-driver-qat
> @@ -41,8 +41,8 @@ Description:	(RW) Reports the current configuration of the QAT device.
>  
>  		Service combinations are permitted for all services except dcc.
>  		On QAT GEN4 devices (qat_4xxx driver) a maximum of two services can be
> -		combined and on QAT GEN6 devices (qat_6xxx driver ) a maximum of three
> -		services can be combined.
> +		combined. On QAT GEN5 devices (qat_420xx driver) and GEN6 devices
> +		(qat_6xxx driver) a maximum of three services can be combined.

[Severity: Medium]
When reconfiguring cfg_services on a QAT 420xx device from three-service
mode (sym;asym;dc) back to a one- or two-service mode via sysfs, does
rl_data.svc_ae_mask retain stale three-service engine counts?

During probe, adf_probe() -> adf_init_hw_data_420xx() -> adf_init_rl_data()
calls adf_gen4_init_num_svc_aes(), initializing hw_data->rl_data.svc_ae_mask
to 16 worker engines for each service.

When a user configures three-service mode via cfg_services and brings the
device up via state_store() -> adf_dev_up() -> adf_420xx_dev_config(),
rl_data.svc_ae_mask is overwritten in-place by adf_update_rl_data() with the
three-service engine counts (4 for asym, 4 for sym, and 8 for dc):

drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c:
static int adf_420xx_dev_config(struct adf_accel_dev *accel_dev)
{
	int ret;

	ret = adf_gen4_dev_config(accel_dev);
	if (ret)
		return ret;

	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
		adf_update_rl_data(&GET_HW_DATA(accel_dev)->rl_data);

	return 0;
}

If the user then brings the device down, changes cfg_services back to a
one- or two-service configuration (such as sym;asym or dc), and brings the
device up again, adf_get_service_enabled(accel_dev) is no longer
SVC_SYM_ASYM_DC, and nothing in adf_420xx_dev_config(), adf_dev_down(), or
adf_dev_up() resets rl_data.svc_ae_mask.

When adf_rl_start() or subsequent SLA operations calculate rate-limiting
cycle budgets in drivers/crypto/intel/qat/qat_common/adf_rl.c:

adf_rl_calculate_ae_cycles() {
	...
	avail_ae_cycles = hw_data->clock_frequency;
	avail_ae_cycles *= adf_rl_get_num_svc_aes(accel_dev, svc_type);
	do_div(avail_ae_cycles, device_data->scan_interval);
	...
}

wouldn't adf_rl_get_num_svc_aes() read the stale three-service svc_ae_mask
values and under-allocate SLA cycle budgets (1/4 of the intended budget for
sym/asym and 1/2 for dc), and should adf_420xx_dev_config() have an else
branch calling adf_gen4_init_num_svc_aes(&GET_HW_DATA(accel_dev)->rl_data)
to restore svc_ae_mask?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023144835.170453-1-ahsan.atta@intel.com?part=2

Re: [PATCH RESEND 1/2] crypto: qat - enable three-service mode for QAT GEN5 devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] crypto: qat - unrestored rl_data.svc_ae_mask in adf_420xx_dev_config()
--

commit 84452d0273c635ae08ee4c142e1d46cbf85da2de
Author: Bairavi Alagappan <bairavix.alagappan@intel.com>

crypto: qat - enable three-service mode for QAT GEN5 devices

This commit adds support for enabling symmetric crypto, asymmetric crypto,
and compression services concurrently on QAT GEN5 (qat_420xx) devices. It
defines the firmware object layout, ring-to-service mapping,
thread-to-arbiter mapping, and rate-limiting accelerator engine accounting
for three-service mode.

> diff --git a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> index 19f9f738630b1..281067f5b60c5 100644
> --- a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> +++ b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c

[ ... ]

> @@ -425,6 +450,79 @@ static void adf_gen4_set_err_mask(struct adf_dev_err_mask *dev_err_mask)

[ ... ]

> +static void adf_update_rl_data(struct adf_rl_hw_data *rl_data)
> +{
> +	unsigned int num_groups;
> +	unsigned int i;
> +
> +	num_groups = ARRAY_SIZE(adf_fw_asym_sym_dc_config) - ADF_GEN4_ADMIN_ACCELENGINES;
> +
> +	memset(rl_data->svc_ae_mask, 0, sizeof(rl_data->svc_ae_mask));
> +
> +	for (i = 0; i < num_groups; i++) {
> +		switch (adf_fw_asym_sym_dc_config[i].obj) {
> +		case ADF_FW_ASYM_OBJ:
> +			rl_data->svc_ae_mask[SVC_ASYM] +=
> +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> +			break;
> +		case ADF_FW_SYM_OBJ:
> +			rl_data->svc_ae_mask[SVC_SYM] +=
> +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> +			break;
> +		case ADF_FW_DC_OBJ:
> +			rl_data->svc_ae_mask[SVC_DC] +=
> +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +static int adf_420xx_dev_config(struct adf_accel_dev *accel_dev)
> +{
> +	int ret;
> +
> +	ret = adf_gen4_dev_config(accel_dev);
> +	if (ret)
> +		return ret;
> +
> +	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
> +		adf_update_rl_data(&GET_HW_DATA(accel_dev)->rl_data);
> +
> +	return 0;
> +}

[Severity: Medium]
During probe, adf_init_hw_data_420xx() -> adf_init_rl_data() ->
adf_gen4_init_num_svc_aes() initializes rl_data.svc_ae_mask to ae_cnt - 1
(16 on an unfused 420xx device) for SVC_ASYM, SVC_SYM, and SVC_DC.

When the device is brought down via sysfs, configured to SVC_SYM_ASYM_DC,
and brought back up via state_store() -> adf_dev_up(accel_dev, true) ->
adf_420xx_dev_config(), adf_update_rl_data() overwrites rl_data.svc_ae_mask
in-place with {SVC_ASYM: 4, SVC_SYM: 4, SVC_DC: 8, SVC_DECOMP: 0}.

If the device is subsequently brought down, reconfigured via cfg_services
to a 1-service or 2-service mode, and brought back up,
adf_420xx_dev_config() has no else branch (such as calling
adf_gen4_init_num_svc_aes()) to restore rl_data.svc_ae_mask.

Additionally, if cfg_services is modified while the device is down and
SR-IOV is then enabled, adf_sriov_configure() -> adf_do_enable_sriov()
brings the device up with config set to false:

drivers/crypto/intel/qat/qat_common/adf_sriov.c:adf_do_enable_sriov() {
	...
	ret = adf_dev_up(accel_dev, false);
	...
}

which bypasses adf_420xx_dev_config() and adf_update_rl_data() before
adf_rl_start() initializes rate limiting.

In both cases, adf_rl_calculate_ae_cycles() will read stale AE counts via
adf_rl_get_num_svc_aes():

drivers/crypto/intel/qat/qat_common/adf_rl.c:adf_rl_calculate_ae_cycles() {
	...
	avail_ae_cycles = hw_data->clock_frequency;
	avail_ae_cycles *= adf_rl_get_num_svc_aes(accel_dev, svc_type);
	do_div(avail_ae_cycles, device_data->scan_interval);
	...
}

Can this cause miscalculated rate-limiting AE cycle budgets (ae_util_cir
and ae_util_pir) to be programmed into firmware after service
reconfiguration or when enabling SR-IOV?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023144835.170453-1-ahsan.atta@intel.com?part=1

Re: [PATCH RESEND 1/2] crypto: qat - enable three-service mode for QAT GEN5 devices

[Ahsan Atta]

On Fri, Oct 23, 2026 at 03:03:08PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] crypto: qat - unrestored rl_data.svc_ae_mask in adf_420xx_dev_config()
> --
> 
> commit 84452d0273c635ae08ee4c142e1d46cbf85da2de
> Author: Bairavi Alagappan <bairavix.alagappan@intel.com>
> 
> crypto: qat - enable three-service mode for QAT GEN5 devices
> 
> This commit adds support for enabling symmetric crypto, asymmetric crypto,
> and compression services concurrently on QAT GEN5 (qat_420xx) devices. It
> defines the firmware object layout, ring-to-service mapping,
> thread-to-arbiter mapping, and rate-limiting accelerator engine accounting
> for three-service mode.
> 
> > diff --git a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> > index 19f9f738630b1..281067f5b60c5 100644
> > --- a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> > +++ b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> 
> [ ... ]
> 
> > @@ -425,6 +450,79 @@ static void adf_gen4_set_err_mask(struct adf_dev_err_mask *dev_err_mask)
> 
> [ ... ]
> 
> > +static void adf_update_rl_data(struct adf_rl_hw_data *rl_data)
> > +{
> > +	unsigned int num_groups;
> > +	unsigned int i;
> > +
> > +	num_groups = ARRAY_SIZE(adf_fw_asym_sym_dc_config) - ADF_GEN4_ADMIN_ACCELENGINES;
> > +
> > +	memset(rl_data->svc_ae_mask, 0, sizeof(rl_data->svc_ae_mask));
> > +
> > +	for (i = 0; i < num_groups; i++) {
> > +		switch (adf_fw_asym_sym_dc_config[i].obj) {
> > +		case ADF_FW_ASYM_OBJ:
> > +			rl_data->svc_ae_mask[SVC_ASYM] +=
> > +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> > +			break;
> > +		case ADF_FW_SYM_OBJ:
> > +			rl_data->svc_ae_mask[SVC_SYM] +=
> > +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> > +			break;
> > +		case ADF_FW_DC_OBJ:
> > +			rl_data->svc_ae_mask[SVC_DC] +=
> > +				hweight32(adf_fw_asym_sym_dc_config[i].ae_mask);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +}
> > +
> > +static int adf_420xx_dev_config(struct adf_accel_dev *accel_dev)
> > +{
> > +	int ret;
> > +
> > +	ret = adf_gen4_dev_config(accel_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (adf_get_service_enabled(accel_dev) == SVC_SYM_ASYM_DC)
> > +		adf_update_rl_data(&GET_HW_DATA(accel_dev)->rl_data);
> > +
> > +	return 0;
> > +}
> 
> [Severity: Medium]
> During probe, adf_init_hw_data_420xx() -> adf_init_rl_data() ->
> adf_gen4_init_num_svc_aes() initializes rl_data.svc_ae_mask to ae_cnt - 1
> (16 on an unfused 420xx device) for SVC_ASYM, SVC_SYM, and SVC_DC.
> 
> When the device is brought down via sysfs, configured to SVC_SYM_ASYM_DC,
> and brought back up via state_store() -> adf_dev_up(accel_dev, true) ->
> adf_420xx_dev_config(), adf_update_rl_data() overwrites rl_data.svc_ae_mask
> in-place with {SVC_ASYM: 4, SVC_SYM: 4, SVC_DC: 8, SVC_DECOMP: 0}.
> 
> If the device is subsequently brought down, reconfigured via cfg_services
> to a 1-service or 2-service mode, and brought back up,
> adf_420xx_dev_config() has no else branch (such as calling
> adf_gen4_init_num_svc_aes()) to restore rl_data.svc_ae_mask.
> 
> Additionally, if cfg_services is modified while the device is down and
> SR-IOV is then enabled, adf_sriov_configure() -> adf_do_enable_sriov()
> brings the device up with config set to false:
>

Hi All,
I will re-work the series and submit a V1.
The feedback here from sashiko is valid.

Regards,
Ahsan