linux-kernel - RE: [PATCH] i2c: cadence: Avoid fifo clear after start

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 17 Jan 2024 18:06:55 +0000
From: "Boddu, Sai Pavan" <sai.pavan.boddu@....com>
To: Andi Shyti <andi.shyti@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "Simek, Michal"
	<michal.simek@....com>, Lars-Peter Clausen <lars@...afoo.de>, Wolfram Sang
	<wsa@...nel.org>
Subject: RE: [PATCH] i2c: cadence: Avoid fifo clear after start

Hi Andi,

>-----Original Message-----
>From: Andi Shyti <andi.shyti@...nel.org>
>Sent: Wednesday, January 17, 2024 6:50 PM
>To: Boddu, Sai Pavan <sai.pavan.boddu@....com>
>Cc: linux-kernel@...r.kernel.org; linux-i2c@...r.kernel.org; linux-arm-
>kernel@...ts.infradead.org; Simek, Michal <michal.simek@....com>; Lars-
>Peter Clausen <lars@...afoo.de>; Wolfram Sang <wsa@...nel.org>
>Subject: Re: [PATCH] i2c: cadence: Avoid fifo clear after start
>
>Hi Sai,
>
>sorry, but I'm not really understanding the issue here.
>On Fri, Jan 05, 2024 at 06:22:58PM +0530, Sai Pavan Boddu wrote:
>> Driver unintentionally programs ctrl reg to clear fifo which is
>> happening after start of transaction
>
>what does it mean "unintentionally"?
[Boddu, Sai Pavan] I mean, the previous patch which introduced the issue, was unintentional.
>
>> this was not the case previously
>> as it was read-modified-write. This issue breaks i2c reads on QEMU as
>> i2c-read is done before guest starts programming ctrl register.
>
>this log can be improved. How about something like
>
>The driver unintentionally programs the control register to clear the FIFO,
>which occurs after the start of the transaction.
>Previously, this was not an issue as it involved read-modify-write operations.
>However, this current issue disrupts I2C reads on QEMU, as the I2C read is
>executed before the guest starts programming the control register.
[Boddu, Sai Pavan] Looks good. I will mention as above.

>> Fixes: ff0cf7bca6309 ("i2c: cadence: Remove unnecessary register
>> reads")
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@....com>
>> ---
>>  drivers/i2c/busses/i2c-cadence.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c
>> b/drivers/i2c/busses/i2c-cadence.c
>> index de3f58b60dce..6f7d753a8197 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -633,6 +633,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>>
>>  	if (hold_clear) {
>>  		ctrl_reg &= ~CDNS_I2C_CR_HOLD;
>> +		ctrl_reg &= ~CDNS_I2C_CR_CLR_FIFO;
>
>I'm wondering whether the whole ctrl_reg should be reset after the first write.
[Boddu, Sai Pavan] previous implementation of read-modify-write was good then ?

Regards,
Sai Pavan
>
>Andi
>
>>  		/*
>>  		 * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer
>size
>>  		 * register reaches '0'. This is an IP bug which causes transfer
>> size
>> --
>> 2.25.1
>>