linux-kernel - RE: [PATCH] i3c: dw: Disable IBI IRQ depends on hot-join and SIR enabling

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: Thu, 2 May 2024 05:24:38 +0000
From: Dylan Hung <dylan_hung@...eedtech.com>
To: Jeremy Kerr <jk@...econstruct.com.au>, "alexandre.belloni@...tlin.com"
	<alexandre.belloni@...tlin.com>, "joel@....id.au" <joel@....id.au>,
	"u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
	"gustavoars@...nel.org" <gustavoars@...nel.org>,
	"krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
	"zenghuchen@...gle.com" <zenghuchen@...gle.com>, "matt@...econstruct.com.au"
	<matt@...econstruct.com.au>, "linux-i3c@...ts.infradead.org"
	<linux-i3c@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: BMC-SW <BMC-SW@...eedtech.com>
Subject: RE: [PATCH] i3c: dw: Disable IBI IRQ depends on hot-join and SIR
 enabling

Hi Jeremy,

> -----Original Message-----
> From: Jeremy Kerr <jk@...econstruct.com.au>
> Sent: Wednesday, May 1, 2024 2:22 PM
> To: Dylan Hung <dylan_hung@...eedtech.com>;
> alexandre.belloni@...tlin.com; joel@....id.au; u.kleine-
> koenig@...gutronix.de; gustavoars@...nel.org;
> krzysztof.kozlowski@...aro.org; zenghuchen@...gle.com;
> matt@...econstruct.com.au; linux-i3c@...ts.infradead.org; linux-
> kernel@...r.kernel.org
> Cc: BMC-SW <BMC-SW@...eedtech.com>
> Subject: Re: [PATCH] i3c: dw: Disable IBI IRQ depends on hot-join and SIR
> enabling
> 
> Hi Dylan,
> 
> Just a question on a prior patch you sent:
> 
> > Disable IBI IRQ signal and status only when hot-join and SIR enabling
> > of all target devices attached to the bus are disabled.
> >
> > Fixes: e389b1d72a62 ("i3c: dw: Add support for in-band interrupts")
> 
> [...]
> 
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
> > @@ -1163,8 +1163,10 @@ static void
> > dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
> >                 global = reg == 0xffffffff;
> >                 reg &= ~BIT(idx);
> >         } else {
> > -               global = reg == 0;
> > +               bool hj_rejected = !!(readl(master->regs +
> > +DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK);
> > +
> >                 reg |= BIT(idx);
> > +               global = (reg == 0xffffffff) && hj_rejected;
> >         }
> >         writel(reg, master->regs + IBI_SIR_REQ_REJECT);
> >
>

```
	if (global)
		dw_i3c_master_enable_sir_signal(master, enable);
```

> My interpretation of this change is that we keep the "global" IBI irq enabled if
> hot-join-nack is set (ie, always, because we don't support hot join, and
> configure the hardware to nack all hot join requests).
> 
I would like to clarify the control logic, incorporating the principle of disabling the SIR interrupt signal:

Case 1:
When `DEV_CTRL_HOT_JOIN_NACK` is set, indicating `hj_rejected` is true, it signifies the controller's non-receptiveness to the hot-join event. Consequently, we can safely disable the SIR interrupt signal if none of the target devices request SIR (reg == 0xffffffff).

Case 2:
When `DEV_CTRL_HOT_JOIN_NACK` is unset, indicating `hj_rejected` is false, this indicates the controller's readiness to engage with the hot-join event. Therefore, it's imperative to keep the SIR interrupt signal enabled, even if not all target devices request SIR. In this case, `global` is false and `enable` is false.

Here's a summary table to illustrate the logic:

| enable | reg==0xffffffff? | hj_rejected? | global | result                            |
|---         |---                          |---                   |---         |---                                  |
| false     | true                      | true                | true     | disable SIR interrupt |
| false     | false                     | true                | false     | keep SIR interrupt    |
| false     | true                      | false               | false     | keep SIR interrupt    |
| false     | false                     | false               | false     | keep SIR interrupt    |

> However, we never enable the hot-join NACK interrupt - IBI_QUEUE_CTRL bit
> 0 is never set. So I can't see how we would ever get an interrupt for the hot
> join NACK case anyway. Because of that, this patch is just keeping the IBI
> threshold interrupt always enabled for no reason.
> 

Billy recently submitted a change to implement the hot-join enabling/disabling. Therefore, it is timely to consider the hot-join functionality.
https://patchwork.kernel.org/project/linux-i3c/patch/20240429073624.256830-1-billy_tsai@aspeedtech.com/

> Or is something else happening here? Is there another cause for the IBI
> threshold IRQs?
> 
> Cheers,
> 
> 
> Jeremy