[PATCH] hw/intc: Fix KVM VM start failure when AIA is configured as aplic-imsic

liu.xuemei1@zte.com.cn posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260601111637608gw-0SX._5FSUzpTzaHRaZftH@zte.com.cn
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
hw/intc/riscv_aplic.c | 52 ++++++++++++++++++++++---------------------
hw/intc/riscv_imsic.c | 16 +++++++------
2 files changed, 36 insertions(+), 32 deletions(-)
[PATCH] hw/intc: Fix KVM VM start failure when AIA is configured as aplic-imsic
Posted by liu.xuemei1@zte.com.cn 1 week ago
From: Xuemei Liu <liu.xuemei1@zte.com.cn>

Since commits 99bfcd329a ("hw/intc: riscv_aplic: Add reset API to APLIC")
and 766391483b ("hw/intc: riscv_imsic: Add reset API to IMSIC"), when KVM
acceleration is used and AIA is configured as aplic-imsic, the APLIC and
IMSIC devices are not emulated (they are handled in-kernel). In such
cases, the realize functions do not allocate memory for their internal
register state, leaving the corresponding pointers NULL.

The reset functions riscv_aplic_reset_enter() and riscv_imsic_reset_enter()
subsequently attempt to access these NULL pointers (e.g., via memset),
causing a segmentation fault and VM start failure.

Fix this by guarding the reset operations with condition checks that
skip the emulated‑specific logic when the in‑kernel mode is active:
- For APLIC, check riscv_use_emulated_aplic(aplic->msimode).
- For IMSIC, check !kvm_irqchip_in_kernel().

Fixes: 99bfcd329a ("hw/intc: riscv_aplic: Add reset API to APLIC")
Fixes: 766391483b ("hw/intc: riscv_imsic: Add reset API to IMSIC")

Signed-off-by: Xuemei Liu <liu.xuemei1@zte.com.cn>
---
 hw/intc/riscv_aplic.c | 52 ++++++++++++++++++++++---------------------
 hw/intc/riscv_imsic.c | 16 +++++++------
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index c2c67c29e6..0f61b67fc5 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -910,35 +910,37 @@ static void riscv_aplic_reset_enter(Object *obj, ResetType type)
     RISCVAPLICState *aplic = RISCV_APLIC(obj);
     int i;

