[v3] irqchip/riscv-imsic: Adjust the number of available guest irq files

[PATCH v3] irqchip/riscv-imsic: Adjust the number of available guest irq files

Posted by Xu Lu 1 month, 2 weeks ago

During initialization, kernel maps the MMIO resources of IMSIC, which is
parsed from ACPI or DTS and may not strictly contains all guest
interrupt files. Page fault happens when KVM wrongly allocates an
unmapped guest interrupt file and writes it.

Thus, during initialization, we calculate the number of available guest
interrupt files according to MMIO resources and constrain the number of
guest interrupt files that can be allocated by KVM.

Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
---
 arch/riscv/kvm/aia.c                    | 2 +-
 drivers/irqchip/irq-riscv-imsic-state.c | 7 ++++++-
 include/linux/irqchip/riscv-imsic.h     | 3 +++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index dad3181856600..cac3c2b51d724 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -630,7 +630,7 @@ int kvm_riscv_aia_init(void)
 	 */
 	if (gc)
 		kvm_riscv_aia_nr_hgei = min((ulong)kvm_riscv_aia_nr_hgei,
-					    BIT(gc->guest_index_bits) - 1);
+					    gc->nr_guest_files);
 	else
 		kvm_riscv_aia_nr_hgei = 0;
 
diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
index dc95ad856d80a..1e982ce024a47 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.c
+++ b/drivers/irqchip/irq-riscv-imsic-state.c
@@ -794,7 +794,7 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
 
 int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
 {
-	u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
+	u32 i, j, index, nr_parent_irqs, nr_mmios, nr_guest_files, nr_handlers = 0;
 	struct imsic_global_config *global;
 	struct imsic_local_config *local;
 	void __iomem **mmios_va = NULL;
@@ -888,6 +888,7 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
 	}
 
 	/* Configure handlers for target CPUs */
