Re: [PATCH V2 1/4] RISC-V: KVM: AIA: Make HGEI number and management fully per-CPU
Guo Ren <[email protected]>
| Newsgroups | org.infradead.lists.kvm-riscv,org.infradead.lists.linux-riscv,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAJF2gTSO1T2-vfA61wKTc9OVfBJmoNE9a+UEeLgth0fhVEidkA@mail.gmail.com> |
On Sun, May 24, 2026 at 1:23 PM Anup Patel <[email protected]> wrote: > > On Sat, Apr 25, 2026 at 6:29 AM <[email protected]> wrote: > > > > From: "Guo Ren (Alibaba DAMO Academy)" <[email protected]> > > > > Previously the number of Hypervisor Guest External Interrupt (HGEI) > > lines was stored in a single global variable `kvm_riscv_aia_nr_hgei` > > and assumed to be the same for all HARTs. This assumption does not > > hold on heterogeneous RISC-V SoCs where different cores may expose > > different HGEIE CSR widths. > > > > Introduce `nr_hgei` field into the per-CPU `struct aia_hgei_control` > > and probe the actual supported HGEI count for the current HART in > > `kvm_riscv_aia_enable()` using the standard RISC-V CSR probe > > technique: > > > > csr_write(CSR_HGEIE, -1UL); > > nr = fls_long(csr_read(CSR_HGEIE)); > > if (nr) > > nr--; > > > > All HGEI allocation, free and disable paths (`kvm_riscv_aia_free_hgei()`, > > `kvm_riscv_aia_disable()`, etc.) now use the per-CPU value instead of > > the global one. > > > > The early global `kvm_riscv_aia_nr_hgei` is kept only for deciding > > whether SGEI interrupt registration is needed; the real per-HART > > initialization of lock and free_bitmap is moved to enable time. > > We should keep the `kvm_riscv_aia_nr_hgei` to imply two things: > 1) Minimum number of HGEI lines available across all HARTs > 2) Non-zero kvm_riscv_aia_nr_hgei means HGEI is enabled > > The PATCH2 is redundant and must be droped. I will take care > care of this at the time of merging. Okay. > > > > > This makes KVM AIA robust on big.LITTLE-style and multi-vendor > > asymmetric platforms. > > > > Signed-off-by: Guo Ren (Alibaba DAMO Academy) <[email protected]> > > --- > > arch/riscv/kvm/aia.c | 40 ++++++++++++++++++++++++---------------- > > 1 file changed, 24 insertions(+), 16 deletions(-) > > > > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c > > index 5ec503288555..a23729052cfb 100644 > > --- a/arch/riscv/kvm/aia.c > > +++ b/arch/riscv/kvm/aia.c > > @@ -23,6 +23,7 @@ struct aia_hgei_control { > > raw_spinlock_t lock; > > unsigned long free_bitmap; > > struct kvm_vcpu *owners[BITS_PER_LONG]; > > + unsigned int nr_hgei; > > }; > > static DEFINE_PER_CPU(struct aia_hgei_control, aia_hgei); > > static int hgei_parent_irq; > > @@ -452,7 +453,7 @@ void kvm_riscv_aia_free_hgei(int cpu, int hgei) > > > > raw_spin_lock_irqsave(&hgctrl->lock, flags); > > > > - if (hgei > 0 && hgei <= kvm_riscv_aia_nr_hgei) { > > + if (hgei > 0 && hgei <= hgctrl->nr_hgei) { > > if (!(hgctrl->free_bitmap & BIT(hgei))) { > > hgctrl->free_bitmap |= BIT(hgei); > > hgctrl->owners[hgei] = NULL; > > @@ -486,21 +487,8 @@ static irqreturn_t hgei_interrupt(int irq, void *dev_id) > > > > static int aia_hgei_init(void) > > { > > - int cpu, rc; > > + int rc; > > struct irq_domain *domain; > > - struct aia_hgei_control *hgctrl; > > - > > - /* Initialize per-CPU guest external interrupt line management */ > > - for_each_possible_cpu(cpu) { > > - hgctrl = per_cpu_ptr(&aia_hgei, cpu); > > - raw_spin_lock_init(&hgctrl->lock); > > - if (kvm_riscv_aia_nr_hgei) { > > - hgctrl->free_bitmap = > > - BIT(kvm_riscv_aia_nr_hgei + 1) - 1; > > - hgctrl->free_bitmap &= ~BIT(0); > > - } else > > - hgctrl->free_bitmap = 0; > > - } > > > > /* Skip SGEI interrupt setup for zero guest external interrupts */ > > if (!kvm_riscv_aia_nr_hgei) > > @@ -545,9 +533,29 @@ static void aia_hgei_exit(void) > > > > void kvm_riscv_aia_enable(void) > > { > > + struct aia_hgei_control *hgctrl; > > + > > if (!kvm_riscv_aia_available()) > > return; > > > > + hgctrl = this_cpu_ptr(&aia_hgei); > > + > > + /* Figure-out number of bits in HGEIE */ > > + csr_write(CSR_HGEIE, -1UL); > > + hgctrl->nr_hgei = fls_long(csr_read(CSR_HGEIE)); > > + csr_write(CSR_HGEIE, 0); > > + if (hgctrl->nr_hgei) > > + hgctrl->nr_hgei--; > > For completness of this patch, we should also have the following: > > if (gc) > hgctrl->nr_hgei= min((ulong)hgctrl->nr_hgei, gc->nr_guest_files); Thanks for the review! On keeping kvm_riscv_aia_nr_hgei as the minimum across all HARTs: I agree to keep kvm_riscv_aia_nr_hgei as a non-zero indicator for HGEI enablement. However, I have a concern about making it track the per-HART minimum. The purpose of this patch is precisely to let each HART use its own nr_hgei independently, so heterogeneous HARTs with more HGEI lines are not artificially limited by weaker ones. Collapsing that back into a global minimum would partially defeat the goal. ref: global->nr_guest_files = min(global->nr_guest_files, local->nr_guest_files); > else > hgctrl->nr_hgei = 0; > > Also, over here we should check and update kvm_riscv_aia_nr_hgei > to keep it minimum accross HARTs: > > if (hgctrl->nr_hgei && hgctrl->nr_hgei < kvm_riscv_aia_nr_hgei) > kvm_riscv_aia_nr_hgei = hgctrl->nr_hgei; > > > + > > + if (hgctrl->nr_hgei) { > > + hgctrl->free_bitmap = BIT(hgctrl->nr_hgei + 1) - 1; > > + hgctrl->free_bitmap &= ~BIT(0); > > + } else { > > + hgctrl->free_bitmap = 0; > > + } > > + > > + raw_spin_lock_init(&hgctrl->lock); > > + > > csr_write(CSR_HVICTL, aia_hvictl_value(false)); > > csr_write(CSR_HVIPRIO1, 0x0); > > csr_write(CSR_HVIPRIO2, 0x0); > > @@ -588,7 +596,7 @@ void kvm_riscv_aia_disable(void) > > > > raw_spin_lock_irqsave(&hgctrl->lock, flags); > > > > - for (i = 0; i <= kvm_riscv_aia_nr_hgei; i++) { > > + for (i = 0; i <= hgctrl->nr_hgei; i++) { > > vcpu = hgctrl->owners[i]; > > if (!vcpu) > > continue; > > -- > > 2.43.0 > > > > Regards, > Anup -- Best Regards Guo Ren -- kvm-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kvm-riscv