On Mon, Jan 10, 2022 at 6:38 PM Frank Chang <
frank.chang@sifive.com> wrote:
>
> Anup Patel <
anup@brainfault.org> 於 2021年12月30日 週四 下午8:38寫道：
>>
>> From: Anup Patel <
anup.patel@wdc.com>
>>
>> The AIA spec defines programmable 8-bit priority for each local interrupt
>> at M-level, S-level and VS-level so we extend local interrupt processing
>> to consider AIA interrupt priorities. The AIA CSRs which help software
>> configure local interrupt priorities will be added by subsequent patches.
>>
>> Signed-off-by: Anup Patel <
anup.patel@wdc.com>
>> Signed-off-by: Anup Patel <
anup@brainfault.org>
>> Reviewed-by: Alistair Francis <
alistair.francis@wdc.com>
>> ---
>> target/riscv/cpu.c | 19 ++++
>> target/riscv/cpu.h | 12 ++
>> target/riscv/cpu_helper.c | 231 ++++++++++++++++++++++++++++++++++----
>> target/riscv/machine.c | 3 +
>> 4 files changed, 244 insertions(+), 21 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 9f1a4d1088..9ad26035e1 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -348,6 +348,10 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
>>
>> static void riscv_cpu_reset(DeviceState *dev)
>> {
>> +#ifndef CONFIG_USER_ONLY
>> + uint8_t iprio;
>> + int i, irq, rdzero;
>> +#endif
>> CPUState *cs = CPU(dev);
>> RISCVCPU *cpu = RISCV_CPU(cs);
>> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
>> @@ -370,6 +374,21 @@ static void riscv_cpu_reset(DeviceState *dev)
>> env->miclaim = MIP_SGEIP;
>> env->pc = env->resetvec;
>> env->two_stage_lookup = false;
>> +
>> + /* Initialized default priorities of local interrupts. */
>> + for (i = 0; i < ARRAY_SIZE(env->miprio); i++) {
>> + iprio = riscv_cpu_default_priority(i);
>> + env->miprio[i] = (i == IRQ_M_EXT) ? 0 : iprio;
>> + env->siprio[i] = (i == IRQ_S_EXT) ? 0 : iprio;
>> + env->hviprio[i] = 0;
>> + }
>> + i = 0;
>> + while (!riscv_cpu_hviprio_index2irq(i, &irq, &rdzero)) {
>> + if (!rdzero) {
>> + env->hviprio[irq] = env->miprio[irq];
>> + }
>> + i++;
>> + }
>> /* mmte is supposed to have pm.current hardwired to 1 */
>> env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
>> #endif
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 02f3ef2c3c..140fabfdf9 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -182,6 +182,10 @@ struct CPURISCVState {
>> target_ulong mcause;
>> target_ulong mtval; /* since: priv-1.10.0 */
>>
>> + /* Machine and Supervisor interrupt priorities */
>> + uint8_t miprio[64];
>> + uint8_t siprio[64];
>> +
>> /* Hypervisor CSRs */
>> target_ulong hstatus;
>> target_ulong hedeleg;
>> @@ -194,6 +198,9 @@ struct CPURISCVState {
>> target_ulong hgeip;
>> uint64_t htimedelta;
>>
>> + /* Hypervisor controlled virtual interrupt priorities */
>> + uint8_t hviprio[64];
>> +
>> /* Virtual CSRs */
>> /*
>> * For RV32 this is 32-bit vsstatus and 32-bit vsstatush.
>> @@ -379,6 +386,11 @@ int riscv_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>> int cpuid, void *opaque);
>> int riscv_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>> int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>> +int riscv_cpu_hviprio_index2irq(int index, int *out_irq, int *out_rdzero);
>> +uint8_t riscv_cpu_default_priority(int irq);
>> +int riscv_cpu_mirq_pending(CPURISCVState *env);
>> +int riscv_cpu_sirq_pending(CPURISCVState *env);
>> +int riscv_cpu_vsirq_pending(CPURISCVState *env);
>> bool riscv_cpu_fp_enabled(CPURISCVState *env);
>> target_ulong riscv_cpu_get_geilen(CPURISCVState *env);
>> void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen);
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index f94a36fa89..e3532de4cf 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -151,36 +151,225 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>> }
>>
>> #ifndef CONFIG_USER_ONLY
>> -static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>> +
>> +/*
>> + * The HS-mode is allowed to configure priority only for the
>> + * following VS-mode local interrupts:
>> + *
>> + * 0 (Reserved interrupt, reads as zero)
>> + * 1 Supervisor software interrupt
>> + * 4 (Reserved interrupt, reads as zero)
>> + * 5 Supervisor timer interrupt
>> + * 8 (Reserved interrupt, reads as zero)
>> + * 13 (Reserved interrupt)
>> + * 14 "
>> + * 15 "
>> + * 16 "
>> + * 18 Debug/trace interrupt
>> + * 20 (Reserved interrupt)
>> + * 22 "
>> + * 24 "
>> + * 26 "
>> + * 28 "
>> + * 30 (Reserved for standard reporting of bus or system errors)
>> + */
>> +
>> +static int hviprio_index2irq[] =
>> + { 0, 1, 4, 5, 8, 13, 14, 15, 16, 18, 20, 22, 24, 26, 28, 30 };
>> +static int hviprio_index2rdzero[] =
>> + { 1, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>> +
>> +int riscv_cpu_hviprio_index2irq(int index, int *out_irq, int *out_rdzero)
>> {
>> - target_ulong virt_enabled = riscv_cpu_virt_enabled(env);
>> + if (index < 0 || ARRAY_SIZE(hviprio_index2irq) <= index) {
>> + return -EINVAL;
>> + }
>> +
>> + if (out_irq) {
>> + *out_irq = hviprio_index2irq[index];
>> + }
>>
>> - target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
>> - target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
>> + if (out_rdzero) {
>> + *out_rdzero = hviprio_index2rdzero[index];
>> + }
>>
>> - target_ulong vsgemask =
>> - (target_ulong)1 << get_field(env->hstatus, HSTATUS_VGEIN);
>> - target_ulong vsgein = (env->hgeip & vsgemask) ? MIP_VSEIP : 0;
>> + return 0;
>> +}
>>
>> - target_ulong pending = (env->mip | vsgein) & env->mie;
>> +uint8_t riscv_cpu_default_priority(int irq)
>> +{
>> + int u, l;
>> + uint8_t iprio = IPRIO_MMAXIPRIO;
>>
>> - target_ulong mie = env->priv < PRV_M ||
>> - (env->priv == PRV_M && mstatus_mie);
>> - target_ulong sie = env->priv < PRV_S ||
>> - (env->priv == PRV_S && mstatus_sie);
>> - target_ulong hsie = virt_enabled || sie;
>> - target_ulong vsie = virt_enabled && sie;
>> + if (irq < 0 || irq > 63) {
>> + return iprio;
>> + }
>>
>> - target_ulong irqs =
>> - (pending & ~env->mideleg & -mie) |
>> - (pending & env->mideleg & ~env->hideleg & -hsie) |
>> - (pending & env->mideleg & env->hideleg & -vsie);
>> + /*
>> + * Default priorities of local interrupts are defined in the
>> + * RISC-V Advanced Interrupt Architecture specification.
>> + *
>> + * ----------------------------------------------------------------
>> + * Default |
>> + * Priority | Major Interrupt Numbers
>> + * ----------------------------------------------------------------
>> + * Highest | 63 (3f), 62 (3e), 31 (1f), 30 (1e), 61 (3d), 60 (3c),
>> + * | 59 (3b), 58 (3a), 29 (1d), 28 (1c), 57 (39), 56 (38),
>> + * | 55 (37), 54 (36), 27 (1b), 26 (1a), 53 (35), 52 (34),
>> + * | 51 (33), 50 (32), 25 (19), 24 (18), 49 (31), 48 (30)
>> + * |
>> + * | 11 (0b), 3 (03), 7 (07)
>> + * | 9 (09), 1 (01), 5 (05)
>> + * | 12 (0c)
>> + * | 10 (0a), 2 (02), 6 (06)
>> + * |
>> + * | 47 (2f), 46 (2e), 23 (17), 22 (16), 45 (2d), 44 (2c),
>> + * | 43 (2b), 42 (2a), 21 (15), 20 (14), 41 (29), 40 (28),
>> + * | 39 (27), 38 (26), 19 (13), 18 (12), 37 (25), 36 (24),
>> + * Lowest | 35 (23), 34 (22), 17 (11), 16 (10), 33 (21), 32 (20)
>> + * ----------------------------------------------------------------
>> + */
>>
>> - if (irqs) {
>> - return ctz64(irqs); /* since non-zero */
>> + u = IPRIO_DEFAULT_U(irq);
>> + l = IPRIO_DEFAULT_L(irq);
>> + if (u == 0) {
>> + if (irq == IRQ_VS_EXT || irq == IRQ_VS_TIMER ||
>> + irq == IRQ_VS_SOFT) {
>> + iprio = IPRIO_DEFAULT_VS;
>> + } else if (irq == IRQ_S_GEXT) {
>> + iprio = IPRIO_DEFAULT_SGEXT;
>> + } else if (irq == IRQ_S_EXT || irq == IRQ_S_TIMER ||
>> + irq == IRQ_S_SOFT) {
>> + iprio = IPRIO_DEFAULT_S;
>> + } else if (irq == IRQ_M_EXT || irq == IRQ_M_TIMER ||
>> + irq == IRQ_M_SOFT) {
>> + iprio = IPRIO_DEFAULT_M;
>> + } else {
>> + iprio = IPRIO_DEFAULT_VS;
>> + }
>> + } else if (u == 1) {
>> + if (l < 8) {
>> + iprio = IPRIO_DEFAULT_16_23(irq);
>> + } else {
>> + iprio = IPRIO_DEFAULT_24_31(irq);
>> + }
>> + } else if (u == 2) {
>> + iprio = IPRIO_DEFAULT_32_47(irq);
>> + } else if (u == 3) {
>> + iprio = IPRIO_DEFAULT_48_63(irq);
>> + }
>> +
>> + return iprio;
>> +}
>> +
>> +static int riscv_cpu_pending_to_irq(CPURISCVState *env,
>> + uint64_t pending, uint8_t *iprio)
>> +{
>> + int irq, best_irq = RISCV_EXCP_NONE;
>> + unsigned int prio, best_prio = UINT_MAX;
>> +
>> + if (!pending) {
>> + return RISCV_EXCP_NONE;
>> + }
>> +
>> + irq = ctz64(pending);
>> + if (!riscv_feature(env, RISCV_FEATURE_AIA)) {
>> + return irq;
>> + }
>> +
>> + pending = pending >> irq;
>> + while (pending) {
>> + prio = iprio[irq];
>> + if (!prio) {
>> + prio = (riscv_cpu_default_priority(irq) < IPRIO_DEFAULT_M) ?
>> + 1 : IPRIO_MMAXIPRIO;
>
>
> Hi Anup,
>
> I have a question regarding configuring priorities of major interrupts.
>
> riscv_cpu_default_priority() for M-mode external interrupt would return IPRIO_DEFAULT_M (8).
> but the comparison here is < IPRIO_DEFAULT_M,
> so if M-mode external interrupt is pending, the prio would be assigned to IPRIO_MMAXIPRIO (255).
> Is this the expected behavior?
Actually, for irq == IRQ_M_EXT the prio should be IPRIO_DEFAULT_M
same for irq == IRQ_S_EXT.
Goot catch, I will fix this in next revision.
>
> Also, why is IPRIO_DEFAULT_M been compared?
> Should IPRIO_DEFAULT_S and IPRIO_DEFAULT_VS be used for S-mode and VS-mode interrupts?
For M-mode, we should compare IPRIO_DEFAULT_M.
For S/VS-mode, we should compare IPRIO_DEFAULT_S
but the priority registers used for S-mode and VS-mode
are different.
>
>>
>> + }
>> + if ((pending & 0x1) && (prio < best_prio)) {
>
>
> If both interrupt 63 and 62 are pending-and-enabled,
> with their iprio values both assigned to zero.
> Interrupt 62 would be picked, because best_prio will be 1 when interrupt 62 is been checked.
> As the default priority order for interrupt 63 is also less than IPRIO_DEFAULT_M (8),
> so prio would be assigned to 1 as well.
> But as prio is not less than best_prio, so best_irq would still be interrupt 62.
>
> However, according to the default priority table in AIA spec,
> interrupt 63 should have a higher priority than interrupt 62:
> Interrupt 63 has the highest default priority, and interrupt 32 has the lowest default priority.
Argh, the comparision "prio < best_prio" should have been
"prio <= best_prio".
I am certainly aware of the fact that if two interrupts
have same priority then higher interrupt number should be
preferred. This is silly mistake on my part.
I really appreciate your detailed review. Thanks.
>
>>
>> + best_irq = irq;
>> + best_prio = prio;
>> + }
>> + irq++;
>> + pending = pending >> 1;
>> + }
>> +
>> + return best_irq;
>> +}
>
>
> Yet I have another question for major interrupts priorities.
>
> According to AIA spec (v0.2-draft.28):
> ==========
> The priority number for a machine-level external interrupt (bits 31:24 of register iprio2)
> must also be read-only zero.
>
> When an interrupt’s priority number in the array is zero (either read-only zero or set to zero),
> its priority is the default order from Section 6.1.
> Setting an interrupt’s priority number instead to a nonzero value p gives that interrupt nominally
> the same priority as a machine-level external interrupt with priority number p.
> For a major interrupt that defaults to a higher priority than machine external interrupts,
> setting its priority number to a nonzero value lowers its priority.
> For a major interrupt that defaults to a lower priority than machine external interrupts,
> setting its priority number to a nonzero value raises its priority.
> ==========
>
> From the above sentences,
> The M-mode external interrupt has iprio value of 0, so the default priority order is applied.
Yes, that's correct and same logic applies to S/VS-mode external interrupts.
>
> What are the priorities for the major interrupt interrupts which are higher/lower than
> M-mode external interrupt when their iprio values are not 0?
For a major interrupt X (which is not an external interrupt) having iprio value 0,
we take iprio of X as 1 or 255 based of comparison of default priority of X
with default priority of external interrupt.
(Refer, documentation of "mtopi" (Section 6.3.2) and "stopi" (Section 6.5.2))
>
> Which interrupt will be picked when:
> M-mode external interrupt is pending-and-enabled,
> Interrupt with higher default priority than M-mode external interrupt is pending-and-enabled, and
> Interrupt with lower default priority than M-mode external interrupt is pending-and-enabled/
>
> From the above riscv_cpu_pending_to_irq(),
> If both interrupt 63 and interrupt 18's iprio values are assigned to the value of 10,
You mean both these interrupt have iprio 0, right ?
> and interrupt 63, interrupt 18 and M-mode external interrupts are pending-and-enabled.
> Interrupt 18 would be picked.
> Is this the correct expectation?
If both 63 and 18 have iprio 0 then interrupt 63 would be picked
because 63 has higher default priority compared to default
priorities of both M-mode external interrupt and interrupt 18.
> I'm a bit confused with the correct priority order of the interrupts with/without non-zero value iprio and the default priority.
Whenever iprio of an interrupt X is 0, the default priority of
interrupt X compared against default external interrupt
priority and based on this comparison 1 (highest) or 255 (lowest)
priority is used for interrupt X.
In other, a saturated priority is used on interrupt X when
it's iprio is set to zero.
Regards,
Anup
>
> Thanks,
> Frank Chang
>
>>
>> +
>> +static uint64_t riscv_cpu_all_pending(CPURISCVState *env)
>> +{
>> + uint32_t gein = get_field(env->hstatus, HSTATUS_VGEIN);
>> + uint64_t vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0;
>> +
>> + return (env->mip | vsgein) & env->mie;
>> +}
>> +
>> +int riscv_cpu_mirq_pending(CPURISCVState *env)
>> +{
>> + uint64_t irqs = riscv_cpu_all_pending(env) & ~env->mideleg &
>> + ~(MIP_SGEIP | MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
>> +
>> + return riscv_cpu_pending_to_irq(env, irqs, env->miprio);
>> +}
>> +
>> +int riscv_cpu_sirq_pending(CPURISCVState *env)
>> +{
>> + uint64_t irqs = riscv_cpu_all_pending(env) & env->mideleg &
>> + ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
>> +
>> + return riscv_cpu_pending_to_irq(env, irqs, env->siprio);
>> +}
>> +
>> +int riscv_cpu_vsirq_pending(CPURISCVState *env)
>> +{
>> + uint64_t irqs = riscv_cpu_all_pending(env) & env->mideleg &
>> + (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
>> +
>> + return riscv_cpu_pending_to_irq(env, irqs >> 1, env->hviprio);
>> +}
>> +
>> +static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>> +{
>> + int virq;
>> + uint64_t irqs, pending, mie, hsie, vsie;
>> +
>> + /* Determine interrupt enable state of all privilege modes */
>> + if (riscv_cpu_virt_enabled(env)) {
>> + mie = 1;
>> + hsie = 1;
>> + vsie = (env->priv < PRV_S) ||
>> + (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_SIE));
>> } else {
>> - return RISCV_EXCP_NONE; /* indicates no pending interrupt */
>> + mie = (env->priv < PRV_M) ||
>> + (env->priv == PRV_M && get_field(env->mstatus, MSTATUS_MIE));
>> + hsie = (env->priv < PRV_S) ||
>> + (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_SIE));
>> + vsie = 0;
>> }
>> +
>> + /* Determine all pending interrupts */
>> + pending = riscv_cpu_all_pending(env);
>> +
>> + /* Check M-mode interrupts */
>> + irqs = pending & ~env->mideleg & -mie;
>> + if (irqs) {
>> + return riscv_cpu_pending_to_irq(env, irqs, env->miprio);
>> + }
>> +
>> + /* Check HS-mode interrupts */
>> + irqs = pending & env->mideleg & ~env->hideleg & -hsie;
>> + if (irqs) {
>> + return riscv_cpu_pending_to_irq(env, irqs, env->siprio);
>> + }
>> +
>> + /* Check VS-mode interrupts */
>> + irqs = pending & env->mideleg & env->hideleg & -vsie;
>> + if (irqs) {
>> + virq = riscv_cpu_pending_to_irq(env, irqs >> 1, env->hviprio);
>> + return (virq <= 0) ? virq : virq + 1;
>> + }
>> +
>> + /* Indicate no pending interrupt */
>> + return RISCV_EXCP_NONE;
>> }
>>
>> bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> index 76dd0d415c..cffc444969 100644
>> --- a/target/riscv/machine.c
>> +++ b/target/riscv/machine.c
>> @@ -92,6 +92,7 @@ static const VMStateDescription vmstate_hyper = {
>> VMSTATE_UINTTL(env.hgeie, RISCVCPU),
>> VMSTATE_UINTTL(env.hgeip, RISCVCPU),
>> VMSTATE_UINT64(env.htimedelta, RISCVCPU),
>> + VMSTATE_UINT8_ARRAY(env.hviprio, RISCVCPU, 64),
>>
>> VMSTATE_UINT64(env.vsstatus, RISCVCPU),
>> VMSTATE_UINTTL(env.vstvec, RISCVCPU),
>> @@ -173,6 +174,8 @@ const VMStateDescription vmstate_riscv_cpu = {
>> .fields = (VMStateField[]) {
>> VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
>> VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32),
>> + VMSTATE_UINT8_ARRAY(env.miprio, RISCVCPU, 64),
>> + VMSTATE_UINT8_ARRAY(env.siprio, RISCVCPU, 64),
>> VMSTATE_UINTTL(env.pc, RISCVCPU),
>> VMSTATE_UINTTL(env.load_res, RISCVCPU),
>> VMSTATE_UINTTL(env.load_val, RISCVCPU),
>> --
>> 2.25.1
>>
>>