Adding Crypto support #2477

UtsavAgarwalADI · 2024-04-19T14:09:24Z

PR Description

Please replace this comment with a summary of your changes, and add any context
necessary to understand them. List any dependencies required for this change.
To check the checkboxes below, insert a 'x' between square brackets (without
any space), or simply check them after publishing the PR.
If you changes include a breaking change, please specify dependent PRs in the
description and try to push all related PRs simultaneously.

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly (if there is the case)

NOTE: hardware cryptographic functions were not tested at the moment due to absence of the devcrypto engine userspace library

Hashing passes test, cipher untested - Correcting error handling - Changing ahash_algs to ahash_engine_algs Since this is a hardware driven engine, it is more approriate to be using ahash_engine_algs - Removing busy wait While loops now yield while waiting for device status - Changing interrupt handler to threaded context CRC tests now pass (for upto 4KB input) - Changing interrupt to threaded context - Removing unneeded function call Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

commodo · 2024-04-19T15:38:27Z

drivers/crypto/adi/adi-pkte-hash.c

+{
+	u32 i, nLength, SIZE, nPadLength = 0, nTotalSize;
+	u32 *temp;
+	u32 temp1;


the naming of temp1 and temp could be improved a bit;
maybe source -> source_offset and temp1 -> data_offset ?
or something more representative of what they are

commodo · 2024-04-19T15:40:36Z

drivers/crypto/adi/adi-pkte-hash.c

+	SIZE = (nLength > 128) ? 128 : (nLength);
+
+	if (SIZE % 4) {
+		for (i = 0; i < 1+SIZE/4; i++) {


these 2 for-loop look like it could be condensed into one;
maybe re-used nTotalSize = !!(SIZE % 4) + (SIZE / 4) here?

commodo · 2024-04-19T15:41:59Z

drivers/crypto/adi/adi-pkte-hash.c

+		break;
+	}
+
+	pkte->pPkteList.pSource += SIZE/(u32)4;


explicit casts are discouraged in kernel code (in most cases);

commodo · 2024-04-19T15:43:20Z

drivers/crypto/adi/adi-pkte-hash.c

+
+void adi_write_packet(struct adi_dev *pkte_dev, u32 *source)
+{
+	u32 i, nLength, SIZE, nPadLength = 0, nTotalSize;


the naming of variables is a mix of camel-case or C-style (with underscores);
if this needs to be upstreamed, C-style is preferred for the naming;

commodo · 2024-04-19T15:44:48Z

drivers/crypto/adi/adi-pkte-hash.c

+	u32 *temp;
+	u32 temp1 = 0, temp2 = 0;
+	u8 pos;
+	struct ADI_PKTE_DEVICE *pkte;


some upstream people prefer reverse-christmas-tree ordering for variable names;
and IIRC, initialization-on-declaration is not preferred;

commodo · 2024-04-19T15:48:08Z

drivers/crypto/adi/adi-pkte-skcipher.c

+
+static int adi_crypt_aes_cbc_decrypt(struct skcipher_request *req)
+{
+


nitpick: remove these empty lines at the start of these functions?

commodo · 2024-04-19T15:49:43Z

drivers/crypto/adi/adi-pkte.c

+u32 Key[8] = {0x0, 0x0, 0x0, 0x0, 0, 0, 0, 0};
+u32 IV[4] = {0x0, 0x0, 0x0, 0x0};
+u32 IDigest[8] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
+u32 ODigest[8] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};


these look like global;
should they be static const?
if they are device-state-specific, then malloc-ing is preferred, in case there are 2 similar devices in the same system

commodo · 2024-04-19T15:50:23Z

drivers/crypto/adi/adi-pkte.c

+
+void adi_init_ring(struct adi_dev *pkte_dev)
+{
+	u32 temp;


u32 temp -> u32 addr ?

commodo · 2024-04-19T15:51:26Z

drivers/crypto/adi/adi-pkte-hash.c

+{
+	u32 i = 0;
+	u32 nLength = 0, SIZE = 0;
+	uint8_t iBufIncrement = 0;


there's a mix of uintXX_t and uXX types;
it's preferred to have one of the 2;

commodo · 2024-04-19T15:52:11Z

drivers/crypto/adi/adi_crc.c

+#include <crypto/algapi.h>
+#include <crypto/hash.h>
+#include <crypto/internal/hash.h>
+#include <asm/unaligned.h>


are all headers needed?
it's preferred to use the minimum required

commodo · 2024-04-19T15:53:08Z

drivers/crypto/adi/adi_crc.c

+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	crc->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR((void *)crc->regs)) {


casting here is redundant

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI · 2024-04-22T11:45:35Z

Thank you for your review.

I have made the requested changes in addition to some indentation for the source files - to allow it being more in line with the kernel coding style(used gnu indent with kernel specific settings).

Please let me know if there are any further changes that you feel may be necessary.

commodo · 2024-04-22T12:30:53Z

@UtsavAgarwalADI
I have a question here :)
What is the desired level of review here?

For upstream kernel (review level), quite a bit more work would be required (to review, and re-spin).
Then, when/if upstreaming this driver, upstream maintainers would still have some feedback (which is usually more about preference).
For internal (non-upstream) use of this driver, the review can be more relaxed.

The initial review (which I did) was a bit more late-Friday quick-scan.
I'll need to make some time for something more thorough.

commodo · 2024-04-22T13:36:49Z

drivers/crypto/Kconfig

@@ -21,7 +21,7 @@ config CRYPTO_DEV_PADLOCK
 	  (so called VIA PadLock ACE, Advanced Cryptography Engine)
 	  that provides instructions for very fast cryptographic
 	  operations with supported algorithms.
-	  
+


this change looks like patch noise;
maybe remove it?

commodo · 2024-04-22T13:37:20Z

drivers/crypto/adi/adi-pkte-hash.c

@@ -0,0 +1,1196 @@
+//#define DEBUG


left over debug;
remove?

commodo · 2024-04-22T13:41:28Z

drivers/crypto/adi/adi-pkte-hash.c

+static int adi_one_request(struct crypto_engine *engine, void *areq);
+static int adi_prepare_req(struct crypto_engine *engine, void *areq);
+
+void adi_write_packet(struct adi_dev *pkte_dev, u32 * source)


this adi_write_packet() function looks like it does a lot of work;

is there a way to make this return error codes?
(just asking; no need to change if this is not doable)

usually with both read & write function (in C), it's a good idea to provide the length of the buffer;
to be able to do some checks, in case the caller may goof something

void adi_write_packet(struct adi_dev *pkte_dev, u32 * source, u32 source_len)

commodo · 2024-04-22T13:42:21Z

drivers/crypto/adi/adi-pkte-hash.c

+	n_length = pkte_dev->src_bytes_available;
+	SIZE = (n_length > 128) ? 128 : (n_length);
+
+	for (i = 0; i < (!!(SIZE % 4) + SIZE / 4); i++) {


i would use an intermediate variable here;
``(!!(SIZE % 4) + SIZE / 4)` looks a bit quirky;
but feel free to disregard here; it may be just my preference

UtsavAgarwalADI · 2024-04-22T13:43:58Z

@commodo

Ideally these files(this and a series of others before - these are basically ports of a non-upstream LTS branch to the current kernel version) are intended to be eventually submitted to the kernel mailing list.

As you had correctly mentioned, there is a lot of work needed for re-spinning it in addition to feedback from the kernel maintainers.

Hence, if possible, I would request an upstream review, however, any feedback at all would be appreciated.

commodo · 2024-04-22T13:45:00Z

drivers/crypto/adi/adi-pkte-hash.c

+	data_offset += DATAIO_BUF_OFFSET;
+
+	n_length = pkte_dev->src_bytes_available;
+	SIZE = (n_length > 128) ? 128 : (n_length);


usually upper-case names are constants or macros;

also, this looks like a min() function;
could this re-use a macro or generic min() function from the kernel?

commodo · 2024-04-22T13:46:04Z

drivers/crypto/adi/adi-pkte-hash.c

+	default:
+		//Pad to 16 byte alignment/boundaries
+		if (SIZE % 16 != 0) {
+			nTotalSize = ((SIZE / 16) + 1) * 16;


nPadLength, nTotalSize are used here;
they could be declared in this scope (since it's a new scope), to reduce the number of variables declared in the function;

commodo · 2024-04-22T13:48:16Z

drivers/crypto/adi/adi-pkte-hash.c

+	imsk_stat_offset = adi_read(pkte_dev, IMSK_STAT_OFFSET);
+	pos =
+	    pkte_dev->flags & PKTE_AUTONOMOUS_MODE ? pkte_dev->
+	    ring_pos_consume : 0;


ternary operators aren't preferred too often;

also, in this case, this looks more preferred:

if (pkte_dev->flags & PKTE_AUTONOMOUS_MODE) pos = pkte_dev->ring_pos_consume; else pos = 0;

commodo · 2024-04-22T13:49:31Z

drivers/crypto/adi/adi-pkte-hash.c

+								     pkte->
+								     pPkteList.
+								     pDestination));
+		SIZE = (n_length > 128) ? 128 : n_length;


min() function?

commodo · 2024-04-22T13:50:48Z

drivers/crypto/adi/adi-pkte-hash.c

+			if (pkte->pPkteList.pCommand.hash == hash_sha256)
+				SIZE = 8 * 4;	/* Destination will be 8 word hash value */
+
+			if (SIZE % 4) {


is there a way this could happen? i.e. (SIZE % 4) != 0 for a valid hash command?

if this could only happen for an invalid command, then maybe convert this to a switch statement?

or use the get_hash_command_params() or get_hash_command_size() approach?

commodo · 2024-04-22T13:57:48Z

drivers/crypto/adi/adi-pkte-hash.c

+		SIZE = (n_length > 128) ? 128 : n_length;
+
+		if (pkte->pPkteList.pCommand.opcode == opcode_hash) {
+			if (pkte->pPkteList.pCommand.hash == hash_md5)


could this become a lookup table?

struct hash_command_param { uint8_t hash_cmd; uint32_t size; }

then

static const struct hash_command_param hash_command_params[] = { { hash_md5, 4 * 4 } , { hash_sha1, 5 * 4 } , { hash_sha224, 7 * 4 }, { hash_sha256, 8 * 4 }, };

then have a lookup table

static struct hash_command_param* get_hash_command_params(int hash_cmd) { for (i = 0; i < ARARY_SIZE(hash_command_params); i++) { struct hash_command_param *p = &hash_command_params[i]; if (p->cmd == hash_cmd) return p; } return NULL; }

the get_hash_command_params() function could also just be a get_has_command_size() function if there will be no other per-command parameter;

Changing implementation for a more efficient and modular approach. adi_read_packet() now calls adi_read_hash_to_dest() for handling various opcode and algorithm combinations. This also allows reusing code and increasing readibility. Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Adding a check to adi_read_hash_to_dest() for making sure an invalid memory address is not written to Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Added more variables for improved readability and removed unnecessary conditional statements for more code reuse Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Changing how code waits for pkte device by introducing adi_wait_for_bit() - it can now timeout rather than waiting indefinitely, which then returns an error - code is now reused Changing implementation for adi_write_packet(). - now packet alignment to 16B, if required, is handled by a dedicated function. Changing implementation for adi_process_packet(). - added a dedicated debug function which can be enabled during compile time - added a dedicated function to handle resuming a previous packet processing operation - added checks which can return an error value Adding some general comments for increased readability All hash and CRC tests pass Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Changing adi_wait_for_bit() to a common function between multiple kernel modules for increased code reuse - now used in adi-pkte-skcipher.c as well Modularising implementation of adi_crypt_configure_pkte - Replaced multiple if statements with LUT and switch statements for more efficient and compact code - Split function into multiple parts for code reuse and better readability Adding dedicated debug print functions Changing variable names for consistency with other modules - changed crypt to pkte_dev, as it is represented in adi-pkte-hash.c Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI · 2024-04-25T11:23:35Z

I have tried addressing all of the above-mentioned concerns/suggestions. In addition, I have also attempted to try and remove any other redundant code and added checks for long/complex functions which previously could never fail.

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

commodo · 2024-04-25T14:00:04Z

drivers/crypto/adi/adi-pkte-hash.c

+{
+	u32 i, n_pad_len = 0;
+
+	if (packet_size % 16 != 0) {


early returns are preferred in kernel code;

if (packet_size % 16 == 0) return 0;

the rationale is, that it creates less indentation

commodo · 2024-04-25T14:00:42Z

drivers/crypto/adi/adi-pkte-hash.c

+	if((hash_type < ARRAY_SIZE(hash_params)) && (hash_type > 0))
+		return hash_params[hash_type].packet_size;
+
+	//invalid hash command


this comment is redundant;
but feel free to keep it;

commodo · 2024-04-25T14:01:38Z

drivers/crypto/adi/adi-pkte-hash.c

+
+	pkte = pkte_dev->pkte_device;
+	imsk_stat_offset = adi_read(pkte_dev, IMSK_STAT_OFFSET);
+	if(pkte_dev->flags & PKTE_AUTONOMOUS_MODE)


some kernel guidelines are also covered by the ./scripts/checkpatch.pl script;

commodo · 2024-04-25T14:03:21Z

drivers/crypto/adi/adi-pkte-hash.c

+	else
+		pos = 0;
+
+	if ((imsk_stat_offset & BITM_PKTE_IMSK_EN_OPDN) !=


inverting this conditio, also for an early return;
and can be moved right after reading imsk_stat_offset = adi_read(pkte_dev, IMSK_STAT_OFFSET);

so

imsk_stat_offset = adi_read(pkte_dev, IMSK_STAT_OFFSET); imsk_stat_offset &= BITM_PKTE_IMSK_EN_OPDN; if (imsk_stat_offset == BITM_PKTE_IMSK_EN_OPDN) return 0; ```

oh, and obviously, less indentation in the next block :)

commodo · 2024-04-25T14:05:41Z

drivers/crypto/adi/adi-pkte-hash.c

+		*dev_final_hash |=
+			hash_source_state << BITP_PKTE_SA_CMD0_HASHSRC;
+		sa_record->SA_Para.SA_CMD0 = *dev_final_hash;
+	} else {


not that it makes much difference here, but coming back to early returns:

if (pkte_dev->flags & PKTE_AUTONOMOUS_MODE) { pos = pkte_dev->ring_pos_consume; sa_record = &pkte_dev->pkte_device->pPkteDescriptor.SARecord[pos]; *dev_final_hash = sa_record->SA_Para.SA_CMD0; *dev_final_hash &= ~BITM_PKTE_SA_CMD0_HASHSRC; *dev_final_hash |= hash_source_state << BITP_PKTE_SA_CMD0_HASHSRC; sa_record->SA_Para.SA_CMD0 = *dev_final_hash; + return; }

then the rest of the block can be un-indented one-level

commodo · 2024-04-25T14:07:38Z

drivers/crypto/adi/adi-pkte-hash.c

+	} else {
+		dev_err(pkte_dev->dev, "Key length exceeds limit!\n");
+		return -ENOMEM;
+	}


same here:

if (keylen > PKTE_MAX_KEY_SIZE) { dev_err(pkte_dev->dev, "Key length exceeds limit!\n"); return -ENOMEM; }

then more code can be un-indented one level

commodo · 2024-04-25T14:09:05Z

drivers/crypto/adi/adi-pkte-hash.c

+
+	//Endian Swap!
+	if (pkte->pPkteList.pCommand.hash == hash_md5) {
+		memcpy(hash, pkte->pPkteList.pDestination, rctx->digcnt);


a return here can allow the un-indentation of the else block;
then hash[i] = __builtin_bswap32(pkte->pPkteList.pDestination[i]); this may fit on one line

commodo · 2024-04-25T14:10:01Z

drivers/crypto/adi/adi-pkte-hash.c

+	dev_dbg(pkte_dev->dev, "%s flags %lx\n", __func__, pkte_dev->flags);
+	pkte = pkte_dev->pkte_device;
+
+	if (!err && (PKTE_FLAGS_FINAL & pkte_dev->flags)) {


especially here it looks useful

if (err || (PKTE_FLAGS_FINAL & pkte_dev->flags)) return:

and all the rest can be un-indented

commodo · 2024-04-25T14:10:22Z

drivers/crypto/adi/adi-pkte-hash.c

+		adi_finish_req(req, err);
+
+	return err;
+


nitpick: empty line

commodo · 2024-04-25T14:12:15Z

drivers/crypto/adi/adi_crc.c

+
+	ret = dmaengine_slave_config(crc->dma_ch, &dma_config);
+	if (ret) {
+		dev_err(crc->dev, "Error configuring DMA channel\n");


it may be useful to also print the ret value in the error message;
for debugging

commodo · 2024-04-25T14:12:48Z

drivers/crypto/adi/adi_crc.c

+	if (!req->nbytes)
+		return 0;
+
+	if (ctx->total + req->nbytes <= BUFLEN) {


if (ctx->total + req->nbytes > BUFLEN) return -ENOBUFS;

commodo · 2024-04-26T05:58:48Z

drivers/crypto/adi/adi_crc.h

+#define POLYMIRR		0x00200000	/* CRC poly register is mirrored. */
+#define POLYMIRR_OFFSET		21		/* Mirror CRC poly offset. */
+#define CMPMIRR			0x00400000	/* CRC compare register is mirrored. */
+#define CMPMIRR_OFFSET		22		/* Mirror CRC compare offset. */


as a note: in the kernel, there are some neat macros for bits/mask
for example
#define CMPMIRR BIT(22) ; or if it's a mask (bit-range) it would be (for example) #define CMPMIRR_MASK GETMASK(22, 20) (in this example, bits 22 and 20 are pare of the mask);

then they can be used to mask values with the FIELD_PREP() macro

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Adding Early returns for functions where possible Adding more variables to avoid long pointer strings Replacing ternary operators with if else statements for simplicity/readibility Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Adding better indentation Adding early returns where possible Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

commodo · 2024-05-01T12:03:52Z

drivers/crypto/adi/adi-pkte.c

+const u32 IDigest[8] = { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
+const u32 ODigest[8] = { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
+
+u32 IV[4] = { 0x0, 0x0, 0x0, 0x0 };


are these global variables, state variables for the chip?
if yes, it is recommended to make part of the struct adi_dev or some instance struct;

the reason being: in some cases there could be more than 1 chip in the system;
then 1 chip state-info would overwrite the state info for all chips;

Adding Key, Idigest, Odigest, IV as members of adi_dev. - Allows managing multiple devices by assigning each one a separate context Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Making minor changes in accordance with kernel guidelines(checkpatch) Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI requested review from artursartamonovsadi and vasbimpikasadi April 19, 2024 14:09

commodo reviewed Apr 19, 2024

View reviewed changes

drivers/crypto/adi/adi-pkte.c Outdated

void adi_init_ring(struct adi_dev *pkte_dev)

{

u32 temp;

Copy link

Contributor

commodo Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u32 temp -> u32 addr ?

commodo reviewed Apr 19, 2024

View reviewed changes

Formatting code with respect to kernel guidelines

77c8307

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI force-pushed the adsp-main-crypto_support branch from c70f836 to 77c8307 Compare April 22, 2024 11:39

UtsavAgarwalADI requested a review from commodo April 22, 2024 11:46

commodo reviewed Apr 22, 2024

View reviewed changes

drivers/crypto/adi/adi-pkte-hash.c Outdated

@@ -0,0 +1,1196 @@

//#define DEBUG

Copy link

Contributor

commodo Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left over debug;
remove?

commodo reviewed Apr 22, 2024

View reviewed changes

UtsavAgarwalADI added 5 commits April 23, 2024 14:59

Adding check for buffer overflow

acbeb86

Adding a check to adi_read_hash_to_dest() for making sure an invalid memory address is not written to Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Changing adi_append_sg

47b93df

Added more variables for improved readability and removed unnecessary conditional statements for more code reuse Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI requested a review from commodo April 25, 2024 11:21

Removing patch noise

f9c4dd9

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI force-pushed the adsp-main-crypto_support branch from 04f6a49 to f9c4dd9 Compare April 25, 2024 11:29

Removing debug remnant

4e54148

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

commodo reviewed Apr 25, 2024

View reviewed changes

drivers/crypto/adi/adi-pkte-hash.c Outdated

adi_finish_req(req, err);

return err;

Copy link

Contributor

commodo Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: empty line

commodo reviewed Apr 25, 2024

View reviewed changes

commodo reviewed Apr 26, 2024

View reviewed changes

UtsavAgarwalADI added 3 commits May 1, 2024 11:52

Formatting adi CRC driver

550532d

Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Formatting adi pkte driver

63afe56

Adding Early returns for functions where possible Adding more variables to avoid long pointer strings Replacing ternary operators with if else statements for simplicity/readibility Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Formatting adi pkte-hash driver

1fb33d3

Adding better indentation Adding early returns where possible Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI requested a review from commodo May 1, 2024 11:28

commodo reviewed May 1, 2024

View reviewed changes

UtsavAgarwalADI added 2 commits May 1, 2024 13:50

Changing global state variables to struct members

2e67ca2

Adding Key, Idigest, Odigest, IV as members of adi_dev. - Allows managing multiple devices by assigning each one a separate context Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

Formatting adi-skcipher driver

c68204a

Making minor changes in accordance with kernel guidelines(checkpatch) Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI requested a review from commodo May 1, 2024 13:00


		static int adi_crypt_aes_cbc_decrypt(struct skcipher_request *req)
		{

Adding Crypto support #2477

Are you sure you want to change the base?

Adding Crypto support #2477

Conversation

UtsavAgarwalADI commented Apr 19, 2024

PR Description

PR Type

PR Checklist

Choose a reason for hiding this comment

commodo Apr 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UtsavAgarwalADI commented Apr 22, 2024

commodo commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UtsavAgarwalADI commented Apr 22, 2024

Choose a reason for hiding this comment

commodo Apr 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UtsavAgarwalADI commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

commodo May 1, 2024 • edited

Choose a reason for hiding this comment

commodo Apr 19, 2024 •

edited

commodo Apr 22, 2024 •

edited

commodo May 1, 2024 •

edited