[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48556d36-947b-4e07-aa7e-602bb9fe052d@arm.com>
Date: Thu, 7 Aug 2025 19:26:19 +0100
From: James Morse <james.morse@....com>
To: Ben Horgan <ben.horgan@....com>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: Rob Herring <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
Shanker Donthineni <sdonthineni@...dia.com>, Zeng Heng
<zengheng4@...wei.com>, Lecopzer Chen <lecopzerc@...dia.com>,
Carl Worth <carl@...amperecomputing.com>,
shameerali.kolothum.thodi@...wei.com,
D Scott Phillips OS <scott@...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>, Rex Nie <rex.nie@...uarmicro.com>,
Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>
Subject: Re: [RFC PATCH 20/36] arm_mpam: Probe the hardware features resctrl
supports
Hi Ben,
On 24/07/2025 16:08, Ben Horgan wrote:
> On 7/11/25 19:36, James Morse wrote:
>> Expand the probing support with the control and monitor types
>> we can use with resctrl.
>> diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/
>> mpam_devices.c
>> index 8646fb85ad09..61911831ab39 100644
>> --- a/drivers/platform/arm64/mpam/mpam_devices.c
>> +++ b/drivers/platform/arm64/mpam/mpam_devices.c
>> @@ -645,6 +659,137 @@ static struct mpam_msc_ris *mpam_get_or_create_ris(struct mpam_msc
>> *msc,
>> return found;
>> }
>> +/*
>> + * IHI009A.a has this nugget: "If a monitor does not support automatic behaviour
>> + * of NRDY, software can use this bit for any purpose" - so hardware might not
>> + * implement this - but it isn't RES0.
>> + *
>> + * Try and see what values stick in this bit. If we can write either value,
>> + * its probably not implemented by hardware.
>> + */
>> +#define mpam_ris_hw_probe_hw_nrdy(_ris, _mon_reg, _result) \
>> +do { \
>> + u32 now; \
>> + u64 mon_sel; \
>> + bool can_set, can_clear; \
>> + struct mpam_msc *_msc = _ris->vmsc->msc; \
>> + \
>> + if (WARN_ON_ONCE(!mpam_mon_sel_inner_lock(_msc))) { \
>> + _result = false; \
>> + break; \
>> + } \
>> + mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, 0) | \
>> + FIELD_PREP(MSMON_CFG_MON_SEL_RIS, _ris->ris_idx); \
>> + mpam_write_monsel_reg(_msc, CFG_MON_SEL, mon_sel); \
>> + \
>> + mpam_write_monsel_reg(_msc, _mon_reg, MSMON___NRDY); \
>> + now = mpam_read_monsel_reg(_msc, _mon_reg); \
>> + can_set = now & MSMON___NRDY; \
>> + \
>> + mpam_write_monsel_reg(_msc, _mon_reg, 0); \
>> + now = mpam_read_monsel_reg(_msc, _mon_reg); \
>> + can_clear = !(now & MSMON___NRDY); \
>> + mpam_mon_sel_inner_unlock(_msc); \
>> + \
>> + _result = (!can_set || !can_clear); \
>> +} while (0)
> It is a bit surprising that something that looks like a function modifies a boolean passed
> by value.
> Consider continuing the pattern you have above:
> #define mpam_ris_hw_probe_hw_nrdy(_ris, _mon_reg, _result)
> _mpam_ris_hw_probe_hw_nrdy(_ris, MSMON##_mon_reg, _result)
>
> with signature:
> void _mpam_ris_hw_probe_hw_nrdy(struct mpam_msc *msc, u16 reg, bool *hw_managed);
>
> and using the _mpam functions from the new _mpam_ris_hw_probe_hw_nrdy().
With that, it may as well return the result.
Done.
Thanks,
James
Powered by blists - more mailing lists