-    aplic->domaincfg = 0;
-    memset(aplic->sourcecfg, 0, sizeof(uint32_t) * aplic->num_irqs);
-    memset(aplic->target, 0, sizeof(uint32_t) * aplic->num_irqs);
-    if (!aplic->msimode) {
-        for (i = 0; i < aplic->num_irqs; i++) {
-            aplic->target[i] = 1;
+    if (riscv_use_emulated_aplic(aplic->msimode)) {
+        aplic->domaincfg = 0;
+        memset(aplic->sourcecfg, 0, sizeof(uint32_t) * aplic->num_irqs);
+        memset(aplic->target, 0, sizeof(uint32_t) * aplic->num_irqs);
+        if (!aplic->msimode) {
+            for (i = 0; i < aplic->num_irqs; i++) {
+                aplic->target[i] = 1;
+            }
         }
-    }

-    for (i = 0; i < aplic->num_irqs ; i++) {
-        riscv_aplic_set_enabled_raw(aplic, i, false);
-    }
-
-    /* Need to unlock [ms]msicfgaddrh.L */
-    aplic->mmsicfgaddr = 0;
-    aplic->mmsicfgaddrH = 0;
-    aplic->smsicfgaddr = 0;
-    aplic->smsicfgaddrH = 0;
-
-    if (!aplic->msimode) {
-        /* Reset IDC registers only in non-MSI mode */
-        for (i = 0; i < aplic->num_harts; i++) {
-            aplic->idelivery[i] = 0;
-            aplic->iforce[i] = 0;
-            aplic->ithreshold[i] = 0;
+        for (i = 0; i < aplic->num_irqs ; i++) {
+            riscv_aplic_set_enabled_raw(aplic, i, false);
         }

-        for (i = 0; i < aplic->num_harts; i++) {
-            qemu_irq_lower(aplic->external_irqs[i]);
+        /* Need to unlock [ms]msicfgaddrh.L */
+        aplic->mmsicfgaddr = 0;
+        aplic->mmsicfgaddrH = 0;
+        aplic->smsicfgaddr = 0;
+        aplic->smsicfgaddrH = 0;
+
+        if (!aplic->msimode) {
+            /* Reset IDC registers only in non-MSI mode */
+            for (i = 0; i < aplic->num_harts; i++) {
+                aplic->idelivery[i] = 0;
+                aplic->iforce[i] = 0;
+                aplic->ithreshold[i] = 0;
+            }
+
+            for (i = 0; i < aplic->num_harts; i++) {
+                qemu_irq_lower(aplic->external_irqs[i]);
+            }
         }
     }
 }
diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
index ac59496c22..6dbbc152a2 100644
--- a/hw/intc/riscv_imsic.c
+++ b/hw/intc/riscv_imsic.c
@@ -347,15 +347,17 @@ static void riscv_imsic_reset_enter(Object *obj, ResetType type)
     RISCVIMSICState *imsic = RISCV_IMSIC(obj);
     int i;

-    memset(imsic->eidelivery, 0, sizeof(uint32_t) * imsic->num_pages);
-    memset(imsic->eithreshold, 0, sizeof(uint32_t) * imsic->num_pages);
+    if (!kvm_irqchip_in_kernel()) {
+        memset(imsic->eidelivery, 0, sizeof(uint32_t) * imsic->num_pages);
+        memset(imsic->eithreshold, 0, sizeof(uint32_t) * imsic->num_pages);

-    for (i = 0; i < imsic->num_eistate; i++) {
-        imsic->eistate[i] &= ~IMSIC_EISTATE_ENABLED;
-    }
+        for (i = 0; i < imsic->num_eistate; i++) {
+            imsic->eistate[i] &= ~IMSIC_EISTATE_ENABLED;
+        }

-    for (i = 0; i < imsic->num_pages; i++) {
-        qemu_irq_lower(imsic->external_irqs[i]);
+        for (i = 0; i < imsic->num_pages; i++) {
+            qemu_irq_lower(imsic->external_irqs[i]);
+        }
     }
 }

-- 
2.27.0

Re: [PATCH] hw/intc: Fix KVM VM start failure when AIA is configured as aplic-imsic
Posted by Nutty.Liu 4 days, 21 hours ago
On 6/1/2026 11:16 AM, liu.xuemei1@zte.com.cn wrote:
> From: Xuemei Liu <liu.xuemei1@zte.com.cn>
>
> Since commits 99bfcd329a ("hw/intc: riscv_aplic: Add reset API to APLIC")
> and 766391483b ("hw/intc: riscv_imsic: Add reset API to IMSIC"), when KVM
> acceleration is used and AIA is configured as aplic-imsic, the APLIC and
> IMSIC devices are not emulated (they are handled in-kernel). In such
> cases, the realize functions do not allocate memory for their internal
> register state, leaving the corresponding pointers NULL.
>
> The reset functions riscv_aplic_reset_enter() and riscv_imsic_reset_enter()
> subsequently attempt to access these NULL pointers (e.g., via memset),
> causing a segmentation fault and VM start failure.
>
> Fix this by guarding the reset operations with condition checks that
> skip the emulated‑specific logic when the in‑kernel mode is active:
> - For APLIC, check riscv_use_emulated_aplic(aplic->msimode).
> - For IMSIC, check !kvm_irqchip_in_kernel().
>
> Fixes: 99bfcd329a ("hw/intc: riscv_aplic: Add reset API to APLIC")
> Fixes: 766391483b ("hw/intc: riscv_imsic: Add reset API to IMSIC")
>
> Signed-off-by: Xuemei Liu <liu.xuemei1@zte.com.cn>
It would be better to split this patch into two fix patches.
Otherwise,
Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>

Thanks,
Nutty
> ---
>   hw/intc/riscv_aplic.c | 52 ++++++++++++++++++++++---------------------
>   hw/intc/riscv_imsic.c | 16 +++++++------
>   2 files changed, 36 insertions(+), 32 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index c2c67c29e6..0f61b67fc5 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -910,35 +910,37 @@ static void riscv_aplic_reset_enter(Object *obj, ResetType type)
>       RISCVAPLICState *aplic = RISCV_APLIC(obj);
>       int i;
>
> -    aplic->domaincfg = 0;
> -    memset(aplic->sourcecfg, 0, sizeof(uint32_t) * aplic->num_irqs);
> -    memset(aplic->target, 0, sizeof(uint32_t) * aplic->num_irqs);
> -    if (!aplic->msimode) {
> -        for (i = 0; i < aplic->num_irqs; i++) {
> -            aplic->target[i] = 1;
> +    if (riscv_use_emulated_aplic(aplic->msimode)) {
> +        aplic->domaincfg = 0;
> +        memset(aplic->sourcecfg, 0, sizeof(uint32_t) * aplic->num_irqs);
> +        memset(aplic->target, 0, sizeof(uint32_t) * aplic->num_irqs);
> +        if (!aplic->msimode) {
> +            for (i = 0; i < aplic->num_irqs; i++) {
> +                aplic->target[i] = 1;
> +            }
>           }
> -    }
>
> -    for (i = 0; i < aplic->num_irqs ; i++) {
> -        riscv_aplic_set_enabled_raw(aplic, i, false);
> -    }
> -
> -    /* Need to unlock [ms]msicfgaddrh.L */
> -    aplic->mmsicfgaddr = 0;
> -    aplic->mmsicfgaddrH = 0;
> -    aplic->smsicfgaddr = 0;
> -    aplic->smsicfgaddrH = 0;
> -
> -    if (!aplic->msimode) {
> -        /* Reset IDC registers only in non-MSI mode */
> -        for (i = 0; i < aplic->num_harts; i++) {
> -            aplic->idelivery[i] = 0;
> -            aplic->iforce[i] = 0;
> -            aplic->ithreshold[i] = 0;
> +        for (i = 0; i < aplic->num_irqs ; i++) {
> +            riscv_aplic_set_enabled_raw(aplic, i, false);
>           }
>
> -        for (i = 0; i < aplic->num_harts; i++) {
> -            qemu_irq_lower(aplic->external_irqs[i]);
> +        /* Need to unlock [ms]msicfgaddrh.L */
> +        aplic->mmsicfgaddr = 0;
> +        aplic->mmsicfgaddrH = 0;
> +        aplic->smsicfgaddr = 0;
> +        aplic->smsicfgaddrH = 0;
> +
> +        if (!aplic->msimode) {
> +            /* Reset IDC registers only in non-MSI mode */
> +            for (i = 0; i < aplic->num_harts; i++) {
> +                aplic->idelivery[i] = 0;
> +                aplic->iforce[i] = 0;
> +                aplic->ithreshold[i] = 0;
> +            }
> +
> +            for (i = 0; i < aplic->num_harts; i++) {
> +                qemu_irq_lower(aplic->external_irqs[i]);
> +            }
>           }
>       }
>   }
> diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
> index ac59496c22..6dbbc152a2 100644
> --- a/hw/intc/riscv_imsic.c
> +++ b/hw/intc/riscv_imsic.c
> @@ -347,15 +347,17 @@ static void riscv_imsic_reset_enter(Object *obj, ResetType type)
>       RISCVIMSICState *imsic = RISCV_IMSIC(obj);
>       int i;
>
> -    memset(imsic->eidelivery, 0, sizeof(uint32_t) * imsic->num_pages);
> -    memset(imsic->eithreshold, 0, sizeof(uint32_t) * imsic->num_pages);
> +    if (!kvm_irqchip_in_kernel()) {
> +        memset(imsic->eidelivery, 0, sizeof(uint32_t) * imsic->num_pages);
> +        memset(imsic->eithreshold, 0, sizeof(uint32_t) * imsic->num_pages);
>
> -    for (i = 0; i < imsic->num_eistate; i++) {
> -        imsic->eistate[i] &= ~IMSIC_EISTATE_ENABLED;
> -    }
> +        for (i = 0; i < imsic->num_eistate; i++) {
> +            imsic->eistate[i] &= ~IMSIC_EISTATE_ENABLED;
> +        }
>
> -    for (i = 0; i < imsic->num_pages; i++) {
> -        qemu_irq_lower(imsic->external_irqs[i]);
> +        for (i = 0; i < imsic->num_pages; i++) {
> +            qemu_irq_lower(imsic->external_irqs[i]);
> +        }
>       }
>   }
>