Re: [RFC PATCH v3 16/27] hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)

[yangxiaojuan]

Hi,Mark

On 12/18/2021 07:54 AM, Mark Cave-Ayland wrote:
> On 04/12/2021 12:07, Xiaojuan Yang wrote:
> 
>> Loongson-3A5000 support 14 interrupts from 64 - 77(Timer->75 IPI->76)
>> Loongson-3A5000 and ls7a form a legacy model and extended model irq
>> hierarchy.Tcg mode emulate a simplified extended model which
>> has no Legacy I/O Interrupt Controller(LIOINTC) and LPC.
>> e.g:
>>
>>   |    +-----+    +---------+     +-------+             |
>>   |    | IPI |--> | CPUINTC | <-- | Timer |             |
>>   |    +-----+    +---------+     +-------+             |
>>   |                    ^                                |
>>   |                    |                                |
>>   |               +---------+
>>   |               | EIOINTC |
>>   |               +---------+
>>   |                ^       ^                            |
>>   |                |       |                            |
>>   |         +---------+ +---------+                     |
>>   |         | PCH-PIC | | PCH-MSI |                     |
>>   |         +---------+ +---------+                     |
>>   |           ^     ^           ^                       |
>>   |           |     |           |                       |
>>   |   +---------+ +---------+ +---------+               |
>>   |   | UARTs | | Devices | | Devices |                 |
>>   |   +---------+ +---------+ +---------+               |
>>   |        ^                                            |
>>
>> The following series patch will realize the interrupt
>> controller in this model.
>>
>> More detailed info can be found at the kernel doc or manual
>> 1.https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/
>> linux-loongson.git/tree/Documentation/loongarch?h=loongarch-next
>> 2.https://github.com/loongson/LoongArch-Documentation
>>
>> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   target/loongarch/cpu.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index 62c2a4d813..afa550c950 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -504,11 +504,39 @@ static void loongarch_cpu_realizefn(DeviceState *dev, 
>> Error **errp)
>>       lacc->parent_realize(dev, errp);
>>   }
>>   +#ifndef CONFIG_USER_ONLY
>> +static void loongarch_cpu_set_irq(void *opaque, int irq, int level)
>> +{
>> +    LoongArchCPU *cpu = opaque;
>> +    CPULoongArchState *env = &cpu->env;
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    if (irq < 0 || irq > N_IRQS) {
>> +        return;
>> +    }
>> +
>> +    if (level) {
>> +        env->CSR_ESTAT |= 1 << irq;
>> +    } else {
>> +        env->CSR_ESTAT &= ~(1 << irq);
>> +    }
>> +
>> +    if (FIELD_EX64(env->CSR_ESTAT, CSR_ESTAT, IS)) {
>> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> +    } else {
>> +        cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>> +    }
>> +}
>> +#endif
>> +
>>   static void loongarch_cpu_initfn(Object *obj)
>>   {
>>       LoongArchCPU *cpu = LOONGARCH_CPU(obj);
>>         cpu_set_cpustate_pointers(cpu);
>> +#ifndef CONFIG_USER_ONLY
>> +    qdev_init_gpio_in(DEVICE(cpu), loongarch_cpu_set_irq, N_IRQS);
>> +#endif
>>   }
>>     static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
> 
> Rather than use defines to split out user mode, I would suggest using a 
> separate function in a similar way to sparc64_cpu_devinit() in 
> hw/sparc64/sparc64.c to set up the parts of the CPU that are only required in 
> system mode. This function can then be called as part of the board setup.
> 
yes, put the code to the board setup stage is more suitable, thank you!
> 
> ATB,
> 
> Mark.