Re: [PATCH] coresight: Fix support for sparsely populated ports

[Suzuki K Poulose <suzuki.poulose@arm.com>]

Hi Mathieu,

On 04/10/2020 07:17 PM, Mathieu Poirier wrote:
> Hi Suzuki,
> 
> On Thu, Apr 09, 2020 at 12:03:16PM +0100, Suzuki K Poulose wrote:
>> On some systems the firmware may not describe all the ports
>> connected to a component (e.g, for security reasons). This
>> could be especially problematic for "funnels" where we could
>> end up in modifying memory beyond the allocated space for
>> refcounts.

...

>> @@ -672,10 +687,14 @@ static int acpi_coresight_parse_graph(struct acpi_device *adev,
>>   			return dir;
>>   
>>   		if (dir == ACPI_CORESIGHT_LINK_MASTER) {
>> -			pdata->nr_outport++;
>> +			if (ptr->outport > pdata->nr_outport)
>> +				pdata->nr_outport = ptr->outport;
>>   			ptr++;
>>   		} else {
>> -			pdata->nr_inport++;
>> +			WARN_ON(pdata->nr_inport == ptr->child_port);
>> +			/* Do not move the ptr for input connections */
>> +			if (ptr->child_port > pdata->nr_inport)
>> +				pdata->nr_inport = ptr->child_port;
> 
> How you are using the current ptr as a scratch pad for input port was definitely
> a brain twister this morning...  I would certainly appreciate a richer comment
> so that I (or anyone else) doesn't have to go through the same process the next
> time around.
> 

Sure, it deserves a better comment. I will add something like :

	/*
	 * We don't track input connection details for a device,
	 * except for the highest input port number. Thus we could
	 * reuse the current record as a scratch pad and reuse it
	 * by not moving the ptr ahead.
	 */



>>   /**
>> - * struct coresight_platform_data - data harvested from the DT specification
>> - * @nr_inport:	number of input ports for this component.
>> - * @nr_outport:	number of output ports for this component.
>> - * @conns:	Array of nr_outport connections from this component
>> + * struct coresight_platform_data - data harvested from the firmware
>> + * specification.
>> + *
>> + * @nr_inport:	Number of elements for the input connections.
>> + * @nr_outport:	Number of elements for the output connections.
>> + * @conns:	Sparse arrray of nr_outport connections from this component.
> 
> s/arrray/array
> 
> Please rebase your work on my the coresight-next branch.  Other than the above
> this patch looks fine to me.

Thanks for the heads up, will do.

Cheers
Suzuki