Received-SPF: pass (google.com: domain of linux-crypto+bounces-1291-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <563b1dd6-8e91-4cb1-8981-5f18dc1d7130@intel.com>
Date: Mon, 8 Jan 2024 16:07:20 -0700
User-Agent: Betterbird (Linux)
Subject: Re: [PATCH] crypto: iaa - Remove header table code
To: Tom Zanussi <tom.zanussi@linux.intel.com>, <herbert@gondor.apana.org.au>,
	<davem@davemloft.net>, <fenghua.yu@intel.com>
CC: <dan.carpenter@linaro.org>, <tony.luck@intel.com>,
	<linux-kernel@vger.kernel.org>, <linux-crypto@vger.kernel.org>,
	<dmaengine@vger.kernel.org>
References: <8bde35bf981a1e490114c6b50fc4755a64da55a5.camel@linux.intel.com>
Content-Language: en-US
From: Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <8bde35bf981a1e490114c6b50fc4755a64da55a5.camel@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eGFjdVJnRTRKUWNwYzZIN3JhdnJQbFYydTFOaWZzajVCdFNtLzdHbEtMbWQy?=
 =?utf-8?B?RGlxUkcyMksyTFRDZWc1N3U1K0dJdW9VUlEwSGNiUmlocWNCZnl6N2FoRURG?=
 =?utf-8?B?M0pRMWVMTnJqbDRueE51U3ZKcXAzRVZ4TnV2UzZiZi9SUG5yOU1reUdRREZ6?=
 =?utf-8?B?VERSTlpzMERQQ0F5YkppNzhnbnBoa3BXVktwRU40S3RGUlZIRmJ1Y0ZIdkNW?=
 =?utf-8?B?ZmxiZjVMYUVIRkhnRUl4ZTFnWnQ0YUg1NitMNDQ2MzBBNFltaWxMT2VtdUow?=
 =?utf-8?B?M1BNUC96VmRRYkN2Rm1pdnVnelV2WllNK0RnZTIxcnh3bWdNdVJVeXo0U2p4?=
 =?utf-8?B?Qlp5QU9kcDg4d1JDQ0tEdnNqZDI1dzM4QkpWYkZ1UlU5dEFvOUlCcU5VU3Zi?=
 =?utf-8?B?YkNNMU5Fa0xZb3NJdS81bnVFSmluN3FCaGRWRUdKVXVZRi9JRXNFZ1hRamVp?=
 =?utf-8?B?U2srWEVzQkJJcDl1cUJVZ01RTms1dTFkbmFwUitPV0ZtRGJrenFNS2psVlM4?=
 =?utf-8?B?WWl6SndXaW1WNGlHQXBnNG8yUlBHbGR4U0ZPSm5jYitUb0paWTZsazAyQytm?=
 =?utf-8?B?SzczWDVPdEd2dFZPOHY4M3FCcVYzTHEyNkN1S0ZSODlCWmFpZXVoUGRvVnpq?=
 =?utf-8?B?VzVPbmppeFpVVCtnZFNzSDh1cTNwdnk0bmprVXZJUmo0eS9aaDlnQnZvWlNr?=
 =?utf-8?B?d1BXdHplZ2RSM0JGM094d0JDcFlyL2NuazVCTzJGWjQra1UvQlRxS2JUT0E4?=
 =?utf-8?B?Mk5SZFB6bTRIVVV2M3JpdVQ4RHFCbHdhaU1jTmRDWkUxT2Q5UC8yWUxldHNX?=
 =?utf-8?B?UTYxdUdmQ2RLdnYzWFo2eUFBYm1ZRGh2MVc1K21kVFA0RFp4azFFMkt6U2NQ?=
 =?utf-8?B?SFVUUy9CZC9HNEQzN0hsSHJMRHh4WGx5MGNvYVY1dHgxSHJmdW9sTEs0aUt2?=
 =?utf-8?B?QjJGZFRJeHE1K3JhNm1WV2FOdUxhd0ZCSmUxd3dIWmZqRVNUd3R3VFBER0hD?=
 =?utf-8?B?V21Cd1lLeGpYQXlqRURwNUlidjF2SkxOQW0wVjVUUVNsVk1aUzI3QW9PZFBH?=
 =?utf-8?B?WFUzam1XOVJiOGxreDdBN2tlV2x3UXNENTc3TXRNYXRQdUh2dzB1TnJZUnZr?=
 =?utf-8?B?aW1JaTdkOTIvWGc3VkdaRWpxY2o4d0VYNXFjK0tacXU2OW5kMzFWUUU3YUY1?=
 =?utf-8?B?YUhYd3c3T2JRTm52SkdNYVpCWDU4bGhyam5GT2gxbVdRL2xIUE5OWkVRRlBT?=
 =?utf-8?B?aW9kQk5wWG8vOHVXUnJMRU1pdHJwcWRwd2dmSGN5MHZoa0Q1ODl2aE1HMHAy?=
 =?utf-8?B?Z2xQZnpOWG9RY3V4aysvR1JjUGNZSkNDTnlCQW1oYklrOXU5KzlWQVFPTjBt?=
 =?utf-8?B?cnNzWHd3UmpBd2ZYOCtPUkxhb252RFNMQTlQK2wxNmFXc3ZhZDV5bE81WGda?=
 =?utf-8?B?a3ZCLzVnYXZETmZUNS9ESll3MzFJaC9zQytGQ3hEZmN4Rk1qdWg1ZmpuU3pV?=
 =?utf-8?B?bEtBS0IxZkp0K0xrcmplY1FoVlZVME5XZzhNK25jUm16S1drMnFEM1oyOTFO?=
 =?utf-8?B?SUNMQ3RJWlVBZ2JWa3FYaWROeEt4OEJIRVFXZm43dTNpWVJJb25VRGRxM0FY?=
 =?utf-8?B?Szk1Nlk2OWNQVWw0dzEvK1g4UGVEK2d2UTFFWEZZN1lscStkVHFEVnRPSlBM?=
 =?utf-8?B?b21TY29qb2lHN0Z1S25IMVRDTDQ3TDBsYjVrNnhjY2RvdllzSWtMQ3IzQjlz?=
 =?utf-8?B?Z0dEaEdGMVVEWnFvOWg2YXgvNnJDYTFMbGxmWGtnbFpVenNwL285VUhLZm1H?=
 =?utf-8?B?UlRwS1h1amdnb3Z4SkdJWXd0Q0JUV3VkL1llb0dpbTdjNmhJazgrRjhISTBB?=
 =?utf-8?B?MUdoRlY5YlR1d3QvclRvWXpNV0Z0RjFEVWFpWDdkcFV5S3A0OVNzbVN3Zi9W?=
 =?utf-8?B?UE9tWXBSbmVoY2cvdHJMVXZJbVA1ZTE3RUlRdWo1SjNzN1F6eG1YYStqZ3Zj?=
 =?utf-8?B?TTBoWlJOOHQ4eUJnZU8wYUVVQVJTT2pKSnBzaHBFVURVM01UVWNsdU43Mlhs?=
 =?utf-8?B?MkdwaGlmS0s5QXY3QTl6cC9tL3ZqdzJIRmVvY0dNWU5HMElEdHlMODAxam9a?=
 =?utf-8?Q?xNMamMtDksL9zdI8p3k8xIbzG?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 510b98c9-bc92-4fd9-8223-08dc109e932a
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jan 2024 23:07:24.2782
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: sXyWX/8+nOVdDYk3wT/0Z20RICOXCPWm/QgEfAyQkgzR3nWrD3KWje0zrnIuc4JcggchFsUTGCPuaVg5wpNSyQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6878
X-OriginatorOrg: intel.com


On 1/8/24 15:53, Tom Zanussi wrote:
> The header table and related code is currently unused - it was
> included and used for canned mode, but canned mode has been removed,
> so this code can be safely removed as well.
> 
> This indirectly fixes a bug reported by Dan Carpenter.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-crypto/b2e0bd974981291e16882686a2b9b1db3986abe4.camel@linux.intel.com/T/#m4403253d6a4347a925fab4fc1cdb4ef7c095fb86
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/crypto/intel/iaa/iaa_crypto.h         |  25 ----
>  .../crypto/intel/iaa/iaa_crypto_comp_fixed.c  |   1 -
>  drivers/crypto/intel/iaa/iaa_crypto_main.c    | 108 +-----------------
>  3 files changed, 3 insertions(+), 131 deletions(-)
> 
> diff --git a/drivers/crypto/intel/iaa/iaa_crypto.h b/drivers/crypto/intel/iaa/iaa_crypto.h
> index 014420f7beb0..2524091a5f70 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto.h
> +++ b/drivers/crypto/intel/iaa/iaa_crypto.h
> @@ -59,10 +59,8 @@ struct iaa_device_compression_mode {
>  	const char			*name;
>  
>  	struct aecs_comp_table_record	*aecs_comp_table;
> -	struct aecs_decomp_table_record	*aecs_decomp_table;
>  
>  	dma_addr_t			aecs_comp_table_dma_addr;
> -	dma_addr_t			aecs_decomp_table_dma_addr;
>  };
>  
>  /* Representation of IAA device with wqs, populated by probe */
> @@ -107,23 +105,6 @@ struct aecs_comp_table_record {
>  	u32 reserved_padding[2];
>  } __packed;
>  
> -/* AECS for decompress */
> -struct aecs_decomp_table_record {
> -	u32 crc;
> -	u32 xor_checksum;
> -	u32 low_filter_param;
> -	u32 high_filter_param;
> -	u32 output_mod_idx;
> -	u32 drop_init_decomp_out_bytes;
> -	u32 reserved[36];
> -	u32 output_accum_data[2];
> -	u32 out_bits_valid;
> -	u32 bit_off_indexing;
> -	u32 input_accum_data[64];
> -	u8  size_qw[32];
> -	u32 decomp_state[1220];
> -} __packed;
> -
>  int iaa_aecs_init_fixed(void);
>  void iaa_aecs_cleanup_fixed(void);
>  
> @@ -136,9 +117,6 @@ struct iaa_compression_mode {
>  	int			ll_table_size;
>  	u32			*d_table;
>  	int			d_table_size;
> -	u32			*header_table;
> -	int			header_table_size;
> -	u16			gen_decomp_table_flags;
>  	iaa_dev_comp_init_fn_t	init;
>  	iaa_dev_comp_free_fn_t	free;
>  };
> @@ -148,9 +126,6 @@ int add_iaa_compression_mode(const char *name,
>  			     int ll_table_size,
>  			     const u32 *d_table,
>  			     int d_table_size,
> -			     const u8 *header_table,
> -			     int header_table_size,
> -			     u16 gen_decomp_table_flags,
>  			     iaa_dev_comp_init_fn_t init,
>  			     iaa_dev_comp_free_fn_t free);
>  
> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_comp_fixed.c b/drivers/crypto/intel/iaa/iaa_crypto_comp_fixed.c
> index 45cf5d74f0fb..19d9a333ac49 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_comp_fixed.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_comp_fixed.c
> @@ -78,7 +78,6 @@ int iaa_aecs_init_fixed(void)
>  				       sizeof(fixed_ll_sym),
>  				       fixed_d_sym,
>  				       sizeof(fixed_d_sym),
> -				       NULL, 0, 0,
>  				       init_fixed_mode, NULL);
>  	if (!ret)
>  		pr_debug("IAA fixed compression mode initialized\n");
> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index dfd3baf0a8d8..39a5fc905c4d 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> @@ -258,16 +258,14 @@ static void free_iaa_compression_mode(struct iaa_compression_mode *mode)
>  	kfree(mode->name);
>  	kfree(mode->ll_table);
>  	kfree(mode->d_table);
> -	kfree(mode->header_table);
>  
>  	kfree(mode);
>  }
>  
>  /*
> - * IAA Compression modes are defined by an ll_table, a d_table, and an
> - * optional header_table.  These tables are typically generated and
> - * captured using statistics collected from running actual
> - * compress/decompress workloads.
> + * IAA Compression modes are defined by an ll_table and a d_table.
> + * These tables are typically generated and captured using statistics
> + * collected from running actual compress/decompress workloads.
>   *
>   * A module or other kernel code can add and remove compression modes
>   * with a given name using the exported @add_iaa_compression_mode()
> @@ -315,9 +313,6 @@ EXPORT_SYMBOL_GPL(remove_iaa_compression_mode);
>   * @ll_table_size: The ll table size in bytes
>   * @d_table: The d table
>   * @d_table_size: The d table size in bytes
> - * @header_table: Optional header table
> - * @header_table_size: Optional header table size in bytes
> - * @gen_decomp_table_flags: Otional flags used to generate the decomp table
>   * @init: Optional callback function to init the compression mode data
>   * @free: Optional callback function to free the compression mode data
>   *
> @@ -330,9 +325,6 @@ int add_iaa_compression_mode(const char *name,
>  			     int ll_table_size,
>  			     const u32 *d_table,
>  			     int d_table_size,
> -			     const u8 *header_table,
> -			     int header_table_size,
> -			     u16 gen_decomp_table_flags,
>  			     iaa_dev_comp_init_fn_t init,
>  			     iaa_dev_comp_free_fn_t free)
>  {
> @@ -370,16 +362,6 @@ int add_iaa_compression_mode(const char *name,
>  		mode->d_table_size = d_table_size;
>  	}
>  
> -	if (header_table) {
> -		mode->header_table = kzalloc(header_table_size, GFP_KERNEL);
> -		if (!mode->header_table)
> -			goto free;
> -		memcpy(mode->header_table, header_table, header_table_size);
> -		mode->header_table_size = header_table_size;
> -	}
> -
> -	mode->gen_decomp_table_flags = gen_decomp_table_flags;
> -
>  	mode->init = init;
>  	mode->free = free;
>  
> @@ -420,10 +402,6 @@ static void free_device_compression_mode(struct iaa_device *iaa_device,
>  	if (device_mode->aecs_comp_table)
>  		dma_free_coherent(dev, size, device_mode->aecs_comp_table,
>  				  device_mode->aecs_comp_table_dma_addr);
> -	if (device_mode->aecs_decomp_table)
> -		dma_free_coherent(dev, size, device_mode->aecs_decomp_table,
> -				  device_mode->aecs_decomp_table_dma_addr);
> -
>  	kfree(device_mode);
>  }
>  
> @@ -440,73 +418,6 @@ static int check_completion(struct device *dev,
>  			    bool compress,
>  			    bool only_once);
>  
> -static int decompress_header(struct iaa_device_compression_mode *device_mode,
> -			     struct iaa_compression_mode *mode,
> -			     struct idxd_wq *wq)
> -{
> -	dma_addr_t src_addr, src2_addr;
> -	struct idxd_desc *idxd_desc;
> -	struct iax_hw_desc *desc;
> -	struct device *dev;
> -	int ret = 0;
> -
> -	idxd_desc = idxd_alloc_desc(wq, IDXD_OP_BLOCK);
> -	if (IS_ERR(idxd_desc))
> -		return PTR_ERR(idxd_desc);
> -
> -	desc = idxd_desc->iax_hw;
> -
> -	dev = &wq->idxd->pdev->dev;
> -
> -	src_addr = dma_map_single(dev, (void *)mode->header_table,
> -				  mode->header_table_size, DMA_TO_DEVICE);
> -	dev_dbg(dev, "%s: mode->name %s, src_addr %llx, dev %p, src %p, slen %d\n",
> -		__func__, mode->name, src_addr,	dev,
> -		mode->header_table, mode->header_table_size);
> -	if (unlikely(dma_mapping_error(dev, src_addr))) {
> -		dev_dbg(dev, "dma_map_single err, exiting\n");
> -		ret = -ENOMEM;
> -		return ret;
> -	}
> -
> -	desc->flags = IAX_AECS_GEN_FLAG;
> -	desc->opcode = IAX_OPCODE_DECOMPRESS;
> -
> -	desc->src1_addr = (u64)src_addr;
> -	desc->src1_size = mode->header_table_size;
> -
> -	src2_addr = device_mode->aecs_decomp_table_dma_addr;
> -	desc->src2_addr = (u64)src2_addr;
> -	desc->src2_size = 1088;
> -	dev_dbg(dev, "%s: mode->name %s, src2_addr %llx, dev %p, src2_size %d\n",
> -		__func__, mode->name, desc->src2_addr, dev, desc->src2_size);
> -	desc->max_dst_size = 0; // suppressed output
> -
> -	desc->decompr_flags = mode->gen_decomp_table_flags;
> -
> -	desc->priv = 0;
> -
> -	desc->completion_addr = idxd_desc->compl_dma;
> -
> -	ret = idxd_submit_desc(wq, idxd_desc);
> -	if (ret) {
> -		pr_err("%s: submit_desc failed ret=0x%x\n", __func__, ret);
> -		goto out;
> -	}
> -
> -	ret = check_completion(dev, idxd_desc->iax_completion, false, false);
> -	if (ret)
> -		dev_dbg(dev, "%s: mode->name %s check_completion failed ret=%d\n",
> -			__func__, mode->name, ret);
> -	else
> -		dev_dbg(dev, "%s: mode->name %s succeeded\n", __func__,
> -			mode->name);
> -out:
> -	dma_unmap_single(dev, src_addr, 1088, DMA_TO_DEVICE);
> -
> -	return ret;
> -}
> -
>  static int init_device_compression_mode(struct iaa_device *iaa_device,
>  					struct iaa_compression_mode *mode,
>  					int idx, struct idxd_wq *wq)
> @@ -529,24 +440,11 @@ static int init_device_compression_mode(struct iaa_device *iaa_device,
>  	if (!device_mode->aecs_comp_table)
>  		goto free;
>  
> -	device_mode->aecs_decomp_table = dma_alloc_coherent(dev, size,
> -							    &device_mode->aecs_decomp_table_dma_addr, GFP_KERNEL);
> -	if (!device_mode->aecs_decomp_table)
> -		goto free;
> -
>  	/* Add Huffman table to aecs */
>  	memset(device_mode->aecs_comp_table, 0, sizeof(*device_mode->aecs_comp_table));
>  	memcpy(device_mode->aecs_comp_table->ll_sym, mode->ll_table, mode->ll_table_size);
>  	memcpy(device_mode->aecs_comp_table->d_sym, mode->d_table, mode->d_table_size);
>  
> -	if (mode->header_table) {
> -		ret = decompress_header(device_mode, mode, wq);
> -		if (ret) {
> -			pr_debug("iaa header decompression failed: ret=%d\n", ret);
> -			goto free;
> -		}
> -	}
> -
>  	if (mode->init) {
>  		ret = mode->init(device_mode);
>  		if (ret)