+	global->nr_guest_files = BIT(global->guest_index_bits) - 1;
 	for (i = 0; i < nr_parent_irqs; i++) {
 		rc = imsic_get_parent_hartid(fwnode, i, &hartid);
 		if (rc) {
@@ -928,6 +929,10 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
 		local->msi_pa = mmios[index].start + reloff;
 		local->msi_va = mmios_va[index] + reloff;
 
+		nr_guest_files = (resource_size(&mmios[index]) - reloff) / IMSIC_MMIO_PAGE_SZ - 1;
+		global->nr_guest_files = global->nr_guest_files > nr_guest_files ? nr_guest_files :
+					 global->nr_guest_files;
+
 		nr_handlers++;
 	}
 
diff --git a/include/linux/irqchip/riscv-imsic.h b/include/linux/irqchip/riscv-imsic.h
index 7494952c55187..43aed52385008 100644
--- a/include/linux/irqchip/riscv-imsic.h
+++ b/include/linux/irqchip/riscv-imsic.h
@@ -69,6 +69,9 @@ struct imsic_global_config {
 	/* Number of guest interrupt identities */
 	u32					nr_guest_ids;
 
+	/* Number of guest interrupt files per core */
+	u32					nr_guest_files;
+
 	/* Per-CPU IMSIC addresses */
 	struct imsic_local_config __percpu	*local;
 };
-- 
2.20.1

Re: [PATCH v3] irqchip/riscv-imsic: Adjust the number of available guest irq files

Posted by Anup Patel 1 month, 2 weeks ago

On Sat, Dec 20, 2025 at 10:07 PM Xu Lu <luxu.kernel@bytedance.com> wrote:
>
> During initialization, kernel maps the MMIO resources of IMSIC, which is
> parsed from ACPI or DTS and may not strictly contains all guest
> interrupt files. Page fault happens when KVM wrongly allocates an
> unmapped guest interrupt file and writes it.

The motivation is not clear from the above text and needs to improve.

How about the following ?

Currently, KVM assumes the minimum of implemented HGEIE bits and
"BIT(gc->guest_index_bits) - 1" as the number of guest files available
across all CPUs. This will not work only when CPUs have different number
of guest files because KVM may incorrectly allocate a guest file on a CPU
with fewer guest files.

>
> Thus, during initialization, we calculate the number of available guest

s/Thus, during initialization/To address the above/

> interrupt files according to MMIO resources and constrain the number of
> guest interrupt files that can be allocated by KVM.
>
> Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> ---
>  arch/riscv/kvm/aia.c                    | 2 +-
>  drivers/irqchip/irq-riscv-imsic-state.c | 7 ++++++-
>  include/linux/irqchip/riscv-imsic.h     | 3 +++
>  3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> index dad3181856600..cac3c2b51d724 100644
> --- a/arch/riscv/kvm/aia.c
> +++ b/arch/riscv/kvm/aia.c
> @@ -630,7 +630,7 @@ int kvm_riscv_aia_init(void)
>          */
>         if (gc)
>                 kvm_riscv_aia_nr_hgei = min((ulong)kvm_riscv_aia_nr_hgei,
> -                                           BIT(gc->guest_index_bits) - 1);
> +                                           gc->nr_guest_files);
>         else
>                 kvm_riscv_aia_nr_hgei = 0;
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index dc95ad856d80a..1e982ce024a47 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -794,7 +794,7 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
>
>  int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
>  {
> -       u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
> +       u32 i, j, index, nr_parent_irqs, nr_mmios, nr_guest_files, nr_handlers = 0;
>         struct imsic_global_config *global;
>         struct imsic_local_config *local;
>         void __iomem **mmios_va = NULL;
> @@ -888,6 +888,7 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
>         }
>
>         /* Configure handlers for target CPUs */
> +       global->nr_guest_files = BIT(global->guest_index_bits) - 1;
>         for (i = 0; i < nr_parent_irqs; i++) {
>                 rc = imsic_get_parent_hartid(fwnode, i, &hartid);
>                 if (rc) {
> @@ -928,6 +929,10 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
>                 local->msi_pa = mmios[index].start + reloff;
>                 local->msi_va = mmios_va[index] + reloff;
>

Need comments explaining why we are taking the
minimum number of guest files across CPUs.

> +               nr_guest_files = (resource_size(&mmios[index]) - reloff) / IMSIC_MMIO_PAGE_SZ - 1;
> +               global->nr_guest_files = global->nr_guest_files > nr_guest_files ? nr_guest_files :
> +                                        global->nr_guest_files;

global->nr_guest_files = min(nr_guest_files, global->nr_guest_files);

> +
>                 nr_handlers++;
>         }
>
> diff --git a/include/linux/irqchip/riscv-imsic.h b/include/linux/irqchip/riscv-imsic.h
> index 7494952c55187..43aed52385008 100644
> --- a/include/linux/irqchip/riscv-imsic.h
> +++ b/include/linux/irqchip/riscv-imsic.h
> @@ -69,6 +69,9 @@ struct imsic_global_config {
>         /* Number of guest interrupt identities */
>         u32                                     nr_guest_ids;
>
> +       /* Number of guest interrupt files per core */
> +       u32                                     nr_guest_files;
> +
>         /* Per-CPU IMSIC addresses */
>         struct imsic_local_config __percpu      *local;
>  };
> --
> 2.20.1
>

Regards,
Anup

Re: [External] Re: [PATCH v3] irqchip/riscv-imsic: Adjust the number of available guest irq files

Posted by Xu Lu 1 month, 2 weeks ago

Roger that!

On Mon, Dec 22, 2025 at 1:04 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sat, Dec 20, 2025 at 10:07 PM Xu Lu <luxu.kernel@bytedance.com> wrote:
> >
> > During initialization, kernel maps the MMIO resources of IMSIC, which is
> > parsed from ACPI or DTS and may not strictly contains all guest
> > interrupt files. Page fault happens when KVM wrongly allocates an
> > unmapped guest interrupt file and writes it.
>
> The motivation is not clear from the above text and needs to improve.
>
> How about the following ?
>
> Currently, KVM assumes the minimum of implemented HGEIE bits and
> "BIT(gc->guest_index_bits) - 1" as the number of guest files available
> across all CPUs. This will not work only when CPUs have different number
> of guest files because KVM may incorrectly allocate a guest file on a CPU
> with fewer guest files.
>
> >
> > Thus, during initialization, we calculate the number of available guest
>
> s/Thus, during initialization/To address the above/
>
> > interrupt files according to MMIO resources and constrain the number of
> > guest interrupt files that can be allocated by KVM.
> >
> > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> > ---
> >  arch/riscv/kvm/aia.c                    | 2 +-
> >  drivers/irqchip/irq-riscv-imsic-state.c | 7 ++++++-
> >  include/linux/irqchip/riscv-imsic.h     | 3 +++
> >  3 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> > index dad3181856600..cac3c2b51d724 100644
> > --- a/arch/riscv/kvm/aia.c
> > +++ b/arch/riscv/kvm/aia.c
> > @@ -630,7 +630,7 @@ int kvm_riscv_aia_init(void)
> >          */
> >         if (gc)
> >                 kvm_riscv_aia_nr_hgei = min((ulong)kvm_riscv_aia_nr_hgei,
> > -                                           BIT(gc->guest_index_bits) - 1);
> > +                                           gc->nr_guest_files);
> >         else
> >                 kvm_riscv_aia_nr_hgei = 0;
> >
> > diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> > index dc95ad856d80a..1e982ce024a47 100644
> > --- a/drivers/irqchip/irq-riscv-imsic-state.c
> > +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> > @@ -794,7 +794,7 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
> >
> >  int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
> >  {
> > -       u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
> > +       u32 i, j, index, nr_parent_irqs, nr_mmios, nr_guest_files, nr_handlers = 0;
> >         struct imsic_global_config *global;
> >         struct imsic_local_config *local;
> >         void __iomem **mmios_va = NULL;
> > @@ -888,6 +888,7 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
> >         }
> >
> >         /* Configure handlers for target CPUs */
> > +       global->nr_guest_files = BIT(global->guest_index_bits) - 1;
> >         for (i = 0; i < nr_parent_irqs; i++) {
> >                 rc = imsic_get_parent_hartid(fwnode, i, &hartid);
> >                 if (rc) {
> > @@ -928,6 +929,10 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
> >                 local->msi_pa = mmios[index].start + reloff;
> >                 local->msi_va = mmios_va[index] + reloff;
> >
>
> Need comments explaining why we are taking the
> minimum number of guest files across CPUs.
>
> > +               nr_guest_files = (resource_size(&mmios[index]) - reloff) / IMSIC_MMIO_PAGE_SZ - 1;
> > +               global->nr_guest_files = global->nr_guest_files > nr_guest_files ? nr_guest_files :
> > +                                        global->nr_guest_files;
>
> global->nr_guest_files = min(nr_guest_files, global->nr_guest_files);
>
> > +
> >                 nr_handlers++;
> >         }
> >
> > diff --git a/include/linux/irqchip/riscv-imsic.h b/include/linux/irqchip/riscv-imsic.h
> > index 7494952c55187..43aed52385008 100644
> > --- a/include/linux/irqchip/riscv-imsic.h
> > +++ b/include/linux/irqchip/riscv-imsic.h
> > @@ -69,6 +69,9 @@ struct imsic_global_config {
> >         /* Number of guest interrupt identities */
> >         u32                                     nr_guest_ids;
> >
> > +       /* Number of guest interrupt files per core */
> > +       u32                                     nr_guest_files;
> > +
> >         /* Per-CPU IMSIC addresses */
> >         struct imsic_local_config __percpu      *local;
> >  };
> > --
> > 2.20.1
> >
>
> Regards,
> Anup