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]

Message-ID:
 <TYZPR06MB65678EE7611DDB825E5404C19C1C2@TYZPR06MB6567.apcprd06.prod.outlook.com>
Date: Mon, 6 May 2024 05:48:49 +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: Monday, May 6, 2024 1:10 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,
> 
> Thanks for the response! I have a couple of follow-up things though:
> 
> > > 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.
> 
> Yep, I see what you're doing there, but it looks like the correct state would
> never be set if we're not enabling/disabling the IBIs separately; with this code,
> we would only ever enable the SIR for the HJ if we
> *also* happen to enable IBIs.

When we want to enable the SIR, the current code doesn't check whether the
hot-join is enable or not. The modification I made pertains to disabling the SIR interrupt.
It doesn't alter the logic for enabling the SIR.

```
	reg = readl(master->regs + IBI_SIR_REQ_REJECT);
	if (enable) {
		global = reg == 0xffffffff;
		reg &= ~BIT(idx);
	}
```

> 
> The initial state would be to have all SIRs masked.
> 

Yes, indeed. The "global" variable is also true because "reg == 0xffffffff" is true.
Therefore, the INTR_IBI_THLD_STAT bit will be set in the following code.

> > 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.25
> > 6830-1-billy_tsai@...eedtech.com/
> 
> Yep, I saw that, excellent! It's next on my list to take a look at.
> 
> It's just a little unusual that we're enabling the HJ interrupt before actually
> having the HJ support though.
> 
> Cheers,
> 
> 
> Jeremy