[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/intc: sifive_plic: fix hard-coded max priority level
From: |
Clément Chigot |
Subject: |
Re: [PATCH] hw/intc: sifive_plic: fix hard-coded max priority level |
Date: |
Mon, 26 Sep 2022 09:52:40 +0200 |
Hi Jim,
On Sun, Sep 25, 2022 at 3:26 PM Jim Shu <jim.shu@sifive.com> wrote:
>
> The maximum priority level is hard-coded when writing to interrupt
> priority register. However, when writing to priority threshold register,
> the maximum priority level is from num_priorities Property which is
> configured by platform.
>
> Also change interrupt priority register to use num_priorities Property
> in maximum priority level.
>
> Signed-off-by: Emmanuel Blot <emmanuel.blot@sifive.com>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> ---
> hw/intc/sifive_plic.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index af4ae3630e..f864efa761 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
> uint64_t value,
> if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
> uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
>
> - plic->source_priority[irq] = value & 7;
> - sifive_plic_update(plic);
> + if (value <= plic->num_priorities) {
> + plic->source_priority[irq] = value;
> + sifive_plic_update(plic);
> + }
If I'm not mistaking the documentation [1], these registers are WARL
(Write-Any-Read-Legal). However, in your case, any value above
"num_priorities" will be ignored.
We had an issue related to that and ended up making a local patch.
However, we are assuming that "num_priorities+1" is a power of 2
(which is currently the case)
- plic->source_priority[irq] = value & 7;
+ /* Interrupt Priority registers are Write-Any Read-Legal. Cleanup
+ * incoming values before storing them.
+ */
+ plic->source_priority[irq] = value % (plic->num_priorities + 1);
Note that it should also be done for target_priority a bit below.
- if (value <= plic->num_priorities) {
- plic->target_priority[addrid] = value;
- sifive_plic_update(plic);
- }
+ /* Priority Thresholds registers are Write-Any Read-Legal. Cleanup
+ * incoming values before storing them.
+ */
+ plic->target_priority[addrid] = value % (plic->num_priorities + 1);
+ sifive_plic_update(plic);
[1] https://static.dev.sifive.com/FE310-G000.pdf
Thanks,
Clément