Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
Hi Suzuki, On Thu, 18 Feb 2021 at 15:14, Suzuki K Poulose wrote: > > On 2/18/21 2:30 PM, Mike Leach wrote: > > HI Suzuki, > > > > On Thu, 18 Feb 2021 at 07:50, Suzuki K Poulose > > wrote: > >> > >> Hi Mike > >> > >> On 2/16/21 9:00 AM, Mike Leach wrote: > >>> Hi Anshuman, > >>> > >>> There have been plenty of detailed comments so I will restrict mine to > >>> a few general issues:- > >>> > >>> 1) Currently there appears to be no sysfs support (I cannot see the > >>> MODE_SYSFS constants running alongside the MODE_PERF ones present in > >>> the other sink drivers). This is present on all other coresight > >>> devices, and must be provided for this device. It is useful for > >>> testing, and there are users out there who will have scripts to use > >>> it. It is not essential it makes it into this set, but should be a > >>> follow up set. > >> > >> This is mentioned in the cover-letter and as you rightly said > >> we could add this in a later series. > >> > > > > Yes - I see that it was mentioned at the end as an open question - so > > I guess this is my answer! > > > >>> > >>> 2) Using FILL mode for TRBE means that the trace will by definition be > >>> lossy. Fill mode will halt collection without cleanly stopping and > >>> flushing the source. This will result in the sink missing the last of > >>> the data from the source as it stops. Even if taking the exception > >>> moves into a prohibited region there is still the possibility the last > >>> trace operations will not be seen. Further it is possible that the > >> > >> Correct. > >> > >>> last few bytes of trace will be an incomplete packet, and indeed the > >>> start of the next buffer could contain incomplete packets too. > >> > >> Yes, this is possible. > >> > >>> > >>> This operation differs from the other sinks which will only halt after > >>> the sources have stopped and the path has been flushed. This ensures > >>> that the latest trace is complete. The weakness with the older sinks > >>> is the lack of interrupt meaning buffers were frequently wrapped so > >>> that only the latest trace is available. > >> > >> This is true, when there was no overflow. i.e, we follow the normal > >> source-stop-flush, sink-stop. > >> > >>> > >>> By using TRBE WRAP mode, with a watermark as described in the TRBE > >>> spec, using the interrupts it is possible to approach lossless trace > >>> in a way that is not possible with earlier ETR/ETB. This is something > >> > >> It may be possible to do lossless trace, but not without double buffering > >> in perf mode. In perf mode, with a single buffer, we have to honor the > >> boundaries set by the aux_buffer head and tail, otherwise we could be > >> corrupting the trace being consumed by the userland. > >> > >> Please remember that the "water mark" is considered as the END of the > >> buffer by TRBE (unlike the SoC-600 ETR). So the LIMIT pointer could be > >> one of : > >> > >> * Tail pointer ( of the handle space, <= End_of_the_Buffer) > >> * Wake up pointer ( when the userspace would like to be woken up ,<= > >> End_of_the_Buffer) > >> > >> So, if we use WRAP mode for perf, the TRBE would overwrite the from > >> the Base, after we hit the LIMIT, where we should have started > >> writing *after* the LIMIT (when LIMIT < End_of_the_Buffer). Moreover > >> restarting from the Base is going to be even more trouble some > >> as it is most likely the data, perf is still collecting. > >> > > > > I agree that the TRBE must write inbetween head and tail / wakeup. > > Howver, there is no reason that I can see why the trbe_base register > > has to remain constant @ the start of the vmapped aux buffer. > > A valid trbe write buffer could be set by: > > trbe_base >= head (rounded up to page boundary) > > trbe_limit <= min(tail, wakeup) (rounded down to page boundary) > > trbe_write is then trbe_base + "watermark" offset. - as suggested in > > the TRBE spec. > > The problem is we are dealing with separate entities. The producer > and the consumer are separate entities playing with a single, > infinite running ring buffer. Had this been a double buffering scheme, > this would work really nice. In fact, I had some plans to do this for > SoC-600 ETR. > > Coming back, with the proposed approach : > > head write (watermark) end-of-real buffer >/ | > ^---v---^--v---| > /\ > base(aligned)limit (wakeup/tail) > > In this case, assuming single shared buffer, the TRBE would start > writing from "write" watermark and WRAP at limit, going back to base. > > The issues here are : >- If there were no overflow, we cant update the AUX handle > unless we pad from head to write (which might be significant). > > If there was overflow : >- You have wrongly ordered data. i.e, the older trace is at "write" > and newer trace is at "base". Unless we copy the data written from > base to the end of "limit", the userspace can't consume it. Or
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/18/21 2:30 PM, Mike Leach wrote: HI Suzuki, On Thu, 18 Feb 2021 at 07:50, Suzuki K Poulose wrote: Hi Mike On 2/16/21 9:00 AM, Mike Leach wrote: Hi Anshuman, There have been plenty of detailed comments so I will restrict mine to a few general issues:- 1) Currently there appears to be no sysfs support (I cannot see the MODE_SYSFS constants running alongside the MODE_PERF ones present in the other sink drivers). This is present on all other coresight devices, and must be provided for this device. It is useful for testing, and there are users out there who will have scripts to use it. It is not essential it makes it into this set, but should be a follow up set. This is mentioned in the cover-letter and as you rightly said we could add this in a later series. Yes - I see that it was mentioned at the end as an open question - so I guess this is my answer! 2) Using FILL mode for TRBE means that the trace will by definition be lossy. Fill mode will halt collection without cleanly stopping and flushing the source. This will result in the sink missing the last of the data from the source as it stops. Even if taking the exception moves into a prohibited region there is still the possibility the last trace operations will not be seen. Further it is possible that the Correct. last few bytes of trace will be an incomplete packet, and indeed the start of the next buffer could contain incomplete packets too. Yes, this is possible. This operation differs from the other sinks which will only halt after the sources have stopped and the path has been flushed. This ensures that the latest trace is complete. The weakness with the older sinks is the lack of interrupt meaning buffers were frequently wrapped so that only the latest trace is available. This is true, when there was no overflow. i.e, we follow the normal source-stop-flush, sink-stop. By using TRBE WRAP mode, with a watermark as described in the TRBE spec, using the interrupts it is possible to approach lossless trace in a way that is not possible with earlier ETR/ETB. This is something It may be possible to do lossless trace, but not without double buffering in perf mode. In perf mode, with a single buffer, we have to honor the boundaries set by the aux_buffer head and tail, otherwise we could be corrupting the trace being consumed by the userland. Please remember that the "water mark" is considered as the END of the buffer by TRBE (unlike the SoC-600 ETR). So the LIMIT pointer could be one of : * Tail pointer ( of the handle space, <= End_of_the_Buffer) * Wake up pointer ( when the userspace would like to be woken up ,<= End_of_the_Buffer) So, if we use WRAP mode for perf, the TRBE would overwrite the from the Base, after we hit the LIMIT, where we should have started writing *after* the LIMIT (when LIMIT < End_of_the_Buffer). Moreover restarting from the Base is going to be even more trouble some as it is most likely the data, perf is still collecting. I agree that the TRBE must write inbetween head and tail / wakeup. Howver, there is no reason that I can see why the trbe_base register has to remain constant @ the start of the vmapped aux buffer. A valid trbe write buffer could be set by: trbe_base >= head (rounded up to page boundary) trbe_limit <= min(tail, wakeup) (rounded down to page boundary) trbe_write is then trbe_base + "watermark" offset. - as suggested in the TRBE spec. The problem is we are dealing with separate entities. The producer and the consumer are separate entities playing with a single, infinite running ring buffer. Had this been a double buffering scheme, this would work really nice. In fact, I had some plans to do this for SoC-600 ETR. Coming back, with the proposed approach : head write (watermark) end-of-real buffer / | ^---v---^--v---| /\ base(aligned)limit (wakeup/tail) In this case, assuming single shared buffer, the TRBE would start writing from "write" watermark and WRAP at limit, going back to base. The issues here are : - If there were no overflow, we cant update the AUX handle unless we pad from head to write (which might be significant). If there was overflow : - You have wrongly ordered data. i.e, the older trace is at "write" and newer trace is at "base". Unless we copy the data written from base to the end of "limit", the userspace can't consume it. Or else it thinks the data at base is older and this gets the trace decoding gone for a toss. As you can see, two players dealing with a single buffer doesn't allow for the kind of flow possible with WRAP. On the other hand this works perfectly with double buffering. We could copy the data from the "write" to limit, followed by from "base" to the current write ptr. This could well be used for sysfs, but since we have asynchronous collection, we don't need the interrupt and thus fall back to CIRCULAR_BUFFER mode. The issue
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
HI Suzuki, On Thu, 18 Feb 2021 at 07:50, Suzuki K Poulose wrote: > > Hi Mike > > On 2/16/21 9:00 AM, Mike Leach wrote: > > Hi Anshuman, > > > > There have been plenty of detailed comments so I will restrict mine to > > a few general issues:- > > > > 1) Currently there appears to be no sysfs support (I cannot see the > > MODE_SYSFS constants running alongside the MODE_PERF ones present in > > the other sink drivers). This is present on all other coresight > > devices, and must be provided for this device. It is useful for > > testing, and there are users out there who will have scripts to use > > it. It is not essential it makes it into this set, but should be a > > follow up set. > > This is mentioned in the cover-letter and as you rightly said > we could add this in a later series. > Yes - I see that it was mentioned at the end as an open question - so I guess this is my answer! > > > > 2) Using FILL mode for TRBE means that the trace will by definition be > > lossy. Fill mode will halt collection without cleanly stopping and > > flushing the source. This will result in the sink missing the last of > > the data from the source as it stops. Even if taking the exception > > moves into a prohibited region there is still the possibility the last > > trace operations will not be seen. Further it is possible that the > > Correct. > > > last few bytes of trace will be an incomplete packet, and indeed the > > start of the next buffer could contain incomplete packets too. > > Yes, this is possible. > > > > > This operation differs from the other sinks which will only halt after > > the sources have stopped and the path has been flushed. This ensures > > that the latest trace is complete. The weakness with the older sinks > > is the lack of interrupt meaning buffers were frequently wrapped so > > that only the latest trace is available. > > This is true, when there was no overflow. i.e, we follow the normal > source-stop-flush, sink-stop. > > > > > By using TRBE WRAP mode, with a watermark as described in the TRBE > > spec, using the interrupts it is possible to approach lossless trace > > in a way that is not possible with earlier ETR/ETB. This is something > > It may be possible to do lossless trace, but not without double buffering > in perf mode. In perf mode, with a single buffer, we have to honor the > boundaries set by the aux_buffer head and tail, otherwise we could be > corrupting the trace being consumed by the userland. > > Please remember that the "water mark" is considered as the END of the > buffer by TRBE (unlike the SoC-600 ETR). So the LIMIT pointer could be > one of : > >* Tail pointer ( of the handle space, <= End_of_the_Buffer) >* Wake up pointer ( when the userspace would like to be woken up ,<= > End_of_the_Buffer) > > So, if we use WRAP mode for perf, the TRBE would overwrite the from > the Base, after we hit the LIMIT, where we should have started > writing *after* the LIMIT (when LIMIT < End_of_the_Buffer). Moreover > restarting from the Base is going to be even more trouble some > as it is most likely the data, perf is still collecting. > I agree that the TRBE must write inbetween head and tail / wakeup. Howver, there is no reason that I can see why the trbe_base register has to remain constant @ the start of the vmapped aux buffer. A valid trbe write buffer could be set by: trbe_base >= head (rounded up to page boundary) trbe_limit <= min(tail, wakeup) (rounded down to page boundary) trbe_write is then trbe_base + "watermark" offset. - as suggested in the TRBE spec. The issue then becomes unravelling the buffer. Given what we know now, and the work on aux buffers, I would suggest that we can easily insert meta data to do this in the front of the buffer, saving any trace overwirtten at the end of the buffer, and setting a new flag in the aux buffer to tell userspace decode to sort it out. Thus the only copy needed is in the region of 8 bytes perhaps. Of course there are potential inefficiencies here in usage of buffer space, and yes we cannot guarantee lossless trace, but FILL mode guarantees lossy trace and a truncated buffer for every time it wraps (by definition, if FILL mode wraps then you cannot be sure that you have all the possible trace so it has to be marked as truncated in the same way we mark wrapped ETR buffers as truncated whenever they are wrapped). > > that has been requested by partners since trace became available in > > linux systems. (There is still a possibility of loss due to filling > > the buffer completely and overflowing the watermark, but that can be > > flagged). > > > > While FILL mode trace is a good start, and suitable for some scenarios > > - WRAP mode needs implementing as well. > > Using WRAP mode makes sense only in the case of double buffering. Even > with that, we are not guaranteed that we wouldn't loose trace data, with > significantly larger buffer than the AUX buffer. So this may not be the > right choice looking at the performance
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
Hi Mike On 2/16/21 9:00 AM, Mike Leach wrote: Hi Anshuman, There have been plenty of detailed comments so I will restrict mine to a few general issues:- 1) Currently there appears to be no sysfs support (I cannot see the MODE_SYSFS constants running alongside the MODE_PERF ones present in the other sink drivers). This is present on all other coresight devices, and must be provided for this device. It is useful for testing, and there are users out there who will have scripts to use it. It is not essential it makes it into this set, but should be a follow up set. This is mentioned in the cover-letter and as you rightly said we could add this in a later series. 2) Using FILL mode for TRBE means that the trace will by definition be lossy. Fill mode will halt collection without cleanly stopping and flushing the source. This will result in the sink missing the last of the data from the source as it stops. Even if taking the exception moves into a prohibited region there is still the possibility the last trace operations will not be seen. Further it is possible that the Correct. last few bytes of trace will be an incomplete packet, and indeed the start of the next buffer could contain incomplete packets too. Yes, this is possible. This operation differs from the other sinks which will only halt after the sources have stopped and the path has been flushed. This ensures that the latest trace is complete. The weakness with the older sinks is the lack of interrupt meaning buffers were frequently wrapped so that only the latest trace is available. This is true, when there was no overflow. i.e, we follow the normal source-stop-flush, sink-stop. By using TRBE WRAP mode, with a watermark as described in the TRBE spec, using the interrupts it is possible to approach lossless trace in a way that is not possible with earlier ETR/ETB. This is something It may be possible to do lossless trace, but not without double buffering in perf mode. In perf mode, with a single buffer, we have to honor the boundaries set by the aux_buffer head and tail, otherwise we could be corrupting the trace being consumed by the userland. Please remember that the "water mark" is considered as the END of the buffer by TRBE (unlike the SoC-600 ETR). So the LIMIT pointer could be one of : * Tail pointer ( of the handle space, <= End_of_the_Buffer) * Wake up pointer ( when the userspace would like to be woken up ,<= End_of_the_Buffer) So, if we use WRAP mode for perf, the TRBE would overwrite the from the Base, after we hit the LIMIT, where we should have started writing *after* the LIMIT (when LIMIT < End_of_the_Buffer). Moreover restarting from the Base is going to be even more trouble some as it is most likely the data, perf is still collecting. that has been requested by partners since trace became available in linux systems. (There is still a possibility of loss due to filling the buffer completely and overflowing the watermark, but that can be flagged). While FILL mode trace is a good start, and suitable for some scenarios - WRAP mode needs implementing as well. Using WRAP mode makes sense only in the case of double buffering. Even with that, we are not guaranteed that we wouldn't loose trace data, with significantly larger buffer than the AUX buffer. So this may not be the right choice looking at the performance and the software expectations. When it comes to sysfs mode, I believe we could use the CIRCULAR_BUFFER mode, as the collection is asynchronous. I understand WRAP is suitable for lossless collection, but unfortunately the Linux sof 3) Padding: To be clear, it is not safe for the decoder to run off the end of one buffer, into the padding area and continue decoding, or continue through the padding into the next buffer. However I believe the buffer start / stop points are demarked by the aux_output_start / aux_output_end calls? Yes. Each session is marked by RECORD_AUX. So, as long as we fix the decoding to use the limit, we should be fine. Thanks for raising this point. Suzuki With upcoming perf decode updates this should enable the decoder to correctly be started and stopped on the buffer boundaries. The padding is there primarily to ensure that the decoder does not synchronize with the data stream until a genuine sync point is found. 4) TRBE needs to be a loadable module like the rest of coresight. Regards Mike On Mon, 15 Feb 2021 at 09:46, Anshuman Khandual wrote: On 2/13/21 1:56 AM, Mathieu Poirier wrote: On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is accessible via the system registers. The TRBE supports different addressing modes including CPU virtual address and buffer modes including the circular buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the access to the trace buffer could be pr
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
Hi Anshuman, On Tue, 16 Feb 2021 at 09:44, Anshuman Khandual wrote: > > Hello Mike, > > On 2/16/21 2:30 PM, Mike Leach wrote: > > Hi Anshuman, > > > > There have been plenty of detailed comments so I will restrict mine to > > a few general issues:- > > > > 1) Currently there appears to be no sysfs support (I cannot see the > > MODE_SYSFS constants running alongside the MODE_PERF ones present in > > the other sink drivers). This is present on all other coresight > > devices, and must be provided for this device. It is useful for > > testing, and there are users out there who will have scripts to use > > it. It is not essential it makes it into this set, but should be a > > follow up set. > > Sure, will try and add it in a follow up series. > > > > > 2) Using FILL mode for TRBE means that the trace will by definition be > > lossy. Fill mode will halt collection without cleanly stopping and > > flushing the source. This will result in the sink missing the last of > > the data from the source as it stops. Even if taking the exception > > moves into a prohibited region there is still the possibility the last > > trace operations will not be seen. Further it is possible that the > > last few bytes of trace will be an incomplete packet, and indeed the > > start of the next buffer could contain incomplete packets too. > > Just wondering why TRBE and ETE would not sync with each other in order > for the ETE to possibly resend all the lost trace data, when the TRBE > runs out of buffer and wrappers around ? The ETE and TRBE are separate devices - there is no feedback between them. The ETE can also send to external sinks. Given the rate of trace generation, buffering enough trace in the ETE to resend is not realistic, and would be very complicated in terms of hardware. Therefore the solution is to stop the source (disable ETE or prohibit using TFR), flush (TSB CSYNC), then stop collection. A TSB CSYNC without stopping the ETE, or after TRBE has stopped collection will have no effect in terms of getting cleanly stopped trace into the buffer. > Is this ETE/TRBE behavior same > for all implementations in the FILL mode ? Just wondering. > Yes - there is nothing in either spec that would suggest otherwise. > > > > This operation differs from the other sinks which will only halt after > > the sources have stopped and the path has been flushed. This ensures > > that the latest trace is complete. The weakness with the older sinks > > is the lack of interrupt meaning buffers were frequently wrapped so > > that only the latest trace is available. > > Right. > > > > > By using TRBE WRAP mode, with a watermark as described in the TRBE > > spec, using the interrupts it is possible to approach lossless trace > > in a way that is not possible with earlier ETR/ETB. This is somethin > Using TRBTRG_EL1 as the above mentioned watermark ? > Using TRBTRG_EL1 precludes using the ETE Event triggers for activating and marking trace. It is preferable to use the write pointer offset from the initial base to allow a portion of the buffer to be filled after wrap. This a little more complex but more flexible in terms of ETE usage. > > that has been requested by partners since trace became available in > > linux systems. (There is still a possibility of loss due to filling > > the buffer completely and overflowing the watermark, but that can be > > flagged). > > > > While FILL mode trace is a good start, and suitable for some scenarios > > - WRAP mode needs implementing as well. > > I would like to understand this mechanism more. Besides how the perf > interface suppose to choose between FILL and WRAP mode ? via a new > event attribute ? > That is an open question. Event option is one possibility, configfs or compile time options are others. Probably have to look at the performance of wrap mode and decide if it could be used all the time or if FILL still has value. We are in the early days of ETE / TRBE development here. I do not think there is anything wrong with using FILL as a first step. as long as the limitations are well understood. Regards Mike > > > > 3) Padding: To be clear, it is not safe for the decoder to run off the > > end of one buffer, into the padding area and continue decoding, or > > continue through the padding into the next buffer. However I believe > > the buffer start / stop points are demarked by the aux_output_start / > > aux_output_end calls? > > Yes. > > > > > With upcoming perf decode updates this should enable the decoder to > > correctly be started and stopped on the buffer boundaries. The padding > > is there primarily to ensure that the decoder does not synchronize > > with the data stream until a genuine sync point is found. > > Right. > > > > > 4) TRBE needs to be a loadable module like the rest of coresight. > > Even though the driver has all the module constructs, the Kconfig was > missing a tristate value, which is being fixed for the next version. > > - Anshuman -- Mike Leach Principal Engin
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
Hello Mike, On 2/16/21 2:30 PM, Mike Leach wrote: > Hi Anshuman, > > There have been plenty of detailed comments so I will restrict mine to > a few general issues:- > > 1) Currently there appears to be no sysfs support (I cannot see the > MODE_SYSFS constants running alongside the MODE_PERF ones present in > the other sink drivers). This is present on all other coresight > devices, and must be provided for this device. It is useful for > testing, and there are users out there who will have scripts to use > it. It is not essential it makes it into this set, but should be a > follow up set. Sure, will try and add it in a follow up series. > > 2) Using FILL mode for TRBE means that the trace will by definition be > lossy. Fill mode will halt collection without cleanly stopping and > flushing the source. This will result in the sink missing the last of > the data from the source as it stops. Even if taking the exception > moves into a prohibited region there is still the possibility the last > trace operations will not be seen. Further it is possible that the > last few bytes of trace will be an incomplete packet, and indeed the > start of the next buffer could contain incomplete packets too. Just wondering why TRBE and ETE would not sync with each other in order for the ETE to possibly resend all the lost trace data, when the TRBE runs out of buffer and wrappers around ? Is this ETE/TRBE behavior same for all implementations in the FILL mode ? Just wondering. > > This operation differs from the other sinks which will only halt after > the sources have stopped and the path has been flushed. This ensures > that the latest trace is complete. The weakness with the older sinks > is the lack of interrupt meaning buffers were frequently wrapped so > that only the latest trace is available. Right. > > By using TRBE WRAP mode, with a watermark as described in the TRBE > spec, using the interrupts it is possible to approach lossless trace > in a way that is not possible with earlier ETR/ETB. This is somethin Using TRBTRG_EL1 as the above mentioned watermark ? > that has been requested by partners since trace became available in > linux systems. (There is still a possibility of loss due to filling > the buffer completely and overflowing the watermark, but that can be > flagged). > > While FILL mode trace is a good start, and suitable for some scenarios > - WRAP mode needs implementing as well. I would like to understand this mechanism more. Besides how the perf interface suppose to choose between FILL and WRAP mode ? via a new event attribute ? > > 3) Padding: To be clear, it is not safe for the decoder to run off the > end of one buffer, into the padding area and continue decoding, or > continue through the padding into the next buffer. However I believe > the buffer start / stop points are demarked by the aux_output_start / > aux_output_end calls? Yes. > > With upcoming perf decode updates this should enable the decoder to > correctly be started and stopped on the buffer boundaries. The padding > is there primarily to ensure that the decoder does not synchronize > with the data stream until a genuine sync point is found. Right. > > 4) TRBE needs to be a loadable module like the rest of coresight. Even though the driver has all the module constructs, the Kconfig was missing a tristate value, which is being fixed for the next version. - Anshuman
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
Hi Anshuman, There have been plenty of detailed comments so I will restrict mine to a few general issues:- 1) Currently there appears to be no sysfs support (I cannot see the MODE_SYSFS constants running alongside the MODE_PERF ones present in the other sink drivers). This is present on all other coresight devices, and must be provided for this device. It is useful for testing, and there are users out there who will have scripts to use it. It is not essential it makes it into this set, but should be a follow up set. 2) Using FILL mode for TRBE means that the trace will by definition be lossy. Fill mode will halt collection without cleanly stopping and flushing the source. This will result in the sink missing the last of the data from the source as it stops. Even if taking the exception moves into a prohibited region there is still the possibility the last trace operations will not be seen. Further it is possible that the last few bytes of trace will be an incomplete packet, and indeed the start of the next buffer could contain incomplete packets too. This operation differs from the other sinks which will only halt after the sources have stopped and the path has been flushed. This ensures that the latest trace is complete. The weakness with the older sinks is the lack of interrupt meaning buffers were frequently wrapped so that only the latest trace is available. By using TRBE WRAP mode, with a watermark as described in the TRBE spec, using the interrupts it is possible to approach lossless trace in a way that is not possible with earlier ETR/ETB. This is something that has been requested by partners since trace became available in linux systems. (There is still a possibility of loss due to filling the buffer completely and overflowing the watermark, but that can be flagged). While FILL mode trace is a good start, and suitable for some scenarios - WRAP mode needs implementing as well. 3) Padding: To be clear, it is not safe for the decoder to run off the end of one buffer, into the padding area and continue decoding, or continue through the padding into the next buffer. However I believe the buffer start / stop points are demarked by the aux_output_start / aux_output_end calls? With upcoming perf decode updates this should enable the decoder to correctly be started and stopped on the buffer boundaries. The padding is there primarily to ensure that the decoder does not synchronize with the data stream until a genuine sync point is found. 4) TRBE needs to be a loadable module like the rest of coresight. Regards Mike On Mon, 15 Feb 2021 at 09:46, Anshuman Khandual wrote: > > > On 2/13/21 1:56 AM, Mathieu Poirier wrote: > > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: > >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is > >> accessible via the system registers. The TRBE supports different addressing > >> modes including CPU virtual address and buffer modes including the circular > >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), > >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the > >> access to the trace buffer could be prohibited by a higher exception level > >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU > >> private interrupt (PPI) on address translation errors and when the buffer > >> is full. Overall implementation here is inspired from the Arm SPE driver. > >> > >> Cc: Mathieu Poirier > >> Cc: Mike Leach > >> Cc: Suzuki K Poulose > >> Signed-off-by: Anshuman Khandual > >> --- > >> Changes in V3: > >> > >> - Added new DT bindings document TRBE.yaml > >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 > >> - Dropped isb() from trbe_reset_local() > >> - Dropped gap between (void *) and buf->trbe_base > >> - Changed 'int' to 'unsigned int' in is_trbe_available() > >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), > >> set_trbe_enabled() and set_trbe_limit_pointer() > >> - Changed get_trbe_flag_update(), is_trbe_programmable() and > >> get_trbe_address_align() to accept TRBIDR value > >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), > >> is_trbe_trg(), > >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value > >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() > >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled > >> - Compute trbe_limit before trbe_write to get the updated handle > >> - Added trbe_stop_and_truncate_event() > >> - Dropped trbe_handle_fatal() > >> > >> Documentation/trace/coresight/coresight-trbe.rst | 39 + > >> arch/arm64/include/asm/sysreg.h |1 + > >> drivers/hwtracing/coresight/Kconfig | 11 + > >> drivers/hwtracing/coresight/Makefile |1 + > >> drivers/hwtracing/coresight/coresight-trbe.c | 1023 > >> ++ > >> drivers/hwtracing/coresight/coresight-trbe.h | 16
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/13/21 1:56 AM, Mathieu Poirier wrote: > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is >> accessible via the system registers. The TRBE supports different addressing >> modes including CPU virtual address and buffer modes including the circular >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the >> access to the trace buffer could be prohibited by a higher exception level >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU >> private interrupt (PPI) on address translation errors and when the buffer >> is full. Overall implementation here is inspired from the Arm SPE driver. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Suzuki K Poulose >> Signed-off-by: Anshuman Khandual >> --- >> Changes in V3: >> >> - Added new DT bindings document TRBE.yaml >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 >> - Dropped isb() from trbe_reset_local() >> - Dropped gap between (void *) and buf->trbe_base >> - Changed 'int' to 'unsigned int' in is_trbe_available() >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), >> set_trbe_enabled() and set_trbe_limit_pointer() >> - Changed get_trbe_flag_update(), is_trbe_programmable() and >> get_trbe_address_align() to accept TRBIDR value >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled >> - Compute trbe_limit before trbe_write to get the updated handle >> - Added trbe_stop_and_truncate_event() >> - Dropped trbe_handle_fatal() >> >> Documentation/trace/coresight/coresight-trbe.rst | 39 + >> arch/arm64/include/asm/sysreg.h |1 + >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile |1 + >> drivers/hwtracing/coresight/coresight-trbe.c | 1023 >> ++ >> drivers/hwtracing/coresight/coresight-trbe.h | 160 >> 6 files changed, 1235 insertions(+) >> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h >> >> diff --git a/Documentation/trace/coresight/coresight-trbe.rst >> b/Documentation/trace/coresight/coresight-trbe.rst >> new file mode 100644 >> index 000..1cbb819 >> --- /dev/null >> +++ b/Documentation/trace/coresight/coresight-trbe.rst >> @@ -0,0 +1,39 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +== >> +Trace Buffer Extension (TRBE). >> +== >> + >> +:Author: Anshuman Khandual >> +:Date: November 2020 >> + >> +Hardware Description >> + >> + >> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system >> +memory, CPU traces generated from a corresponding percpu tracing unit. This >> +gets plugged in as a coresight sink device because the corresponding trace >> +genarators (ETE), are plugged in as source device. >> + >> +The TRBE is not compliant to CoreSight architecture specifications, but is >> +driven via the CoreSight driver framework to support the ETE (which is >> +CoreSight compliant) integration. >> + >> +Sysfs files and directories >> +--- >> + >> +The TRBE devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> +>$ ls /sys/bus/coresight/devices >> +trbe0 trbe1 trbe2 trbe3 >> + >> +The ``trbe`` named TRBEs are associated with a CPU.:: >> + >> +>$ ls /sys/bus/coresight/devices/trbe0/ >> +align dbm >> + >> +*Key file items are:-* >> + * ``align``: TRBE write pointer alignment >> + * ``dbm``: TRBE updates memory with access and dirty flags >> + >> diff --git a/arch/arm64/include/asm/sysreg.h >> b/arch/arm64/include/asm/sysreg.h >> index 85ae4db..9e2e9b7 100644 >> --- a/arch/arm64/include/asm/sysreg.h >> +++ b/arch/arm64/include/asm/sysreg.h >> @@ -97,6 +97,7 @@ >> #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_TCO(x) __emit_inst(0xd500401f | PSTATE_TCO | >> ((!!x) << PSTATE_Imm_shift)) >> +#define TSB_CSYNC __emit_inst(0xd503225f) >> >> #define set_pstate_pan(x) asm volatile(SET_PSTATE_PAN(x)) >> #define set_pstate_uao(x) asm volatile(SET_PSTATE_UAO(x)) >> diff --git a/drivers/hwtracing/coresight/Kconfig >> b/drivers/hwtracing/coresight/Kconfig >> index f15
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/12/21 10:27 PM, Mathieu Poirier wrote:
> [...]
>
>>>
>>>
+ if (nr_pages < 2)
+ return NULL;
+
+ buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event));
+ if (IS_ERR(buf))
+ return ERR_PTR(-ENOMEM);
+
+ pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL);
+ if (IS_ERR(pglist)) {
+ kfree(buf);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ for (i = 0; i < nr_pages; i++)
+ pglist[i] = virt_to_page(pages[i]);
+
+ buf->trbe_base = (unsigned long) vmap(pglist, nr_pages, VM_MAP,
PAGE_KERNEL);
+ if (IS_ERR((void *)buf->trbe_base)) {
>>>
>>> Why not simply make buf->trbe_base a void * instead of having to do all this
>>
>> There are many arithmetic and comparison operations involving trbe_base
>> element. Hence it might be better to keep it as unsigned long, also to
>> keeps it consistent with other pointers i.e trbe_write, trbe_limit.
>
> That is a fair point. Please add a comment to explain your design choice and
> make sure the sparse checker is happy with all of it.
Added a comment.
>
>>
>> Snippet from $cat drivers/hwtracing/coresight/coresight-trbe.c | grep
>> "trbe_base"
>> There are just two places type casting trbe_base back to (void *).
>>
>> memset((void *)buf->trbe_base + head, ETE_IGNORE_PACKET, len);
>> return buf->trbe_base + offset;
>> WARN_ON(buf->trbe_write < buf->trbe_base);
>> set_trbe_base_pointer(buf->trbe_base);
>> buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP,
>> PAGE_KERNEL);
>> if (IS_ERR((void *)buf->trbe_base)) {
>> return ERR_PTR(buf->trbe_base);
>> buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
>> buf->trbe_write = buf->trbe_base;
>> vunmap((void *)buf->trbe_base);
>> base = get_trbe_base_pointer();
>> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
>> if (buf->trbe_limit == buf->trbe_base) {
>> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
>> if (buf->trbe_limit == buf->trbe_base) {
>> offset = get_trbe_limit_pointer() - get_trbe_base_pointer();
>> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
>> if (buf->trbe_limit == buf->trbe_base) {
>> WARN_ON(buf->trbe_base != get_trbe_base_pointer());
>> if (get_trbe_write_pointer() == get_trbe_base_pointer())
>>
>>> casting? And IS_ERR() doesn't work with vmap().
>>
>> Sure, will drop IS_ERR() here.
>>
>
> [...]
>
>
>>>
+
+static ssize_t dbm_show(struct device *dev, struct device_attribute
*attr, char *buf)
+{
+ struct trbe_cpudata *cpudata = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", cpudata->trbe_dbm);
+}
+static DEVICE_ATTR_RO(dbm);
>>>
>>> What does "dbm" stand for? Looking at the documentation for TRBIDR_EL1.F, I
>>> don't see what "dbm" relates to.
>>
>> I made it up to refer TRBIDR_EL1.F as "Dirty (and Access Flag) Bit
>> Management".
>> Could change it as "afdbm" to be more specific or if it is preferred.
>>
>
> I don't see "afdbm" being a better solution - why not simply "flag"?
Replaced all reference for "dbm" with "flag".
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: > Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is > accessible via the system registers. The TRBE supports different addressing > modes including CPU virtual address and buffer modes including the circular > buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), > an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the > access to the trace buffer could be prohibited by a higher exception level > (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU > private interrupt (PPI) on address translation errors and when the buffer > is full. Overall implementation here is inspired from the Arm SPE driver. > > Cc: Mathieu Poirier > Cc: Mike Leach > Cc: Suzuki K Poulose > Signed-off-by: Anshuman Khandual > --- > Changes in V3: > > - Added new DT bindings document TRBE.yaml > - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 > - Dropped isb() from trbe_reset_local() > - Dropped gap between (void *) and buf->trbe_base > - Changed 'int' to 'unsigned int' in is_trbe_available() > - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), > set_trbe_enabled() and set_trbe_limit_pointer() > - Changed get_trbe_flag_update(), is_trbe_programmable() and > get_trbe_address_align() to accept TRBIDR value > - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), > is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value > - Dropped snapshot mode condition in arm_trbe_alloc_buffer() > - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled > - Compute trbe_limit before trbe_write to get the updated handle > - Added trbe_stop_and_truncate_event() > - Dropped trbe_handle_fatal() > > Documentation/trace/coresight/coresight-trbe.rst | 39 + > arch/arm64/include/asm/sysreg.h |1 + > drivers/hwtracing/coresight/Kconfig | 11 + > drivers/hwtracing/coresight/Makefile |1 + > drivers/hwtracing/coresight/coresight-trbe.c | 1023 > ++ > drivers/hwtracing/coresight/coresight-trbe.h | 160 > 6 files changed, 1235 insertions(+) > create mode 100644 Documentation/trace/coresight/coresight-trbe.rst > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h > > diff --git a/Documentation/trace/coresight/coresight-trbe.rst > b/Documentation/trace/coresight/coresight-trbe.rst > new file mode 100644 > index 000..1cbb819 > --- /dev/null > +++ b/Documentation/trace/coresight/coresight-trbe.rst > @@ -0,0 +1,39 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +== > +Trace Buffer Extension (TRBE). > +== > + > +:Author: Anshuman Khandual > +:Date: November 2020 > + > +Hardware Description > + > + > +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system > +memory, CPU traces generated from a corresponding percpu tracing unit. This > +gets plugged in as a coresight sink device because the corresponding trace > +genarators (ETE), are plugged in as source device. > + > +The TRBE is not compliant to CoreSight architecture specifications, but is > +driven via the CoreSight driver framework to support the ETE (which is > +CoreSight compliant) integration. > + > +Sysfs files and directories > +--- > + > +The TRBE devices appear on the existing coresight bus alongside the other > +coresight devices:: > + > + >$ ls /sys/bus/coresight/devices > + trbe0 trbe1 trbe2 trbe3 > + > +The ``trbe`` named TRBEs are associated with a CPU.:: > + > + >$ ls /sys/bus/coresight/devices/trbe0/ > +align dbm > + > +*Key file items are:-* > + * ``align``: TRBE write pointer alignment > + * ``dbm``: TRBE updates memory with access and dirty flags > + > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 85ae4db..9e2e9b7 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -97,6 +97,7 @@ > #define SET_PSTATE_UAO(x)__emit_inst(0xd500401f | PSTATE_UAO | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_TCO(x)__emit_inst(0xd500401f | PSTATE_TCO | > ((!!x) << PSTATE_Imm_shift)) > +#define TSB_CSYNC__emit_inst(0xd503225f) > > #define set_pstate_pan(x)asm volatile(SET_PSTATE_PAN(x)) > #define set_pstate_uao(x)asm volatile(SET_PSTATE_UAO(x)) > diff --git a/drivers/hwtracing/coresight/Kconfig > b/drivers/hwtracing/coresight/Kconfig > index f154ae7..aa657ab 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -164,6 +164,17 @@ config CORESIGHT
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Fri, Feb 12, 2021 at 11:13:01AM +0530, Anshuman Khandual wrote:
>
>
> On 2/11/21 12:30 AM, Mathieu Poirier wrote:
> > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote:
> >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is
> >> accessible via the system registers. The TRBE supports different addressing
> >> modes including CPU virtual address and buffer modes including the circular
> >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1),
> >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the
> >> access to the trace buffer could be prohibited by a higher exception level
> >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU
> >> private interrupt (PPI) on address translation errors and when the buffer
> >> is full. Overall implementation here is inspired from the Arm SPE driver.
> >>
> >> Cc: Mathieu Poirier
> >> Cc: Mike Leach
> >> Cc: Suzuki K Poulose
> >> Signed-off-by: Anshuman Khandual
> >> ---
> >> Changes in V3:
> >>
> >> - Added new DT bindings document TRBE.yaml
> >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3
> >> - Dropped isb() from trbe_reset_local()
> >> - Dropped gap between (void *) and buf->trbe_base
> >> - Changed 'int' to 'unsigned int' in is_trbe_available()
> >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(),
> >> set_trbe_enabled() and set_trbe_limit_pointer()
> >> - Changed get_trbe_flag_update(), is_trbe_programmable() and
> >> get_trbe_address_align() to accept TRBIDR value
> >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(),
> >> is_trbe_trg(),
> >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value
> >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer()
> >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled
> >> - Compute trbe_limit before trbe_write to get the updated handle
> >> - Added trbe_stop_and_truncate_event()
> >> - Dropped trbe_handle_fatal()
> >>
> >> Documentation/trace/coresight/coresight-trbe.rst | 39 +
> >> arch/arm64/include/asm/sysreg.h |1 +
> >> drivers/hwtracing/coresight/Kconfig | 11 +
> >> drivers/hwtracing/coresight/Makefile |1 +
> >> drivers/hwtracing/coresight/coresight-trbe.c | 1023
> >> ++
> >> drivers/hwtracing/coresight/coresight-trbe.h | 160
> >> 6 files changed, 1235 insertions(+)
> >> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst
> >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c
> >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h
> >> +
> >
> > [...]
> >
> >> +static void arm_trbe_probe_coresight_cpu(void *info)
> >> +{
> >> + struct trbe_drvdata *drvdata = info;
> >> + struct coresight_desc desc = { 0 };
> >> + int cpu = smp_processor_id();
> >> + struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> >> + struct coresight_device *trbe_csdev = per_cpu(csdev_sink, cpu);
> >> + u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
> >> + struct device *dev;
> >> +
> >> + if (WARN_ON(!cpudata))
> >> + goto cpu_clear;
> >
> > There is already a check for this in arm_trbe_probe_coresight(), we
> > couldn't be
> > here if there was a problem with the allocation.
>
> Right but just to be extra cautious. Do you really want this to be dropped ?
I don't think it is necessary but there is no harm in keeping it if you are keen
on it.
>
> >
> >> +
> >> + if (trbe_csdev)
> >> + return;
> >
> > Now that's a reason to have a WARN_ON(). If we are probing and a sink is
> > already present in this cpu's slot, something went seriously wrong and we
> > should
> > be clear about it.
>
> Right, will add an WARN_ON().
>
> >
> >> +
> >> + cpudata->cpu = smp_processor_id();
> >> + cpudata->drvdata = drvdata;
> >> + dev = &cpudata->drvdata->pdev->dev;
> >> +
> >> + if (!is_trbe_available()) {
> >> + pr_err("TRBE is not implemented on cpu %d\n", cpudata->cpu);
> >> + goto cpu_clear;
> >> + }
> >> +
> >> + if (!is_trbe_programmable(trbidr)) {
> >> + pr_err("TRBE is owned in higher exception level on cpu %d\n",
> >> cpudata->cpu);
> >> + goto cpu_clear;
> >> + }
> >> + desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", DRVNAME,
> >> smp_processor_id());
> >
> > We will end up with "arm_trbe0", "arm_trbe1" and so on in sysfs... Is the
> > "arm_" part absolutely needed? I think this should be like what we do for
> > etmv3
> > and etmv4 where only "etmX" shows up in sysfs.
>
> Okay, will drop arm_ here. IIRC this was originally trbeX where X is the cpu
> number
> but then ended up using DRVNAME as prefix.
>
> >
> >> + if (IS_ERR(desc.name))
> >> + goto cpu_clear;
> >> +
> >> + desc.type = CORESIGHT_DEV_TYPE_SINK;
> >> + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
> >> + des
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
[...]
> >
> >
> >> + if (nr_pages < 2)
> >> + return NULL;
> >> +
> >> + buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event));
> >> + if (IS_ERR(buf))
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL);
> >> + if (IS_ERR(pglist)) {
> >> + kfree(buf);
> >> + return ERR_PTR(-ENOMEM);
> >> + }
> >> +
> >> + for (i = 0; i < nr_pages; i++)
> >> + pglist[i] = virt_to_page(pages[i]);
> >> +
> >> + buf->trbe_base = (unsigned long) vmap(pglist, nr_pages, VM_MAP,
> >> PAGE_KERNEL);
> >> + if (IS_ERR((void *)buf->trbe_base)) {
> >
> > Why not simply make buf->trbe_base a void * instead of having to do all this
>
> There are many arithmetic and comparison operations involving trbe_base
> element. Hence it might be better to keep it as unsigned long, also to
> keeps it consistent with other pointers i.e trbe_write, trbe_limit.
That is a fair point. Please add a comment to explain your design choice and
make sure the sparse checker is happy with all of it.
>
> Snippet from $cat drivers/hwtracing/coresight/coresight-trbe.c | grep
> "trbe_base"
> There are just two places type casting trbe_base back to (void *).
>
> memset((void *)buf->trbe_base + head, ETE_IGNORE_PACKET, len);
> return buf->trbe_base + offset;
> WARN_ON(buf->trbe_write < buf->trbe_base);
> set_trbe_base_pointer(buf->trbe_base);
> buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP,
> PAGE_KERNEL);
> if (IS_ERR((void *)buf->trbe_base)) {
> return ERR_PTR(buf->trbe_base);
> buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
> buf->trbe_write = buf->trbe_base;
> vunmap((void *)buf->trbe_base);
> base = get_trbe_base_pointer();
> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
> if (buf->trbe_limit == buf->trbe_base) {
> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
> if (buf->trbe_limit == buf->trbe_base) {
> offset = get_trbe_limit_pointer() - get_trbe_base_pointer();
> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
> if (buf->trbe_limit == buf->trbe_base) {
> WARN_ON(buf->trbe_base != get_trbe_base_pointer());
> if (get_trbe_write_pointer() == get_trbe_base_pointer())
>
> > casting? And IS_ERR() doesn't work with vmap().
>
> Sure, will drop IS_ERR() here.
>
[...]
> >
> >> +
> >> +static ssize_t dbm_show(struct device *dev, struct device_attribute
> >> *attr, char *buf)
> >> +{
> >> + struct trbe_cpudata *cpudata = dev_get_drvdata(dev);
> >> +
> >> + return sprintf(buf, "%d\n", cpudata->trbe_dbm);
> >> +}
> >> +static DEVICE_ATTR_RO(dbm);
> >
> > What does "dbm" stand for? Looking at the documentation for TRBIDR_EL1.F, I
> > don't see what "dbm" relates to.
>
> I made it up to refer TRBIDR_EL1.F as "Dirty (and Access Flag) Bit
> Management".
> Could change it as "afdbm" to be more specific or if it is preferred.
>
I don't see "afdbm" being a better solution - why not simply "flag"?
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/11/21 12:30 AM, Mathieu Poirier wrote:
> On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote:
>> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is
>> accessible via the system registers. The TRBE supports different addressing
>> modes including CPU virtual address and buffer modes including the circular
>> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1),
>> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the
>> access to the trace buffer could be prohibited by a higher exception level
>> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU
>> private interrupt (PPI) on address translation errors and when the buffer
>> is full. Overall implementation here is inspired from the Arm SPE driver.
>>
>> Cc: Mathieu Poirier
>> Cc: Mike Leach
>> Cc: Suzuki K Poulose
>> Signed-off-by: Anshuman Khandual
>> ---
>> Changes in V3:
>>
>> - Added new DT bindings document TRBE.yaml
>> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3
>> - Dropped isb() from trbe_reset_local()
>> - Dropped gap between (void *) and buf->trbe_base
>> - Changed 'int' to 'unsigned int' in is_trbe_available()
>> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(),
>> set_trbe_enabled() and set_trbe_limit_pointer()
>> - Changed get_trbe_flag_update(), is_trbe_programmable() and
>> get_trbe_address_align() to accept TRBIDR value
>> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(),
>> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value
>> - Dropped snapshot mode condition in arm_trbe_alloc_buffer()
>> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled
>> - Compute trbe_limit before trbe_write to get the updated handle
>> - Added trbe_stop_and_truncate_event()
>> - Dropped trbe_handle_fatal()
>>
>> Documentation/trace/coresight/coresight-trbe.rst | 39 +
>> arch/arm64/include/asm/sysreg.h |1 +
>> drivers/hwtracing/coresight/Kconfig | 11 +
>> drivers/hwtracing/coresight/Makefile |1 +
>> drivers/hwtracing/coresight/coresight-trbe.c | 1023
>> ++
>> drivers/hwtracing/coresight/coresight-trbe.h | 160
>> 6 files changed, 1235 insertions(+)
>> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst
>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c
>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h
>> +
>
> [...]
>
>> +static void arm_trbe_probe_coresight_cpu(void *info)
>> +{
>> +struct trbe_drvdata *drvdata = info;
>> +struct coresight_desc desc = { 0 };
>> +int cpu = smp_processor_id();
>> +struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
>> +struct coresight_device *trbe_csdev = per_cpu(csdev_sink, cpu);
>> +u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
>> +struct device *dev;
>> +
>> +if (WARN_ON(!cpudata))
>> +goto cpu_clear;
>
> There is already a check for this in arm_trbe_probe_coresight(), we couldn't
> be
> here if there was a problem with the allocation.
Right but just to be extra cautious. Do you really want this to be dropped ?
>
>> +
>> +if (trbe_csdev)
>> +return;
>
> Now that's a reason to have a WARN_ON(). If we are probing and a sink is
> already present in this cpu's slot, something went seriously wrong and we
> should
> be clear about it.
Right, will add an WARN_ON().
>
>> +
>> +cpudata->cpu = smp_processor_id();
>> +cpudata->drvdata = drvdata;
>> +dev = &cpudata->drvdata->pdev->dev;
>> +
>> +if (!is_trbe_available()) {
>> +pr_err("TRBE is not implemented on cpu %d\n", cpudata->cpu);
>> +goto cpu_clear;
>> +}
>> +
>> +if (!is_trbe_programmable(trbidr)) {
>> +pr_err("TRBE is owned in higher exception level on cpu %d\n",
>> cpudata->cpu);
>> +goto cpu_clear;
>> +}
>> +desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", DRVNAME,
>> smp_processor_id());
>
> We will end up with "arm_trbe0", "arm_trbe1" and so on in sysfs... Is the
> "arm_" part absolutely needed? I think this should be like what we do for
> etmv3
> and etmv4 where only "etmX" shows up in sysfs.
Okay, will drop arm_ here. IIRC this was originally trbeX where X is the cpu
number
but then ended up using DRVNAME as prefix.
>
>> +if (IS_ERR(desc.name))
>> +goto cpu_clear;
>> +
>> +desc.type = CORESIGHT_DEV_TYPE_SINK;
>> +desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
>> +desc.ops = &arm_trbe_cs_ops;
>> +desc.pdata = dev_get_platdata(dev);
>> +desc.groups = arm_trbe_groups;
>> +desc.dev = dev;
>> +trbe_csdev = coresight_register(&desc);
>> +if (IS_ERR(trbe_csdev))
>> +goto cpu_clear;
>> +
>> +dev_set_drvdata(&trbe_csdev->dev, cpudata);
>> +cpudata->trbe_dbm = get_trbe_flag_
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/12/21 12:30 AM, Mathieu Poirier wrote: > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is >> accessible via the system registers. The TRBE supports different addressing >> modes including CPU virtual address and buffer modes including the circular >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the >> access to the trace buffer could be prohibited by a higher exception level >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU >> private interrupt (PPI) on address translation errors and when the buffer >> is full. Overall implementation here is inspired from the Arm SPE driver. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Suzuki K Poulose >> Signed-off-by: Anshuman Khandual >> --- >> Changes in V3: >> >> - Added new DT bindings document TRBE.yaml >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 >> - Dropped isb() from trbe_reset_local() >> - Dropped gap between (void *) and buf->trbe_base >> - Changed 'int' to 'unsigned int' in is_trbe_available() >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), >> set_trbe_enabled() and set_trbe_limit_pointer() >> - Changed get_trbe_flag_update(), is_trbe_programmable() and >> get_trbe_address_align() to accept TRBIDR value >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled >> - Compute trbe_limit before trbe_write to get the updated handle >> - Added trbe_stop_and_truncate_event() >> - Dropped trbe_handle_fatal() >> >> Documentation/trace/coresight/coresight-trbe.rst | 39 + >> arch/arm64/include/asm/sysreg.h |1 + >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile |1 + >> drivers/hwtracing/coresight/coresight-trbe.c | 1023 >> ++ >> drivers/hwtracing/coresight/coresight-trbe.h | 160 >> 6 files changed, 1235 insertions(+) >> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h >> >> diff --git a/Documentation/trace/coresight/coresight-trbe.rst >> b/Documentation/trace/coresight/coresight-trbe.rst >> new file mode 100644 >> index 000..1cbb819 >> --- /dev/null >> +++ b/Documentation/trace/coresight/coresight-trbe.rst >> @@ -0,0 +1,39 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +== >> +Trace Buffer Extension (TRBE). >> +== >> + >> +:Author: Anshuman Khandual >> +:Date: November 2020 >> + >> +Hardware Description >> + >> + >> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system >> +memory, CPU traces generated from a corresponding percpu tracing unit. This >> +gets plugged in as a coresight sink device because the corresponding trace >> +genarators (ETE), are plugged in as source device. >> + >> +The TRBE is not compliant to CoreSight architecture specifications, but is >> +driven via the CoreSight driver framework to support the ETE (which is >> +CoreSight compliant) integration. >> + >> +Sysfs files and directories >> +--- >> + >> +The TRBE devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> +>$ ls /sys/bus/coresight/devices >> +trbe0 trbe1 trbe2 trbe3 >> + >> +The ``trbe`` named TRBEs are associated with a CPU.:: >> + >> +>$ ls /sys/bus/coresight/devices/trbe0/ >> +align dbm >> + >> +*Key file items are:-* >> + * ``align``: TRBE write pointer alignment >> + * ``dbm``: TRBE updates memory with access and dirty flags >> + >> diff --git a/arch/arm64/include/asm/sysreg.h >> b/arch/arm64/include/asm/sysreg.h >> index 85ae4db..9e2e9b7 100644 >> --- a/arch/arm64/include/asm/sysreg.h >> +++ b/arch/arm64/include/asm/sysreg.h >> @@ -97,6 +97,7 @@ >> #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_TCO(x) __emit_inst(0xd500401f | PSTATE_TCO | >> ((!!x) << PSTATE_Imm_shift)) >> +#define TSB_CSYNC __emit_inst(0xd503225f) >> >> #define set_pstate_pan(x) asm volatile(SET_PSTATE_PAN(x)) >> #define set_pstate_uao(x) asm volatile(SET_PSTATE_UAO(x)) >> diff --git a/drivers/hwtracing/coresight/Kconfig >> b/drivers/hwtracing/coresight/Kconfig >> index f
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: > Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is > accessible via the system registers. The TRBE supports different addressing > modes including CPU virtual address and buffer modes including the circular > buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), > an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the > access to the trace buffer could be prohibited by a higher exception level > (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU > private interrupt (PPI) on address translation errors and when the buffer > is full. Overall implementation here is inspired from the Arm SPE driver. > > Cc: Mathieu Poirier > Cc: Mike Leach > Cc: Suzuki K Poulose > Signed-off-by: Anshuman Khandual > --- > Changes in V3: > > - Added new DT bindings document TRBE.yaml > - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 > - Dropped isb() from trbe_reset_local() > - Dropped gap between (void *) and buf->trbe_base > - Changed 'int' to 'unsigned int' in is_trbe_available() > - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), > set_trbe_enabled() and set_trbe_limit_pointer() > - Changed get_trbe_flag_update(), is_trbe_programmable() and > get_trbe_address_align() to accept TRBIDR value > - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), > is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value > - Dropped snapshot mode condition in arm_trbe_alloc_buffer() > - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled > - Compute trbe_limit before trbe_write to get the updated handle > - Added trbe_stop_and_truncate_event() > - Dropped trbe_handle_fatal() > > Documentation/trace/coresight/coresight-trbe.rst | 39 + > arch/arm64/include/asm/sysreg.h |1 + > drivers/hwtracing/coresight/Kconfig | 11 + > drivers/hwtracing/coresight/Makefile |1 + > drivers/hwtracing/coresight/coresight-trbe.c | 1023 > ++ > drivers/hwtracing/coresight/coresight-trbe.h | 160 > 6 files changed, 1235 insertions(+) > create mode 100644 Documentation/trace/coresight/coresight-trbe.rst > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h > > diff --git a/Documentation/trace/coresight/coresight-trbe.rst > b/Documentation/trace/coresight/coresight-trbe.rst > new file mode 100644 > index 000..1cbb819 > --- /dev/null > +++ b/Documentation/trace/coresight/coresight-trbe.rst > @@ -0,0 +1,39 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +== > +Trace Buffer Extension (TRBE). > +== > + > +:Author: Anshuman Khandual > +:Date: November 2020 > + > +Hardware Description > + > + > +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system > +memory, CPU traces generated from a corresponding percpu tracing unit. This > +gets plugged in as a coresight sink device because the corresponding trace > +genarators (ETE), are plugged in as source device. > + > +The TRBE is not compliant to CoreSight architecture specifications, but is > +driven via the CoreSight driver framework to support the ETE (which is > +CoreSight compliant) integration. > + > +Sysfs files and directories > +--- > + > +The TRBE devices appear on the existing coresight bus alongside the other > +coresight devices:: > + > + >$ ls /sys/bus/coresight/devices > + trbe0 trbe1 trbe2 trbe3 > + > +The ``trbe`` named TRBEs are associated with a CPU.:: > + > + >$ ls /sys/bus/coresight/devices/trbe0/ > +align dbm > + > +*Key file items are:-* > + * ``align``: TRBE write pointer alignment > + * ``dbm``: TRBE updates memory with access and dirty flags > + > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 85ae4db..9e2e9b7 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -97,6 +97,7 @@ > #define SET_PSTATE_UAO(x)__emit_inst(0xd500401f | PSTATE_UAO | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_TCO(x)__emit_inst(0xd500401f | PSTATE_TCO | > ((!!x) << PSTATE_Imm_shift)) > +#define TSB_CSYNC__emit_inst(0xd503225f) > > #define set_pstate_pan(x)asm volatile(SET_PSTATE_PAN(x)) > #define set_pstate_uao(x)asm volatile(SET_PSTATE_UAO(x)) > diff --git a/drivers/hwtracing/coresight/Kconfig > b/drivers/hwtracing/coresight/Kconfig > index f154ae7..aa657ab 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -164,6 +164,17 @@ config CORESIGHT
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote:
> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is
> accessible via the system registers. The TRBE supports different addressing
> modes including CPU virtual address and buffer modes including the circular
> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1),
> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the
> access to the trace buffer could be prohibited by a higher exception level
> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU
> private interrupt (PPI) on address translation errors and when the buffer
> is full. Overall implementation here is inspired from the Arm SPE driver.
>
> Cc: Mathieu Poirier
> Cc: Mike Leach
> Cc: Suzuki K Poulose
> Signed-off-by: Anshuman Khandual
> ---
> Changes in V3:
>
> - Added new DT bindings document TRBE.yaml
> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3
> - Dropped isb() from trbe_reset_local()
> - Dropped gap between (void *) and buf->trbe_base
> - Changed 'int' to 'unsigned int' in is_trbe_available()
> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(),
> set_trbe_enabled() and set_trbe_limit_pointer()
> - Changed get_trbe_flag_update(), is_trbe_programmable() and
> get_trbe_address_align() to accept TRBIDR value
> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(),
> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value
> - Dropped snapshot mode condition in arm_trbe_alloc_buffer()
> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled
> - Compute trbe_limit before trbe_write to get the updated handle
> - Added trbe_stop_and_truncate_event()
> - Dropped trbe_handle_fatal()
>
> Documentation/trace/coresight/coresight-trbe.rst | 39 +
> arch/arm64/include/asm/sysreg.h |1 +
> drivers/hwtracing/coresight/Kconfig | 11 +
> drivers/hwtracing/coresight/Makefile |1 +
> drivers/hwtracing/coresight/coresight-trbe.c | 1023
> ++
> drivers/hwtracing/coresight/coresight-trbe.h | 160
> 6 files changed, 1235 insertions(+)
> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst
> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c
> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h
> +
[...]
> +static void arm_trbe_probe_coresight_cpu(void *info)
> +{
> + struct trbe_drvdata *drvdata = info;
> + struct coresight_desc desc = { 0 };
> + int cpu = smp_processor_id();
> + struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
> + struct coresight_device *trbe_csdev = per_cpu(csdev_sink, cpu);
> + u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
> + struct device *dev;
> +
> + if (WARN_ON(!cpudata))
> + goto cpu_clear;
There is already a check for this in arm_trbe_probe_coresight(), we couldn't be
here if there was a problem with the allocation.
> +
> + if (trbe_csdev)
> + return;
Now that's a reason to have a WARN_ON(). If we are probing and a sink is
already present in this cpu's slot, something went seriously wrong and we should
be clear about it.
> +
> + cpudata->cpu = smp_processor_id();
> + cpudata->drvdata = drvdata;
> + dev = &cpudata->drvdata->pdev->dev;
> +
> + if (!is_trbe_available()) {
> + pr_err("TRBE is not implemented on cpu %d\n", cpudata->cpu);
> + goto cpu_clear;
> + }
> +
> + if (!is_trbe_programmable(trbidr)) {
> + pr_err("TRBE is owned in higher exception level on cpu %d\n",
> cpudata->cpu);
> + goto cpu_clear;
> + }
> + desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", DRVNAME,
> smp_processor_id());
We will end up with "arm_trbe0", "arm_trbe1" and so on in sysfs... Is the
"arm_" part absolutely needed? I think this should be like what we do for etmv3
and etmv4 where only "etmX" shows up in sysfs.
> + if (IS_ERR(desc.name))
> + goto cpu_clear;
> +
> + desc.type = CORESIGHT_DEV_TYPE_SINK;
> + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM;
> + desc.ops = &arm_trbe_cs_ops;
> + desc.pdata = dev_get_platdata(dev);
> + desc.groups = arm_trbe_groups;
> + desc.dev = dev;
> + trbe_csdev = coresight_register(&desc);
> + if (IS_ERR(trbe_csdev))
> + goto cpu_clear;
> +
> + dev_set_drvdata(&trbe_csdev->dev, cpudata);
> + cpudata->trbe_dbm = get_trbe_flag_update(trbidr);
> + cpudata->trbe_align = 1ULL << get_trbe_address_align(trbidr);
> + if (cpudata->trbe_align > SZ_2K) {
> + pr_err("Unsupported alignment on cpu %d\n", cpudata->cpu);
> + goto cpu_clear;
Here coresight_unregister() should be called. The other option is to call
coresight_register() when everything else is known to be fine, which is the
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Wed, Feb 10, 2021 at 09:42:29AM +0530, Anshuman Khandual wrote:
>
>
> On 2/9/21 11:09 PM, Mathieu Poirier wrote:
> > On Fri, Feb 05, 2021 at 10:53:30AM -0700, Mathieu Poirier wrote:
> >> On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote:
> >>> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is
> >>> accessible via the system registers. The TRBE supports different
> >>> addressing
> >>> modes including CPU virtual address and buffer modes including the
> >>> circular
> >>> buffer mode. The TRBE buffer is addressed by a base pointer
> >>> (TRBBASER_EL1),
> >>> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the
> >>> access to the trace buffer could be prohibited by a higher exception level
> >>> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU
> >>> private interrupt (PPI) on address translation errors and when the buffer
> >>> is full. Overall implementation here is inspired from the Arm SPE driver.
> >>>
> >>
> >> I got this message when applying the patch:
> >>
> >> Applying: coresight: sink: Add TRBE driver
> >> .git/rebase-apply/patch:76: new blank line at EOF.
> >> +
> >> warning: 1 line adds whitespace errors.
> >>
> >>> Cc: Mathieu Poirier
> >>> Cc: Mike Leach
> >>> Cc: Suzuki K Poulose
> >>> Signed-off-by: Anshuman Khandual
> >>> ---
> >>> Changes in V3:
> >>>
> >>> - Added new DT bindings document TRBE.yaml
> >>> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3
> >>> - Dropped isb() from trbe_reset_local()
> >>> - Dropped gap between (void *) and buf->trbe_base
> >>> - Changed 'int' to 'unsigned int' in is_trbe_available()
> >>> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(),
> >>> set_trbe_enabled() and set_trbe_limit_pointer()
> >>> - Changed get_trbe_flag_update(), is_trbe_programmable() and
> >>> get_trbe_address_align() to accept TRBIDR value
> >>> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(),
> >>> is_trbe_trg(),
> >>> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value
> >>> - Dropped snapshot mode condition in arm_trbe_alloc_buffer()
> >>> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled
> >>> - Compute trbe_limit before trbe_write to get the updated handle
> >>> - Added trbe_stop_and_truncate_event()
> >>> - Dropped trbe_handle_fatal()
> >>>
> >>> Documentation/trace/coresight/coresight-trbe.rst | 39 +
> >>> arch/arm64/include/asm/sysreg.h |1 +
> >>> drivers/hwtracing/coresight/Kconfig | 11 +
> >>> drivers/hwtracing/coresight/Makefile |1 +
> >>> drivers/hwtracing/coresight/coresight-trbe.c | 1023
> >>> ++
> >>> drivers/hwtracing/coresight/coresight-trbe.h | 160
> >>> 6 files changed, 1235 insertions(+)
> >>> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst
> >>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c
> >>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h
> >>>
[...]
> >>> +
> >>> +static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
> >>> +{
> >>> + struct perf_output_handle **handle_ptr = dev;
> >>> + struct perf_output_handle *handle = *handle_ptr;
> >>> + enum trbe_fault_action act;
> >>> +
> >>> + WARN_ON(!is_trbe_irq(read_sysreg_s(SYS_TRBSR_EL1)));
> >>> + clr_trbe_irq();
> >>> +
> >>> + /*
> >>> + * Ensure the trace is visible to the CPUs and
> >>> + * any external aborts have been resolved.
> >>> + */
> >>> + trbe_drain_buffer();
> >>> + isb();
> >>> +
> >>> + if (!perf_get_aux(handle))
> >>> + return IRQ_NONE;
> >>> +
> >>> + if (!is_perf_trbe(handle))
> >>> + return IRQ_NONE;
> >>> +
> >>> + irq_work_run();
> >
> > There is a comment in the SPE driver about this. Since this driver closely
> > follows that implementation it would be nice to have the comments as well.
> > Otherwise the reader has to constantly go back to the original driver.
>
> Sure, will add the following comment before irq_work_run().
>
> /*
> * Ensure perf callbacks have completed, which may disable the
> * profiling buffer in response to a TRUNCATION flag.
> */
>
> >
> > I will come back to this function later.
>
> Okay.
>
> >
> >>> +
> >>> + act = trbe_get_fault_act(handle);
> >>> + switch (act) {
> >>> + case TRBE_FAULT_ACT_WRAP:
> >>> + trbe_handle_overflow(handle);
> >>> + break;
> >>> + case TRBE_FAULT_ACT_SPURIOUS:
> >>> + trbe_handle_spurious(handle);
> >>> + break;
> >>> + case TRBE_FAULT_ACT_FATAL:
> >>> + trbe_stop_and_truncate_event(handle);
> >>> + break;
> >>> + }
> >>> + return IRQ_HANDLED;
> >>> +}
> >>> +
> >>> +static const struct coresight_ops_sink arm_trbe_sink_ops = {
> >>> + .enable = arm_trbe_enable,
> >>> + .disable= arm_trbe_disable,
> >>> + .alloc_buffer = arm_trbe_alloc_buffer,
> >>> + .free_buffer= arm_trbe_
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/9/21 11:09 PM, Mathieu Poirier wrote: > On Fri, Feb 05, 2021 at 10:53:30AM -0700, Mathieu Poirier wrote: >> On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: >>> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is >>> accessible via the system registers. The TRBE supports different addressing >>> modes including CPU virtual address and buffer modes including the circular >>> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), >>> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the >>> access to the trace buffer could be prohibited by a higher exception level >>> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU >>> private interrupt (PPI) on address translation errors and when the buffer >>> is full. Overall implementation here is inspired from the Arm SPE driver. >>> >> >> I got this message when applying the patch: >> >> Applying: coresight: sink: Add TRBE driver >> .git/rebase-apply/patch:76: new blank line at EOF. >> + >> warning: 1 line adds whitespace errors. >> >>> Cc: Mathieu Poirier >>> Cc: Mike Leach >>> Cc: Suzuki K Poulose >>> Signed-off-by: Anshuman Khandual >>> --- >>> Changes in V3: >>> >>> - Added new DT bindings document TRBE.yaml >>> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 >>> - Dropped isb() from trbe_reset_local() >>> - Dropped gap between (void *) and buf->trbe_base >>> - Changed 'int' to 'unsigned int' in is_trbe_available() >>> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), >>> set_trbe_enabled() and set_trbe_limit_pointer() >>> - Changed get_trbe_flag_update(), is_trbe_programmable() and >>> get_trbe_address_align() to accept TRBIDR value >>> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), >>> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value >>> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() >>> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled >>> - Compute trbe_limit before trbe_write to get the updated handle >>> - Added trbe_stop_and_truncate_event() >>> - Dropped trbe_handle_fatal() >>> >>> Documentation/trace/coresight/coresight-trbe.rst | 39 + >>> arch/arm64/include/asm/sysreg.h |1 + >>> drivers/hwtracing/coresight/Kconfig | 11 + >>> drivers/hwtracing/coresight/Makefile |1 + >>> drivers/hwtracing/coresight/coresight-trbe.c | 1023 >>> ++ >>> drivers/hwtracing/coresight/coresight-trbe.h | 160 >>> 6 files changed, 1235 insertions(+) >>> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst >>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c >>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h >>> >>> diff --git a/Documentation/trace/coresight/coresight-trbe.rst >>> b/Documentation/trace/coresight/coresight-trbe.rst >>> new file mode 100644 >>> index 000..1cbb819 >>> --- /dev/null >>> +++ b/Documentation/trace/coresight/coresight-trbe.rst >>> @@ -0,0 +1,39 @@ >>> +.. SPDX-License-Identifier: GPL-2.0 >>> + >>> +== >>> +Trace Buffer Extension (TRBE). >>> +== >>> + >>> +:Author: Anshuman Khandual >>> +:Date: November 2020 >>> + >>> +Hardware Description >>> + >>> + >>> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system >>> +memory, CPU traces generated from a corresponding percpu tracing unit. This >>> +gets plugged in as a coresight sink device because the corresponding trace >>> +genarators (ETE), are plugged in as source device. >>> + >>> +The TRBE is not compliant to CoreSight architecture specifications, but is >>> +driven via the CoreSight driver framework to support the ETE (which is >>> +CoreSight compliant) integration. >>> + >>> +Sysfs files and directories >>> +--- >>> + >>> +The TRBE devices appear on the existing coresight bus alongside the other >>> +coresight devices:: >>> + >>> + >$ ls /sys/bus/coresight/devices >>> + trbe0 trbe1 trbe2 trbe3 >>> + >>> +The ``trbe`` named TRBEs are associated with a CPU.:: >>> + >>> + >$ ls /sys/bus/coresight/devices/trbe0/ >>> +align dbm >>> + >>> +*Key file items are:-* >>> + * ``align``: TRBE write pointer alignment >>> + * ``dbm``: TRBE updates memory with access and dirty flags >>> + >> >> Please add documentation for these, the same way it was done for all the >> other CS >> components [1]. >> >> [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing >> (sysfs-bus-coresight-device-xyz) >> >>> diff --git a/arch/arm64/include/asm/sysreg.h >>> b/arch/arm64/include/asm/sysreg.h >>> index 85ae4db..9e2e9b7 100644 >>> --- a/arch/arm64/include/asm/sysreg.h >>> +++ b/arch/arm64/include/asm/sysreg.h >>> @@ -97,6 +97,7 @@ >>> #define SET_PSTATE_UAO(x) __emit_i
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Fri, Feb 05, 2021 at 10:53:30AM -0700, Mathieu Poirier wrote: > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: > > Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is > > accessible via the system registers. The TRBE supports different addressing > > modes including CPU virtual address and buffer modes including the circular > > buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), > > an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the > > access to the trace buffer could be prohibited by a higher exception level > > (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU > > private interrupt (PPI) on address translation errors and when the buffer > > is full. Overall implementation here is inspired from the Arm SPE driver. > > > > I got this message when applying the patch: > > Applying: coresight: sink: Add TRBE driver > .git/rebase-apply/patch:76: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > > > Cc: Mathieu Poirier > > Cc: Mike Leach > > Cc: Suzuki K Poulose > > Signed-off-by: Anshuman Khandual > > --- > > Changes in V3: > > > > - Added new DT bindings document TRBE.yaml > > - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 > > - Dropped isb() from trbe_reset_local() > > - Dropped gap between (void *) and buf->trbe_base > > - Changed 'int' to 'unsigned int' in is_trbe_available() > > - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), > > set_trbe_enabled() and set_trbe_limit_pointer() > > - Changed get_trbe_flag_update(), is_trbe_programmable() and > > get_trbe_address_align() to accept TRBIDR value > > - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), > > is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value > > - Dropped snapshot mode condition in arm_trbe_alloc_buffer() > > - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled > > - Compute trbe_limit before trbe_write to get the updated handle > > - Added trbe_stop_and_truncate_event() > > - Dropped trbe_handle_fatal() > > > > Documentation/trace/coresight/coresight-trbe.rst | 39 + > > arch/arm64/include/asm/sysreg.h |1 + > > drivers/hwtracing/coresight/Kconfig | 11 + > > drivers/hwtracing/coresight/Makefile |1 + > > drivers/hwtracing/coresight/coresight-trbe.c | 1023 > > ++ > > drivers/hwtracing/coresight/coresight-trbe.h | 160 > > 6 files changed, 1235 insertions(+) > > create mode 100644 Documentation/trace/coresight/coresight-trbe.rst > > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c > > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h > > > > diff --git a/Documentation/trace/coresight/coresight-trbe.rst > > b/Documentation/trace/coresight/coresight-trbe.rst > > new file mode 100644 > > index 000..1cbb819 > > --- /dev/null > > +++ b/Documentation/trace/coresight/coresight-trbe.rst > > @@ -0,0 +1,39 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +== > > +Trace Buffer Extension (TRBE). > > +== > > + > > +:Author: Anshuman Khandual > > +:Date: November 2020 > > + > > +Hardware Description > > + > > + > > +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system > > +memory, CPU traces generated from a corresponding percpu tracing unit. This > > +gets plugged in as a coresight sink device because the corresponding trace > > +genarators (ETE), are plugged in as source device. > > + > > +The TRBE is not compliant to CoreSight architecture specifications, but is > > +driven via the CoreSight driver framework to support the ETE (which is > > +CoreSight compliant) integration. > > + > > +Sysfs files and directories > > +--- > > + > > +The TRBE devices appear on the existing coresight bus alongside the other > > +coresight devices:: > > + > > + >$ ls /sys/bus/coresight/devices > > + trbe0 trbe1 trbe2 trbe3 > > + > > +The ``trbe`` named TRBEs are associated with a CPU.:: > > + > > + >$ ls /sys/bus/coresight/devices/trbe0/ > > +align dbm > > + > > +*Key file items are:-* > > + * ``align``: TRBE write pointer alignment > > + * ``dbm``: TRBE updates memory with access and dirty flags > > + > > Please add documentation for these, the same way it was done for all the > other CS > components [1]. > > [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing > (sysfs-bus-coresight-device-xyz) > > > diff --git a/arch/arm64/include/asm/sysreg.h > > b/arch/arm64/include/asm/sysreg.h > > index 85ae4db..9e2e9b7 100644 > > --- a/arch/arm64/include/asm/sysreg.h > > +++ b/arch/arm64/include/asm/sysreg.h > > @@ -97,6 +97,7 @@ > > #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | > > ((!!x) << PSTATE_Imm_s
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 2/5/21 11:23 PM, Mathieu Poirier wrote: > On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is >> accessible via the system registers. The TRBE supports different addressing >> modes including CPU virtual address and buffer modes including the circular >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the >> access to the trace buffer could be prohibited by a higher exception level >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU >> private interrupt (PPI) on address translation errors and when the buffer >> is full. Overall implementation here is inspired from the Arm SPE driver. >> > > I got this message when applying the patch: > > Applying: coresight: sink: Add TRBE driver > .git/rebase-apply/patch:76: new blank line at EOF. > + > warning: 1 line adds whitespace errors. It could be the additional blank line at the end of documentation file i.e Documentation/trace/coresight/coresight-trbe.rst, will drop it. > >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Suzuki K Poulose >> Signed-off-by: Anshuman Khandual >> --- >> Changes in V3: >> >> - Added new DT bindings document TRBE.yaml >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 >> - Dropped isb() from trbe_reset_local() >> - Dropped gap between (void *) and buf->trbe_base >> - Changed 'int' to 'unsigned int' in is_trbe_available() >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), >> set_trbe_enabled() and set_trbe_limit_pointer() >> - Changed get_trbe_flag_update(), is_trbe_programmable() and >> get_trbe_address_align() to accept TRBIDR value >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled >> - Compute trbe_limit before trbe_write to get the updated handle >> - Added trbe_stop_and_truncate_event() >> - Dropped trbe_handle_fatal() >> >> Documentation/trace/coresight/coresight-trbe.rst | 39 + >> arch/arm64/include/asm/sysreg.h |1 + >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile |1 + >> drivers/hwtracing/coresight/coresight-trbe.c | 1023 >> ++ >> drivers/hwtracing/coresight/coresight-trbe.h | 160 >> 6 files changed, 1235 insertions(+) >> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h >> >> diff --git a/Documentation/trace/coresight/coresight-trbe.rst >> b/Documentation/trace/coresight/coresight-trbe.rst >> new file mode 100644 >> index 000..1cbb819 >> --- /dev/null >> +++ b/Documentation/trace/coresight/coresight-trbe.rst >> @@ -0,0 +1,39 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +== >> +Trace Buffer Extension (TRBE). >> +== >> + >> +:Author: Anshuman Khandual >> +:Date: November 2020 >> + >> +Hardware Description >> + >> + >> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system >> +memory, CPU traces generated from a corresponding percpu tracing unit. This >> +gets plugged in as a coresight sink device because the corresponding trace >> +genarators (ETE), are plugged in as source device. >> + >> +The TRBE is not compliant to CoreSight architecture specifications, but is >> +driven via the CoreSight driver framework to support the ETE (which is >> +CoreSight compliant) integration. >> + >> +Sysfs files and directories >> +--- >> + >> +The TRBE devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> +>$ ls /sys/bus/coresight/devices >> +trbe0 trbe1 trbe2 trbe3 >> + >> +The ``trbe`` named TRBEs are associated with a CPU.:: >> + >> +>$ ls /sys/bus/coresight/devices/trbe0/ >> +align dbm >> + >> +*Key file items are:-* >> + * ``align``: TRBE write pointer alignment >> + * ``dbm``: TRBE updates memory with access and dirty flags >> + > > Please add documentation for these, the same way it was done for all the > other CS > components [1]. > > [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing > (sysfs-bus-coresight-device-xyz) Sure, will add the following new sysfs doc file in this regard. Marked the KernelVersion as 5.12, will change if required. new file mode 100644 index 000..5cb090f --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-trbe @@ -0,0 +1,14 @@ +What: /sys/bus/coresight/devic
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote: > Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is > accessible via the system registers. The TRBE supports different addressing > modes including CPU virtual address and buffer modes including the circular > buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), > an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the > access to the trace buffer could be prohibited by a higher exception level > (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU > private interrupt (PPI) on address translation errors and when the buffer > is full. Overall implementation here is inspired from the Arm SPE driver. > I got this message when applying the patch: Applying: coresight: sink: Add TRBE driver .git/rebase-apply/patch:76: new blank line at EOF. + warning: 1 line adds whitespace errors. > Cc: Mathieu Poirier > Cc: Mike Leach > Cc: Suzuki K Poulose > Signed-off-by: Anshuman Khandual > --- > Changes in V3: > > - Added new DT bindings document TRBE.yaml > - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 > - Dropped isb() from trbe_reset_local() > - Dropped gap between (void *) and buf->trbe_base > - Changed 'int' to 'unsigned int' in is_trbe_available() > - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), > set_trbe_enabled() and set_trbe_limit_pointer() > - Changed get_trbe_flag_update(), is_trbe_programmable() and > get_trbe_address_align() to accept TRBIDR value > - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), > is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value > - Dropped snapshot mode condition in arm_trbe_alloc_buffer() > - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled > - Compute trbe_limit before trbe_write to get the updated handle > - Added trbe_stop_and_truncate_event() > - Dropped trbe_handle_fatal() > > Documentation/trace/coresight/coresight-trbe.rst | 39 + > arch/arm64/include/asm/sysreg.h |1 + > drivers/hwtracing/coresight/Kconfig | 11 + > drivers/hwtracing/coresight/Makefile |1 + > drivers/hwtracing/coresight/coresight-trbe.c | 1023 > ++ > drivers/hwtracing/coresight/coresight-trbe.h | 160 > 6 files changed, 1235 insertions(+) > create mode 100644 Documentation/trace/coresight/coresight-trbe.rst > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c > create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h > > diff --git a/Documentation/trace/coresight/coresight-trbe.rst > b/Documentation/trace/coresight/coresight-trbe.rst > new file mode 100644 > index 000..1cbb819 > --- /dev/null > +++ b/Documentation/trace/coresight/coresight-trbe.rst > @@ -0,0 +1,39 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +== > +Trace Buffer Extension (TRBE). > +== > + > +:Author: Anshuman Khandual > +:Date: November 2020 > + > +Hardware Description > + > + > +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system > +memory, CPU traces generated from a corresponding percpu tracing unit. This > +gets plugged in as a coresight sink device because the corresponding trace > +genarators (ETE), are plugged in as source device. > + > +The TRBE is not compliant to CoreSight architecture specifications, but is > +driven via the CoreSight driver framework to support the ETE (which is > +CoreSight compliant) integration. > + > +Sysfs files and directories > +--- > + > +The TRBE devices appear on the existing coresight bus alongside the other > +coresight devices:: > + > + >$ ls /sys/bus/coresight/devices > + trbe0 trbe1 trbe2 trbe3 > + > +The ``trbe`` named TRBEs are associated with a CPU.:: > + > + >$ ls /sys/bus/coresight/devices/trbe0/ > +align dbm > + > +*Key file items are:-* > + * ``align``: TRBE write pointer alignment > + * ``dbm``: TRBE updates memory with access and dirty flags > + Please add documentation for these, the same way it was done for all the other CS components [1]. [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing (sysfs-bus-coresight-device-xyz) > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 85ae4db..9e2e9b7 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -97,6 +97,7 @@ > #define SET_PSTATE_UAO(x)__emit_inst(0xd500401f | PSTATE_UAO | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | > ((!!x) << PSTATE_Imm_shift)) > #define SET_PSTATE_TCO(x)__emit_inst(0xd500401f | PSTATE_TCO | > ((!!x) << PSTATE_Imm_shift)) > +#define TSB_CSYNC__emit_inst(0xd503225f) >
Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver
On 1/29/21 3:53 PM, Suzuki K Poulose wrote: > Hi Anshuman > > On 1/27/21 8:55 AM, Anshuman Khandual wrote: >> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is >> accessible via the system registers. The TRBE supports different addressing >> modes including CPU virtual address and buffer modes including the circular >> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), >> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the >> access to the trace buffer could be prohibited by a higher exception level >> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU >> private interrupt (PPI) on address translation errors and when the buffer >> is full. Overall implementation here is inspired from the Arm SPE driver. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Suzuki K Poulose >> Signed-off-by: Anshuman Khandual > > This version looks functionally correct to me. There are some minor > issues with the devm_ allocated memory and some driver hardening comments. > I ran this on a model and have tested this with various scenarios. Okay. > >> --- >> Changes in V3: >> >> - Added new DT bindings document TRBE.yaml >> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 >> - Dropped isb() from trbe_reset_local() >> - Dropped gap between (void *) and buf->trbe_base >> - Changed 'int' to 'unsigned int' in is_trbe_available() >> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), >> set_trbe_enabled() and set_trbe_limit_pointer() >> - Changed get_trbe_flag_update(), is_trbe_programmable() and >> get_trbe_address_align() to accept TRBIDR value >> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), >> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value >> - Dropped snapshot mode condition in arm_trbe_alloc_buffer() >> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled >> - Compute trbe_limit before trbe_write to get the updated handle >> - Added trbe_stop_and_truncate_event() >> - Dropped trbe_handle_fatal() >> >> Documentation/trace/coresight/coresight-trbe.rst | 39 + >> arch/arm64/include/asm/sysreg.h | 1 + >> drivers/hwtracing/coresight/Kconfig | 11 + >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/coresight-trbe.c | 1023 >> ++ >> drivers/hwtracing/coresight/coresight-trbe.h | 160 >> 6 files changed, 1235 insertions(+) >> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c >> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h >> >> diff --git a/Documentation/trace/coresight/coresight-trbe.rst >> b/Documentation/trace/coresight/coresight-trbe.rst >> new file mode 100644 >> index 000..1cbb819 >> --- /dev/null >> +++ b/Documentation/trace/coresight/coresight-trbe.rst >> @@ -0,0 +1,39 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +== >> +Trace Buffer Extension (TRBE). >> +== >> + >> + :Author: Anshuman Khandual >> + :Date: November 2020 >> + >> +Hardware Description >> + >> + >> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system >> +memory, CPU traces generated from a corresponding percpu tracing unit. This >> +gets plugged in as a coresight sink device because the corresponding trace >> +genarators (ETE), are plugged in as source device. >> + >> +The TRBE is not compliant to CoreSight architecture specifications, but is >> +driven via the CoreSight driver framework to support the ETE (which is >> +CoreSight compliant) integration. >> + >> +Sysfs files and directories >> +--- >> + >> +The TRBE devices appear on the existing coresight bus alongside the other >> +coresight devices:: >> + >> + >$ ls /sys/bus/coresight/devices >> + trbe0 trbe1 trbe2 trbe3 >> + >> +The ``trbe`` named TRBEs are associated with a CPU.:: >> + >> + >$ ls /sys/bus/coresight/devices/trbe0/ >> + align dbm >> + >> +*Key file items are:-* >> + * ``align``: TRBE write pointer alignment >> + * ``dbm``: TRBE updates memory with access and dirty flags >> + >> diff --git a/arch/arm64/include/asm/sysreg.h >> b/arch/arm64/include/asm/sysreg.h >> index 85ae4db..9e2e9b7 100644 >> --- a/arch/arm64/include/asm/sysreg.h >> +++ b/arch/arm64/include/asm/sysreg.h >> @@ -97,6 +97,7 @@ >> #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | >> ((!!x) << PSTATE_Imm_shift)) >> #define SET_PSTATE_TCO(x) __emit_inst(0xd500401f | PSTATE_TCO | >> ((!!x) << PSTATE_Imm_shift)) >> +#define TSB_CSYNC __emit_inst(0xd503225f) >> #define set_pstate_pa
[PATCH V3 11/14] coresight: sink: Add TRBE driver
Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is accessible via the system registers. The TRBE supports different addressing modes including CPU virtual address and buffer modes including the circular buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1), an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the access to the trace buffer could be prohibited by a higher exception level (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU private interrupt (PPI) on address translation errors and when the buffer is full. Overall implementation here is inspired from the Arm SPE driver. Cc: Mathieu Poirier Cc: Mike Leach Cc: Suzuki K Poulose Signed-off-by: Anshuman Khandual --- Changes in V3: - Added new DT bindings document TRBE.yaml - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3 - Dropped isb() from trbe_reset_local() - Dropped gap between (void *) and buf->trbe_base - Changed 'int' to 'unsigned int' in is_trbe_available() - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(), set_trbe_enabled() and set_trbe_limit_pointer() - Changed get_trbe_flag_update(), is_trbe_programmable() and get_trbe_address_align() to accept TRBIDR value - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(), is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value - Dropped snapshot mode condition in arm_trbe_alloc_buffer() - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled - Compute trbe_limit before trbe_write to get the updated handle - Added trbe_stop_and_truncate_event() - Dropped trbe_handle_fatal() Documentation/trace/coresight/coresight-trbe.rst | 39 + arch/arm64/include/asm/sysreg.h |1 + drivers/hwtracing/coresight/Kconfig | 11 + drivers/hwtracing/coresight/Makefile |1 + drivers/hwtracing/coresight/coresight-trbe.c | 1023 ++ drivers/hwtracing/coresight/coresight-trbe.h | 160 6 files changed, 1235 insertions(+) create mode 100644 Documentation/trace/coresight/coresight-trbe.rst create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h diff --git a/Documentation/trace/coresight/coresight-trbe.rst b/Documentation/trace/coresight/coresight-trbe.rst new file mode 100644 index 000..1cbb819 --- /dev/null +++ b/Documentation/trace/coresight/coresight-trbe.rst @@ -0,0 +1,39 @@ +.. SPDX-License-Identifier: GPL-2.0 + +== +Trace Buffer Extension (TRBE). +== + +:Author: Anshuman Khandual +:Date: November 2020 + +Hardware Description + + +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system +memory, CPU traces generated from a corresponding percpu tracing unit. This +gets plugged in as a coresight sink device because the corresponding trace +genarators (ETE), are plugged in as source device. + +The TRBE is not compliant to CoreSight architecture specifications, but is +driven via the CoreSight driver framework to support the ETE (which is +CoreSight compliant) integration. + +Sysfs files and directories +--- + +The TRBE devices appear on the existing coresight bus alongside the other +coresight devices:: + + >$ ls /sys/bus/coresight/devices + trbe0 trbe1 trbe2 trbe3 + +The ``trbe`` named TRBEs are associated with a CPU.:: + + >$ ls /sys/bus/coresight/devices/trbe0/ +align dbm + +*Key file items are:-* + * ``align``: TRBE write pointer alignment + * ``dbm``: TRBE updates memory with access and dirty flags + diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 85ae4db..9e2e9b7 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -97,6 +97,7 @@ #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | ((!!x) << PSTATE_Imm_shift)) #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | ((!!x) << PSTATE_Imm_shift)) #define SET_PSTATE_TCO(x) __emit_inst(0xd500401f | PSTATE_TCO | ((!!x) << PSTATE_Imm_shift)) +#define TSB_CSYNC __emit_inst(0xd503225f) #define set_pstate_pan(x) asm volatile(SET_PSTATE_PAN(x)) #define set_pstate_uao(x) asm volatile(SET_PSTATE_UAO(x)) diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index f154ae7..aa657ab 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -164,6 +164,17 @@ config CORESIGHT_CTI To compile this driver as a module, choose M here: the module will be called coresight-cti. +config CORESIGHT_TRBE + bool "Trace Buffer Extension (TRBE) driver" + depends on ARM64 + help + This driver provides support for p
