lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <63cbf052-86cb-74d2-f189-2eedc05343ba@quicinc.com>
Date: Mon, 9 Dec 2024 11:08:07 +0530
From: Md Sadre Alam <quic_mdalam@...cinc.com>
To: Vinod Koul <vkoul@...nel.org>
CC: <thara.gopinath@...il.com>, <herbert@...dor.apana.org.au>,
        <davem@...emloft.net>, <kees@...nel.org>, <robin.murphy@....com>,
        <fenghua.yu@...el.com>, <av2082000@...il.com>,
        <u.kleine-koenig@...gutronix.d>, <linux-crypto@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <dmaengine@...r.kernel.org>, <quic_varada@...cinc.com>,
        <quic_srichara@...cinc.com>
Subject: Re: [PATCH v4 02/11] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag
 support



On 12/4/2024 6:15 PM, Vinod Koul wrote:
> On 09-09-24, 14:56, Md Sadre Alam wrote:
>> Add lock and unlock flag support on command descriptor.
>> Once lock set in requester pipe, then the bam controller
>> will lock all others pipe and process the request only
>> from requester pipe. Unlocking only can be performed from
>> the same pipe.
>>
>> If DMA_PREP_LOCK flag passed in command descriptor then requester
>> of this transaction wanted to lock the BAM controller for this
>> transaction so BAM driver should set LOCK bit for the HW descriptor.
>>
>> If DMA_PREP_UNLOCK flag passed in command descriptor then requester
>> of this transaction wanted to unlock the BAM controller.so BAM driver
>> should set UNLOCK bit for the HW descriptor.
>>
>> BAM IP version 1.4.0 and above only supports this LOCK/UNLOCK
>> feature. So adding check for the same and setting bam_pipe_lock
>> based on BAM SW Version.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@...cinc.com>
>> ---
>>
>> Change in [v4]
>>
>> * Added BAM_SW_VERSION read for major & minor
>>    version
>>
>> * Added bam_pipe_lock flag
>>
>> Change in [v3]
>>
>> * Moved lock/unlock bit set inside loop
>>
>> Change in [v2]
>>
>> * No change
>>   
>> Change in [v1]
>>
>> * Added initial support for BAM pipe lock/unlock
>>
>>   drivers/dma/qcom/bam_dma.c | 25 ++++++++++++++++++++++++-
>>   include/linux/dmaengine.h  |  6 ++++++
>>   2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 3a2965939531..d30416a26d8e 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -53,11 +53,20 @@ struct bam_desc_hw {
>>   
>>   #define BAM_DMA_AUTOSUSPEND_DELAY 100
>>   
>> +#define SW_VERSION_MAJOR_SHIFT	28
>> +#define SW_VERSION_MINOR_SHIFT	16
>> +#define SW_VERSION_MAJOR_MASK	(0xf << SW_VERSION_MAJOR_SHIFT)
> 
> Use GENMASK for this
Ok
> 
>> +#define SW_VERSION_MINOR_MASK	0xf
> 
> Can you define only masks and use FIELD_PREP etc to handle this
Ok
> 
>> +#define SW_MAJOR_1	0x1
>> +#define SW_VERSION_4	0x4
>> +
>>   #define DESC_FLAG_INT BIT(15)
>>   #define DESC_FLAG_EOT BIT(14)
>>   #define DESC_FLAG_EOB BIT(13)
>>   #define DESC_FLAG_NWD BIT(12)
>>   #define DESC_FLAG_CMD BIT(11)
>> +#define DESC_FLAG_LOCK BIT(10)
>> +#define DESC_FLAG_UNLOCK BIT(9)
>>   
>>   struct bam_async_desc {
>>   	struct virt_dma_desc vd;
>> @@ -393,6 +402,7 @@ struct bam_device {
>>   	u32 ee;
>>   	bool controlled_remotely;
>>   	bool powered_remotely;
>> +	bool bam_pipe_lock;
>>   	u32 active_channels;
>>   	u32 bam_sw_version;
>>   
>> @@ -696,9 +706,15 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
>>   		unsigned int curr_offset = 0;
>>   
>>   		do {
>> -			if (flags & DMA_PREP_CMD)
>> +			if (flags & DMA_PREP_CMD) {
>>   				desc->flags |= cpu_to_le16(DESC_FLAG_CMD);
>>   
>> +				if (bdev->bam_pipe_lock && flags & DMA_PREP_LOCK)
>> +					desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
>> +				else if (bdev->bam_pipe_lock && flags & DMA_PREP_UNLOCK)
>> +					desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
>> +			}
>> +
>>   			desc->addr = cpu_to_le32(sg_dma_address(sg) +
>>   						 curr_offset);
>>   
>> @@ -1242,6 +1258,7 @@ static int bam_dma_probe(struct platform_device *pdev)
>>   {
>>   	struct bam_device *bdev;
>>   	const struct of_device_id *match;
>> +	u32 sw_major, sw_minor;
>>   	int ret, i;
>>   
>>   	bdev = devm_kzalloc(&pdev->dev, sizeof(*bdev), GFP_KERNEL);
>> @@ -1305,6 +1322,12 @@ static int bam_dma_probe(struct platform_device *pdev)
>>   
>>   	bdev->bam_sw_version = readl_relaxed(bam_addr(bdev, 0, BAM_SW_VERSION));
>>   	dev_info(bdev->dev, "BAM software version:0x%08x\n", bdev->bam_sw_version);
>> +	sw_major = (bdev->bam_sw_version & SW_VERSION_MAJOR_MASK) >> SW_VERSION_MAJOR_SHIFT;
>> +	sw_minor = (bdev->bam_sw_version >> SW_VERSION_MINOR_SHIFT) & SW_VERSION_MINOR_MASK;
> 
> FIELD_GET
Ok
> 
>> +
>> +	if (sw_major == SW_MAJOR_1 && sw_minor >= SW_VERSION_4)
>> +		bdev->bam_pipe_lock = true;
>> +
>>   
>>   	ret = bam_init(bdev);
>>   	if (ret)
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index b137fdb56093..70f23068bfdc 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -200,6 +200,10 @@ struct dma_vec {
>>    *  transaction is marked with DMA_PREP_REPEAT will cause the new transaction
>>    *  to never be processed and stay in the issued queue forever. The flag is
>>    *  ignored if the previous transaction is not a repeated transaction.
>> + *  @DMA_PREP_LOCK: tell the driver that there is a lock bit set on command
>> + *  descriptor.
>> + *  @DMA_PREP_UNLOCK: tell the driver that there is a un-lock bit set on command
>> + *  descriptor.
>>    */
>>   enum dma_ctrl_flags {
>>   	DMA_PREP_INTERRUPT = (1 << 0),
>> @@ -212,6 +216,8 @@ enum dma_ctrl_flags {
>>   	DMA_PREP_CMD = (1 << 7),
>>   	DMA_PREP_REPEAT = (1 << 8),
>>   	DMA_PREP_LOAD_EOT = (1 << 9),
>> +	DMA_PREP_LOCK = (1 << 10),
>> +	DMA_PREP_UNLOCK = (1 << 11),
> 
> this should be a separate patch to define how to use these new flags,
> what it means etc...
Ok , will prepare separate patch for these flags in next revision.
> 
> 
>>   };
>>   
>>   /**
>> -- 
>> 2.34.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