From: Ben Horgan <ben.horgan@arm.com>
To: Andre Przywara <andre.przywara@arm.com>,
Lorenzo Pieralisi <lpieralisi@kernel.org>,
Hanjun Guo <guohanjun@huawei.com>,
Sudeep Holla <sudeep.holla@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Len Brown <lenb@kernel.org>, James Morse <james.morse@arm.com>,
Reinette Chatre <reinette.chatre@intel.com>,
Fenghua Yu <fenghuay@nvidia.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
Srivathsa L Rao <srivathsa.rao@oss.qualcomm.com>,
Ganapatrao Kulkarni <ganapatrao.kulkarni@oss.qualcomm.com>,
Trilok Soni <tsoni@quicinc.com>,
Srinivas Ramana <sramana@qti.qualcomm.com>,
Niyas Sait <niyas.sait@arm.com>,
linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 03/15] arm_mpam: propagate MSC read errors for hw_probe functions
Date: Wed, 1 Jul 2026 21:00:56 +0100 [thread overview]
Message-ID: <a628a34f-fd1a-4bd1-ba45-73d49922cc13@arm.com> (raw)
In-Reply-To: <20260702162229.4008659-4-andre.przywara@arm.com>
Hi Andre,
On 7/2/26 17:22, Andre Przywara wrote:
> Allow the functions probing for MSC hardware and features to return an
> error, and propagate read errors from the lower level up.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> drivers/resctrl/mpam_devices.c | 62 ++++++++++++++++++++++++----------
> 1 file changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index ce8738adb6ff..011d1e3544d7 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -800,20 +800,22 @@ static bool mpam_ris_hw_probe_csu_nrdy(struct mpam_msc_ris *ris)
> mpam_write_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
>
> _mpam_write_monsel_reg(msc, MSMON_CSU, MSMON___NRDY);
> - _mpam_read_monsel_reg(msc, MSMON_CSU, &now);
> + if (_mpam_read_monsel_reg(msc, MSMON_CSU, &now))
> + return false;
This early return leaves the mon_sel lock held.
> can_set = now & MSMON___NRDY;
As you're adding early returns to this function does it make sense to
return early if can_set is false?
Thanks,
Ben
>
> _mpam_write_monsel_reg(msc, MSMON_CSU, 0);
> /* Configuration change to try and coax hardware into setting nrdy */
> mpam_write_monsel_reg(msc, CFG_CSU_FLT, 0x1);
> - _mpam_read_monsel_reg(msc, MSMON_CSU, &now);
> + if (_mpam_read_monsel_reg(msc, MSMON_CSU, &now))
> + return false;
> can_clear = !(now & MSMON___NRDY);
> mpam_mon_sel_unlock(msc);
>
> return (!can_set || !can_clear);
> }
>
> -static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> +static int mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> {
> int err;
> struct mpam_msc *msc = ris->vmsc->msc;
> @@ -828,7 +830,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> if (FIELD_GET(MPAMF_IDR_HAS_CCAP_PART, ris->idr)) {
> u32 ccap_features;
>
> - mpam_read_partsel_reg(msc, CCAP_IDR, &ccap_features);
> + err = mpam_read_partsel_reg(msc, CCAP_IDR, &ccap_features);
> + if (err)
> + return err;
>
> props->cmax_wd = FIELD_GET(MPAMF_CCAP_IDR_CMAX_WD, ccap_features);
> if (props->cmax_wd &&
> @@ -853,7 +857,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> if (FIELD_GET(MPAMF_IDR_HAS_CPOR_PART, ris->idr)) {
> u32 cpor_features;
>
> - mpam_read_partsel_reg(msc, CPOR_IDR, &cpor_features);
> + err = mpam_read_partsel_reg(msc, CPOR_IDR, &cpor_features);
> + if (err)
> + return err;
>
> props->cpbm_wd = FIELD_GET(MPAMF_CPOR_IDR_CPBM_WD, cpor_features);
> if (props->cpbm_wd)
> @@ -863,8 +869,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> /* Memory bandwidth partitioning */
> if (FIELD_GET(MPAMF_IDR_HAS_MBW_PART, ris->idr)) {
> u32 mbw_features;
> -
> - mpam_read_partsel_reg(msc, MBW_IDR, &mbw_features);
> + err = mpam_read_partsel_reg(msc, MBW_IDR, &mbw_features);
> + if (err)
> + return err;
>
> /* portion bitmap resolution */
> props->mbw_pbm_bits = FIELD_GET(MPAMF_MBW_IDR_BWPBM_WD, mbw_features);
> @@ -893,8 +900,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> /* Priority partitioning */
> if (FIELD_GET(MPAMF_IDR_HAS_PRI_PART, ris->idr)) {
> u32 pri_features;
> -
> - mpam_read_partsel_reg(msc, PRI_IDR, &pri_features);
> + err = mpam_read_partsel_reg(msc, PRI_IDR, &pri_features);
> + if (err)
> + return err;
>
> props->intpri_wd = FIELD_GET(MPAMF_PRI_IDR_INTPRI_WD, pri_features);
> if (props->intpri_wd && FIELD_GET(MPAMF_PRI_IDR_HAS_INTPRI, pri_features)) {
> @@ -915,7 +923,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> if (FIELD_GET(MPAMF_IDR_HAS_MSMON, ris->idr)) {
> u32 msmon_features;
>
> - mpam_read_partsel_reg(msc, MSMON_IDR, &msmon_features);
> + err = mpam_read_partsel_reg(msc, MSMON_IDR, &msmon_features);
> + if (err)
> + return err;
>
> /*
> * If the firmware max-nrdy-us property is missing, the
> @@ -928,7 +938,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> if (FIELD_GET(MPAMF_MSMON_IDR_MSMON_CSU, msmon_features)) {
> u32 csumonidr;
>
> - mpam_read_partsel_reg(msc, CSUMON_IDR, &csumonidr);
> + err = mpam_read_partsel_reg(msc, CSUMON_IDR, &csumonidr);
> + if (err)
> + return err;
>
> props->num_csu_mon = FIELD_GET(MPAMF_CSUMON_IDR_NUM_MON, csumonidr);
> if (props->num_csu_mon) {
> @@ -954,7 +966,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> bool has_long;
> u32 mbwumon_idr;
>
> - mpam_read_partsel_reg(msc, MBWUMON_IDR, &mbwumon_idr);
> + err = mpam_read_partsel_reg(msc, MBWUMON_IDR, &mbwumon_idr);
> + if (err)
> + return err;
>
> props->num_mbwu_mon = FIELD_GET(MPAMF_MBWUMON_IDR_NUM_MON, mbwumon_idr);
> if (props->num_mbwu_mon) {
> @@ -987,16 +1001,22 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> u16 partid_max;
> u32 nrwidr;
>
> - mpam_read_partsel_reg(msc, PARTID_NRW_IDR, &nrwidr);
> + err = mpam_read_partsel_reg(msc, PARTID_NRW_IDR, &nrwidr);
> + if (err)
> + return err;
> +
> partid_max = FIELD_GET(MPAMF_PARTID_NRW_IDR_INTPARTID_MAX, nrwidr);
>
> mpam_set_feature(mpam_feat_partid_nrw, props);
> msc->partid_max = min(msc->partid_max, partid_max);
> }
> +
> + return 0;
> }
>
> static int mpam_msc_hw_probe(struct mpam_msc *msc)
> {
> + int ret;
> u64 idr;
> u16 partid_max;
> u8 ris_idx, pmg_max;
> @@ -1012,10 +1032,12 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>
> /* Grab an IDR value to find out how many RIS there are */
> mutex_lock(&msc->part_sel_lock);
> - mpam_msc_read_idr(msc, &idr);
> - mpam_read_partsel_reg(msc, IIDR, &msc->iidr);
> -
> + ret = mpam_msc_read_idr(msc, &idr);
> + if (!ret)
> + ret = mpam_read_partsel_reg(msc, IIDR, &msc->iidr);
> mutex_unlock(&msc->part_sel_lock);
> + if (ret)
> + return ret;
>
> mpam_enable_quirks(msc);
>
> @@ -1028,8 +1050,10 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
> for (ris_idx = 0; ris_idx <= msc->ris_max; ris_idx++) {
> mutex_lock(&msc->part_sel_lock);
> __mpam_part_sel(ris_idx, 0, msc);
> - mpam_msc_read_idr(msc, &idr);
> + ret = mpam_msc_read_idr(msc, &idr);
> mutex_unlock(&msc->part_sel_lock);
> + if (ret)
> + return ret;
>
> partid_max = FIELD_GET(MPAMF_IDR_PARTID_MAX, idr);
> pmg_max = FIELD_GET(MPAMF_IDR_PMG_MAX, idr);
> @@ -1046,8 +1070,10 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>
> mutex_lock(&msc->part_sel_lock);
> __mpam_part_sel(ris_idx, 0, msc);
> - mpam_ris_hw_probe(ris);
> + ret = mpam_ris_hw_probe(ris);
> mutex_unlock(&msc->part_sel_lock);
> + if (ret)
> + return ret;
> }
>
> /* Clear any stale errors */
next prev parent reply other threads:[~2026-07-03 8:42 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-02 16:22 [PATCH v2 00/15] arm_mpam: Add MPAM-Fb firmware support Andre Przywara
2026-07-02 16:22 ` [PATCH v2 01/15] arm_mpam: let low level MSC read accessors return an error Andre Przywara
2026-07-02 16:22 ` [PATCH v2 02/15] arm_mpam: propagate MSC read errors for wrapper functions Andre Przywara
2026-07-01 19:56 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 03/15] arm_mpam: propagate MSC read errors for hw_probe functions Andre Przywara
2026-07-01 20:00 ` Ben Horgan [this message]
2026-07-02 16:22 ` [PATCH v2 04/15] arm_mpam: propagate MSC read errors for mpam_msc_read_mbwu_l() Andre Przywara
2026-07-01 20:06 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 05/15] arm_mpam: propagate MSC read errors for msmon helpers Andre Przywara
2026-07-02 16:22 ` [PATCH v2 06/15] arm_mpam: propagate MSC read errors for __ris_msmon_read() Andre Przywara
2026-07-01 20:14 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 07/15] arm_mpam: propagate MSC read errors for state saving functions Andre Przywara
2026-07-01 20:19 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 08/15] arm_mpam: let low level MSC write accessors return an error Andre Przywara
2026-07-02 16:22 ` [PATCH v2 09/15] arm_mpam: propagate MSC write errors for ESR and part_sel wrappers Andre Przywara
2026-07-02 16:22 ` [PATCH v2 10/15] arm_mpam: propagate MSC write errors for hardware probe functions Andre Przywara
2026-07-02 16:22 ` [PATCH v2 11/15] arm_mpam: propagate MSC write errors for remaining MSC write users Andre Przywara
2026-07-02 16:22 ` [PATCH v2 12/15] arm_mpam: Split the locking around the mon_sel registers Andre Przywara
2026-07-01 21:01 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 13/15] arm_mpam: add MPAM-Fb MSC firmware access support Andre Przywara
2026-07-02 16:22 ` [PATCH v2 14/15] arm_mpam: prevent MPAM-Fb accesses inside IRQ handler Andre Przywara
2026-07-03 10:54 ` Ben Horgan
2026-07-02 16:22 ` [PATCH v2 15/15] arm_mpam: detect and enable MPAM-Fb PCC support Andre Przywara
2026-07-03 11:00 ` Ben Horgan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a628a34f-fd1a-4bd1-ba45-73d49922cc13@arm.com \
--to=ben.horgan@arm.com \
--cc=andre.przywara@arm.com \
--cc=catalin.marinas@arm.com \
--cc=fenghuay@nvidia.com \
--cc=ganapatrao.kulkarni@oss.qualcomm.com \
--cc=guohanjun@huawei.com \
--cc=james.morse@arm.com \
--cc=jic23@kernel.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=niyas.sait@arm.com \
--cc=rafael@kernel.org \
--cc=reinette.chatre@intel.com \
--cc=sramana@qti.qualcomm.com \
--cc=srivathsa.rao@oss.qualcomm.com \
--cc=sudeep.holla@kernel.org \
--cc=tsoni@quicinc.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox