On 4/12/2023 11:50 AM, Marijn Suijten wrote:
On 2023-04-12 10:48:18, Abhinav Kumar wrote:
[..]
The only way to trigger this newly introduced range check is by omitting
the DSC_x constants and manually writing e.g. an out-of-range value 10
here, or setting DSC_NONE. This is only allowed for interfaces.
Correct, its just working on an implicit understanding that the indices
in the catalog
.. this sentence appears to be incomplete: what did you want to say? ..
Its complete.
"Correct, its just working on an implicit understanding that the indices
in the catalog which might still be right stick to the RM limits.
Thats why this is not bad to have."
which might still be right stick to the RM limits.
Thats why this is not bad to have.
What do you mean by "RM limits"? We have constants in the kernel that
both define the maximum number of blocks in these arrays and a
predefined set of ids that block can have. These are all used in
constant structs in the catalog, so there's nothing "software" or
SoC-specific limiting about this (except what is available in the
arrays).
WB_MAX, DSC_MAX, LM_MAX etc are RM limits not catalog limits.
For example, LM_MAX is 8 but in the future if could have a HW which has
10 LMs. That time if LM_MAX is not increased, its just a SW number.
Catalog on the other hand, can still list 10 LMs but with the catch that
it uses the indices from the rm. So its just an implicit understanding
here that catalog uses indices from RM.
Nothing prevents someone from manually adding an entry and forgetting to
update the *_MAX in the RM.
Although, yes we will catch that in reviews.
[..]
I think kuogee just added this to keep it consistent with other checks
present in the RM. So I didnt see any harm with that.
Yep, that's the only reason
If he did see an issue, i will let him report that here.
If so an out-of-bounds constant was hardcoded in dpu_hw_catalog.c.
Otherwise, I dont want to spend more time discussing this bounds check
when other blocks already have it.
I'll whip up a patch to clear out the extraneous lookup (assuming there
is no other reason/dependency for it to be there...) and can follow that
up with removing these range checks of known-good values in `const
struct` fields.
Lets see what you have in mind. As I said, I am not too obsessed with
this patch. So i dont want to spend more time convincing why it should
be there.
- Marijn
