[PATCH] lib: sbi: Enable M-mode external interrupt only for AIA

[Jessica Clarke]

On 18 Feb 2022, at 17:11, Anup Patel <apatel at ventanamicro.com> wrote:
> 
> On Fri, Feb 18, 2022 at 10:34 PM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>> 
>> On 18 Feb 2022, at 16:56, Atish Patra <atishp at rivosinc.com> wrote:
>>> 
>>> Some platforms may inject spurious M-mode external interrupt if
>>> the external interrupt bit is enabled in mie. We really don't
>>> need to enable M-mode external interrupts for platforms without
>>> AIA. This issue is observed in HiFive unmatched.
>>> 
>>> Keep the M-mode external interrupt disabled for non-AIA platforms.
>> 
>> Is this for UART1 (https://forums.sifive.com/t/spurious-interrupts-from-m-2-e-key-uart1/5478)?
>> 
>> Seems to be OpenSBI should not be enabling any external interrupts in
>> the PLIC that it doesn’t want, which should render this patch
>> unnecessary (though you could perhaps argue you still might as well not
>> unmask external interrupts if you don’t expect any). Otherwise you just
>> risk the same thing happening for AIA hardware and needing to do the
>> proper fix at a later date.
> 
> On platforms with IMSIC, the IPIs are through IMSIC since there is no
> CLINT or MSWI on these platforms. To support IPIs over IMSIC, we have
> to enable external interrupt on platforms with IMSIC.
> 
> Unfortunately, this leads to possible spurious interrupts on platforms
> (e.g. SiFive Unmatched) with traditional PLIC.

Why? If you correctly configure the PLIC you shouldn’t get any, surely?

Jess

> Regards,
> Anup
> 
>> 
>> As it stands this patch is quite dubious without further exploration
>> and explanation of where these external interrupts are coming from.
>> 
>> Jess
>> 
>>> Signed-off-by: Atish Patra <atishp at rivosinc.com>
>>> ---
>>> lib/sbi/sbi_init.c | 14 ++++++++++++--
>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
>>> index 6876eb2f0f22..76b7c5d953ec 100644
>>> --- a/lib/sbi/sbi_init.c
>>> +++ b/lib/sbi/sbi_init.c
>>> @@ -276,7 +276,12 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
>>>                         __func__, rc);
>>>              sbi_hart_hang();
>>>      }
>>> -     csr_set(CSR_MIE, MIP_MEIP);
>>> +     /**
>>> +      * HiFive unmatched platform generates spurious interrupt if external
>>> +      * interrupt is enabled in mie. Enable it only if AIA is present.
>>> +      */
>>> +     if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
>>> +             csr_set(CSR_MIE, MIP_MEIP);
>>> 
>>>      rc = sbi_ipi_init(scratch, TRUE);
>>>      if (rc) {
>>> @@ -377,7 +382,12 @@ static void init_warm_startup(struct sbi_scratch *scratch, u32 hartid)
>>>      rc = sbi_platform_irqchip_init(plat, FALSE);
>>>      if (rc)
>>>              sbi_hart_hang();
>>> -     csr_set(CSR_MIE, MIP_MEIP);
>>> +     /**
>>> +      * HiFive unmatched platform generates spurious interrupt if external
>>> +      * interrupt is enabled in mie. Enable it only if AIA is present.
>>> +      */
>>> +     if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
>>> +             csr_set(CSR_MIE, MIP_MEIP);
>>> 
>>>      rc = sbi_ipi_init(scratch, FALSE);
>>>      if (rc)
>>> --
>>> 2.30.2
>>> 
>>> 
>>> --
>>> opensbi mailing list
>>> opensbi at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/opensbi
>> 
>> 
>> --
>> opensbi mailing list
>> opensbi at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/opensbi