RE: [RFC 1/5] crypto: qat - add bank save/restore and RP drain

From: "Tian, Kevin" <kevin.tian@intel.com>
To: "Zeng, Xin" <xin.zeng@intel.com>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc: "Cabiddu, Giovanni" <giovanni.cabiddu@intel.com>,
	"andriy.shevchenko@linux.intel.com" 
	<andriy.shevchenko@linux.intel.com>,
	"Wan, Siming" <siming.wan@intel.com>,
	"Pankratov, Svyatoslav" <svyatoslav.pankratov@intel.com>,
	"Zeng, Xin" <xin.zeng@intel.com>
Subject: RE: [RFC 1/5] crypto: qat - add bank save/restore and RP drain
Date: Fri, 4 Aug 2023 07:51:02 +0000	[thread overview]
Message-ID: <BN9PR11MB527649234A93384F1EB11F478C09A@BN9PR11MB5276.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230630131304.64243-2-xin.zeng@intel.com>

> From: Xin Zeng <xin.zeng@intel.com>
> Sent: Friday, June 30, 2023 9:13 PM
> ---
>  .../intel/qat/qat_4xxx/adf_4xxx_hw_data.c     |   5 +-
>  .../intel/qat/qat_c3xxx/adf_c3xxx_hw_data.c   |   2 +-
>  .../qat/qat_c3xxxvf/adf_c3xxxvf_hw_data.c     |   2 +-
>  .../intel/qat/qat_c62x/adf_c62x_hw_data.c     |   2 +-
>  .../intel/qat/qat_c62xvf/adf_c62xvf_hw_data.c |   2 +-
>  .../intel/qat/qat_common/adf_accel_devices.h  |  60 ++-
>  .../intel/qat/qat_common/adf_gen2_hw_data.c   |  17 +-
>  .../intel/qat/qat_common/adf_gen2_hw_data.h   |  10 +-
>  .../intel/qat/qat_common/adf_gen4_hw_data.c   | 362 +++++++++++++++++-
>  .../intel/qat/qat_common/adf_gen4_hw_data.h   | 131 ++++++-
>  .../intel/qat/qat_common/adf_transport.c      |  11 +-
>  .../crypto/intel/qat/qat_common/adf_vf_isr.c  |   2 +-
>  .../qat/qat_dh895xcc/adf_dh895xcc_hw_data.c   |   2 +-
>  .../qat_dh895xccvf/adf_dh895xccvf_hw_data.c   |   2 +-
>  14 files changed, 584 insertions(+), 26 deletions(-)

this could be split into 3 patches.

one is moving from hw_data->csr_ops to hw_data->csr_info. apply to
all qat drivers.

the 2nd is adding new csr_ops.

the last one then covers bank save/restore.

> +
> +#define ADF_RP_INT_SRC_SEL_F_RISE_MASK BIT(2)
> +#define ADF_RP_INT_SRC_SEL_F_FALL_MASK GENMASK(2, 0)
> +static int gen4_bank_state_restore(void __iomem *csr, u32 bank_number,
> +				   struct bank_state *state, u32 num_rings,
> +				   int tx_rx_gap)
> +{

restore is the most tricky part. it's worth of some comments to help
understand the flow, e.g. what is rx/tx layout, why there are multiple
ring tails writes, etc. 

> +	u32 val, tmp_val, i;
> +
> +	write_csr_ring_srv_arb_en(csr, bank_number, 0);
> +
> +	for (i = 0; i < num_rings; i++)
> +		write_csr_ring_base(csr, bank_number, i, state-
> >rings[i].base);
> +
> +	for (i = 0; i < num_rings; i++)
> +		write_csr_ring_config(csr, bank_number, i, state-
> >rings[i].config);
> +
> +	for (i = 0; i < num_rings / 2; i++) {
> +		int tx = i * (tx_rx_gap + 1);
> +		int rx = tx + tx_rx_gap;
> +		u32 tx_idx = tx / ADF_RINGS_PER_INT_SRCSEL;
> +		u32 rx_idx = rx / ADF_RINGS_PER_INT_SRCSEL;
> +
> +		write_csr_ring_head(csr, bank_number, tx, state-
> >rings[tx].head);
> +
> +		write_csr_ring_tail(csr, bank_number, tx, state->rings[tx].tail);
> +
> +		if (state->ringestat & (BIT(tx))) {
> +			val = read_csr_int_srcsel(csr, bank_number, tx_idx);
> +			val |= (ADF_RP_INT_SRC_SEL_F_RISE_MASK << (8 *
> tx));
> +			write_csr_int_srcsel(csr, bank_number, tx_idx, val);
> +			write_csr_ring_head(csr, bank_number, tx, state-
> >rings[tx].head);
> +		}
> +
> +		write_csr_ring_tail(csr, bank_number, rx, state-
> >rings[rx].tail);
> +
> +		val = read_csr_int_srcsel(csr, bank_number, rx_idx);
> +		val |= (ADF_RP_INT_SRC_SEL_F_RISE_MASK << (8 * rx));
> +		write_csr_int_srcsel(csr, bank_number, rx_idx, val);
> +
> +		write_csr_ring_head(csr, bank_number, rx, state-
> >rings[rx].head);
> +
> +		val = read_csr_int_srcsel(csr, bank_number, rx_idx);
> +		val |= (ADF_RP_INT_SRC_SEL_F_FALL_MASK << (8 * rx));
> +		write_csr_int_srcsel(csr, bank_number, rx_idx, val);
> +
> +		if (state->ringfstat & BIT(rx))
> +			write_csr_ring_tail(csr, bank_number, rx, state-
> >rings[rx].tail);
> +	}
> +
> +	write_csr_int_flag_and_col(csr, bank_number, state-
> >iaintflagandcolen);
> +	write_csr_int_en(csr, bank_number, state->iaintflagen);
> +	write_csr_int_col_en(csr, bank_number, state->iaintcolen);
> +	write_csr_int_srcsel(csr, bank_number, 0, state->iaintflagsrcsel0);
> +	write_csr_exp_int_en(csr, bank_number, state->ringexpintenable);
> +	write_csr_int_col_ctl(csr, bank_number, state->iaintcolctl);
> +
> +	/* Check that all ring statuses are restored into a saved state. */
> +	tmp_val = read_csr_stat(csr, bank_number);
> +	val = state->ringstat0;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringstat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_e_stat(csr, bank_number);
> +	val = state->ringestat;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringestat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_ne_stat(csr, bank_number);
> +	val = state->ringnestat;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringnestat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_nf_stat(csr, bank_number);
> +	val = state->ringnfstat;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringnfstat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_f_stat(csr, bank_number);
> +	val = state->ringfstat;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringfstat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_c_stat(csr, bank_number);
> +	val = state->ringcstat0;
> +	if (tmp_val != val) {
> +		pr_err("Fail to restore ringcstat register. Expected 0x%x, but
> actual is 0x%x\n",
> +		       tmp_val, val);
> +		return -EINVAL;
> +	}
> +
> +	tmp_val = read_csr_exp_stat(csr, bank_number);
> +	val = state->ringexpstat;
> +	if (tmp_val && !val) {
> +		pr_err("Bank was restored with exception: 0x%x\n", val);
> +		return -EINVAL;
> +	}

above checks could be wrapped in macros.