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

[Bo Gan <ganboing@gmail.com>]

Just sent the patch. I've changed the function names slightly:

https://lore.kernel.org/opensbi/20260913100128.2437-1-ganboing@gmail.com/

On 9/12/26 02:09, Troy Mitchell wrote:
> 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.
> 

Bo

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