[PATCH 02/13] lib: sbi: Detect AIA CSRs at boot-time

[Anup Patel]

On Fri, Jan 28, 2022 at 1:17 PM Atish Patra <atishp at atishpatra.org> wrote:
>
>
>
> On Tue, Jan 4, 2022 at 2:10 AM Anup Patel <apatel at ventanamicro.com> wrote:
>>
>> We extend HART feature detection to discover AIA CSRs at boot-time.
>>
>> Signed-off-by: Anup Patel <anup.patel at wdc.com>
>> Signed-off-by: Anup Patel <apatel at ventanamicro.com>
>> ---
>>  include/sbi/sbi_hart.h |  4 +++-
>>  lib/sbi/sbi_hart.c     | 23 +++++++++++++++++++++++
>>  2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
>> index d2db9d6..a83b45b 100644
>> --- a/include/sbi/sbi_hart.h
>> +++ b/include/sbi/sbi_hart.h
>> @@ -24,9 +24,11 @@ enum sbi_hart_features {
>>         SBI_HART_HAS_SSCOFPMF = (1 << 3),
>>         /** HART has timer csr implementation in hardware */
>>         SBI_HART_HAS_TIME = (1 << 4),
>> +       /** HART has AIA local interrupt CSRs */
>> +       SBI_HART_HAS_AIA = (1 << 5),
>>
>>         /** Last index of Hart features*/
>> -       SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_TIME,
>> +       SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_AIA,
>>  };
>>
>>  struct sbi_scratch;
>> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
>> index d9a15d9..c16b395 100644
>> --- a/lib/sbi/sbi_hart.c
>> +++ b/lib/sbi/sbi_hart.c
>> @@ -283,6 +283,9 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
>>         case SBI_HART_HAS_TIME:
>>                 fstr = "time";
>>                 break;
>> +       case SBI_HART_HAS_AIA:
>> +               fstr = "aia";
>> +               break;
>>         default:
>>                 break;
>>         }
>> @@ -506,6 +509,26 @@ __mhpm_skip:
>>         csr_read_allowed(CSR_TIME, (unsigned long)&trap);
>>         if (!trap.cause)
>>                 hfeatures->features |= SBI_HART_HAS_TIME;
>> +
>> +       /* Detect if hart has AIA local interrupt CSRs */
>> +       csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
>> +       if (trap.cause)
>> +               goto __aia_skip;
>> +       csr_read_allowed(CSR_MISELECT, (unsigned long)&trap);
>> +       if (trap.cause)
>> +               goto __aia_skip;
>> +       csr_write_allowed(CSR_MISELECT, (unsigned long)&trap, 0x30);
>
>
> You can use __check_csr directly here as you are not using the value returned from csr_read_allowed.
> Some comment for the magic value  0x30 is necessary as well.

We don't __check_csr() here because some CSRs (such as MTOPI) are read-only.

>
> I have a slightly orthogonal question as well. AFAIK, all these CSRs are mandatory for AIA.
> Do we need to detect all the CSRs every time? Absence of 1 CSR is enough to indicate that AIA is not
> present. I guess you want all the CSR to cover the buggy hardware use case.
>
> However, that is a corner case which can be handled in a platform specific manner. I am just worried
> that this may blow up in the future which would hurt the boot time in the long run.

You are right. Checking all these CSRs for AIA presence is an overkill.

We can simply check read-only MTOPI CSR and be done with it. I will
update this in the next patch revision.

Regards,
Anup

>
>> +       if (trap.cause)
>> +               goto __aia_skip;
>> +       val = csr_read_allowed(CSR_MIREG, (unsigned long)&trap);
>> +       if (trap.cause)
>> +               goto __aia_skip;
>> +       csr_write_allowed(CSR_MIREG, (unsigned long)&trap, val);
>> +       if (trap.cause)
>> +               goto __aia_skip;
>> +       hfeatures->features |= SBI_HART_HAS_AIA;
>> +__aia_skip:
>> +       return;
>>  }
>>
>>  int sbi_hart_reinit(struct sbi_scratch *scratch)
>> --
>> 2.25.1
>>
>>
>> --
>> opensbi mailing list
>> opensbi at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/opensbi
>
>
>
> --
> Regards,
> Atish