2024-01-08 22:54:01

by Tom Zanussi

[permalink] [raw]
Subject: [PATCH] crypto: iaa - Remove header table code

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 <[email protected]>
Closes: https://lore.kernel.org/linux-crypto/[email protected]/T/#m4403253d6a4347a925fab4fc1cdb4ef7c095fb86
Signed-off-by: Tom Zanussi <[email protected]>
---
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)
--
2.34.1




2024-01-08 23:07:35

by Dave Jiang

[permalink] [raw]
Subject: Re: [PATCH] crypto: iaa - Remove header table code



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 <[email protected]>
> Closes: https://lore.kernel.org/linux-crypto/[email protected]/T/#m4403253d6a4347a925fab4fc1cdb4ef7c095fb86
> Signed-off-by: Tom Zanussi <[email protected]>

Reviewed-by: Dave Jiang <[email protected]>

> ---
> 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)

2024-01-26 09:57:34

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: iaa - Remove header table code

On Mon, Jan 08, 2024 at 04:53:48PM -0600, 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 <[email protected]>
> Closes: https://lore.kernel.org/linux-crypto/[email protected]/T/#m4403253d6a4347a925fab4fc1cdb4ef7c095fb86
> Signed-off-by: Tom Zanussi <[email protected]>
> ---
> 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(-)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt