LIU Zhiwei <[email protected]> 於 2021年6月11日 週五 下午4:56寫道:

>
> On 6/11/21 4:42 PM, Frank Chang wrote:
>
> LIU Zhiwei <[email protected]> 於 2021年6月11日 週五 下午4:30寫道:
>
>>
>> On 6/11/21 4:15 PM, Frank Chang wrote:
>>
>> LIU Zhiwei <[email protected]> 於 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 <[email protected]>
>>> ---
>>>  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.

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

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