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

[Lucas De Marchi]

On Thu, May 25, 2023 at 10:14:21PM -0300, Gustavo Sousa wrote:
>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()?

As long as xe_reg_sr_apply_mmio() is there, I think we could leave this
one in xe_reg_sr.c. If we are going to migrate it somewhere, then
probably xe_reg_whitelist.c would be a better home so the RING_FORCE_*
are all contained in that unit.... We could then add a func() action and 
remove XE_RTP_ACTION_WHITELIST() too. Not sure sure it's worth though.

Lucas De Marchi

>
>--
>Gustavo Sousa