* [PATCH v4 1/6] arm_mpam: Fix the RIS index range check in mpam_ris_create_locked
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-28 11:45 ` [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size() Yin Li
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree
The RIS selector field is defined as MPAMCFG_PART_SEL_RIS (bits 27:24),
which is 4 bits wide and gives valid indices 0..15. MPAM_MSC_MAX_NUM_RIS
is defined as 16, so the check should use >= to reject index 16 and
above. The previous > check incorrectly accepted index 16 as valid.
Fixes: 01fb4b822472 ("arm_mpam: Add the class and component structures for firmware described ris")
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
---
drivers/resctrl/mpam_devices.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 48adc49b833c..347a5a6d6a1d 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -700,7 +700,7 @@ static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
lockdep_assert_held(&mpam_list_lock);
- if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
+ if (ris_idx >= MPAM_MSC_MAX_NUM_RIS)
return -EINVAL;
if (test_and_set_bit(ris_idx, &msc->ris_idxs))
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size()
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
2026-10-28 11:45 ` [PATCH v4 1/6] arm_mpam: Fix the RIS index range check in mpam_ris_create_locked Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-29 8:13 ` Jinjie Ruan
2026-10-29 12:02 ` Ilpo Järvinen
2026-10-28 11:45 ` [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding Yin Li
` (3 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree, kernel test robot
struct resource uses an inclusive end address, so the window size is
end - start + 1. do_mpam_msc_drv_probe() computed it as end - start,
which is one byte too small.
mapped_hwpage_sz is used by the register access bounds checks, which
have the form "offset + width > mapped_hwpage_sz". With the size one
byte too small, an access to the last register in the window is
incorrectly flagged as out of bounds: for a 0x1000 window, a 4-byte
access at offset 0xFFC (bytes 0xFFC..0xFFF, legal) computes
0xFFC + 4 = 0x1000 > 0xFFF and falsely warns.
Use resource_size() so the size is correct. The ">" bounds checks are
correct as-is once the size is accurate and are left unchanged.
Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202609030809.ObirDhR3-lkp@intel.com/
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
---
drivers/resctrl/mpam_devices.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 347a5a6d6a1d..0f984fcbcae9 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -2508,7 +2508,7 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
dev_err_once(dev, "Failed to map MSC base address\n");
return ERR_CAST(io);
}
- msc->mapped_hwpage_sz = msc_res->end - msc_res->start;
+ msc->mapped_hwpage_sz = resource_size(msc_res);
msc->mapped_hwpage = io;
} else if (msc->iface == MPAM_IFACE_PCC) {
u32 mpam_fb_msc_id;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size()
2026-10-28 11:45 ` [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size() Yin Li
@ 2026-10-29 8:13 ` Jinjie Ruan
2026-10-29 12:02 ` Ilpo Järvinen
1 sibling, 0 replies; 12+ messages in thread
From: Jinjie Ruan @ 2026-10-29 8:13 UTC (permalink / raw)
To: Yin Li, James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Andre Przywara, Drew Fustini, Ilpo Järvinen, Shaopeng Tan,
trilok.soni, aiqun.yu, ganapatrao.kulkarni, Srivathsa L Rao,
Huang Yiwei, linux-arm-kernel, linux-arm-msm, linux-kernel,
driver-core, devicetree, kernel test robot
在 2026/10/28 19:45, Yin Li 写道:
> struct resource uses an inclusive end address, so the window size is
> end - start + 1. do_mpam_msc_drv_probe() computed it as end - start,
> which is one byte too small.
>
> mapped_hwpage_sz is used by the register access bounds checks, which
> have the form "offset + width > mapped_hwpage_sz". With the size one
> byte too small, an access to the last register in the window is
> incorrectly flagged as out of bounds: for a 0x1000 window, a 4-byte
> access at offset 0xFFC (bytes 0xFFC..0xFFF, legal) computes
> 0xFFC + 4 = 0x1000 > 0xFFF and falsely warns.
>
> Use resource_size() so the size is correct. The ">" bounds checks are
> correct as-is once the size is accurate and are left unchanged.
>
> Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202609030809.ObirDhR3-lkp@intel.com/
> Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Ben Horgan <ben.horgan@arm.com>
> ---
> drivers/resctrl/mpam_devices.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 347a5a6d6a1d..0f984fcbcae9 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -2508,7 +2508,7 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
> dev_err_once(dev, "Failed to map MSC base address\n");
> return ERR_CAST(io);
> }
> - msc->mapped_hwpage_sz = msc_res->end - msc_res->start;
> + msc->mapped_hwpage_sz = resource_size(msc_res);
Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
> msc->mapped_hwpage = io;
> } else if (msc->iface == MPAM_IFACE_PCC) {
> u32 mpam_fb_msc_id;
>
--
Best regards,
Jinjie
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size()
2026-10-28 11:45 ` [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size() Yin Li
2026-10-29 8:13 ` Jinjie Ruan
@ 2026-10-29 12:02 ` Ilpo Järvinen
1 sibling, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2026-10-29 12:02 UTC (permalink / raw)
To: Yin Li
Cc: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan, Andre Przywara, Drew Fustini,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
LKML, driver-core, devicetree, kernel test robot
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
On Wed, 28 Oct 2026, Yin Li wrote:
> struct resource uses an inclusive end address, so the window size is
> end - start + 1. do_mpam_msc_drv_probe() computed it as end - start,
> which is one byte too small.
>
> mapped_hwpage_sz is used by the register access bounds checks, which
> have the form "offset + width > mapped_hwpage_sz". With the size one
> byte too small, an access to the last register in the window is
> incorrectly flagged as out of bounds: for a 0x1000 window, a 4-byte
> access at offset 0xFFC (bytes 0xFFC..0xFFF, legal) computes
> 0xFFC + 4 = 0x1000 > 0xFFF and falsely warns.
>
> Use resource_size() so the size is correct. The ">" bounds checks are
> correct as-is once the size is accurate and are left unchanged.
>
> Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202609030809.ObirDhR3-lkp@intel.com/
> Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Ben Horgan <ben.horgan@arm.com>
> ---
> drivers/resctrl/mpam_devices.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 347a5a6d6a1d..0f984fcbcae9 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -2508,7 +2508,7 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
> dev_err_once(dev, "Failed to map MSC base address\n");
> return ERR_CAST(io);
> }
> - msc->mapped_hwpage_sz = msc_res->end - msc_res->start;
> + msc->mapped_hwpage_sz = resource_size(msc_res);
> msc->mapped_hwpage = io;
> } else if (msc->iface == MPAM_IFACE_PCC) {
> u32 mpam_fb_msc_id;
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
2026-10-28 11:45 ` [PATCH v4 1/6] arm_mpam: Fix the RIS index range check in mpam_ris_create_locked Yin Li
2026-10-28 11:45 ` [PATCH v4 2/6] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size() Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-28 11:56 ` sashiko-bot
2026-10-28 11:45 ` [PATCH v4 4/6] cacheinfo: Expose the code to generate a cache-id from a device_node Yin Li
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree
From: Rob Herring <robh@kernel.org>
The binding is designed around the assumption that an MSC will be a
sub-block of something else such as a memory controller or cache
controller.
A key part of MPAM is we need to know about all of the MSCs in the system
before it can be enabled. This drives the need for the genericish
'arm,mpam-msc' compatible. Though we can't assume an MSC is accessible
until a h/w specific driver potentially enables the h/w.
This binding covers only the properties currently used in practice; it
can be extended as additional features are needed.
Cc: James Morse <james.morse@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: James Morse <james.morse@arm.com>
[ Yin Li and Huang Yiwei: fix schema field order and yamllint spacing;
fix example reg addresses; reduce to minimal binding covering only
properties in current use, dropping RIS child nodes, memory controller
support, arm,mpam-device, cpus and interrupt-names ]
Co-developed-by: Huang Yiwei <huang.yiwei@oss.qualcomm.com>
Signed-off-by: Huang Yiwei <huang.yiwei@oss.qualcomm.com>
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
---
.../devicetree/bindings/arm/arm,mpam-msc.yaml | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
new file mode 100644
index 000000000000..523978900d3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,mpam-msc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Memory System Resource Partitioning and Monitoring (MPAM)
+
+maintainers:
+ - Rob Herring <robh@kernel.org>
+
+description: |
+ The Arm MPAM specification can be found here:
+
+ https://developer.arm.com/documentation/ddi0598/latest
+
+properties:
+ compatible:
+ const: arm,mpam-msc
+
+ reg:
+ maxItems: 1
+ description: A memory region containing registers as defined in the MPAM
+ specification.
+
+ arm,not-ready-us:
+ description: The maximum time in microseconds for monitoring data to be
+ accurate after a settings change. For more information, see the
+ Not-Ready (NRDY) bit description in the MPAM specification.
+
+additionalProperties: false
+
+examples:
+ - |
+ L3: cache-controller@30000000 {
+ compatible = "foo,a-l3-cache", "cache";
+ cache-level = <3>;
+ cache-unified;
+ reg = <0x0 0x30000000 0x0 0x2000>;
+ ranges = <0x0 0x30000000 0x2000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ msc@0 {
+ compatible = "arm,mpam-msc";
+ reg = <0x0 0x2000>;
+ arm,not-ready-us = <1>;
+ /* CPU affinity implied by parent cache node */
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding
2026-10-28 11:45 ` [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding Yin Li
@ 2026-10-28 11:56 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-10-28 11:56 UTC (permalink / raw)
To: Yin Li; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] dt-bindings: arm: arm,mpam-msc: mismatched reg and ranges cell counts in example
- [Low] dt-bindings: arm: arm,mpam-msc: missing top-level required property list
- [Low] dt-bindings: arm: arm,mpam-msc: example violates dt-check-style rules
--
commit f7533674b3629d7c4309338f82390e5f0a91878a
Author: Rob Herring <robh@kernel.org>
dt-bindings: arm: Add MPAM MSC binding
This commit adds the devicetree binding schema for the Arm Memory System
Resource Partitioning and Monitoring (MPAM) Memory System Component (MSC)
using the arm,mpam-msc compatible string.
> diff --git a/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
> new file mode 100644
> index 0000000000000..523978900d3de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
> @@ -0,0 +1,52 @@
[ ... ]
> +properties:
> + compatible:
> + const: arm,mpam-msc
> +
> + reg:
> + maxItems: 1
> + description: A memory region containing registers as defined in the MPAM
> + specification.
> +
> + arm,not-ready-us:
> + description: The maximum time in microseconds for monitoring data to be
> + accurate after a settings change. For more information, see the
> + Not-Ready (NRDY) bit description in the MPAM specification.
> +
> +additionalProperties: false
[Severity: Low]
Should this schema include a top-level required block listing compatible and
reg before additionalProperties: false?
In drivers/resctrl/mpam_devices.c, do_mpam_msc_drv_probe() unconditionally
maps MMIO resource 0 for these nodes:
if (msc->iface == MPAM_IFACE_MMIO) {
void __iomem *io;
io = devm_platform_get_and_ioremap_resource(pdev, 0,
&msc_res);
if (IS_ERR(io)) {
dev_err_once(dev, "Failed to map MSC base address\n");
return ERR_CAST(io);
}
Without a required list in Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml,
devicetree nodes matching arm,mpam-msc that omit reg will pass dtbs_check
validation and fail at runtime during probe.
> +
> +examples:
> + - |
> + L3: cache-controller@30000000 {
> + compatible = "foo,a-l3-cache", "cache";
> + cache-level = <3>;
> + cache-unified;
[Severity: Low]
Does this example trigger check_property_order() and check_unused_labels()
warnings when running scripts/dtc/dt-check-style --mode=strict on
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml?
In cache-controller@30000000, the L3 label is never referenced in the example
(and uses uppercase characters), and cache-level and cache-unified are placed
before reg and ranges rather than after standard addressing properties.
> + reg = <0x0 0x30000000 0x0 0x2000>;
> + ranges = <0x0 0x30000000 0x2000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
[Severity: Low]
Are the cell counts in reg and ranges consistent with the default example bus
here?
When make dt_binding_check extracts the example in
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml, the surrounding
example-0 parent node defaults to #address-cells = <1> and #size-cells = <1>.
In cache-controller@30000000, ranges = <0x0 0x30000000 0x2000> uses 3 cells
(1 child address cell, 1 parent address cell, and 1 child size cell), which
matches a 1-cell parent bus. However, reg = <0x0 0x30000000 0x0 0x2000> uses
4 cells as if the parent bus had #address-cells = <2> and #size-cells = <2>,
causing it to be decoded under the 1-cell parent as two separate 32-bit
regions at base address 0x0 (<0x0 0x30000000> and <0x0 0x2000>) rather than a
single region at 0x30000000.
Should reg be <0x30000000 0x2000> to match the 1-cell parent bus and ranges?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-mpam-resctrl-dt-knp-support-v4-0-bdda1e445cba@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 4/6] cacheinfo: Expose the code to generate a cache-id from a device_node
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
` (2 preceding siblings ...)
2026-10-28 11:45 ` [PATCH v4 3/6] dt-bindings: arm: Add MPAM MSC binding Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-28 11:45 ` [PATCH v4 5/6] arm_mpam: Add device tree support for MSC probing Yin Li
2026-10-28 11:45 ` [PATCH DNM v4 6/6] arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches Yin Li
5 siblings, 0 replies; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree
From: James Morse <james.morse@arm.com>
The MPAM driver identifies caches by id for use with resctrl. It
needs to know the cache-id when probe-ing, but the value isn't set
in cacheinfo until device_initcall(). Even after device_initcall(),
the cache-id is only available if at least one CPU associated with
the cache is online.
Instead of making the driver wait, expose the code that generates the
cache-id. The parts of the MPAM driver that run early can use this to
set up the resctrl structures before cacheinfo is ready in
device_initcall().
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
---
drivers/base/cacheinfo.c | 17 ++++++++++++-----
include/linux/cacheinfo.h | 1 +
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index eb8a907f6d50..a4b10f3a9007 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -243,8 +243,7 @@ static bool match_cache_node(struct device_node *cpu,
#define arch_compact_of_hwid(_x) (_x)
#endif
-static void cache_of_set_id(struct cacheinfo *this_leaf,
- struct device_node *cache_node)
+u32 cache_of_calculate_id(struct device_node *cache_node)
{
struct device_node *cpu;
u32 min_id = ~0;
@@ -255,15 +254,23 @@ static void cache_of_set_id(struct cacheinfo *this_leaf,
id = arch_compact_of_hwid(id);
if (FIELD_GET(GENMASK_ULL(63, 32), id)) {
of_node_put(cpu);
- return;
+ return ~0;
}
if (match_cache_node(cpu, cache_node))
min_id = min(min_id, id);
}
- if (min_id != ~0) {
- this_leaf->id = min_id;
+ return min_id;
+}
+
+static void cache_of_set_id(struct cacheinfo *this_leaf,
+ struct device_node *cache_node)
+{
+ u32 id = cache_of_calculate_id(cache_node);
+
+ if (id != ~0) {
+ this_leaf->id = id;
this_leaf->attributes |= CACHE_ID;
}
}
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index 56fa646df0d1..469d490968ac 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -114,6 +114,7 @@ int acpi_get_cache_info(unsigned int cpu,
#endif
const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
+u32 cache_of_calculate_id(struct device_node *np);
/*
* Get the cacheinfo structure for the cache associated with @cpu at
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/6] arm_mpam: Add device tree support for MSC probing
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
` (3 preceding siblings ...)
2026-10-28 11:45 ` [PATCH v4 4/6] cacheinfo: Expose the code to generate a cache-id from a device_node Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-28 12:04 ` sashiko-bot
2026-10-28 11:45 ` [PATCH DNM v4 6/6] arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches Yin Li
5 siblings, 1 reply; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree
From: James Morse <james.morse@arm.com>
The MPAM driver currently discovers MSCs only via ACPI. Add a device
tree path so MSCs can be probed on DT-based platforms: parse MSC nodes
from the device tree, compute cache-id and affinity from the cache
nodes and create the RIS entries.
Signed-off-by: James Morse <james.morse@arm.com>
[ Yin Li: drop the ACPI-only stub of mpam_get_cpumask_from_cache_id();
use of_node_get() for the cache parent to avoid a refcount underflow;
reject out-of-range ris_idx after of_property_read_reg() before
narrowing to u8; use u32 instead of unsigned long for cache-id values
and compare against ~0U; fix mpam_dt_create_foundling_msc() to iterate
MSC child nodes rather than creating a device for the cache node itself ]
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
---
drivers/resctrl/mpam_devices.c | 250 +++++++++++++++++++++++++++++++++++++---
drivers/resctrl/mpam_internal.h | 2 +-
2 files changed, 234 insertions(+), 18 deletions(-)
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 0f984fcbcae9..091c8ba2e974 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -22,6 +22,9 @@
#include <linux/lockdep.h>
#include <linux/mailbox_client.h>
#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/property.h>
@@ -263,6 +266,171 @@ static void mpam_free_garbage(void)
}
}
+/* Called recursively to walk the list of caches from a particular CPU */
+static void __mpam_get_cpumask_from_cache_id(int cpu, struct device_node *cache_node,
+ u32 cache_id,
+ u32 cache_level,
+ cpumask_t *affinity)
+{
+ int err;
+ u32 iter_level;
+ u32 iter_cache_id;
+ struct device_node *iter_node __free(device_node) = of_find_next_cache_node(cache_node);
+
+ if (!iter_node)
+ return;
+
+ err = of_property_read_u32(iter_node, "cache-level", &iter_level);
+ if (err)
+ return;
+
+ /*
+ * get_cpu_cacheinfo_id() isn't ready until sometime
+ * during device_initcall(). Use cache_of_calculate_id().
+ */
+ iter_cache_id = cache_of_calculate_id(iter_node);
+ if (iter_cache_id == ~0U)
+ return;
+
+ if (iter_level == cache_level && iter_cache_id == cache_id)
+ cpumask_set_cpu(cpu, affinity);
+
+ if (iter_level < cache_level)
+ __mpam_get_cpumask_from_cache_id(cpu, iter_node, cache_id,
+ cache_level, affinity);
+}
+
+/*
+ * The cacheinfo structures are only populated when CPUs are online.
+ * This helper walks the device tree to include offline CPUs too.
+ */
+int mpam_get_cpumask_from_cache_id(u32 cache_id, u32 cache_level,
+ cpumask_t *affinity)
+{
+ int cpu;
+
+ if (!acpi_disabled)
+ return acpi_pptt_get_cpumask_from_cache_id(cache_id, affinity);
+
+ for_each_possible_cpu(cpu) {
+ struct device_node *cpu_node __free(device_node) = of_get_cpu_node(cpu, NULL);
+ if (!cpu_node) {
+ pr_err("Failed to find cpu%d device node\n", cpu);
+ return -ENOENT;
+ }
+
+ __mpam_get_cpumask_from_cache_id(cpu, cpu_node, cache_id,
+ cache_level, affinity);
+ }
+
+ return 0;
+}
+
+static int get_cpumask_from_cache(struct device_node *cache,
+ cpumask_t *affinity)
+{
+ int err;
+ u32 cache_level;
+ u32 cache_id;
+
+ err = of_property_read_u32(cache, "cache-level", &cache_level);
+ if (err) {
+ pr_err("Failed to read cache-level from cache node\n");
+ return -ENOENT;
+ }
+
+ cache_id = cache_of_calculate_id(cache);
+ if (cache_id == ~0U) {
+ pr_err("Failed to calculate cache-id from cache node\n");
+ return -ENOENT;
+ }
+
+ return mpam_get_cpumask_from_cache_id(cache_id, cache_level, affinity);
+}
+
+static int mpam_dt_count_msc(void)
+{
+ int count = 0;
+ struct device_node *np;
+
+ for_each_compatible_node(np, NULL, "arm,mpam-msc") {
+ if (of_device_is_available(np))
+ count++;
+ }
+
+ return count;
+}
+
+static int mpam_dt_parse_resource(struct mpam_msc *msc, struct device_node *np,
+ u8 ris_idx)
+{
+ int err = 0;
+ u32 level = 0;
+ u32 cache_id;
+ struct device *dev = &msc->pdev->dev;
+ struct device_node *cache __free(device_node) = NULL;
+ struct device_node *parent __free(device_node) = of_get_parent(np);
+
+ if (of_device_is_compatible(np, "arm,mpam-cache")) {
+ cache = of_parse_phandle(np, "arm,mpam-device", 0);
+ if (!cache) {
+ dev_err_once(dev, "Failed to read phandle\n");
+ return -EINVAL;
+ }
+ } else if (of_device_is_compatible(parent, "cache")) {
+ cache = of_node_get(parent);
+ } else {
+ /* For now, only caches are supported */
+ cache = NULL;
+ return err;
+ }
+
+ err = of_property_read_u32(cache, "cache-level", &level);
+ if (err) {
+ dev_err_once(dev, "Failed to read cache-level\n");
+ return err;
+ }
+
+ cache_id = cache_of_calculate_id(cache);
+ if (cache_id == ~0U) {
+ dev_err_once(dev, "Failed to calculate cache-id\n");
+ return -ENOENT;
+ }
+
+ return mpam_ris_create(msc, ris_idx, MPAM_CLASS_CACHE, level, cache_id);
+}
+
+static int mpam_dt_parse_resources(struct mpam_msc *msc, void *ignored)
+{
+ u64 ris_idx = 0;
+ int err, num_ris = 0;
+ struct device_node *np;
+
+ np = msc->pdev->dev.of_node;
+ for_each_available_child_of_node_scoped(np, iter) {
+ err = of_property_read_reg(iter, 0, &ris_idx, NULL);
+ if (!err) {
+ /*
+ * ris_idx is read as u64 but indexes a 4-bit RIS selector
+ * (0..MPAM_MSC_MAX_NUM_RIS). Reject out-of-range values here,
+ * before it is narrowed to u8, so a large value cannot be
+ * truncated into a valid-looking index.
+ */
+ if (ris_idx >= MPAM_MSC_MAX_NUM_RIS)
+ return -EINVAL;
+ num_ris++;
+ err = mpam_dt_parse_resource(msc, iter, ris_idx);
+ if (err)
+ return err;
+ }
+ }
+
+ if (!num_ris)
+ err = mpam_dt_parse_resource(msc, np, 0);
+
+ return err;
+}
+
/*
* Once mpam is enabled, new requestors cannot further reduce the available
* partid. Assert that the size is fixed, and new requestors will be turned
@@ -629,16 +797,6 @@ mpam_vmsc_find(struct mpam_component *comp, struct mpam_msc *msc)
return mpam_vmsc_alloc(comp, msc);
}
-/*
- * The cacheinfo structures are only populated when CPUs are online.
- * This helper walks the acpi tables to include offline CPUs too.
- */
-int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
- cpumask_t *affinity)
-{
- return acpi_pptt_get_cpumask_from_cache_id(cache_id, affinity);
-}
-
/*
* cpumask_of_node() only knows about online CPUs. This can't tell us whether
* a class is represented on all possible CPUs.
@@ -2394,15 +2552,34 @@ static int mpam_msc_setup_error_irq(struct mpam_msc *msc)
*/
static void update_msc_accessibility(struct mpam_msc *msc)
{
+ struct device *dev = &msc->pdev->dev;
+ struct device_node *parent;
u32 affinity_id;
int err;
- err = device_property_read_u32(&msc->pdev->dev, "cpu_affinity",
- &affinity_id);
- if (err)
+ if (!acpi_disabled) {
+ err = device_property_read_u32(&msc->pdev->dev, "cpu_affinity",
+ &affinity_id);
+ if (err)
+ cpumask_copy(&msc->accessibility, cpu_possible_mask);
+ else
+ acpi_pptt_get_cpus_from_container(affinity_id,
+ &msc->accessibility);
+
+ return;
+ }
+
+ /* Where an MSC can be accessed from depends on the path to of_node. */
+ parent = of_get_parent(msc->pdev->dev.of_node);
+ if (parent == of_root) {
cpumask_copy(&msc->accessibility, cpu_possible_mask);
- else
- acpi_pptt_get_cpus_from_container(affinity_id, &msc->accessibility);
+ } else {
+ if (of_device_is_compatible(parent, "cache"))
+ get_cpumask_from_cache(parent, &msc->accessibility);
+ else
+ dev_err_once(dev, "Cannot determine accessibility of MSC.\n");
+ }
+ of_node_put(parent);
}
/*
@@ -2563,7 +2740,10 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
return PTR_ERR(msc);
/* Create RIS entries described by firmware */
- err = acpi_mpam_parse_resources(msc, plat_data);
+ if (!acpi_disabled)
+ err = acpi_mpam_parse_resources(msc, plat_data);
+ else
+ err = mpam_dt_parse_resources(msc, plat_data);
if (err) {
mpam_msc_drv_remove(pdev);
return err;
@@ -2576,15 +2756,45 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id mpam_of_match[] = {
+ { .compatible = "arm,mpam-msc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpam_of_match);
+
static struct platform_driver mpam_msc_driver = {
.driver = {
.name = "mpam_msc",
.suppress_bind_attrs = true,
+ .of_match_table = of_match_ptr(mpam_of_match),
},
.probe = mpam_msc_drv_probe,
.remove = mpam_msc_drv_remove,
};
+/*
+ * MSCs that are declared by the firmware as being part of a cache may not
+ * be created automatically as platform devices, since there is no
+ * dedicated cache driver.
+ *
+ * Deal with theo MSCs here.
+ */
+static void mpam_dt_create_foundling_msc(void)
+{
+ for_each_compatible_node_scoped(cache, NULL, "cache") {
+ for_each_child_of_node_scoped(cache, child) {
+ if (!of_match_node(mpam_of_match, child))
+ continue;
+ if (!of_device_is_available(child))
+ continue;
+ if (of_node_check_flag(child, OF_POPULATED))
+ continue;
+ if (!of_platform_device_create(child, NULL, NULL))
+ pr_err("Failed to create MSC device for %pOF\n", child);
+ }
+ }
+}
+
/* Any of these features mean the BWA_WD field is valid. */
static bool mpam_has_bwa_wd_feature(struct mpam_props *props)
{
@@ -3457,12 +3667,18 @@ static int __init mpam_msc_driver_init(void)
init_srcu_struct(&mpam_srcu);
- fw_num_msc = acpi_mpam_count_msc();
+ if (!acpi_disabled)
+ fw_num_msc = acpi_mpam_count_msc();
+ else
+ fw_num_msc = mpam_dt_count_msc();
if (fw_num_msc <= 0) {
pr_err("No MSC devices found in firmware\n");
return -EINVAL;
}
+ if (acpi_disabled)
+ mpam_dt_create_foundling_msc();
+
return platform_driver_register(&mpam_msc_driver);
}
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 1f86cc158100..cd122f308eab 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -519,7 +519,7 @@ int mpam_msmon_read(struct mpam_component *comp, struct mon_cfg *ctx,
enum mpam_device_features, u64 *val);
void mpam_msmon_reset_mbwu(struct mpam_component *comp, struct mon_cfg *ctx);
-int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
+int mpam_get_cpumask_from_cache_id(u32 cache_id, u32 cache_level,
cpumask_t *affinity);
#ifdef CONFIG_RESCTRL_FS
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 5/6] arm_mpam: Add device tree support for MSC probing
2026-10-28 11:45 ` [PATCH v4 5/6] arm_mpam: Add device tree support for MSC probing Yin Li
@ 2026-10-28 12:04 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-10-28 12:04 UTC (permalink / raw)
To: Yin Li; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] resctrl: mpam: DT MSCs all get id -1 and alias in mpam_vmsc_find()
- [Medium] resctrl: mpam: mpam_dt_parse_resources() returns stale error code
- [Medium] resctrl: mpam: undocumented DT bindings and broken parent check in MSC probe
--
commit 551add9668d2f1a68a508a323e1f2913890fe9b8
Author: James Morse <james.morse@arm.com>
arm_mpam: Add device tree support for MSC probing
This commit adds Device Tree support for probing MPAM MSC devices by
parsing MSC nodes, computing cache IDs and CPU affinity from cache nodes,
and creating RIS entries.
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 0f984fcbcae91..091c8ba2e9742 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
[ ... ]
> @@ -263,6 +266,171 @@ static void mpam_free_garbage(void)
> }
> }
>
[ ... ]
> +static int mpam_dt_parse_resource(struct mpam_msc *msc, struct device_node *np,
> + u8 ris_idx)
> +{
> + int err = 0;
> + u32 level = 0;
> + u32 cache_id;
> + struct device *dev = &msc->pdev->dev;
> + struct device_node *cache __free(device_node) = NULL;
> + struct device_node *parent __free(device_node) = of_get_parent(np);
> +
> + if (of_device_is_compatible(np, "arm,mpam-cache")) {
> + cache = of_parse_phandle(np, "arm,mpam-device", 0);
> + if (!cache) {
> + dev_err_once(dev, "Failed to read phandle\n");
> + return -EINVAL;
> + }
> + } else if (of_device_is_compatible(parent, "cache")) {
> + cache = of_node_get(parent);
> + } else {
> + /* For now, only caches are supported */
> + cache = NULL;
> + return err;
> + }
> +
> + err = of_property_read_u32(cache, "cache-level", &level);
> + if (err) {
> + dev_err_once(dev, "Failed to read cache-level\n");
> + return err;
> + }
> +
> + cache_id = cache_of_calculate_id(cache);
> + if (cache_id == ~0U) {
> + dev_err_once(dev, "Failed to calculate cache-id\n");
> + return -ENOENT;
> + }
> +
> + return mpam_ris_create(msc, ris_idx, MPAM_CLASS_CACHE, level, cache_id);
> +}
> +
> +static int mpam_dt_parse_resources(struct mpam_msc *msc, void *ignored)
> +{
> + u64 ris_idx = 0;
> + int err, num_ris = 0;
> + struct device_node *np;
> +
> + np = msc->pdev->dev.of_node;
> + for_each_available_child_of_node_scoped(np, iter) {
> + err = of_property_read_reg(iter, 0, &ris_idx, NULL);
> + if (!err) {
> + /*
> + * ris_idx is read as u64 but indexes a 4-bit RIS selector
> + * (0..MPAM_MSC_MAX_NUM_RIS). Reject out-of-range values here,
> + * before it is narrowed to u8, so a large value cannot be
> + * truncated into a valid-looking index.
> + */
> + if (ris_idx >= MPAM_MSC_MAX_NUM_RIS)
> + return -EINVAL;
> + num_ris++;
> + err = mpam_dt_parse_resource(msc, iter, ris_idx);
> + if (err)
> + return err;
> + }
> + }
> +
> + if (!num_ris)
> + err = mpam_dt_parse_resource(msc, np, 0);
> +
> + return err;
> +}
[Severity: Medium]
Can mpam_dt_parse_resources() return a stale error code when an MSC node
has an available child node without a reg property after a valid RIS child
node?
In the loop over child nodes, err is overwritten on every iteration by
of_property_read_reg(). When a child node has a valid reg property, num_ris
is incremented and mpam_dt_parse_resource() sets err to 0 on success.
However, if a subsequent available child node does not have a reg property,
of_property_read_reg() sets err to -EINVAL and the if (!err) block is
skipped. Because num_ris is already non-zero, if (!num_ris) is not taken,
and mpam_dt_parse_resources() returns -EINVAL to mpam_msc_drv_probe(),
which then calls mpam_msc_drv_remove() and tears down the previously
created RIS entries.
> @@ -2394,15 +2552,34 @@ static int mpam_msc_setup_error_irq(struct mpam_msc *msc)
> */
> static void update_msc_accessibility(struct mpam_msc *msc)
> {
> + struct device *dev = &msc->pdev->dev;
> + struct device_node *parent;
> u32 affinity_id;
> int err;
>
> - err = device_property_read_u32(&msc->pdev->dev, "cpu_affinity",
> - &affinity_id);
> - if (err)
> + if (!acpi_disabled) {
> + err = device_property_read_u32(&msc->pdev->dev, "cpu_affinity",
> + &affinity_id);
> + if (err)
> + cpumask_copy(&msc->accessibility, cpu_possible_mask);
> + else
> + acpi_pptt_get_cpus_from_container(affinity_id,
> + &msc->accessibility);
> +
> + return;
> + }
> +
> + /* Where an MSC can be accessed from depends on the path to of_node. */
> + parent = of_get_parent(msc->pdev->dev.of_node);
> + if (parent == of_root) {
> cpumask_copy(&msc->accessibility, cpu_possible_mask);
> - else
> - acpi_pptt_get_cpus_from_container(affinity_id, &msc->accessibility);
> + } else {
> + if (of_device_is_compatible(parent, "cache"))
> + get_cpumask_from_cache(parent, &msc->accessibility);
> + else
> + dev_err_once(dev, "Cannot determine accessibility of MSC.\n");
> + }
> + of_node_put(parent);
> }
[Severity: Medium]
Does the Device Tree parsing in mpam_dt_parse_resources(),
mpam_dt_parse_resource(), and update_msc_accessibility() match the
documented binding and expected DT hierarchy?
In the companion commit ("dt-bindings: arm: Add MPAM MSC binding"), RIS
child nodes, arm,mpam-cache, and arm,mpam-device were dropped from
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml, which sets
additionalProperties: false and only defines compatible, reg, and
arm,not-ready-us. Yet mpam_dt_parse_resources() and
mpam_dt_parse_resource() still parse child RIS nodes, arm,mpam-cache, and
arm,mpam-device.
Furthermore, when mpam_dt_parse_resources() passes a child node iter as np
to mpam_dt_parse_resource(), parent = of_get_parent(np) resolves to the MSC
node (arm,mpam-msc) rather than its parent cache node, so
of_device_is_compatible(parent, "cache") will evaluate to false.
In addition, update_msc_accessibility() checks whether parent == of_root or
is compatible with "cache". If a standalone MSC node is placed under a
standard bus node such as /soc rather than directly under /, parent ==
of_root is false, leaving msc->accessibility empty and causing
do_mpam_msc_drv_probe() to fail with -EINVAL.
[ ... ]
> @@ -2576,15 +2756,45 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id mpam_of_match[] = {
> + { .compatible = "arm,mpam-msc", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mpam_of_match);
> +
> static struct platform_driver mpam_msc_driver = {
> .driver = {
> .name = "mpam_msc",
> .suppress_bind_attrs = true,
> + .of_match_table = of_match_ptr(mpam_of_match),
> },
> .probe = mpam_msc_drv_probe,
> .remove = mpam_msc_drv_remove,
> };
>
> +/*
> + * MSCs that are declared by the firmware as being part of a cache may not
> + * be created automatically as platform devices, since there is no
> + * dedicated cache driver.
> + *
> + * Deal with theo MSCs here.
> + */
> +static void mpam_dt_create_foundling_msc(void)
> +{
> + for_each_compatible_node_scoped(cache, NULL, "cache") {
> + for_each_child_of_node_scoped(cache, child) {
> + if (!of_match_node(mpam_of_match, child))
> + continue;
> + if (!of_device_is_available(child))
> + continue;
> + if (of_node_check_flag(child, OF_POPULATED))
> + continue;
> + if (!of_platform_device_create(child, NULL, NULL))
> + pr_err("Failed to create MSC device for %pOF\n", child);
> + }
> + }
> +}
[Severity: High]
Will all Device Tree MSCs receive msc->id == -1 and alias each other in
mpam_vmsc_find()?
On ACPI systems, acpi_mpam_parse_msc() allocates each platform_device with
platform_device_alloc("mpam_msc", tbl_msc->identifier), giving each MSC a
unique pdev->id.
On Device Tree systems, platform devices created via standard DT bus
population or via mpam_dt_create_foundling_msc() ->
of_platform_device_create() go through of_device_add(), which sets:
drivers/of/platform.c:of_device_add() {
...
ofdev->name = dev_name(&ofdev->dev);
ofdev->id = PLATFORM_DEVID_NONE;
...
}
When do_mpam_msc_drv_probe() initializes the MSC, it copies pdev->id
directly:
drivers/resctrl/mpam_devices.c:do_mpam_msc_drv_probe() {
...
msc->id = pdev->id;
msc->pdev = pdev;
...
}
As a result, every DT-probed struct mpam_msc gets msc->id == -1. When two
or more MSCs belong to the same struct mpam_component (for example,
multiple MSCs under the same cache node or referencing the same cache, or
multiple MSCs with undescribed RIS indices placed into the shared
MPAM_CLASS_UNKNOWN component during mpam_msc_hw_probe() ->
mpam_get_or_create_ris()), mpam_ris_create_locked() calls mpam_vmsc_find():
drivers/resctrl/mpam_devices.c:mpam_vmsc_find() {
...
list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
if (vmsc->msc->id == msc->id)
return vmsc;
}
return mpam_vmsc_alloc(comp, msc);
}
Because vmsc->msc->id == msc->id evaluates -1 == -1, the second MSC (msc2)
reuses the first MSC's struct mpam_vmsc (vmsc1, where vmsc1->msc == msc1)
instead of allocating its own.
During mpam_msc_hw_probe(msc2), mpam_ris_hw_probe(ris2) resolves
ris->vmsc->msc to msc1 while holding msc2's locks:
drivers/resctrl/mpam_devices.c:mpam_ris_hw_probe() {
struct mpam_msc *msc = ris->vmsc->msc;
...
lockdep_assert_held(&msc->probe_lock);
lockdep_assert_held(&msc->part_sel_lock);
}
This triggers lockdep assertion failures on msc1 and performs unserialized
register accesses against msc1 instead of msc2.
Furthermore, during teardown in mpam_msc_drv_remove() or mpam_disable(),
destroying msc1 leaves vmsc1 alive because ris2 is still linked in
vmsc1->ris. Once msc1 is freed by mpam_free_garbage(), destroying msc2
calls mpam_ris_destroy(ris2), which dereferences the freed msc1 via
vmsc->msc:
drivers/resctrl/mpam_devices.c:mpam_ris_destroy() {
struct mpam_vmsc *vmsc = ris->vmsc;
struct mpam_msc *msc = vmsc->msc;
...
clear_bit(ris->ris_idx, &msc->ris_idxs);
}
causing a use-after-free write. Formatting msc->id with %u in
mpam_msc_setup_error_irq() and __mpam_irq_handler() will also log
msc:4294967295.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-mpam-resctrl-dt-knp-support-v4-0-bdda1e445cba@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH DNM v4 6/6] arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches
2026-10-28 11:45 [PATCH v4 0/6] arm-mpam: Add basic device tree support for resctrl Yin Li
` (4 preceding siblings ...)
2026-10-28 11:45 ` [PATCH v4 5/6] arm_mpam: Add device tree support for MSC probing Yin Li
@ 2026-10-28 11:45 ` Yin Li
2026-10-28 12:05 ` sashiko-bot
5 siblings, 1 reply; 12+ messages in thread
From: Yin Li @ 2026-10-28 11:45 UTC (permalink / raw)
To: James Morse, Rob Herring, Shanker Donthineni, Ben Horgan,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Reinette Chatre, Fenghua Yu, Jonathan Cameron, Bjorn Andersson,
Konrad Dybcio, Gavin Shan
Cc: Yin Li, Andre Przywara, Drew Fustini, Ilpo Järvinen,
Shaopeng Tan, trilok.soni, aiqun.yu, ganapatrao.kulkarni,
Srivathsa L Rao, Huang Yiwei, linux-arm-kernel, linux-arm-msm,
linux-kernel, driver-core, devicetree
Add an MPAM MSC device node for each L2 cache. Each MSC is nested
directly under its L2 cache node so that the cache node is the MSC's
parent device and its CPU affinity is derived from that cache.
The L2 cache nodes are placed directly under the root node, using
'ranges' to map the MSC's register region to the physical address space.
The MSC register size is 0x2000: the Kaanapali platform implements the
MPAM registers up to offset 0x1FFF as permitted by the MPAM
specification when MPAMCFG_MBW_PBM is not implemented.
The nodes are disabled by default. Production firmware does not expose
MSC register access to the OS, so these nodes can only be enabled on a
local test setup where firmware permits direct MMIO access to the MSCs.
This DTS is therefore only an example for other platform DT authors and
is not intended to be merged.
Co-developed-by: Huang Yiwei <huang.yiwei@oss.qualcomm.com>
Signed-off-by: Huang Yiwei <huang.yiwei@oss.qualcomm.com>
Signed-off-by: Yin Li <yin.li@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/kaanapali.dtsi | 52 +++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index e48a179d0722..10aee29a1ba8 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -49,12 +49,6 @@ cpu0: cpu@0 {
power-domains = <&cpu_pd0>, <&scmi_perf 0>;
power-domain-names = "psci", "perf";
#cooling-cells = <2>;
-
- l2_0: l2-cache {
- compatible = "cache";
- cache-level = <2>;
- cache-unified;
- };
};
cpu1: cpu@100 {
@@ -121,12 +115,6 @@ cpu6: cpu@10000 {
power-domains = <&cpu_pd6>, <&scmi_perf 1>;
power-domain-names = "psci", "perf";
#cooling-cells = <2>;
-
- l2_1: l2-cache {
- compatible = "cache";
- cache-level = <2>;
- cache-unified;
- };
};
cpu7: cpu@10100 {
@@ -532,6 +520,46 @@ soccp_smp2p_in: slave-kernel {
};
};
+ l2_0: cache@19870000 {
+ compatible = "qcom,kaanapali-l2-cache", "cache";
+ cache-level = <2>;
+ cache-unified;
+ cache-size = <0xc00000>;
+ reg = <0x0 0x19870000 0x0 0x2000>;
+ ranges = <0x0 0x0 0x19870000 0x2000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ msc@0 {
+ compatible = "arm,mpam-msc";
+ reg = <0x0 0x2000>;
+ arm,not-ready-us = <1>;
+ status = "disabled";
+ };
+
+ };
+
+ l2_1: cache@1a870000 {
+ compatible = "qcom,kaanapali-l2-cache", "cache";
+ cache-level = <2>;
+ cache-unified;
+ cache-size = <0xc00000>;
+ reg = <0x0 0x1a870000 0x0 0x2000>;
+ ranges = <0x0 0x0 0x1a870000 0x2000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ msc@0 {
+ compatible = "arm,mpam-msc";
+ reg = <0x0 0x2000>;
+ arm,not-ready-us = <1>;
+ status = "disabled";
+ };
+
+ };
+
soc: soc@0 {
compatible = "simple-bus";
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH DNM v4 6/6] arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches
2026-10-28 11:45 ` [PATCH DNM v4 6/6] arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches Yin Li
@ 2026-10-28 12:05 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-10-28 12:05 UTC (permalink / raw)
To: Yin Li; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: kaanapali: undocumented qcom,kaanapali-l2-cache compatible
- [Medium] arm64: dts: qcom: kaanapali: invalid root L2 cache MMIO nodes and compatibles
--
commit b790aa2c6d3cc2130d835b988b9078c67a5328af
Author: Yin Li <yin.li@oss.qualcomm.com>
arm64: dts: qcom: kaanapali: Add MPAM MSC nodes for the L2 caches
This commit moves the L2 cache nodes l2_0 and l2_1 from the CPU nodes to the
root node in arch/arm64/boot/dts/qcom/kaanapali.dtsi and adds disabled child
arm,mpam-msc nodes for each L2 cache.
> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index e48a179d07220..10aee29a1ba8a 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
[ ... ]
> @@ -532,6 +520,46 @@ soccp_smp2p_in: slave-kernel {
> };
> };
>
> + l2_0: cache@19870000 {
> + compatible = "qcom,kaanapali-l2-cache", "cache";
> + cache-level = <2>;
> + cache-unified;
> + cache-size = <0xc00000>;
> + reg = <0x0 0x19870000 0x0 0x2000>;
> + ranges = <0x0 0x0 0x19870000 0x2000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + msc@0 {
> + compatible = "arm,mpam-msc";
> + reg = <0x0 0x2000>;
> + arm,not-ready-us = <1>;
> + status = "disabled";
> + };
> +
> + };
[Severity: Medium]
Does adding compatible = "qcom,kaanapali-l2-cache", "cache" to l2_0 and l2_1
in arch/arm64/boot/dts/qcom/kaanapali.dtsi cause dtbs_check and checkpatch.pl
regressions for kaanapali-mtp.dtb and kaanapali-qrd.dtb?
The compatible string "qcom,kaanapali-l2-cache" does not appear to be
documented in Documentation/devicetree/bindings/.
Without a binding schema for "qcom,kaanapali-l2-cache" (and using the node
name cache@... rather than cache-controller@... as shown in
Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml), dtbs_check flags
"qcom,kaanapali-l2-cache" as undocumented and validates l2_0 and l2_1 only
against the generic "cache" schema, which rejects the unit-address on
cache@19870000 and cache@1a870000 as well as the reg, ranges, #address-cells,
#size-cells, and msc@0 properties.
Should a DT binding schema be added for "qcom,kaanapali-l2-cache" (or an
existing documented cache-controller binding be used) before using it in
arch/arm64/boot/dts/qcom/kaanapali.dtsi?
> +
> + l2_1: cache@1a870000 {
> + compatible = "qcom,kaanapali-l2-cache", "cache";
> + cache-level = <2>;
> + cache-unified;
> + cache-size = <0xc00000>;
> + reg = <0x0 0x1a870000 0x0 0x2000>;
> + ranges = <0x0 0x0 0x1a870000 0x2000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + msc@0 {
> + compatible = "arm,mpam-msc";
> + reg = <0x0 0x2000>;
> + arm,not-ready-us = <1>;
> + status = "disabled";
> + };
> +
> + };
> +
> soc: soc@0 {
> compatible = "simple-bus";
[Severity: Medium]
Is it intentional to place the MMIO-mapped l2_0 (cache@19870000) and l2_1
(cache@1a870000) nodes directly under the root node rather than under soc@0,
and to assign the firmware-restricted 0x2000-byte MSC MMIO regions directly to
the enabled parent cache nodes via reg?
On Kaanapali, other MMIO devices in the 0x0..0x10_00000000 address range live
under soc@0:
arch/arm64/boot/dts/qcom/kaanapali.dtsi:
soc: soc@0 {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
dma-ranges = <0x0 0x0 0x0 0x0 0x10 0x0>;
ranges = <0x0 0x0 0x0 0x0 0x10 0x0>;
Because l2_0 and l2_1 are placed directly under the root node and remain
enabled while only their child msc@0 nodes are disabled,
of_platform_default_populate() -> of_platform_device_create_pdata() will
unconditionally instantiate platform devices (19870000.cache and
1a870000.cache) claiming the restricted MSC MMIO resources on every boot.
In addition, the commit message states:
"Each MSC is nested directly under its L2 cache node so that the cache node
is the MSC's parent device and its CPU affinity is derived from that cache."
and:
"This DTS is therefore only an example for other platform DT authors and is
not intended to be merged."
However, in drivers/resctrl/mpam_devices.c:mpam_dt_create_foundling_msc():
if (!of_platform_device_create(child, NULL, NULL))
pr_err("Failed to create MSC device for %pOF\n", child);
NULL is passed as the parent struct device, so the L2 cache node is only the
parent OF device_node rather than the MSC platform device's parent device.
Should the cache nodes omit duplicating the child MSC reg window on the
parent node (or remain under /cpus or /soc@0 as appropriate), and should this
patch be marked with an RFC prefix if it is not intended to be merged?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-mpam-resctrl-dt-knp-support-v4-0-bdda1e445cba@oss.qualcomm.com?part=6
^ permalink raw reply [flat|nested] 12+ messages in thread