Unverified Commit abd35adf authored by Mark Brown's avatar Mark Brown
Browse files

Improve support for sof_ipc{3|4}_bytes_ext_put 

Merge series from Daniel Baluta <daniel.baluta@oss.nxp.com>:

This patch series provides better handling of cases where sending
a data blob to FW results in a validation error.

In this case we restore to the last good known value instead of keeping
the data that firwmare rejected.
parents ad45067a db38d86d

sound/soc/sof/ipc3-control.c

+48 −6
Original line number Diff line number Diff line
@@ -96,6 +96,26 @@ static int sof_ipc3_set_get_kcontrol_data(struct snd_sof_control *scontrol, 
	cdata->elems_remaining = 0;



	ret = iops->set_get_data(sdev, cdata, cdata->rhdr.hdr.size, set);

	if (!set)

		goto unlock;



	/* It is a set-data operation, and we have a backup that we can restore */

	if (ret < 0) {

		if (!scontrol->old_ipc_control_data)

			goto unlock;

		/*

		 * Current ipc_control_data is not valid, we use the last known good

		 * configuration

		 */

		memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data,

		       scontrol->max_size);

		kfree(scontrol->old_ipc_control_data);

		scontrol->old_ipc_control_data = NULL;

		/* Send the last known good configuration to firmware */

		ret = iops->set_get_data(sdev, cdata, cdata->rhdr.hdr.size, set);

		if (ret < 0)

			goto unlock;

	}



unlock:

	if (lock)
@@ -351,6 +371,7 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol, 
	struct sof_ipc_ctrl_data *cdata = scontrol->ipc_control_data;

	struct snd_soc_component *scomp = scontrol->scomp;

	struct snd_ctl_tlv header;

	int ret = -EINVAL;



	/*

	 * The beginning of bytes data contains a header from where
@@ -381,31 +402,52 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol, 
		return -EINVAL;

	}



	if (copy_from_user(cdata->data, tlvd->tlv, header.length))

		return -EFAULT;

	if (!scontrol->old_ipc_control_data) {

		/* Create a backup of the current, valid bytes control */

		scontrol->old_ipc_control_data = kmemdup(scontrol->ipc_control_data,

							 scontrol->max_size, GFP_KERNEL);

		if (!scontrol->old_ipc_control_data)

			return -ENOMEM;

	}



	if (copy_from_user(cdata->data, tlvd->tlv, header.length)) {

		ret = -EFAULT;

		goto err_restore;

	}



	if (cdata->data->magic != SOF_ABI_MAGIC) {

		dev_err_ratelimited(scomp->dev, "Wrong ABI magic 0x%08x\n", cdata->data->magic);

		return -EINVAL;

		goto err_restore;

	}



	if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, cdata->data->abi)) {

		dev_err_ratelimited(scomp->dev, "Incompatible ABI version 0x%08x\n",

				    cdata->data->abi);

		return -EINVAL;

		goto err_restore;

	}



	/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */

	if (cdata->data->size > scontrol->max_size - sizeof(struct sof_abi_hdr)) {

		dev_err_ratelimited(scomp->dev, "Mismatch in ABI data size (truncated?)\n");

		return -EINVAL;

		goto err_restore;

	}



	/* notify DSP of byte control updates */

	if (pm_runtime_active(scomp->dev))

	if (pm_runtime_active(scomp->dev)) {

		/* Actually send the data to the DSP; this is an opportunity to validate the data */

		return sof_ipc3_set_get_kcontrol_data(scontrol, true, true);

	}



	return 0;



err_restore:

	/* If we have an issue, we restore the old, valid bytes control data */

	if (scontrol->old_ipc_control_data) {

		memcpy(cdata->data, scontrol->old_ipc_control_data, scontrol->max_size);

		kfree(scontrol->old_ipc_control_data);

		scontrol->old_ipc_control_data = NULL;

	}

	return ret;

}



static int _sof_ipc3_bytes_ext_get(struct snd_sof_control *scontrol,

sound/soc/sof/ipc4-control.c

+35 −4
Original line number Diff line number Diff line
@@ -54,6 +54,26 @@ static int sof_ipc4_set_get_kcontrol_data(struct snd_sof_control *scontrol, 
	msg->primary |= SOF_IPC4_MOD_INSTANCE(swidget->instance_id);



	ret = iops->set_get_data(sdev, msg, msg->data_size, set);

	if (!set)

		goto unlock;



	/* It is a set-data operation, and we have a valid backup that we can restore */

	if (ret < 0) {

		if (!scontrol->old_ipc_control_data)

			goto unlock;

		/*

		 * Current ipc_control_data is not valid, we use the last known good

		 * configuration

		 */

		memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data,

		       scontrol->max_size);

		kfree(scontrol->old_ipc_control_data);

		scontrol->old_ipc_control_data = NULL;

		/* Send the last known good configuration to firmware */

		ret = iops->set_get_data(sdev, msg, msg->data_size, set);

		if (ret < 0)

			goto unlock;

	}



unlock:

	if (lock)
@@ -327,13 +347,24 @@ static int sof_ipc4_bytes_ext_put(struct snd_sof_control *scontrol, 
		return -EINVAL;

	}



	if (!scontrol->old_ipc_control_data) {

		/* Create a backup of the current, valid bytes control */

		scontrol->old_ipc_control_data = kmemdup(scontrol->ipc_control_data,

							 scontrol->max_size, GFP_KERNEL);

		if (!scontrol->old_ipc_control_data)

			return -ENOMEM;

	}



	/* Copy the whole binary data which includes the ABI header and the payload */

	if (copy_from_user(data, tlvd->tlv, header.length))

	if (copy_from_user(data, tlvd->tlv, header.length)) {

		memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data,

		       scontrol->max_size);

		kfree(scontrol->old_ipc_control_data);

		scontrol->old_ipc_control_data = NULL;

		return -EFAULT;

	}



	sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);



	return 0;

	return sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);

}



static int _sof_ipc4_bytes_ext_get(struct snd_sof_control *scontrol,

sound/soc/sof/sof-audio.h

+1 −0
Original line number Diff line number Diff line
@@ -362,6 +362,7 @@ struct snd_sof_control { 
	size_t priv_size; /* size of private data */

	size_t max_size;

	void *ipc_control_data;

	void *old_ipc_control_data;

	int max; /* applicable to volume controls */

	u32 size;	/* cdata size */

	u32 *volume_table; /* volume table computed from tlv data*/