On Wed, Nov 09, 2022 at 06:45:57PM -0600, Gustavo A. R. Silva wrote:
> On Wed, Nov 09, 2022 at 04:45:42PM +1300, Paulo Miguel Almeida wrote:
>
> Adding Alex, Christian and DRM lists to the thread.
Thanks so much for your reply Gustavo
Yep, that's a good idea.
>
> > struct _ATOM_INIT_REG_BLOCK {
> > USHORT usRegIndexTblSize; /* 0 2 */
> > USHORT usRegDataBlkSize; /* 2 2 */
> > ATOM_INIT_REG_INDEX_FORMAT asRegIndexBuf[1]; /* 4 3 */
> > ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[1]; /* 7 8 */
>
> I didn't find evidence that asRegDataBuf is used anywhere in the
> codebase[1].
> ...
> <snip>
> ...
> As I pointed out above, I don't see asRegDataBuf[] being used in the
> codebase (of course it may describe firmware memory layout). Instead,
> there is this jump to a block of data past asRegIndexBuf[]:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1444:
> 1444: ATOM_MEMORY_SETTING_DATA_BLOCK *reg_data =
> 1445: (ATOM_MEMORY_SETTING_DATA_BLOCK *)
> 1446: ((u8 *)reg_block + (2 * sizeof(u16)) +
> 1447: le16_to_cpu(reg_block->usRegIndexTblSize));
>
> So, it seems the one relevant array, from the kernel side, is
> asRegIndexBuf[]. I wonder if we really need asRegDataBuf[] in that
> structure... or if we could try modifying that struct and only have
> asRegIndexBuf[] as last member? and then we can transform it into a
> flex-array member.
I saw that one too. That would be the way it's currently accessing
asRegDataBuf member as the existing struct would make asRegDataBuf[0] point
to some index within the asRegIndexBuf member (as you probably got it too)
you are right... asRegDataBuff isn't used from a static analysis
point of view but removing it make the code a bit cryptic, right?
That's pickle, ay? :-)
>
> If for any strong reasong we cannot remove asRegDataBuf[] then I think we
> could give it a try and use DECLARE_FLEX_ARRAY() to declare both arrays
> in the structure.
>
Out of curiosity, why both rather than just asRegIndexBuf?
> But first, of course, Alex, Christian, it'd be really great if we can
> have your input and feedback. :)
>
> Thanks!
> --
> Gustavo
>
- Paulo A.
