On 6/11/21 5:07 PM, Frank Chang wrote: > LIU Zhiwei > 於 > 2021年6月11日 週五 下午4:56寫道： > > > On 6/11/21 4:42 PM, Frank Chang wrote: >> LIU Zhiwei > 於 >> 2021年6月11日 週五 下午4:30寫道： >> >> >> On 6/11/21 4:15 PM, Frank Chang wrote: >>> LIU Zhiwei >> > 於 2021年4月9日 週五 >>> 下午3:52寫道： >>> >>> The CSR can be used by software to service the next >>> horizontal interrupt >>> when it has greater level than the saved interrupt context >>> (held in xcause`.pil`) and greater level than the >>> interrupt threshold of >>> the corresponding privilege mode, >>> >>> Signed-off-by: LIU Zhiwei >> > >>> --- >>>  target/riscv/cpu_bits.h |  16 ++++++ >>>  target/riscv/csr.c      | 114 >>> ++++++++++++++++++++++++++++++++++++++++ >>>  2 files changed, 130 insertions(+) >>> >>> diff --git a/target/riscv/cpu_bits.h >>> b/target/riscv/cpu_bits.h >>> index 7922097776..494e41edc9 100644 >>> --- a/target/riscv/cpu_bits.h >>> +++ b/target/riscv/cpu_bits.h >>> @@ -166,6 +166,7 @@ >>>  #define CSR_MCAUSE          0x342 >>>  #define CSR_MTVAL           0x343 >>>  #define CSR_MIP             0x344 >>> +#define CSR_MNXTI           0x345 /* clic-spec-draft */ >>>  #define CSR_MINTSTATUS      0x346 /* clic-spec-draft */ >>>  #define CSR_MINTTHRESH      0x347 /* clic-spec-draft */ >>> >>> @@ -187,6 +188,7 @@ >>>  #define CSR_SCAUSE          0x142 >>>  #define CSR_STVAL           0x143 >>>  #define CSR_SIP             0x144 >>> +#define CSR_SNXTI           0x145 /* clic-spec-draft */ >>>  #define CSR_SINTSTATUS      0x146 /* clic-spec-draft */ >>>  #define CSR_SINTTHRESH      0x147 /* clic-spec-draft */ >>> >>> @@ -596,10 +598,24 @@ >>>  #define MINTSTATUS_SIL    0x0000ff00 /* sil[7:0] */ >>>  #define MINTSTATUS_UIL    0x000000ff /* uil[7:0] */ >>> >>> +/* mcause */ >>> +#define MCAUSE_MINHV    0x40000000 /* minhv */ >>> +#define MCAUSE_MPP    0x30000000 /* mpp[1:0] */ >>> +#define MCAUSE_MPIE     0x08000000 /* mpie */ >>> +#define MCAUSE_MPIL     0x00ff0000 /* mpil[7:0] */ >>> +#define MCAUSE_EXCCODE    0x00000fff /* exccode[11:0] */ >>> + >>>  /* sintstatus */ >>>  #define SINTSTATUS_SIL    0x0000ff00 /* sil[7:0] */ >>>  #define SINTSTATUS_UIL    0x000000ff /* uil[7:0] */ >>> >>> +/* scause */ >>> +#define SCAUSE_SINHV    0x40000000 /* sinhv */ >>> +#define SCAUSE_SPP    0x10000000 /* spp */ >>> +#define SCAUSE_SPIE     0x08000000 /* spie */ >>> +#define SCAUSE_SPIL     0x00ff0000 /* spil[7:0] */ >>> +#define SCAUSE_EXCCODE    0x00000fff /* exccode[11:0] */ >>> + >>>  /* MIE masks */ >>>  #define MIE_SEIE    (1 << IRQ_S_EXT) >>>  #define MIE_UEIE    (1 << IRQ_U_EXT) >>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >>> index e12222b77f..72cba080bf 100644 >>> --- a/target/riscv/csr.c >>> +++ b/target/riscv/csr.c >>> @@ -774,6 +774,80 @@ static int rmw_mip(CPURISCVState >>> *env, int csrno, target_ulong *ret_value, >>>      return 0; >>>  } >>> >>> +static bool get_xnxti_status(CPURISCVState *env) >>> +{ >>> +    CPUState *cs = env_cpu(env); >>> +    int clic_irq, clic_priv, clic_il, pil; >>> + >>> +    if (!env->exccode) { /* No interrupt */ >>> +        return false; >>> +    } >>> +    /* The system is not in a CLIC mode */ >>> +    if (!riscv_clic_is_clic_mode(env)) { >>> +        return false; >>> +    } else { >>> + riscv_clic_decode_exccode(env->exccode, &clic_priv, >>> &clic_il, >>> + &clic_irq); >>> + >>> +        if (env->priv == PRV_M) { >>> +            pil = MAX(get_field(env->mcause, >>> MCAUSE_MPIL), env->mintthresh); >>> +        } else if (env->priv == PRV_S) { >>> +            pil = MAX(get_field(env->scause, >>> SCAUSE_SPIL), env->sintthresh); >>> +        } else { >>> + qemu_log_mask(LOG_GUEST_ERROR, >>> +                          "CSR: rmw xnxti with >>> unsupported mode\n"); >>> +            exit(1); >>> +        } >>> + >>> +        if ((clic_priv != env->priv) || /* No >>> horizontal interrupt */ >>> +            (clic_il <= pil) || /* No higher level >>> interrupt */ >>> + (riscv_clic_shv_interrupt(env->clic, clic_priv, >>> cs->cpu_index, >>> + clic_irq))) { /* CLIC vector mode */ >>> +            return false; >>> +        } else { >>> +            return true; >>> +        } >>> +    } >>> +} >>> + >>> +static int rmw_mnxti(CPURISCVState *env, int csrno, >>> target_ulong *ret_value, >>> +                     target_ulong new_value, >>> target_ulong write_mask) >>> +{ >>> +    int clic_priv, clic_il, clic_irq; >>> +    bool ready; >>> +    CPUState *cs = env_cpu(env); >>> +    if (write_mask) { >>> +        env->mstatus |= new_value & (write_mask & 0b11111); >>> +    } >>> + >>> +    qemu_mutex_lock_iothread(); >>> >>> >>> Hi Zhiwei, >>> >>> May I ask what's the purpose to request the BQL here with >>> /qemu_mutex_lock_iothread()/? >>> Is there any critical data we need to protect in /rmw_mnxti()/? >>> As far I see, /rmw_mnxti()/ won't call /cpu_interrupt()/ >>> which need BQL to be held before calling. >>> Am I missing anything? >> In my opinion, if you read or write any  MMIO register, you >> need to hold the BQL. As you can quickly see, >> it calls riscv_clic_clean_pending. That's why it should hold >> the BQL. >> >> Zhiwei >> >> >> Oh, I see. >> The MMIO register reads and writes should also be protected by BQL. >> Thanks for the explanation. > > I am glad to know that you are reviewing this patch set. As Sifive > implements the initial v0.7 CLIC, I think you may need this patch > set for your SOC. > If you like to, I am happy to see that you connect this patch set > to your SOC, and resend it again. I can also provide the qtest of > this patch set if you need. > > As you may see, the v6.1 soft freeze will come in July. I am > afraid I can't upstream a new SOC in so short time. > > Zhiwei > > Thanks, I think we will leverage the hard works you have done into our > implementation. > However, I'm not sure I can catch up the deadline before v6.1's > soft-freeze. > But I think I can help to review the patches, at least we can speed up > the review process. > Nice. > Regarding qtest, I saw your head commit mentioned the repo you are > using: [1]. > Is it okay to just grab the qtest from this repo? > > [1]: > https://github.com/romanheros/qemu/commit/bce1845ea9b079b4c360440292dc47725d1b24ab > > Yes, it is. The qtest is not so trivial, as it extends current qtest mechanism by intercepting irq_in and irq_out for one device (Normally, you can only intercept irq_in or irq_out for one device). If you have any question, just let me know. Zhiwei > Thanks, > Frank Chang > >> >> Regards, >> Frank Chang >> >>> >>> Regard, >>> Frank Chang >>> >>> + ready = get_xnxti_status(env); >>> +    if (ready) { >>> + riscv_clic_decode_exccode(env->exccode, &clic_priv, >>> &clic_il, >>> + &clic_irq); >>> +        if (write_mask) { >>> +            bool edge = >>> riscv_clic_edge_triggered(env->clic, clic_priv, >>> +           cs->cpu_index, clic_irq); >>> +            if (edge) { >>> + riscv_clic_clean_pending(env->clic, clic_priv, >>> +  cs->cpu_index, clic_irq); >>> +            } >>> +            env->mintstatus = set_field(env->mintstatus, >>> + MINTSTATUS_MIL, clic_il); >>> +            env->mcause = set_field(env->mcause, >>> MCAUSE_EXCCODE, clic_irq); >>> +        } >>> +        if (ret_value) { >>> +            *ret_value = (env->mtvt & ~0x3f) + >>> sizeof(target_ulong) * clic_irq; >>> +        } >>> +    } else { >>> +        if (ret_value) { >>> +            *ret_value = 0; >>> +        } >>> +    } >>> +    qemu_mutex_unlock_iothread(); >>> +    return 0; >>> +} >>> + >>>  static int read_mintstatus(CPURISCVState *env, int >>> csrno, target_ulong *val) >>>  { >>>      *val = env->mintstatus; >>> @@ -982,6 +1056,44 @@ static int rmw_sip(CPURISCVState >>> *env, int csrno, target_ulong *ret_value, >>>      return ret; >>>  } >>> >>> +static int rmw_snxti(CPURISCVState *env, int csrno, >>> target_ulong *ret_value, >>> +                     target_ulong new_value, >>> target_ulong write_mask) >>> +{ >>> +    int clic_priv, clic_il, clic_irq; >>> +    bool ready; >>> +    CPUState *cs = env_cpu(env); >>> +    if (write_mask) { >>> +        env->mstatus |= new_value & (write_mask & 0b11111); >>> +    } >>> + >>> +    qemu_mutex_lock_iothread(); >>> +    ready = get_xnxti_status(env); >>> +    if (ready) { >>> + riscv_clic_decode_exccode(env->exccode, &clic_priv, >>> &clic_il, >>> + &clic_irq); >>> +        if (write_mask) { >>> +            bool edge = >>> riscv_clic_edge_triggered(env->clic, clic_priv, >>> +           cs->cpu_index, clic_irq); >>> +            if (edge) { >>> + riscv_clic_clean_pending(env->clic, clic_priv, >>> +  cs->cpu_index, clic_irq); >>> +            } >>> +            env->mintstatus = set_field(env->mintstatus, >>> + MINTSTATUS_SIL, clic_il); >>> +            env->scause = set_field(env->scause, >>> SCAUSE_EXCCODE, clic_irq); >>> +        } >>> +        if (ret_value) { >>> +            *ret_value = (env->stvt & ~0x3f) + >>> sizeof(target_ulong) * clic_irq; >>> +        } >>> +    } else { >>> +        if (ret_value) { >>> +            *ret_value = 0; >>> +        } >>> +    } >>> +    qemu_mutex_unlock_iothread(); >>> +    return 0; >>> +} >>> + >>>  static int read_sintstatus(CPURISCVState *env, int >>> csrno, target_ulong *val) >>>  { >>>      target_ulong mask = SINTSTATUS_SIL | SINTSTATUS_UIL; >>> @@ -1755,6 +1867,7 @@ riscv_csr_operations >>> csr_ops[CSR_TABLE_SIZE] = { >>> >>>      /* Machine Mode Core Level Interrupt Controller */ >>>      [CSR_MTVT] = { "mtvt", clic, read_mtvt,  >>> write_mtvt      }, >>> +    [CSR_MNXTI] = { "mnxti", clic, NULL,  NULL,  >>> rmw_mnxti   }, >>>      [CSR_MINTSTATUS] = { "mintstatus", clic,  >>> read_mintstatus }, >>>      [CSR_MINTTHRESH] = { "mintthresh", clic,  >>> read_mintthresh, >>> write_mintthresh }, >>> @@ -1766,6 +1879,7 @@ riscv_csr_operations >>> csr_ops[CSR_TABLE_SIZE] = { >>> >>>      /* Supervisor Mode Core Level Interrupt Controller */ >>>      [CSR_STVT] = { "stvt", clic, read_stvt, write_stvt  >>>      }, >>> +    [CSR_SNXTI] = { "snxti", clic, NULL,  NULL,  >>> rmw_snxti   }, >>> >>>  #endif /* !CONFIG_USER_ONLY */ >>>  }; >>> -- >>> 2.25.1 >>> >>>