linux-kernel - Re: [PATCH v4 1/3] platform/chrome: Add struct ec_response_get_next_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZmiTNEjxE-ZCotNu@google.com>
Date: Tue, 11 Jun 2024 11:11:00 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Daisuke Nojiri <dnojiri@...omium.org>
Cc: Benson Leung <bleung@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>,
	Guenter Roeck <groeck@...omium.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Reka Norman <rekanorman@...omium.org>,
	Hans Verkuil <hverkuil-cisco@...all.nl>,
	Pavan Holla <pholla@...omium.org>,
	Gwendal Grignou <gwendal@...omium.org>,
	Lukasz Majczak <lma@...omium.org>,
	Ching-Kang Yen <chingkang@...omium.org>,
	Stephen Boyd <swboyd@...omium.org>,
	Prashant Malani <pmalani@...omium.org>,
	chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-input@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 1/3] platform/chrome: Add struct
 ec_response_get_next_event_v3

Hi Daisuke,

On Fri, Jun 07, 2024 at 10:02:56AM -0700, Daisuke Nojiri wrote:
> Add struct ec_response_get_next_event_v3 to upgrade
> EC_CMD_GET_NEXT_EVENT to version 3.
> 
> Signed-off-by: Daisuke Nojiri <dnojiri@...omium.org>
> ---
>  .../linux/platform_data/cros_ec_commands.h    | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index 070e49c5381e..fff191a8d413 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -3527,6 +3527,34 @@ union __ec_align_offset1 ec_response_get_next_data_v1 {
>  };
>  BUILD_ASSERT(sizeof(union ec_response_get_next_data_v1) == 16);
>  
> +union __ec_align_offset1 ec_response_get_next_data_v3 {
> +	uint8_t key_matrix[18];
> +
> +	/* Unaligned */
> +	uint32_t host_event;
> +	uint64_t host_event64;
> +
> +	struct __ec_todo_unpacked {
> +		/* For aligning the fifo_info */
> +		uint8_t reserved[3];
> +		struct ec_response_motion_sense_fifo_info info;
> +	} sensor_fifo;
> +
> +	uint32_t buttons;
> +
> +	uint32_t switches;
> +
> +	uint32_t fp_events;
> +
> +	uint32_t sysrq;
> +
> +	/* CEC events from enum mkbp_cec_event */
> +	uint32_t cec_events;
> +
> +	uint8_t cec_message[16];
> +};
> +BUILD_ASSERT(sizeof(union ec_response_get_next_data_v3) == 18);
> +
>  struct ec_response_get_next_event {
>  	uint8_t event_type;
>  	/* Followed by event data if any */
> @@ -3539,6 +3567,12 @@ struct ec_response_get_next_event_v1 {
>  	union ec_response_get_next_data_v1 data;
>  } __ec_align1;
>  
> +struct ec_response_get_next_event_v3 {
> +	uint8_t event_type;
> +	/* Followed by event data if any */
> +	union ec_response_get_next_data_v3 data;
> +} __ec_align1;
> +

It is not really obvious that ec_response_get_next_event and
ec_response_get_next_event_v3 are layout compatible. I would simply
extend the union and add key_matrix_v3 field instead of defining
a brand new union.

And I would drop ec_response_get_next_event_v1 and added missing fields
to the original union as well...

Thanks.

-- 
Dmitry