[Sound-open-firmware] [PATCH v2] intel-ipc: fix host ring buffer size not page aligned issue

[Pierre-Louis Bossart]

On 11/15/17 3:41 AM, Keyon Jie wrote:
> The host ring buffer size may be not page aligned,
> but the last page was utilized by host component
> wrongly, which may introduce beating noise.
> 
> Here change to correct size for the last element,
> which will fix the issue.
> 
> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
> ---
>   src/ipc/intel-ipc.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
> index 56bd8ba..7de533a 100644
> --- a/src/ipc/intel-ipc.c
> +++ b/src/ipc/intel-ipc.c
> @@ -175,6 +175,12 @@ static int parse_page_descriptors(struct intel_ipc_data *iipc,
>   		host = (struct sof_ipc_comp_host *)&cd->comp;
>   	}
>   
> +	if (HOST_PAGE_SIZE * ring->pages < ring->size) {
> +		/* host side passed in error ring buffer size */
> +		trace_ipc_error("eBs");
> +		return -EINVAL;
> +	}
> +
>   	for (i = 0; i < ring->pages; i++) {
>   
>   		idx = (((i << 2) + i)) >> 1;
> @@ -192,6 +198,10 @@ static int parse_page_descriptors(struct intel_ipc_data *iipc,
>   		else
>   			elem.dest = phy_addr;
>   
> +		/* the last page may be not full used */
> +		if (i == (ring->pages - 1))
> +			elem.size = ring->size - HOST_PAGE_SIZE * i;

I was thrown off by the two changes, maybe document that the first case 
is indeed a bad configuration where the ring buffer uses too few pages 
and conversely in the second case that the ring buffer relies on an 
additional page which may not be completely used (not an error case).

In the first test, maybe you should also check there is only one page 
which isn't partially used, or the second case will fail.
something like

if (HOST_PAGE_SIZE * ring->pages - ring->size >= HOST_PAGE_SIZE)
	error("too many pages");


> +
>   		if (is_trace)
>   			err = dma_trace_host_buffer(d, &elem, ring->size);
>   		else
>