linux-kernel - Re: [PATCH] perf/arm-cmn: Fix and refactor device mapping resource

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: <Z7QYlUP6nfBNMXsv@U-V2QX163P-2032.local>
Date: Tue, 18 Feb 2025 13:20:21 +0800
From: YinFengwei <fengwei_yin@...ux.alibaba.com>
To: Robin Murphy <robin.murphy@....com>
Cc: Will Deacon <will@...nel.org>, Jing Zhang <renyu.zj@...ux.alibaba.com>,
	ilkka@...amperecomputing.com, Mark Rutland <mark.rutland@....com>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Shuai Xue <xueshuai@...ux.alibaba.com>,
	Zhuo Song <zhuo.song@...ux.alibaba.com>,
	jie.li.linux@...ux.alibaba.com
Subject: Re: [PATCH] perf/arm-cmn: Fix and refactor device mapping resource

Hi Robin,

On Mon, Mar 27, 2023 at 03:27:39PM +0100, Robin Murphy wrote:
> On 2023-03-27 15:05, Will Deacon wrote:
> > [+Robin and Ilkka, as they contribute most to this driver]
> > 
> > On Thu, Feb 16, 2023 at 04:17:50PM +0800, Jing Zhang wrote:
> > > The devm_platform_ioremap_resource() won't let the platform device
> > > claim resource when the ACPI companion device has already claimed it.
> > > If CMN-ANY except CMN600 is ACPI companion device, it will return
> > > -EBUSY in devm_platform_ioremap_resource(), and the driver cannot be
> > > successfully installed.
> > > 
> > > So let ACPI companion device call arm_cmn_acpi_probe and not claim
> > > resource again. In addition, the arm_cmn_acpi_probe() and
> > > arm_cmn_of_probe() functions are refactored to make them compatible
> > > with both CMN600 and CMN-ANY.
> 
> No, the whole point of CMN-600 probing being a special case is that the ACPI
> and DT bindings for CMN-600 are special cases. In ACPI, only ARMHC600 has
> the two nested memory resources; all the other models should only have one
> memory resource because one is all that is meaningful. See table 16 the
> document[1] in where the description of ROOTNODEBASE says "This field is
> specific to the CMN-600 device object."
> 
> Similarly in DT, "arm,root-node" is only required for "arm,cmn-600" - it
> didn't seem worth overcomplicating the schema to actively disallow it for
> other models, but that is supposed to be implied by its description as "not
> relevant for newer CMN/CI products".
> 
> If you're hitting this because you've written your ACPI DSDT incorrectly,
> it's a sign that you should fix your DSDT.
> 
What about following change? Thanks.

-static int arm_cmn600_acpi_probe(struct platform_device *pdev, struct arm_cmn *cmn)
+static int arm_acpi_probe(struct platform_device *pdev, struct arm_cmn *cmn)
 {
-       struct resource *cfg, *root;
+       struct resource *cfg, *root = NULL;

        cfg = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        if (!cfg)
                return -EINVAL;

-       root = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-       if (!root)
-               return -EINVAL;
+       if (cmn->part == PART_CMN600) {
+               /* Per "ACPI for Arm Components" Table 16, CMN600 has
+                * nested root node memory range.
+                */
+               root = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+               if (!root)
+                       return -EINVAL;
+
+               if (!resource_contains(cfg, root))
+                       swap(cfg, root);
+       }

-       if (!resource_contains(cfg, root))
-               swap(cfg, root);
        /*
         * Note that devm_ioremap_resource() is dumb and won't let the platform
         * device claim cfg when the ACPI companion device has already claimed
@@ -2534,7 +2540,7 @@ static int arm_cmn600_acpi_probe(struct platform_device *pdev, struct arm_cmn *c
        if (!cmn->base)
                return -ENOMEM;

-       return root->start - cfg->start;
+       return root ? (root->start - cfg->start) : 0;
 }


Regards
Yin, Fengwei