linux-kernel - Re: [RFC PATCH v2 00/10] RISC-V IOMMU HPM and nested IOMMU support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANXhq0oeUZb7161eJ2z-9aRXSDSmt8P9RpC7QSH4DZ0YvNp_7Q@mail.gmail.com>
Date: Fri, 5 Sep 2025 11:27:10 +0800
From: Zong Li <zong.li@...ive.com>
To: niliqiang <ni_liqiang@....com>
Cc: aou@...s.berkeley.edu, iommu@...ts.linux.dev, jgg@...pe.ca, 
	joro@...tes.org, kevin.tian@...el.com, linux-kernel@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, palmer@...belt.com, paul.walmsley@...ive.com, 
	robin.murphy@....com, tjeznach@...osinc.com, will@...nel.org, 
	chenruisust@...il.com
Subject: Re: [RFC PATCH v2 00/10] RISC-V IOMMU HPM and nested IOMMU support

On Tue, Sep 2, 2025 at 12:01 PM Zong Li <zong.li@...ive.com> wrote:
>
> On Mon, Sep 1, 2025 at 9:37 PM niliqiang <ni_liqiang@....com> wrote:
> >
> > Hi Zong
> >
> > Fri, 14 Jun 2024 22:21:48 +0800, Zong Li <zong.li@...ive.com> wrote:
> >
> > > This patch initialize the pmu stuff and uninitialize it when driver
> > > removing. The interrupt handling is also provided, this handler need to
> > > be primary handler instead of thread function, because pt_regs is empty
> > > when threading the IRQ, but pt_regs is necessary by perf_event_overflow.
> > >
> > > Signed-off-by: Zong Li <zong.li@...ive.com>
> > > ---
> > >  drivers/iommu/riscv/iommu.c | 65 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> > > index 8b6a64c1ad8d..1716b2251f38 100644
> > > --- a/drivers/iommu/riscv/iommu.c
> > > +++ b/drivers/iommu/riscv/iommu.c
> > > @@ -540,6 +540,62 @@ static irqreturn_t riscv_iommu_fltq_process(int irq, void *data)
> > >   return IRQ_HANDLED;
> > >  }
> > >
> > > +/*
> > > + * IOMMU Hardware performance monitor
> > > + */
> > > +
> > > +/* HPM interrupt primary handler */
> > > +static irqreturn_t riscv_iommu_hpm_irq_handler(int irq, void *dev_id)
> > > +{
> > > + struct riscv_iommu_device *iommu = (struct riscv_iommu_device *)dev_id;
> > > +
> > > + /* Process pmu irq */
> > > + riscv_iommu_pmu_handle_irq(&iommu->pmu);
> > > +
> > > + /* Clear performance monitoring interrupt pending */
> > > + riscv_iommu_writel(iommu, RISCV_IOMMU_REG_IPSR, RISCV_IOMMU_IPSR_PMIP);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > > +
> > > +/* HPM initialization */
> > > +static int riscv_iommu_hpm_enable(struct riscv_iommu_device *iommu)
> > > +{
> > > + int rc;
> > > +
> > > + if (!(iommu->caps & RISCV_IOMMU_CAPABILITIES_HPM))
> > > +     return 0;
> > > +
> > > + /*
> > > +  * pt_regs is empty when threading the IRQ, but pt_regs is necessary
> > > +  * by perf_event_overflow. Use primary handler instead of thread
> > > +  * function for PM IRQ.
> > > +  *
> > > +  * Set the IRQF_ONESHOT flag because this IRQ might be shared with
> > > +  * other threaded IRQs by other queues.
> > > +  */
> > > + rc = devm_request_irq(iommu->dev,
> > > +               iommu->irqs[riscv_iommu_queue_vec(iommu, RISCV_IOMMU_IPSR_PMIP)],
> > > +               riscv_iommu_hpm_irq_handler, IRQF_ONESHOT | IRQF_SHARED, NULL, iommu);
> > > + if (rc)
> > > +     return rc;
> > > +
> > > + return riscv_iommu_pmu_init(&iommu->pmu, iommu->reg, dev_name(iommu->dev));
> > > +}
> > > +
> >
> > What are the benefits of initializing the iommu-pmu driver in the iommu driver?
> >
> > It might be better for the RISC-V IOMMU PMU driver to be loaded as a separate module, as this would allow greater flexibility since different vendors may need to add custom events.
> >
> > Also, I'm not quite clear on how custom events should be added if the RISC-V iommu-pmu is placed within the iommu driver.
>
> Hi Liqiang,
> My original idea is that, since the IOMMU HPM is not always present,
> it depends on the capability.HPM bit, if we separate HPM into an
> individual module, I assume that the PMU driver may not have access to
> the IOMMU's complete MMIO region. I’m not sure how we would check the
> capability register in the PMU driver and avoid the following
> situation: capability.HPM is zero, but the IOMMU-PMU driver is still
> loaded because the PMU node is present in the DTS. It will be helpful
> if you have any suggestions on this.
>
> Regarding custom events, since we don’t have the driver data, my
> current rough idea is to add a vendor event map table to list the
> vendor events and use Kconfig to define them respectively. This is
> just an initial thought and may not be the good solution, so feel free
> to share any recommendations. Of course, if we eventually decide to
> move it to drivers/perf as an individual module, then we could use the
> driver data for custom events, similar to what ARM does.

Maybe let's try auxiliary driver framework to resolve this topic in
the next version.

>
> Thanks
>
> >
> >
> > Best regards,
> > Liqiang
> >