[PATCH v3 1/2] drivers/perf: riscv-iommu: add risc-v iommu pmu driver
Zong Li
zong.li at sifive.com
Mon Jul 6 20:49:20 PDT 2026
On Tue, Jul 7, 2026 at 10:58 AM Guo Ren <guoren at kernel.org> wrote:
>
> On Tue, Jul 7, 2026 at 10:01 AM Zong Li <zong.li at sifive.com> wrote:
> >
> > On Fri, Jul 3, 2026 at 9:36 AM Zong Li <zong.li at sifive.com> wrote:
> > >
> > > On Tue, Jun 30, 2026 at 9:17 PM Guo Ren <guoren at kernel.org> wrote:
> > > >
> > > > Hi Zong Li,
> > > >
> > > > On Tue, Jun 30, 2026 at 4:38 PM Zong Li <zong.li at sifive.com> wrote:
> > > > >
> > > > > Add a new driver to support the RISC-V IOMMU PMU. This is an auxiliary
> > > > > device driver created by the parent RISC-V IOMMU driver.
> > > > >
> > > > > The RISC-V IOMMU PMU separates the cycle counter from the event counters.
> > > > > The cycle counter is not associated with iohpmevt0, so a software-defined
> > > > > cycle event is required for the perf subsystem.
> > > > >
> > > > > The number and width of the counters are hardware-implemented and must
> > > > > be detected at runtime.
> > > > >
> > > > > The performance monitor provides counters with filtering support to
> > > > > collect events for specific device ID/process ID, or GSCID/PSCID.
> > > > >
> > > > > PMU-related definitions are moved into the perf driver, where they are
> > > > > used exclusively.
> > > > >
> > > > > Suggested-by: David Laight <david.laight.linux at gmail.com>
> > > > > Suggested-by: Guo Ren <guoren at kernel.org>
> > > > > Link: https://lore.kernel.org/linux-riscv/20260618143634.7f3dd6c5@pumpkin/
> > > > > Signed-off-by: Zong Li <zong.li at sifive.com>
> > > > > ---
> > > > > drivers/iommu/riscv/iommu-bits.h | 61 ---
> > > > > drivers/perf/Kconfig | 12 +
> > > > > drivers/perf/Makefile | 1 +
> > > > > drivers/perf/riscv_iommu_pmu.c | 703 +++++++++++++++++++++++++++++++
> > > > > 4 files changed, 716 insertions(+), 61 deletions(-)
> > > > > create mode 100644 drivers/perf/riscv_iommu_pmu.c
> > > > >
> > > > > diff --git a/drivers/iommu/riscv/iommu-bits.h b/drivers/iommu/riscv/iommu-bits.h
> > > > > index f2ef9bd3cde9..6b5de913a032 100644
> > > > > --- a/drivers/iommu/riscv/iommu-bits.h
> > > > > +++ b/drivers/iommu/riscv/iommu-bits.h
> > > > > @@ -192,67 +192,6 @@ enum riscv_iommu_ddtp_modes {
> > > > > #define RISCV_IOMMU_IPSR_PMIP BIT(RISCV_IOMMU_INTR_PM)
> > > > > #define RISCV_IOMMU_IPSR_PIP BIT(RISCV_IOMMU_INTR_PQ)
> > > > >
> > > > > -/* 5.19 Performance monitoring counter overflow status (32bits) */
> > > > > -#define RISCV_IOMMU_REG_IOCOUNTOVF 0x0058
> > > > > -#define RISCV_IOMMU_IOCOUNTOVF_CY BIT(0)
> > > > > -#define RISCV_IOMMU_IOCOUNTOVF_HPM GENMASK_ULL(31, 1)
> > > > > -
> > > > > -/* 5.20 Performance monitoring counter inhibits (32bits) */
> > > > > -#define RISCV_IOMMU_REG_IOCOUNTINH 0x005C
> > > > > -#define RISCV_IOMMU_IOCOUNTINH_CY BIT(0)
> > > > > -#define RISCV_IOMMU_IOCOUNTINH_HPM GENMASK(31, 1)
> > > > > -
> > > > > -/* 5.21 Performance monitoring cycles counter (64bits) */
> > > > > -#define RISCV_IOMMU_REG_IOHPMCYCLES 0x0060
> > > > > -#define RISCV_IOMMU_IOHPMCYCLES_COUNTER GENMASK_ULL(62, 0)
> > > > > -#define RISCV_IOMMU_IOHPMCYCLES_OF BIT_ULL(63)
> > > > > -
> > > > > -/* 5.22 Performance monitoring event counters (31 * 64bits) */
> > > > > -#define RISCV_IOMMU_REG_IOHPMCTR_BASE 0x0068
> > > > > -#define RISCV_IOMMU_REG_IOHPMCTR(_n) (RISCV_IOMMU_REG_IOHPMCTR_BASE + ((_n) * 0x8))
> > > > > -
> > > > > -/* 5.23 Performance monitoring event selectors (31 * 64bits) */
> > > > > -#define RISCV_IOMMU_REG_IOHPMEVT_BASE 0x0160
> > > > > -#define RISCV_IOMMU_REG_IOHPMEVT(_n) (RISCV_IOMMU_REG_IOHPMEVT_BASE + ((_n) * 0x8))
> > > > > -#define RISCV_IOMMU_IOHPMEVT_EVENTID GENMASK_ULL(14, 0)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_DMASK BIT_ULL(15)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_PID_PSCID GENMASK_ULL(35, 16)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_DID_GSCID GENMASK_ULL(59, 36)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_PV_PSCV BIT_ULL(60)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_DV_GSCV BIT_ULL(61)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_IDT BIT_ULL(62)
> > > > > -#define RISCV_IOMMU_IOHPMEVT_OF BIT_ULL(63)
> > > > > -
> > > > > -/* Number of defined performance-monitoring event selectors */
> > > > > -#define RISCV_IOMMU_IOHPMEVT_CNT 31
> > > > > -
> > > > > -/**
> > > > > - * enum riscv_iommu_hpmevent_id - Performance-monitoring event identifier
> > > > > - *
> > > > > - * @RISCV_IOMMU_HPMEVENT_INVALID: Invalid event, do not count
> > > > > - * @RISCV_IOMMU_HPMEVENT_URQ: Untranslated requests
> > > > > - * @RISCV_IOMMU_HPMEVENT_TRQ: Translated requests
> > > > > - * @RISCV_IOMMU_HPMEVENT_ATS_RQ: ATS translation requests
> > > > > - * @RISCV_IOMMU_HPMEVENT_TLB_MISS: TLB misses
> > > > > - * @RISCV_IOMMU_HPMEVENT_DD_WALK: Device directory walks
> > > > > - * @RISCV_IOMMU_HPMEVENT_PD_WALK: Process directory walks
> > > > > - * @RISCV_IOMMU_HPMEVENT_S_VS_WALKS: First-stage page table walks
> > > > > - * @RISCV_IOMMU_HPMEVENT_G_WALKS: Second-stage page table walks
> > > > > - * @RISCV_IOMMU_HPMEVENT_MAX: Value to denote maximum Event IDs
> > > > > - */
> > > > > -enum riscv_iommu_hpmevent_id {
> > > > > - RISCV_IOMMU_HPMEVENT_INVALID = 0,
> > > > > - RISCV_IOMMU_HPMEVENT_URQ = 1,
> > > > > - RISCV_IOMMU_HPMEVENT_TRQ = 2,
> > > > > - RISCV_IOMMU_HPMEVENT_ATS_RQ = 3,
> > > > > - RISCV_IOMMU_HPMEVENT_TLB_MISS = 4,
> > > > > - RISCV_IOMMU_HPMEVENT_DD_WALK = 5,
> > > > > - RISCV_IOMMU_HPMEVENT_PD_WALK = 6,
> > > > > - RISCV_IOMMU_HPMEVENT_S_VS_WALKS = 7,
> > > > > - RISCV_IOMMU_HPMEVENT_G_WALKS = 8,
> > > > > - RISCV_IOMMU_HPMEVENT_MAX = 9
> > > > > -};
> > > > > -
> > > > > /* 5.24 Translation request IOVA (64bits) */
> > > > > #define RISCV_IOMMU_REG_TR_REQ_IOVA 0x0258
> > > > > #define RISCV_IOMMU_TR_REQ_IOVA_VPN GENMASK_ULL(63, 12)
> > > > > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > > > > index 245e7bb763b9..8cce6c2ea626 100644
> > > > > --- a/drivers/perf/Kconfig
> > > > > +++ b/drivers/perf/Kconfig
> > > > > @@ -105,6 +105,18 @@ config RISCV_PMU_SBI
> > > > > full perf feature support i.e. counter overflow, privilege mode
> > > > > filtering, counter configuration.
> > > > >
> > > > > +config RISCV_IOMMU_PMU
> > > > > + depends on RISCV || COMPILE_TEST
> > > > > + depends on RISCV_IOMMU
> > > > > + bool "RISC-V IOMMU Hardware Performance Monitor"
> > > > > + default y
> > > > > + help
> > > > > + Say Y if you want to use the RISC-V IOMMU performance monitor
> > > > > + implementation. The performance monitor is an optional hardware
> > > > > + feature, and whether it is actually enabled depends on IOMMU
> > > > > + hardware support. If the underlying hardware does not implement
> > > > > + the PMU, this option will have no effect.
> > > > > +
> > > > > config STARFIVE_STARLINK_PMU
> > > > > depends on ARCH_STARFIVE || COMPILE_TEST
> > > > > depends on 64BIT
> > > > > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > > > > index eb8a022dad9a..90c75f3c0ac1 100644
> > > > > --- a/drivers/perf/Makefile
> > > > > +++ b/drivers/perf/Makefile
> > > > > @@ -20,6 +20,7 @@ obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> > > > > obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> > > > > obj-$(CONFIG_RISCV_PMU_LEGACY) += riscv_pmu_legacy.o
> > > > > obj-$(CONFIG_RISCV_PMU_SBI) += riscv_pmu_sbi.o
> > > > > +obj-$(CONFIG_RISCV_IOMMU_PMU) += riscv_iommu_pmu.o
> > > > > obj-$(CONFIG_STARFIVE_STARLINK_PMU) += starfive_starlink_pmu.o
> > > > > obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
> > > > > obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > > > > diff --git a/drivers/perf/riscv_iommu_pmu.c b/drivers/perf/riscv_iommu_pmu.c
> > > > > new file mode 100644
> > > > > index 000000000000..2e856289ddc9
> > > > > --- /dev/null
> > > > > +++ b/drivers/perf/riscv_iommu_pmu.c
> > > > > @@ -0,0 +1,703 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright (C) 2026 SiFive
> > > > > + *
> > > > > + * Authors
> > > > > + * Zong Li <zong.li at sifive.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/auxiliary_bus.h>
> > > > > +#include <linux/io-64-nonatomic-hi-lo.h>
> > > > > +#include <linux/perf_event.h>
> > > > > +
> > > > > +#include "../iommu/riscv/iommu.h"
> > > > > +
> > > > > +/* 5.19 Performance monitoring counter overflow status (32bits) */
> > > > > +#define RISCV_IOMMU_REG_IOCOUNTOVF 0x0058
> > > > > +#define RISCV_IOMMU_IOCOUNTOVF_CY BIT(0)
> > > > > +#define RISCV_IOMMU_IOCOUNTOVF_HPM GENMASK_ULL(31, 1)
> > > > > +
> > > > > +/* 5.20 Performance monitoring counter inhibits (32bits) */
> > > > > +#define RISCV_IOMMU_REG_IOCOUNTINH 0x005C
> > > > > +#define RISCV_IOMMU_IOCOUNTINH_CY BIT(0)
> > > > > +#define RISCV_IOMMU_IOCOUNTINH_HPM GENMASK(31, 0)
> > > > > +
> > > > > +/* 5.21 Performance monitoring cycles counter (64bits) */
> > > > > +#define RISCV_IOMMU_REG_IOHPMCYCLES 0x0060
> > > > > +#define RISCV_IOMMU_IOHPMCYCLES_COUNTER GENMASK_ULL(62, 0)
> > > > > +#define RISCV_IOMMU_IOHPMCYCLES_OF BIT_ULL(63)
> > > > > +#define RISCV_IOMMU_REG_IOHPMCTR(_n) (RISCV_IOMMU_REG_IOHPMCYCLES + ((_n) * 0x8))
> > > > > +
> > > > > +/* 5.22 Performance monitoring event counters (31 * 64bits) */
> > > > > +#define RISCV_IOMMU_REG_IOHPMCTR_BASE 0x0068
> > > > > +#define RISCV_IOMMU_IOHPMCTR_COUNTER GENMASK_ULL(63, 0)
> > > > > +
> > > > > +/* 5.23 Performance monitoring event selectors (31 * 64bits) */
> > > > > +#define RISCV_IOMMU_REG_IOHPMEVT_BASE 0x0160
> > > > > +#define RISCV_IOMMU_REG_IOHPMEVT(_n) (RISCV_IOMMU_REG_IOHPMEVT_BASE + ((_n) * 0x8))
> > > > > +#define RISCV_IOMMU_IOHPMEVT_EVENTID GENMASK_ULL(14, 0)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_DMASK BIT_ULL(15)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_PID_PSCID GENMASK_ULL(35, 16)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_DID_GSCID GENMASK_ULL(59, 36)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_PV_PSCV BIT_ULL(60)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_DV_GSCV BIT_ULL(61)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_IDT BIT_ULL(62)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_OF BIT_ULL(63)
> > > > > +#define RISCV_IOMMU_IOHPMEVT_EVENT GENMASK_ULL(62, 0)
> > > > > +
> > > > > +/* The total number of counters is 31 event counters plus 1 cycle counter */
> > > > > +#define RISCV_IOMMU_HPM_COUNTER_NUM 32
> > > > > +
> > > > > +static int cpuhp_state;
> > > > > +
> > > > > +/**
> > > > > + * enum riscv_iommu_hpmevent_id - Performance-monitoring event identifier
> > > > > + *
> > > > > + * @RISCV_IOMMU_HPMEVENT_CYCLE: Clock cycle counter
> > > > > + * @RISCV_IOMMU_HPMEVENT_URQ: Untranslated requests
> > > > > + * @RISCV_IOMMU_HPMEVENT_TRQ: Translated requests
> > > > > + * @RISCV_IOMMU_HPMEVENT_ATS_RQ: ATS translation requests
> > > > > + * @RISCV_IOMMU_HPMEVENT_TLB_MISS: TLB misses
> > > > > + * @RISCV_IOMMU_HPMEVENT_DD_WALK: Device directory walks
> > > > > + * @RISCV_IOMMU_HPMEVENT_PD_WALK: Process directory walks
> > > > > + * @RISCV_IOMMU_HPMEVENT_S_VS_WALKS: First-stage page table walks
> > > > > + * @RISCV_IOMMU_HPMEVENT_G_WALKS: Second-stage page table walks
> > > > > + * @RISCV_IOMMU_HPMEVENT_MAX: Value to denote maximum Event IDs
> > > > > + *
> > > > > + * The specification does not define an event ID for counting the
> > > > > + * number of clock cycles, meaning there is no associated 'iohpmevt0'.
> > > > > + * Event ID 0 is an invalid event and does not overlap with any valid
> > > > > + * event ID. Let's repurpose ID 0 as the cycle for perf, the cycle
> > > > > + * event is not actually written into any register, it serves solely
> > > > > + * as an identifier.
> > > > > + */
> > > > > +enum riscv_iommu_hpmevent_id {
> > > > > + RISCV_IOMMU_HPMEVENT_CYCLE = 0,
> > > > > + RISCV_IOMMU_HPMEVENT_URQ = 1,
> > > > > + RISCV_IOMMU_HPMEVENT_TRQ = 2,
> > > > > + RISCV_IOMMU_HPMEVENT_ATS_RQ = 3,
> > > > > + RISCV_IOMMU_HPMEVENT_TLB_MISS = 4,
> > > > > + RISCV_IOMMU_HPMEVENT_DD_WALK = 5,
> > > > > + RISCV_IOMMU_HPMEVENT_PD_WALK = 6,
> > > > > + RISCV_IOMMU_HPMEVENT_S_VS_WALKS = 7,
> > > > > + RISCV_IOMMU_HPMEVENT_G_WALKS = 8,
> > > > > + RISCV_IOMMU_HPMEVENT_MAX = 9
> > > > > +};
> > > > > +
> > > > > +struct riscv_iommu_pmu {
> > > > > + struct pmu pmu;
> > > > > + struct hlist_node node;
> > > > > + void __iomem *reg;
> > > > > + unsigned int on_cpu;
> > > > > + int num_counters;
> > > > > + u64 cycle_cntr_mask;
> > > > > + u64 event_cntr_mask;
> > > > > + struct perf_event *events[RISCV_IOMMU_HPM_COUNTER_NUM];
> > > > > + DECLARE_BITMAP(used_counters, RISCV_IOMMU_HPM_COUNTER_NUM);
> > > > > + u32 hi_prev[RISCV_IOMMU_HPM_COUNTER_NUM];
> > > > > + u32 lo_prev[RISCV_IOMMU_HPM_COUNTER_NUM];
> > > > > +};
> > > > > +
> > > > > +#define to_riscv_iommu_pmu(p) (container_of(p, struct riscv_iommu_pmu, pmu))
> > > > > +
> > > > > +#define RISCV_IOMMU_PMU_ATTR_EXTRACTOR(_name, _mask) \
> > > > > + static inline u32 get_##_name(struct perf_event *event) \
> > > > > + { \
> > > > > + return FIELD_GET(_mask, event->attr.config); \
> > > > > + } \
> > > > > +
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(event, RISCV_IOMMU_IOHPMEVT_EVENTID);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(partial_matching, RISCV_IOMMU_IOHPMEVT_DMASK);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(pid_pscid, RISCV_IOMMU_IOHPMEVT_PID_PSCID);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(did_gscid, RISCV_IOMMU_IOHPMEVT_DID_GSCID);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(filter_pid_pscid, RISCV_IOMMU_IOHPMEVT_PV_PSCV);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(filter_did_gscid, RISCV_IOMMU_IOHPMEVT_DV_GSCV);
> > > > > +RISCV_IOMMU_PMU_ATTR_EXTRACTOR(filter_id_type, RISCV_IOMMU_IOHPMEVT_IDT);
> > > > > +
> > > > > +/* Formats */
> > > > > +PMU_FORMAT_ATTR(event, "config:0-14");
> > > > > +PMU_FORMAT_ATTR(partial_matching, "config:15");
> > > > > +PMU_FORMAT_ATTR(pid_pscid, "config:16-35");
> > > > > +PMU_FORMAT_ATTR(did_gscid, "config:36-59");
> > > > > +PMU_FORMAT_ATTR(filter_pid_pscid, "config:60");
> > > > > +PMU_FORMAT_ATTR(filter_did_gscid, "config:61");
> > > > > +PMU_FORMAT_ATTR(filter_id_type, "config:62");
> > > > > +
> > > > > +static struct attribute *riscv_iommu_pmu_formats[] = {
> > > > > + &format_attr_event.attr,
> > > > > + &format_attr_partial_matching.attr,
> > > > > + &format_attr_pid_pscid.attr,
> > > > > + &format_attr_did_gscid.attr,
> > > > > + &format_attr_filter_pid_pscid.attr,
> > > > > + &format_attr_filter_did_gscid.attr,
> > > > > + &format_attr_filter_id_type.attr,
> > > > > + NULL,
> > > > > +};
> > > > > +
> > > > > +static const struct attribute_group riscv_iommu_pmu_format_group = {
> > > > > + .name = "format",
> > > > > + .attrs = riscv_iommu_pmu_formats,
> > > > > +};
> > > > > +
> > > > > +/* Events */
> > > > > +static ssize_t riscv_iommu_pmu_event_show(struct device *dev,
> > > > > + struct device_attribute *attr,
> > > > > + char *page)
> > > > > +{
> > > > > + struct perf_pmu_events_attr *pmu_attr;
> > > > > +
> > > > > + pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
> > > > > +
> > > > > + return sysfs_emit(page, "event=0x%02llx\n", pmu_attr->id);
> > > > > +}
> > > > > +
> > > > > +#define RISCV_IOMMU_PMU_EVENT_ATTR(name, id) \
> > > > > + PMU_EVENT_ATTR_ID(name, riscv_iommu_pmu_event_show, id)
> > > > > +
> > > > > +static struct attribute *riscv_iommu_pmu_events[] = {
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(cycle, RISCV_IOMMU_HPMEVENT_CYCLE),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(untranslated_req, RISCV_IOMMU_HPMEVENT_URQ),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(translated_req, RISCV_IOMMU_HPMEVENT_TRQ),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(ats_trans_req, RISCV_IOMMU_HPMEVENT_ATS_RQ),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(tlb_miss, RISCV_IOMMU_HPMEVENT_TLB_MISS),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(ddt_walks, RISCV_IOMMU_HPMEVENT_DD_WALK),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(pdt_walks, RISCV_IOMMU_HPMEVENT_PD_WALK),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(s_vs_pt_walks, RISCV_IOMMU_HPMEVENT_S_VS_WALKS),
> > > > > + RISCV_IOMMU_PMU_EVENT_ATTR(g_pt_walks, RISCV_IOMMU_HPMEVENT_G_WALKS),
> > > > > + NULL,
> > > > > +};
> > > > > +
> > > > > +static const struct attribute_group riscv_iommu_pmu_events_group = {
> > > > > + .name = "events",
> > > > > + .attrs = riscv_iommu_pmu_events,
> > > > > +};
> > > > > +
> > > > > +/* cpumask */
> > > > > +static ssize_t riscv_iommu_cpumask_show(struct device *dev,
> > > > > + struct device_attribute *attr,
> > > > > + char *buf)
> > > > > +{
> > > > > + struct riscv_iommu_pmu *pmu = to_riscv_iommu_pmu(dev_get_drvdata(dev));
> > > > > +
> > > > > + return cpumap_print_to_pagebuf(true, buf, cpumask_of(pmu->on_cpu));
> > > > > +}
> > > > > +
> > > > > +static struct device_attribute riscv_iommu_cpumask_attr =
> > > > > + __ATTR(cpumask, 0444, riscv_iommu_cpumask_show, NULL);
> > > > > +
> > > > > +static struct attribute *riscv_iommu_cpumask_attrs[] = {
> > > > > + &riscv_iommu_cpumask_attr.attr,
> > > > > + NULL
> > > > > +};
> > > > > +
> > > > > +static const struct attribute_group riscv_iommu_pmu_cpumask_group = {
> > > > > + .attrs = riscv_iommu_cpumask_attrs,
> > > > > +};
> > > > > +
> > > > > +static const struct attribute_group *riscv_iommu_pmu_attr_grps[] = {
> > > > > + &riscv_iommu_pmu_cpumask_group,
> > > > > + &riscv_iommu_pmu_format_group,
> > > > > + &riscv_iommu_pmu_events_group,
> > > > > + NULL,
> > > > > +};
> > > > > +
> > > > > +/* PMU Operations */
> > > > > +static void riscv_iommu_pmu_set_counter(struct riscv_iommu_pmu *pmu, u32 idx,
> > > > > + u64 value)
> > > > > +{
> > > > > + u64 counter_mask = idx ? pmu->event_cntr_mask : pmu->cycle_cntr_mask;
> > > > > +
> > > > > + writeq(value & counter_mask, pmu->reg + RISCV_IOMMU_REG_IOHPMCTR(idx));
> > > > #include <include/linux/io-64-nonatomic-lo-hi.h>
> > > > lo_hi_writeq(value & counter_mask, pmu->reg + RISCV_IOMMU_REG_IOHPMCTR(idx));
> > > >
> > > > For better compatibility, it is recommended to use writel and readl
> > > > instead of readq and writeq. Sorry for overlooking this last time —
> > > > could you please make the change again?
> > > > In the future, your driver could support RV32 directly, as the RISC-V
> > > > IOMMU specification also supports RV32; it's just that the current
> > > > RISC-V IOMMU driver's Kconfig depends on 64BIT.
> > >
> > > Hi Guo,
> > > Thank you for your review. In my implementation for RV32, readq and
> > > writeq are replaced by hi_lo_readq and hi_lo_writeq (from
> > > io-64-nonatomic-hi-lo.h). So, supporting RV32 should be fine.
> > > However, as you mentioned before: 64-bit access might be undefined on
> > > some hardware. To be safe and compatible with all hardware, we should
> > > use 32-bit access instead of 64-bit access.
> > > I will modify them in the next version
> > >
> > > I'd like to make sure I didn't miss anything important. My original
> > > code uses the "hi-lo" order. May I ask if there is a specific reason
> > > why you prefer the "lo-hi" order?
> > > Thanks
> > >
> >
> > Hi all,
> > I have been thinking more about the IOMMU register access behavior and
> > would like to share my thoughts regarding the lo-hi vs. hi-lo access
> > order.
> >
> > It seems to me that the correct 32-bit write order is strictly tied to
> > the underlying hardware implementation. From the software side, we
> > have no way of knowing whether a specific vendor designed their
> > hardware with a low-part trigger or a high-part trigger. Therefore,
> > hardcoding a fixed 32-bit write sequence could result in compatibility
> > issues across different hardware designs.
> >
> > Revisiting the spec regarding the UNSPECIFIED 8-byte atomic access,
> > this might imply that the hardware itself will handle the split into
> > two 4-byte accesses and internally manage its own trigger logic.
> > Because of this, for RV64 systems, software might simply use standard
> > 8-byte writes (e.g., writeq). Delegating the access handling to the
> > hardware is likely the best way to maximize compatibility across all
> > vendor implementations. For the counter reading strategy (hi-lo-hi
> > pattern), I completely agree with using the hi-lo-hi pattern when
> > reading the PMU counters. Since this is a read operation and does not
> > alter hardware state.
> >
> > For RV32 support, we inevitably face the 32-bit access splitting
> > issue. For the RV32 case, maybe we can assume the hi-lo order as the
> > default fallback, because the IOMMU specification uses this sequence
> > as example: "an access may appear, internally to the IOMMU, as if two
> > separate 4 byte accesses - first to the high half and second to the
> > low half - were performed.". Or do you prefer to use the same
> > assumption (hi-lo pattern) in RV64 as well?
> >
> > Please let me know what you think about this.
> >
> > Thanks
>
> You're right, RV32 is not a valid argument here — my wording was
> incorrect. Thanks for pointing that out.
> Also, I have no strong preference for lo-hi; that was just a careless
> copy-paste on my part. I agree hi-lo is the right choice.
>
May I confirm if you still expect to use the hi-lo pattern to 'write'
to 64-bit registers in RV64? Or would you agree that we should let the
hardware handle this behavior, since software couldn't know the high
part/low part trigger behavior?
> Thanks again for your great work.
>
> --
> Best Regards
> Guo Ren
More information about the linux-riscv
mailing list