On 05/07/2020 09:02 PM, Mike Leach wrote:
Hi,
On Thu, 7 May 2020 at 18:45, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Wed, 6 May 2020 at 16:55, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 05/06/2020 08:43 PM, Mathieu Poirier wrote:
On Wed, 6 May 2020 at 11:00, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu,
On Wed, 6 May 2020 at 17:49, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Wed, 6 May 2020 at 04:56, Mike Leach mike.leach@linaro.org wrote: > > Hi Mathieu, > > On Mon, 4 May 2020 at 21:31, Mathieu Poirier mathieu.poirier@linaro.org wrote: >> >> On Fri, May 01, 2020 at 03:02:22PM +0100, Mike Leach wrote:
...
I am happy to change this again if this is no longer agreed as the way forwards.
I think that in 1:1 and 1:1 + global sink topologies the sink closest to the ETM should be selected, regardless of priorities. Things get trickier with N:1 topologies and, given all the possible scenarios, I am wondering if using priorities at all is the way to go.
We could have per CPU sinks and global sinks.
Correct, and that is a topology that creates a lot of problems.
So if we find the closest "sink" from the ETM, we should be fine. Also we need not impose priority among the "sinks" that can route the trace to system memory. i.e, we could think of the sink "subtypes" as the implicit priority.
Given we want to use something that can stream the trace to memory on a sink (ETR, Future IPs) and not something with an internal memory (ETB/ETF/ETB10), could we simply use the coresight_dev_subtype_sink instead of defining priorities ? i.e: Add a new subtypefor SINKs that can push the trace to system memory.
enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
CORESIGHT_DEV_SUBTYPE_SINK_SYS_RAM,
};
I had avoided adding a new sub-type as it implied a lot of rework of code beyond just the current problem. However, a more careful look has shown that CORESIGHT_DEV_SUBTYPE_SINK* are never actually used anywhere at present so this may be a realistic option.,
I like that part as it saves us from adding a new field to struct coresight_device.
Then the priority is implicit by higher subtype_sink value. With this and the closest sink (no of components to the sink) can be easily done.
I can't remember if we have considered the device tree before, but that would be really easy to do. We could introduce a new "default" property, only valid on sink devices, that would indicate what sink to
The first patchset was based on exactly this. It was felt this was not a good approach as ACPI support would be needed too.
use if one isn't explicitly given on the command line. If a sink isn't specified on the command line and a default sink wasn't specified in the DT, then we take the sink closest to the ETM.
Personally I do not prefer DT approach.
Same here.
Something like the below should work for all the cases ?
struct coresight_device * coresight_find_sink(struct coresight_device *dev, int *pdepth) { int i, depth; struct coresight_device *res = NULL;
/* We have reached sink at the given depth, end search */ if (dev->type == SINK) return dev; for (i = 0; i < dev->nr_outports; i ++) { struct coresight_device *tmp; int t = 0; if (!dev->conns[i].child_dev) continue; tmp = coresight_find_sink(dev->conns[i].child_dev, &t); if (!tmp) continue; /* First sink found, set initialize the result */ if (!res) { res = tmp; depth = t; continue; } /* * Update the result if : * a) if we have found a better sink type * b) Or something closer than the current one with * same capability */ if ((res->subtype.sink_subtype < tmp->subtype.sink_subtype) || (t < depth)) { depth = t; res = tmp; }
In a 1:1 + global sink topology where the CPU sinks are ETB and the global sink is an ETR, if the global sink is discovered after a CPU sink, the global sink will be selected. In a N:1 topology, if a TPIU is located closest to the ETMs and discovered after an ETR, the TPIU will be selected.I think
The priority schema used in the current patch set deliberately discounts TPIU as a selectable sink. This must be a feature of any automated scheme as selecting a TPIU (or any off-chip sink) fior purely self hosted trace can never be valid.
If we consider the CS-BSA document, using ETB as a 1:1 sink would not be recommended - it requires that 1:1 topologies use trace capture to system memory. An intervening ETF between ETM and ETR is permitted. Here we can discount a 1:1 schema witn ETM:ETB, and reasonably assume either ETM:<ETR or similar> or <ETM>:<ETF>:<ETR or similar>
The problem is that our search criteria are changing based on the mode we're in. 1:1 we want the shortest distance from source while N:1 a type of sink is preferred. Finding the kind of topology we have on a system will allow us to select the right search heuristic. One way to do that is to keep track of the amount of sinks registered on a system as they get registered. If nr_sinks >= NR_CPUs, we have a 1:1 or a 1:1 + global sink. Otherwise we're back to the good old N:1.
not necessarily true - a 4 core 2 cluster config, with ETF per cluster
- ETR + TPIU actually has 4 sinks and 4 cores, but is N:1. (juno-r1/2
gets close to this with a 6:4 CPU:sink ratio).
I don't think we do need to change the search criteria - just get the prioritisation correct. Use priority + distance to pick, prioritize system memory devices (ETR etc) over local buffer devices (ETF/ETB). This should be sufficient for current and envisaged (by CS-BSA) future systems.
I agree. As long as we prioritise the sinks based on
1) Type of storage . i.e internal limited storage vs system memory
and
2) Distance from the source
for any tie-breakers, we should be fine. It is unreasonable to have an "ETB" per CPU (rather than an ETR). And using an ETB is really bad
* Due to the cost in context switching * Slow word by word reading of the trace data for consumption.
As I mentioned originally, we should concentrate on using the ETR on CSBSA compliant (sane systems) and don't bother about the internal ordering between ETF (as ETB) or ETB. They can still specify the sink and get what they want.
Suzuki