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.

    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 <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


Reply via email to