diff mbox series

[v2,3/3] hw/intc: Fix LoongArch ipi device emulation

Message ID 20220927061225.3566554-4-yangxiaojuan@loongson.cn (mailing list archive)
State New, archived
Headers show
Series Add memmap and fix bugs for LoongArch | expand

Commit Message

Xiaojuan Yang Sept. 27, 2022, 6:12 a.m. UTC
In ipi_send function, it should not to set irq before
writing data to dest cpu iocsr space, as the irq will
trigger after data writing.

Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
---
 hw/intc/loongarch_ipi.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Richard Henderson Sept. 29, 2022, 2:42 a.m. UTC | #1
On 9/26/22 23:12, Xiaojuan Yang wrote:
> In ipi_send function, it should not to set irq before
> writing data to dest cpu iocsr space, as the irq will
> trigger after data writing.
> 
> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> ---
>   hw/intc/loongarch_ipi.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
> index 4f3c58f872..aa4bf9eb74 100644
> --- a/hw/intc/loongarch_ipi.c
> +++ b/hw/intc/loongarch_ipi.c
> @@ -88,7 +88,6 @@ static void ipi_send(uint64_t val)
>       cs = qemu_get_cpu(cpuid);
>       cpu = LOONGARCH_CPU(cs);
>       env = &cpu->env;
> -    loongarch_cpu_set_irq(cpu, IRQ_IPI, 1);
>       address_space_stl(&env->address_space_iocsr, 0x1008,
>                         data, MEMTXATTRS_UNSPECIFIED, NULL);
>   

Did you mean to move the call below the set?
Otherwise, where does the irq get raised?


r~
Xiaojuan Yang Sept. 29, 2022, 3:23 a.m. UTC | #2
在 2022/9/29 上午10:42, Richard Henderson 写道:
> On 9/26/22 23:12, Xiaojuan Yang wrote:
>> In ipi_send function, it should not to set irq before
>> writing data to dest cpu iocsr space, as the irq will
>> trigger after data writing.
>>
>> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> ---
>>   hw/intc/loongarch_ipi.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
>> index 4f3c58f872..aa4bf9eb74 100644
>> --- a/hw/intc/loongarch_ipi.c
>> +++ b/hw/intc/loongarch_ipi.c
>> @@ -88,7 +88,6 @@ static void ipi_send(uint64_t val)
>>       cs = qemu_get_cpu(cpuid);
>>       cpu = LOONGARCH_CPU(cs);
>>       env = &cpu->env;
>> -    loongarch_cpu_set_irq(cpu, IRQ_IPI, 1);
>>       address_space_stl(&env->address_space_iocsr, 0x1008,
>>                         data, MEMTXATTRS_UNSPECIFIED, NULL);
>
> Did you mean to move the call below the set?
> Otherwise, where does the irq get raised?
>
When call this function 'address_space_stl(&env->address_space_iocsr, 
0x1008, ... ...)',  it will trigger  loongarch_ipi_writel(), the addr 
arg is 0x1008 ('CORE_SET_OFFSET'), and qemu_irq_raise will be called in 
this case.

Thanks.
Xiaojuan Yang
Richard Henderson Sept. 29, 2022, 4:12 a.m. UTC | #3
On 9/28/22 20:23, yangxiaojuan wrote:
> 
> 在 2022/9/29 上午10:42, Richard Henderson 写道:
>> On 9/26/22 23:12, Xiaojuan Yang wrote:
>>> In ipi_send function, it should not to set irq before
>>> writing data to dest cpu iocsr space, as the irq will
>>> trigger after data writing.
>>>
>>> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>>> ---
>>>   hw/intc/loongarch_ipi.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
>>> index 4f3c58f872..aa4bf9eb74 100644
>>> --- a/hw/intc/loongarch_ipi.c
>>> +++ b/hw/intc/loongarch_ipi.c
>>> @@ -88,7 +88,6 @@ static void ipi_send(uint64_t val)
>>>       cs = qemu_get_cpu(cpuid);
>>>       cpu = LOONGARCH_CPU(cs);
>>>       env = &cpu->env;
>>> -    loongarch_cpu_set_irq(cpu, IRQ_IPI, 1);
>>>       address_space_stl(&env->address_space_iocsr, 0x1008,
>>>                         data, MEMTXATTRS_UNSPECIFIED, NULL);
>>
>> Did you mean to move the call below the set?
>> Otherwise, where does the irq get raised?
>>
> When call this function 'address_space_stl(&env->address_space_iocsr, 0x1008, ... ...)',  
> it will trigger  loongarch_ipi_writel(), the addr arg is 0x1008 ('CORE_SET_OFFSET'), and 
> qemu_irq_raise will be called in this case.

Ah, I see now, connected to

         qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
index 4f3c58f872..aa4bf9eb74 100644
--- a/hw/intc/loongarch_ipi.c
+++ b/hw/intc/loongarch_ipi.c
@@ -88,7 +88,6 @@  static void ipi_send(uint64_t val)
     cs = qemu_get_cpu(cpuid);
     cpu = LOONGARCH_CPU(cs);
     env = &cpu->env;
-    loongarch_cpu_set_irq(cpu, IRQ_IPI, 1);
     address_space_stl(&env->address_space_iocsr, 0x1008,
                       data, MEMTXATTRS_UNSPECIFIED, NULL);