[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <183a54d5-0f07-4d37-915a-55ff9ab0e86d@nvidia.com>
Date: Wed, 22 Oct 2025 14:00:46 -0500
From: Tushar Dave <tdave@...dia.com>
To: Fenghua Yu <fenghuay@...dia.com>, James Morse <james.morse@....com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-acpi@...r.kernel.org
Cc: D Scott Phillips OS <scott@...amperecomputing.com>,
carl@...amperecomputing.com, lcherian@...vell.com,
bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
dfustini@...libre.com, amitsinght@...vell.com,
David Hildenbrand <david@...hat.com>, Dave Martin <dave.martin@....com>,
Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
baisheng.gao@...soc.com, Jonathan Cameron <jonathan.cameron@...wei.com>,
Rob Herring <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
Rafael Wysocki <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Hanjun Guo
<guohanjun@...wei.com>, Sudeep Holla <sudeep.holla@....com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, Jeremy Linton <jeremy.linton@....com>,
Gavin Shan <gshan@...hat.com>
Subject: Re: [PATCH v3 07/29] arm_mpam: Add probe/remove for mpam msc driver
and kbuild boiler plate
On 10/21/25 7:29 PM, Fenghua Yu wrote:
> Hi, James,
>
> On 10/17/25 11:56, James Morse wrote:
>> Probing MPAM is convoluted. MSCs that are integrated with a CPU may
>> only be accessible from those CPUs, and they may not be online.
>> Touching the hardware early is pointless as MPAM can't be used until
>> the system-wide common values for num_partid and num_pmg have been
>> discovered.
>>
>> Start with driver probe/remove and mapping the MSC.
>>
>> CC: Carl Worth <carl@...amperecomputing.com>
>> Tested-by: Fenghua Yu <fenghuay@...dia.com>
>> Signed-off-by: James Morse <james.morse@....com>
> [SNIP]> +/*
>> + * An MSC can control traffic from a set of CPUs, but may only be accessible
>> + * from a (hopefully wider) set of CPUs. The common reason for this is power
>> + * management. If all the CPUs in a cluster are in PSCI:CPU_SUSPEND, the
>> + * corresponding cache may also be powered off. By making accesses from
>> + * one of those CPUs, we ensure this isn't the case.
>> + */
>> +static int update_msc_accessibility(struct mpam_msc *msc)
>> +{
>> + u32 affinity_id;
>> + int err;
>> +
>> + 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 err;
>
> The error is handled and there is no need to return the error to caller.
> Returning the error causes probe failure and the mpam_msc driver cannot be
> installed.
Ack. I see the probe failure too.
e.g.
[ 7.118297] mpam_msc mpam_msc.183: probe with driver mpam_msc failed with
error -22
[ 7.118383] mpam_msc mpam_msc.370: probe with driver mpam_msc failed with
error -22
[ 10.208127] # Subtest: mpam_devices_test_suite
[ 10.208129] # module: mpam
[ 10.208215] ok 1 test_mpam_reset_msc_bitmap
[ 10.208275] mpam:__props_mismatch: __props_mismatch took the min cmax_wd
[ 10.208285] mpam:__props_mismatch: cleared cpor_part
[ 10.208287] mpam:__props_mismatch: cleared mbw_part
[ 10.208294] mpam:__props_mismatch: took the min bwa_wd
[ 10.208296] mpam:__props_mismatch: __props_mismatch took the min cmax_wd
[ 10.208310] mpam:__props_mismatch: __props_mismatch took the min cmax_wd
[ 10.208345] ok 2 test_mpam_enable_merge_features
[ 10.208411] # mpam_devices_test_suite: pass:3 fail:0 skip:0 total:3
[ 10.208413] ok 1 mpam_devices_test_suite
>
> s/return err;/return 0;/
Yes, this resolve the probe failure.
Tested-by: Tushar Dave <tdave@...dia.com>
>
>> +}
>> +
>> +static int fw_num_msc;
>> +
>> +static void mpam_msc_destroy(struct mpam_msc *msc)
>> +{
>> + struct platform_device *pdev = msc->pdev;
>> +
>> + lockdep_assert_held(&mpam_list_lock);
>> +
>> + list_del_rcu(&msc->all_msc_list);
>> + platform_set_drvdata(pdev, NULL);
>> +}
>> +
>> +static void mpam_msc_drv_remove(struct platform_device *pdev)
>> +{
>> + struct mpam_msc *msc = platform_get_drvdata(pdev);
>> +
>> + if (!msc)
>> + return;
>> +
>> + mutex_lock(&mpam_list_lock);
>> + mpam_msc_destroy(msc);
>> + mutex_unlock(&mpam_list_lock);
>> +
>> + synchronize_srcu(&mpam_srcu);
>> +}
>> +
>> +static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
>> +{
>> + int err;
>> + u32 tmp;
>> + struct mpam_msc *msc;
>> + struct resource *msc_res;
>> + struct device *dev = &pdev->dev;
>> +
>> + lockdep_assert_held(&mpam_list_lock);
>> +
>> + msc = devm_kzalloc(&pdev->dev, sizeof(*msc), GFP_KERNEL);
>> + if (!msc)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + mutex_init(&msc->probe_lock);
>> + mutex_init(&msc->part_sel_lock);
>> + msc->id = pdev->id;
>> + msc->pdev = pdev;
>> + INIT_LIST_HEAD_RCU(&msc->all_msc_list);
>> + INIT_LIST_HEAD_RCU(&msc->ris);
>> +
>> + err = update_msc_accessibility(msc);
>> + if (err)
>> + return ERR_PTR(err);
>
> The returned error causes probe failure and the driver cannot be installed.
> Return 0 will make the probe succeed.
>
> There is no probe failure in mpam/snapshot/v6.18-rc1 because its returned err=0.
>
> [SNIP]
>
> Thanks.
>
> -Fenghua
>
Powered by blists - more mailing lists