Re: [RFC PATCH 03/11] hw/intc: Add CLIC device

[LIU Zhiwei <zhiwei_liu@c-sky.com>]

[-- Attachment #1: Type: text/plain, Size: 6123 bytes --]


On 2021/6/28 下午3:23, Frank Chang wrote:
> LIU Zhiwei <zhiwei_liu@c-sky.com <mailto:zhiwei_liu@c-sky.com>> 於 
> 2021年6月28日 週一 下午3:17寫道：
>
>
>     On 2021/6/26 下午8:56, Frank Chang wrote:
>>     On Wed, Jun 16, 2021 at 10:56 AM LIU Zhiwei <zhiwei_liu@c-sky.com
>>     <mailto:zhiwei_liu@c-sky.com>> wrote:
>>
>>
>>         On 6/13/21 6:10 PM, Frank Chang wrote:
>>>         LIU Zhiwei <zhiwei_liu@c-sky.com
>>>         <mailto:zhiwei_liu@c-sky.com>> 於 2021年4月9日 週五
>>>         下午3:57寫道：
>>>
>>>             +static void riscv_clic_realize(DeviceState *dev, Error
>>>             **errp)
>>>             +{
>>>             +    RISCVCLICState *clic = RISCV_CLIC(dev);
>>>             +    size_t harts_x_sources = clic->num_harts *
>>>             clic->num_sources;
>>>             +    int irqs, i;
>>>             +
>>>             +    if (clic->prv_s && clic->prv_u) {
>>>             +        irqs = 3 * harts_x_sources;
>>>             +    } else if (clic->prv_s || clic->prv_u) {
>>>             +        irqs = 2 * harts_x_sources;
>>>             +    } else {
>>>             +        irqs = harts_x_sources;
>>>             +    }
>>>             +
>>>             +    clic->clic_size = irqs * 4 + 0x1000;
>>>             + memory_region_init_io(&clic->mmio, OBJECT(dev),
>>>             &riscv_clic_ops, clic,
>>>             + TYPE_RISCV_CLIC, clic->clic_size);
>>>             +
>>>             +    clic->clicintip = g_new0(uint8_t, irqs);
>>>             +    clic->clicintie = g_new0(uint8_t, irqs);
>>>             +    clic->clicintattr = g_new0(uint8_t, irqs);
>>>             +    clic->clicintctl = g_new0(uint8_t, irqs);
>>>             +    clic->active_list = g_new0(CLICActiveInterrupt, irqs);
>>>             +    clic->active_count = g_new0(size_t, clic->num_harts);
>>>             +    clic->exccode = g_new0(uint32_t, clic->num_harts);
>>>             +    clic->cpu_irqs = g_new0(qemu_irq, clic->num_harts);
>>>             + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &clic->mmio);
>>>             +
>>>             +    /* Allocate irq through gpio, so that we can use
>>>             qtest */
>>>             +    qdev_init_gpio_in(dev, riscv_clic_set_irq, irqs);
>>>             +    qdev_init_gpio_out(dev, clic->cpu_irqs,
>>>             clic->num_harts);
>>>             +
>>>             +    for (i = 0; i < clic->num_harts; i++) {
>>>             +        RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(i));
>>>
>>>
>>>         As spec says:
>>>           Smaller single-core systems might have only a CLIC,
>>>           while multicore systems might have a CLIC per-core and a
>>>         single shared PLIC.
>>>           The PLIC xeip signals are treated as hart-local interrupt
>>>         sources by the CLIC at each core.
>>>
>>>         It looks like it's possible to have one CLIC instance per core.
>>
>>         If you want to delivery an interrupt to one hart, you should
>>         encode the IRQ by the interrupt number
>>         , the hart number and the interrupt target privilege, then
>>         set the irq.
>>
>>         I think how to calculate the hart number is the task of PLIC
>>         and it can make use of "hartid-base"
>>         to calculate it.
>>
>>         Thanks,
>>         Zhiwei
>>
>>
>>     Hi Zhiwei,
>>
>>     What I mean is if there are multiple CLIC instances, each per
>>     core (CLIC spec allows that).
>>     If you try to bind CLIC with CPU index start from 0,
>>     it will be impossible for CLIC instance to bind CPU from index
>>     other than 0.
>>
>>     For example, for 4 cores system, it's possible to have 4 CLIC
>>     instances:
>>       * CLIC 0 binds to CPU 0
>>       * CLIC 1 binds to CPU 1
>>       * CLIC 2 binds to CPU 2
>>       * CLIC 3 binds to CPU 3
>>
>>     and that's why I said it's possible to pass an extra
>>     "hartid-base" just like PLIC.
>>     I know most of hardid are calculated by the requesing address, so
>>     most hartid usages should be fine.
>>     But I saw two places using qemu_get_cpu(),
>>     which may cause the problem for the scenario I describe above:
>>     i.e. riscv_clic_next_interrupt() and riscv_clic_realize() as my
>>     original reply.
>
>     So what's the problem here?
>
>     Currently all cores share the same CLIC instance. Do you want to
>     give each core  a CLIC instance?
>
> Yes, that's what I mean, which should be supported as what spec says[1]:
>   The CLIC complements the PLIC. Smaller single-core systems might 
> have only a CLIC,
>   while multicore systems might have *a CLIC per-core* and a single 
> shared PLIC.
>   The PLIC xeip signals are treated as hart-local interrupt sources by 
> the CLIC at each core.
>
> [1] 
> https://github.com/riscv/riscv-fast-interrupt/blob/646310a5e4ae055964b4680f12c1c04a7cc0dd56/clic.adoc#12-clic-versus-plic 
> <https://github.com/riscv/riscv-fast-interrupt/blob/646310a5e4ae055964b4680f12c1c04a7cc0dd56/clic.adoc#12-clic-versus-plic>
>
> Thanks,
> Frank Chang

If we give each core a CLIC instance, it is not convenient to access the 
shared memory, such as 0x0-0x1000.
Which CLIC instance should contain this memory region?

Thanks,
Zhiwei

>
>     Thanks,
>     Zhiwei
>
>>     Regards,
>>     Frank Chang
>>
>>>         However if you try to bind CPU reference start from index i = 0.
>>>         It's not possible for each per-core CLIC to bind their own
>>>         CPU instance in multicore system
>>>         as they have to bind from CPU 0.
>>>
>>>         I'm not sure if we add a new "hartid-base" property just
>>>         like what SiFive PLIC is
>>>         implemented would be a good idea or not.
>>>
>>>
>>>         Regards,
>>>         Frank Chang
>>>
>>>             +     qemu_irq irq =
>>>             qemu_allocate_irq(riscv_clic_cpu_irq_handler,
>>>             +  &cpu->env, 1);
>>>             +        qdev_connect_gpio_out(dev, i, irq);
>>>             +        cpu->env.clic = clic;
>>>             +    }
>>>             +}
>>>             +
>>>
>>>

[-- Attachment #2: Type: text/html, Size: 14971 bytes --]