[<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
 
