Re: [PATCH] lib: sbi: Clear all IPI devices when processing an IPI

["Troy Mitchell" <troy.mitchell@linux.dev>]

[-- Attachment #1.1: Type: text/plain, Size: 3067 bytes --]

Hi Bo,

On Fri Sep 11, 2026 at 5:32 PM +08, Bo Gan wrote:
> Hi Troy,
>
> On 9/8/26 06:00, Troy Mitchell wrote:
>> Hart start sends wake-up IPIs through all registered IPI devices, but
>> sbi_ipi_process() only clears the preferred device. A notification from
>> another device can arrive after warm initialization has cleared it.
>> 
>> With both IMSIC and ACLINT MSWI, IMSIC is preferred and has no ipi_clear
>> callback: its interrupts are acknowledged through MTOPEI. A late ACLINT
>> notification therefore leaves MSIP asserted, trapping the hart in the
>> machine-mode interrupt handler and potentially timing out Linux CPU
>> bring-up.
>> 
>> Clear all registered IPI devices before consuming the software IPI event
>> bits so that late wake-up notifications are acknowledged too.
>> 
>> Fixes: 94f0f8465622 ("lib: sbi: Extends sbi_ipi_raw_send() to use all available IPI devices")
>> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
>> ---
>>   lib/sbi/sbi_ipi.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
>> index b04a5877..683d559d 100644
>> --- a/lib/sbi/sbi_ipi.c
>> +++ b/lib/sbi/sbi_ipi.c
>> @@ -263,7 +263,12 @@ void sbi_ipi_process(void)
>>   			sbi_scratch_offset_ptr(scratch, ipi_data_off);
>>   
>>   	sbi_pmu_ctr_incr_fw(SBI_PMU_FW_IPI_RECVD);
>> -	sbi_ipi_raw_clear(false);
>> +	/*
>> +	 * A wake-up IPI is sent through all devices. A notification from a
>> +	 * non-preferred device can arrive after warm-boot initialization
>> +	 * cleared it, so acknowledge all devices when processing the IPI.
>> +	 */
>> +	sbi_ipi_raw_clear(true);
>
> I feel like this is not the proper way to fix the issue. It introduces
> unnecessary overhead for *every* IPI processing, because you need to call
> clear on all IPI devices, even the non-preferred, inactive ones.
>
> IMO, the "int sbi_ipi_raw_send(u32 hartindex, bool all_devices)"
> interface is a bad idea. It opened the door for such issues. AFAIK, the
> only reason such interface exists is that sometimes, some platform
> requires the use of a different IPI device during HSM kicking. E.g.,
> some Sifive cores can't use imsic, but only aclint. Why not introduce
> another interface "int sbi_ipi_raw_send_safe(u32 hartindex)" just for this
> purpose? Then we can have a separate rating on how safe that IPI device
> could be for cold startup, and pick the right one in the function. E.g.,
> favor clint over imsic for sbi_ipi_raw_send_safe.
>
> In this way, we ensure that only 1 IPI is active at any given time, and
> ipi_process can still use sbi_ipi_raw_clear(false) to clear the preferred
> IPI device, and HSM/init code still does sbi_ipi_raw_clear(true). There'd
> be no more cases like Troy encountered, where IPI from another device
> arrives late, and you have no ideal way of dealing with it other than what
> Troy was proposing.
>
> Let me test with this approach and prepare a patchset.
Ok. wait for your feedback.

-- 
Troy Mitchell


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[-- Attachment #2: Type: text/plain, Size: 105 bytes --]

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi