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

[sashiko-bot@kernel.org]

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