[Intel-xe] [PATCH] drm/xe/reg_sr: Apply limit to register whitelisting

[Gustavo Sousa]

Quoting Matt Roper (2023-05-25 21:10:38-03:00)
>On Thu, May 25, 2023 at 08:24:06PM -0300, Gustavo Sousa wrote:
>> If RING_MAX_NONPRIV_SLOTS denotes the maximum number of whitelisting
>> slots, then it makes sense to refuse going above it.
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_reg_sr.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
>> index 0312823101ad..b82f704179a0 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
>> @@ -231,6 +231,13 @@ void xe_reg_sr_apply_whitelist(struct xe_reg_sr *sr, u32 mmio_base,
>>  
>>          p = drm_debug_printer(KBUILD_MODNAME);
>>          xa_for_each(&sr->xa, reg, entry) {
>> +                if (slot == RING_MAX_NONPRIV_SLOTS) {
>> +                        drm_err(&xe->drm,
>
>We probably want xe_gt_err() here so that we'll know which GT we hit
>this error on.  Also, it wouldn't hurt to pass hwe as a parameter to
>this function (instead of mmio_base) and include hwe->name in the error
>message here so that we also know which engine ran out of slots.

Yeah, makes sense. Arguably, we could simply pass hwe as a single
parameter and figure out the rest. Is there any reason not to do that?

If the above is something reasonable, I wonder if xe_reg_sr.c is really
the correct place to have this function. Maybe it should be something
like xe_hw_engine_apply_whilelist()?

--
Gustavo Sousa