Re: [PATCH v11 2/2] iommu/riscv: create a auxiliary device for HPM
From: sashiko-bot@kernel.org
Date: 2026-09-14 04:09:03
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] The PMU auxiliary device is destroyed during the devres phase, after the IOMMU hardware and IRQs have already been torn down in the parent driver's remove callback. Pre-existing issues: - [High] Missing release barrier when locklessly updating the IOMMU Device Directory Table. -- commit 93eb137b97dbeb7d61f4f9e3cac367ef538ba9b7 Author: Zong Li [off-list ref] iommu/riscv: create a auxiliary device for HPM This commit creates an auxiliary device for the hardware performance monitor (HPM) when supported by the RISC-V IOMMU.
quoted hunk ↗ jump to hunk
diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c --- a/drivers/iommu/riscv/iommu.c +++ b/drivers/iommu/riscv/iommu.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -587,6 +591,36 @@ static irqreturn_t riscv_iommu_fltq_process(int irq, void *data) return IRQ_HANDLED; } +/* + * IOMMU Hardware performance monitor + */ +static void riscv_iommu_pmu_id_free(void *data) +{ + ida_free(&riscv_iommu_pmu_ida, (unsigned long)data); +} + +static int riscv_iommu_hpm_enable(struct riscv_iommu_device *iommu) +{ + struct auxiliary_device *auxdev; + int id, ret; + + id = ida_alloc(&riscv_iommu_pmu_ida, GFP_KERNEL); + if (id < 0) + return id; + + ret = devm_add_action_or_reset(iommu->dev, riscv_iommu_pmu_id_free, + (void *)(unsigned long)id); + if (ret) + return ret; + + auxdev = __devm_auxiliary_device_create(iommu->dev, "riscv-iommu", + "pmu", iommu, id);
[Severity: High] Will this devm-managed auxiliary device be destroyed too late during teardown? When the driver is unbound (for example, via sysfs), the parent driver's remove callback in riscv_iommu_platform_remove() is called. This manually disables the IOMMU and calls platform_device_msi_free_irqs_all() to tear down MSIs. Because __devm_auxiliary_device_create() manages the auxiliary device via devres on the parent device, its destruction and the PMU driver's unbind happen after the parent remove callback returns. Could this cause the PMU driver's unbind process to access torn-down MSI descriptors or disabled hardware when it tries to free its IRQ handler or stop the PMU?
+ if (!auxdev) + return -ENODEV; + + return 0; +}
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a missing
release barrier when locklessly updating the IOMMU Device Directory Table.
Looking at riscv_iommu_get_dc(), a new page is allocated and published
using cmpxchg_relaxed():
ptr = riscv_iommu_get_pages(iommu, SZ_4K);
if (!ptr)
return NULL;
new = phys_to_ppn(__pa(ptr)) | RISCV_IOMMU_DDTE_V;
old = cmpxchg_relaxed((unsigned long *)ddtp, ddt, new);
Since cmpxchg_relaxed() lacks release semantics, could the CPU reorder
the page zeroing stores to happen after the pointer is published?
If the IOMMU hardware or another CPU traverses the table concurrently,
is there a risk it might follow the valid pointer and read uninitialized
memory, interpreting it as valid context entries?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914035723.2922057-1-zong.li@sifive.com?part=2