[RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()

Jinjie Ruan via posted 22 patches 9 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
Posted by Jinjie Ruan via 9 months ago
According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so the NMI exception trap entry
behave like IRQ. However, VNMI can be IRQ or FIQ, FIQ can only come from
hcrx_el2.HCRX_VFNMI bit, IRQ can be raised from the GIC or come from the
hcrx_el2.HCRX_VINMI bit.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v4:
- Also handle VNMI in arm_cpu_do_interrupt_aarch64().
v3:
- Remove the FIQ NMI handle.
---
 target/arm/helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b796dbdf21..bd34b3506a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11459,12 +11459,21 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
         break;
     case EXCP_IRQ:
     case EXCP_VIRQ:
+    case EXCP_NMI:
         addr += 0x80;
         break;
     case EXCP_FIQ:
     case EXCP_VFIQ:
         addr += 0x100;
         break;
+    case EXCP_VNMI:
+        if (env->irq_line_state & CPU_INTERRUPT_VNMI ||
+            env->cp15.hcrx_el2 & HCRX_VINMI) {
+            addr += 0x80;
+        } else if (env->cp15.hcrx_el2 & HCRX_VFNMI) {
+            addr += 0x100;
+        }
+        break;
     case EXCP_VSERR:
         addr += 0x180;
         /* Construct the SError syndrome from IDS and ISS fields. */
-- 
2.34.1
Re: [RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
Posted by Richard Henderson 9 months ago
On 2/29/24 03:10, Jinjie Ruan via wrote:
> According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
> with superpriority is always IRQ, never FIQ, so the NMI exception trap entry
> behave like IRQ. However, VNMI can be IRQ or FIQ, FIQ can only come from
> hcrx_el2.HCRX_VFNMI bit, IRQ can be raised from the GIC or come from the
> hcrx_el2.HCRX_VINMI bit.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v4:
> - Also handle VNMI in arm_cpu_do_interrupt_aarch64().
> v3:
> - Remove the FIQ NMI handle.
> ---
>   target/arm/helper.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b796dbdf21..bd34b3506a 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11459,12 +11459,21 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>           break;
>       case EXCP_IRQ:
>       case EXCP_VIRQ:
> +    case EXCP_NMI:
>           addr += 0x80;
>           break;
>       case EXCP_FIQ:
>       case EXCP_VFIQ:
>           addr += 0x100;
>           break;
> +    case EXCP_VNMI:
> +        if (env->irq_line_state & CPU_INTERRUPT_VNMI ||
> +            env->cp15.hcrx_el2 & HCRX_VINMI) {
> +            addr += 0x80;
> +        } else if (env->cp15.hcrx_el2 & HCRX_VFNMI) {
> +            addr += 0x100;
> +        }
> +        break;

By not combining VFNMI with CPU_INTERRUPT_VNMI, you don't need this complication.
Just

      case EXCP_IRQ:
      case EXCP_VIRQ:
+    case EXCP_NMI:


r~
Re: [RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
Posted by Jinjie Ruan via 9 months ago

On 2024/3/1 7:09, Richard Henderson wrote:
> On 2/29/24 03:10, Jinjie Ruan via wrote:
>> According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
>> with superpriority is always IRQ, never FIQ, so the NMI exception trap
>> entry
>> behave like IRQ. However, VNMI can be IRQ or FIQ, FIQ can only come from
>> hcrx_el2.HCRX_VFNMI bit, IRQ can be raised from the GIC or come from the
>> hcrx_el2.HCRX_VINMI bit.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> v4:
>> - Also handle VNMI in arm_cpu_do_interrupt_aarch64().
>> v3:
>> - Remove the FIQ NMI handle.
>> ---
>>   target/arm/helper.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index b796dbdf21..bd34b3506a 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -11459,12 +11459,21 @@ static void
>> arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>           break;
>>       case EXCP_IRQ:
>>       case EXCP_VIRQ:
>> +    case EXCP_NMI:
>>           addr += 0x80;
>>           break;
>>       case EXCP_FIQ:
>>       case EXCP_VFIQ:
>>           addr += 0x100;
>>           break;
>> +    case EXCP_VNMI:
>> +        if (env->irq_line_state & CPU_INTERRUPT_VNMI ||
>> +            env->cp15.hcrx_el2 & HCRX_VINMI) {
>> +            addr += 0x80;
>> +        } else if (env->cp15.hcrx_el2 & HCRX_VFNMI) {
>> +            addr += 0x100;
>> +        }
>> +        break;
> 
> By not combining VFNMI with CPU_INTERRUPT_VNMI, you don't need this
> complication.
> Just
> 
>      case EXCP_IRQ:
>      case EXCP_VIRQ:
> +    case EXCP_NMI:

Not understand it. both VIRQ and VFIQ will set CPU_INTERRUPT_VNMI and
cause EXCP_VNMI if they have Superpriority, the distinction jump here is
necessary.

> 
> 
> r~

Re: [RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
Posted by Richard Henderson 8 months, 4 weeks ago
On 2/29/24 17:42, Jinjie Ruan wrote:
> 
> 
> On 2024/3/1 7:09, Richard Henderson wrote:
>> On 2/29/24 03:10, Jinjie Ruan via wrote:
>>> According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
>>> with superpriority is always IRQ, never FIQ, so the NMI exception trap
>>> entry
>>> behave like IRQ. However, VNMI can be IRQ or FIQ, FIQ can only come from
>>> hcrx_el2.HCRX_VFNMI bit, IRQ can be raised from the GIC or come from the
>>> hcrx_el2.HCRX_VINMI bit.
>>>
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>> ---
>>> v4:
>>> - Also handle VNMI in arm_cpu_do_interrupt_aarch64().
>>> v3:
>>> - Remove the FIQ NMI handle.
>>> ---
>>>    target/arm/helper.c | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index b796dbdf21..bd34b3506a 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -11459,12 +11459,21 @@ static void
>>> arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>>            break;
>>>        case EXCP_IRQ:
>>>        case EXCP_VIRQ:
>>> +    case EXCP_NMI:
>>>            addr += 0x80;
>>>            break;
>>>        case EXCP_FIQ:
>>>        case EXCP_VFIQ:
>>>            addr += 0x100;
>>>            break;
>>> +    case EXCP_VNMI:
>>> +        if (env->irq_line_state & CPU_INTERRUPT_VNMI ||
>>> +            env->cp15.hcrx_el2 & HCRX_VINMI) {
>>> +            addr += 0x80;
>>> +        } else if (env->cp15.hcrx_el2 & HCRX_VFNMI) {
>>> +            addr += 0x100;
>>> +        }
>>> +        break;
>>
>> By not combining VFNMI with CPU_INTERRUPT_VNMI, you don't need this
>> complication.
>> Just
>>
>>       case EXCP_IRQ:
>>       case EXCP_VIRQ:
>> +    case EXCP_NMI:
> 
> Not understand it. both VIRQ and VFIQ will set CPU_INTERRUPT_VNMI and
> cause EXCP_VNMI if they have Superpriority, the distinction jump here is
> necessary.

In my comment against patch 5, that's exactly what I said *not* to do.


r~