Skip to contentSkip to conversation

talitos: allocate channels with main struct

PATCH In review Rosen Penev 3 participants 3 messages May 6 – May 15 Crypto API

lore
Share & tools
lore

Rosen Penev

author
#0 lore raw

PATCH 1/1 talitos: allocate channels with main struct

Use a flexible array member to combine allocations.

Add __counted_by for extra runtime analysis.

Error in case of no channels as they are required.

Signed-off-by Rosen Penev <[email protected]>

---
v2: error when no channels

 drivers/crypto/talitos.c | 19 +++++++------------
 drivers/crypto/talitos.h |  5 +++--
 2 files changed, 10 insertions(+), 14 deletions(-)
drivers/crypto/talitos.c +7 -12
index bc61d0fe3514..bd4cc06ee13c 100644
@@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->dev.of_node;
 	struct talitos_private *priv;
+	unsigned int num_channels;
 	int i, err;
 	int stride;
 	struct resource *res;
 
-	priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
+	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
+		return -EINVAL;
+
+	priv = devm_kzalloc(dev, struct_size(priv, chan, num_channels), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	priv->num_channels = num_channels;
+
 	INIT_LIST_HEAD(&priv->alg_list);
 
 	dev_set_drvdata(dev, priv);

@@ -3436,7 +3442,6 @@ static int talitos_probe(struct platform_device *ofdev)
 	}
 
 	/* get SEC version capabilities from device tree */
-	of_property_read_u32(np, "fsl,num-channels", &priv->num_channels);
 	of_property_read_u32(np, "fsl,channel-fifo-len", &priv->chfifo_len);
 	of_property_read_u32(np, "fsl,exec-units-mask", &priv->exec_units);
 	of_property_read_u32(np, "fsl,descriptor-types-mask",

@@ -3511,16 +3516,6 @@ static int talitos_probe(struct platform_device *ofdev)
 		}
 	}
 
-	priv->chan = devm_kcalloc(dev,
-				  priv->num_channels,
-				  sizeof(struct talitos_channel),
-				  GFP_KERNEL);
-	if (!priv->chan) {
-		dev_err(dev, "failed to allocate channel management space\n");
-		err = -ENOMEM;
-		goto err_out;
-	}
-
 	priv->fifo_len = roundup_pow_of_two(priv->chfifo_len);
 
 	for (i = 0; i < priv->num_channels; i++) {
drivers/crypto/talitos.h +3 -2
index 1a93ee355929..34b0b5fab7e7 100644
@@ -139,8 +139,6 @@ struct talitos_private {
 	 */
 	unsigned int fifo_len;
 
-	struct talitos_channel *chan;
-
 	/* next channel to be assigned next incoming descriptor */
 	atomic_t last_chan ____cacheline_aligned;
 

@@ -153,6 +151,9 @@ struct talitos_private {
 	/* hwrng device */
 	struct hwrng rng;
 	bool rng_registered;
+
+	struct talitos_channel chan[] __counted_by(num_channels);
+
 };
 
 /* .features flag */
Signature · 1 line
2.54.0
#1 lore raw
Quoted 19 lines · Rosen Penev

Use a flexible array member to combine allocations.

Add __counted_by for extra runtime analysis.

Error in case of no channels as they are required.

Signed-off-by Rosen Penev <[email protected]>

---
v2: error when no channels

 drivers/crypto/talitos.c | 19 +++++++------------
 drivers/crypto/talitos.h |  5 +++--
 2 files changed, 10 insertions(+), 14 deletions(-)
drivers/crypto/talitos.c +0 -0
index bc61d0fe3514..bd4cc06ee13c 100644
@@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
 	struct device *dev = &ofdev->dev;

drivers/crypto/talitos.c static int talitos_probe(struct platform_device *ofdev) · line 3418

Quoted 10 lines
drivers/crypto/talitos.c +3 -1
 	struct device_node *np = ofdev->dev.of_node;
 	struct talitos_private *priv;
+	unsigned int num_channels;
 	int i, err;
 	int stride;
 	struct resource *res;
 
-	priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
+	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
+		return -EINVAL;

Actually, this check does not guards against the num-channels property having a value of 0 :
https://elixir.bootlin.com/linux/v7.0.1/source/include/linux/of.h#L1384
This check is done here :
https://elixir.bootlin.com/linux/v7.0.1/source/drivers/crypto/talitos.c#L3367
when checking the property data in the DT node. is_power_of_two does not consider that 0 is a valid power of two.

Channels was not allocated because the DT node value was invalid. Ideally, the driver should validate the DT node before requesting any resources, but this is outside the scope of this patch.

Signature · 3 lines
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#2 lore raw
In reply to Rosen Penev
Quoted 4 lines · Rosen Penev

Use a flexible array member to combine allocations.

Add __counted_by for extra runtime analysis.

Quoted 8 lines

Error in case of no channels as they are required.

Signed-off-by Rosen Penev <[email protected]>

---
v2: error when no channels

 drivers/crypto/talitos.c | 19 +++++++------------
 drivers/crypto/talitos.h |  5 +++--
 2 files changed, 10 insertions(+), 14 deletions(-)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt