HSI: cmt_speech: fix timestamp interface

The user interface for timestamps in the new cms_speech driver is broken in multiple ways:

- The layout is incompatible between 32-bit and 64-bit user space, because of the size differences in 'struct timespec'. This means that the driver can not work when used with 32-bit user space on a 64-bit kernel.

- As there are plans to change 32-bit user space to use a 64-bit time_t type in the future, it will also be incompatible with new 32-bit user space.

As the driver is not yet widely used, now would be a good time to change it to just use a 64-bit nanosecond value, which also happens to be more efficient. In order to make the layout non-ambiguous, we have to also add extra padding so the 64-bit value is naturally aligned.

The change requires the respective update to user space tools using it. If that is for some reason not possible any more, another solution would be to replace the timespec structure with two __u32 values to avoid incompatibilities later.

Signed-off-by Arnd Bergmann <[email protected]>

drivers/hsi/clients/cmt_speech.c +1 -3

index 4983529a9c6c..568dbc117f2e 100644
@@ -451,9 +451,7 @@ static void cs_hsi_read_on_control_complete(struct hsi_msg *msg)
 	dev_dbg(&hi->cl->device, "Read on control: %08X\n", cmd);
 	cs_release_cmd(msg);
 	if (hi->flags & CS_FEAT_TSTAMP_RX_CTRL) {
-		struct timespec *tstamp =
-			&hi->mmap_cfg->tstamp_rx_ctrl;
-		do_posix_clock_monotonic_gettime(tstamp);
+		hi->mmap_cfg->tstamp_rx_ctrl = ktime_get_ns();
 	}
 	spin_unlock(&hi->lock);
 

include/uapi/linux/hsi/cs-protocol.h +2 -2

index 4957bba57cbe..fce633c7c649 100644
@@ -90,12 +90,12 @@ struct cs_mmap_config_block {
 	__u32 tx_offsets[CS_MAX_BUFFERS];
 	__u32 rx_ptr;
 	__u32 rx_ptr_boundary;
-	__u32 reserved3[2];
+	__u32 reserved3[3];
 	/*
 	 * if enabled with CS_FEAT_TSTAMP_RX_CTRL, monotonic
 	 * timestamp taken when the last control command was received
 	 */
-	struct timespec tstamp_rx_ctrl;
+	__u64 tstamp_rx_ctrl;
 };
 
 #define CS_IO_MAGIC		'C'

Quoted 21 lines

- The layout is incompatible between 32-bit and 64-bit user space, because of the size differences in 'struct timespec'. This means that the driver can not work when used with 32-bit user space on a 64-bit kernel.

- As there are plans to change 32-bit user space to use a 64-bit time_t type in the future, it will also be incompatible with new 32-bit user space.

As the driver is not yet widely used, now would be a good time to change it to just use a 64-bit nanosecond value, which also happens to be more efficient. In order to make the layout non-ambiguous, we have to also add extra padding so the 64-bit value is naturally aligned.

The change requires the respective update to user space tools using it. If that is for some reason not possible any more, another solution would be to replace the timespec structure with two __u32 values to avoid incompatibilities later.

Quoted 1 line

Signed-off-by Arnd Bergmann <[email protected]>

I think updating to two __u32 values is the better solution here. While the driver is new in the mainline kernel it has been used for multiple years downstream in the Maemo/MeeGo/Mer communities and I would prefer to keep userspace compatibility.

The user interface for timestamps in the new cmt_speech driver is broken in multiple ways:

- The layout is incompatible between 32-bit and 64-bit user space, because of the size differences in 'struct timespec'. This means that the driver can not work when used with 32-bit user space on a 64-bit kernel.

- As there are plans to change 32-bit user space to use a 64-bit time_t type in the future, it will also be incompatible with new 32-bit user space.

To keep support for the user space tools written for this driver (which have lived many years out-of-tree), the interface has been hardened to unsigned 32-bit values.

Reported-by Arnd Bergmann <[email protected]>

Signed-off-by Sebastian Reichel <[email protected]>

---
 drivers/hsi/clients/cmt_speech.c     |  9 +++++++--
 include/uapi/linux/hsi/cs-protocol.h | 16 +++++++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

drivers/hsi/clients/cmt_speech.c +0 -0

index 4983529..67aef3f 100644

drivers/hsi/clients/cmt_speech.c static void cs_hsi_read_on_control_complete(struct hsi_msg *msg) · line 458

Quoted 3 lines

drivers/hsi/clients/cmt_speech.c +2 -1

-		do_posix_clock_monotonic_gettime(tstamp);
+
+		do_posix_clock_monotonic_gettime(&tspec);

Can you please use ktime_get_ts() instead of do_posix_clock_monotonic_gettime(). I'm desperately trying to get rid of the latter, but it seems to come back in circles.

The user interface for timestamps in the new cmt_speech driver is broken in multiple ways:

- The layout is incompatible between 32-bit and 64-bit user space, because of the size differences in 'struct timespec'. This means that the driver can not work when used with 32-bit user space on a 64-bit kernel.

- As there are plans to change 32-bit user space to use a 64-bit time_t type in the future, it will also be incompatible with new 32-bit user space.

To keep support for the user space tools written for this driver (which have lived many years out-of-tree), the interface has been hardened to unsigned 32-bit values.