On 6/11/21 5:07 PM, Frank Chang wrote:
LIU Zhiwei <zhiwei_...@c-sky.com <mailto:zhiwei_...@c-sky.com>> 於 2021年6月11日 週五 下午4:56寫道:On 6/11/21 4:42 PM, Frank Chang wrote:LIU Zhiwei <zhiwei_...@c-sky.com <mailto:zhiwei_...@c-sky.com>> 於 2021年6月11日 週五 下午4:30寫道: On 6/11/21 4:15 PM, Frank Chang wrote:LIU Zhiwei <zhiwei_...@c-sky.com <mailto:zhiwei_...@c-sky.com>> 於 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 <zhiwei_...@c-sky.com <mailto:zhiwei_...@c-sky.com>> --- 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. ZhiweiThanks, 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].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.Is it okay to just grab the qtest from this repo?[1]: https://github.com/romanheros/qemu/commit/bce1845ea9b079b4c360440292dc47725d1b24ab <https://github.com/romanheros/qemu/commit/bce1845ea9b079b4c360440292dc47725d1b24ab>
Zhiwei
Thanks, Frank ChangRegards, Frank ChangRegard, 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