RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member"

[Winkler, Tomas]

> -----Original Message-----
> From: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx>
> Sent: Wednesday, July 29, 2020 01:15
> To: Winkler, Tomas <tomas.winkler@xxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Usyskin, Alexander <alexander.usyskin@xxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; Gustavo A . R . Silva <gustavoars@xxxxxxxxxx>
> Subject: Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array
> with flexible-array member"
> 
> 
> 
> On 7/28/20 17:01, Winkler, Tomas wrote:
> >
> >
> >> -----Original Message-----
> >> From: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx>
> >> Sent: Wednesday, July 29, 2020 00:57
> >> To: Winkler, Tomas <tomas.winkler@xxxxxxxxx>; Greg Kroah-Hartman
> >> <gregkh@xxxxxxxxxxxxxxxxxxx>
> >> Cc: Usyskin, Alexander <alexander.usyskin@xxxxxxxxx>; linux-
> >> kernel@xxxxxxxxxxxxxxx; Gustavo A . R . Silva <gustavoars@xxxxxxxxxx>
> >> Subject: Re: [char-misc-next] Revert "mei: hdcp: Replace one-element
> >> array with flexible-array member"
> >>
> >> Hi Tomas,
> >>
> >> On 7/28/20 16:41, Tomas Winkler wrote:
> >>> Greg please revert, this commit it changes size of struct
> >>> wired_cmd_repeater_auth_stream_req_in, this is not what firmware is
> >>> expecting.
> >>
> >> Could you elaborate on what's the firmware expecting, exactly?
> > struct wired_cmd_repeater_auth_stream_req_in {
> >   -	struct hdcp2_streamid_type	streams[1];
> >   + 	struct hdcp2_streamid_type	streams[];
> > }
> >
> > But then you have, which you haven't changed to + 1 byte =
> > mei_cldev_send(cldev, (u8 *)&verify_mprime_in,
> >                               sizeof(verify_mprime_in));
> >
> 
> I see, this is the kind of feedback I need from people that knows the code
> better. Thanks!
> 
> > But that's not the major point. Point is that we should be able to
> > review and test the code before it is merged.  You haven't run it, right?
> > There is MAINTAINERS file for a reason.
> >
> 
> I'm using this command:
> 
> $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback
> 
> and this is the output for both files:
> 
> $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback
> drivers/misc/mei/hdcp/mei_hdcp.c Arnd Bergmann <arnd@xxxxxxxx>
> (supporter:CHAR and MISC DRIVERS) Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> (supporter:CHAR and MISC DRIVERS) linux-
> kernel@xxxxxxxxxxxxxxx (open list) $ scripts/get_maintainer.pl --nokeywords --
> nogit --nogit-fallback drivers/misc/mei/hdcp/mei_hdcp.h Arnd Bergmann
> <arnd@xxxxxxxx> (supporter:CHAR and MISC DRIVERS) Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> (supporter:CHAR and MISC DRIVERS) linux-
> kernel@xxxxxxxxxxxxxxx (open list)


> For some reason you don't appear on the list above. Do you see anything
> wrong with the command I'm running to get the maintainers and lists?

Not sure, it should be caught by drivers/misc/mei/* ?  
Maybe it is not recursive? Need to check the script, 
frankly I usually check this manually. 

INTEL MANAGEMENT ENGINE (mei)
M:      Tomas Winkler <tomas.winkler@xxxxxxxxx>
L:      linux-kernel@xxxxxxxxxxxxxxx
S:      Supported
F:      Documentation/driver-api/mei/*
F:      drivers/misc/mei/*
F:      drivers/watchdog/mei_wdt.c
F:      include/linux/mei_cl_bus.h
F:      include/uapi/linux/mei.h
F:      samples/mei/*


> 
> Thanks
> --
> Gustavo
> 
> >>
> >>> I really do not appreciate that the code is bypassing driver
> >>> maintaner review, I think this is a minimum we can ask for, this is
> >>> not for a first time.
> >>>
> >>> This reverts commit c56967d674e361ebe716e66992e3c5332b25ac1f.
> >>>
> >>> Cc: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
> >>> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> >>> ---
> >>>  drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
> >>> drivers/misc/mei/hdcp/mei_hdcp.h | 2 +-
> >>>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> b/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> index d1d3e025ca0e..e6c3dc595617 100644
> >>> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> @@ -572,7 +572,7 @@ static int mei_hdcp_verify_mprime(struct device
> >> *dev,
> >>>  	       HDCP_2_2_MPRIME_LEN);
> >>>  	drm_hdcp_cpu_to_be24(verify_mprime_in.seq_num_m, data-
> seq_num_m);
> >>>  	memcpy(verify_mprime_in.streams, data->streams,
> >>> -	       array_size(data->k, sizeof(*data->streams)));
> >>> +	       (data->k * sizeof(struct hdcp2_streamid_type)));
> >>>
> >>>  	verify_mprime_in.k = cpu_to_be16(data->k);
> >>>
> >>> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> b/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> index 834757f5e072..18ffc773fa18 100644
> >>> --- a/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> +++ b/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> @@ -358,7 +358,7 @@ struct wired_cmd_repeater_auth_stream_req_in
> {
> >>>  	u8
> >> 	seq_num_m[HDCP_2_2_SEQ_NUM_LEN];
> >>>  	u8				m_prime[HDCP_2_2_MPRIME_LEN];
> >>>  	__be16				k;
> >>> -	struct hdcp2_streamid_type	streams[];
> >>> +	struct hdcp2_streamid_type	streams[1];
> >>>  } __packed;
> >>>
> >>>  struct wired_cmd_repeater_auth_stream_req_out {
> >>>