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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----Original Message-----
From: Tian, Kevin <kevin.tian@xxxxxxxxx> 
Sent: Friday, August 4, 2023 3:51 PM
To: Zeng, Xin <xin.zeng@xxxxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
Cc: Cabiddu, Giovanni <giovanni.cabiddu@xxxxxxxxx>; andriy.shevchenko@xxxxxxxxxxxxxxx; Wan, Siming <siming.wan@xxxxxxxxx>; Pankratov, Svyatoslav <svyatoslav.pankratov@xxxxxxxxx>; Zeng, Xin <xin.zeng@xxxxxxxxx>
Subject: RE: [RFC 1/5] crypto: qat - add bank save/restore and RP drain

> From: Xin Zeng <xin.zeng@xxxxxxxxx>
> 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.
->Will split it, thanks.

> +
> +#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. 
->Will add some comments to explain.

> +	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.
->If define macro as below, checkpatch.pl will appear warning-> WARNING: Macros with flow control statements should be avoided.
So do we still need to change?
#define CHECK_STAT(expect, actual, csr_name)    \
do { \
    u32 _expect = expect;                                \
    u32 _actual = actual;                                \
    char *_csr_name = csr_name;                            \
    if (_expect != _actual) {                            \
        pr_err("Fail to restore %s register. Expected 0x%x, but actual is 0x%x\n", \
            _csr_name, _expect, _actual);                    \
        return -EINVAL;                                \
    }                                        \
} while (0)





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux