Re: [PATCH RESEND 2/2] Documentation: qat: document 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 - 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