CoreSight

coresight@lists.linaro.org

2379 discussions

Re: Linux coresight arm64 : Incorrect data in cstrace.bin

by Suzuki K Poulose

Hi Radhe Shyam On 05/07/2023 05:38, Pandey, Radhey Shyam wrote: > Hi, > > I am using linux 6.1 kernel coresight framework to capture ETM trace. > Enabled coresight driver and added coresight component device node > in DTS. With that, we could probe all coresight components. > > But when we do capture using sysfs and then read the trace.bin using > ptm2human we see invalid trace data. Trace content changes on each > capture. Any pointers to help narrow down the issue? > > Tried: sysfs capture and perf (with OpenCSD enabled) > > Development board: VCK190 : > https://docs.xilinx.com/r/en-US/ug1366-vck190-eval-bd https://www.xilinx.com/support/documents/architecture-manuals/am011-versal-… > > xilinx-vck190-20231:/home/petalinux# dmesg | grep -i amba > [ 0.301054] Serial: AMBA PL011 UART driver > [ 0.306226] amba f0c20000.funnel: Fixing up cyclic dependency with f0d70000.etm > [ 0.313624] amba f0c20000.funnel: Fixing up cyclic dependency with f0d30000.etm > [ 0.321110] amba f0c30000.etf: Fixing up cyclic dependency with f0c20000.funnel > [ 1.470247] Serial: AMBA driver > xilinx-vck190-20231:/home/petalinux# dmesg | grep -i coresight > [ 1.865006] cs_system_cfg: CoreSight Configuration manager initialised > [ 1.876858] coresight etm0: CPU0: etm v4.0 initialized > [ 1.885666] coresight etm1: CPU1: etm v4.0 initialized > [ 1.894435] coresight-cpu-debug f0d00000.debug1: Coresight debug-CPU0 initialized > [ 1.902072] coresight-cpu-debug f0d40000.debug1: Coresight debug-CPU1 initialized > > xilinx-vck190-20231:/home/petalinux# ls /sys/bus/coresight/devices/ > etm0 etm1 funnel0 tmc_etf0 > > cd /sys/bus/coresight/devices/ > echo 1 > tmc_etf0/enable_sink > echo 1 > etm0/enable_source > echo 0 > etm0/enable_source > echo 0 > tmc_etf0/enable_sink > cd /root/ dd if=/dev/tmc_etf0 of=cstrace_28Jun.bin > > ./ptm2human/ptm2human -e -i cstrace.bin -d Please note that ptm2human is for PTM trace decoding and ETMv4 uses a different format and thus is not compatible. > > I also tried OpenCSD integration with PERF. Have you made sure the perf is "linked" to the opencsd ? $ ldd $(which perf) | grep opencsd With opencsd in, perf report -D should be able to decode the trace packets. Suzuki > > 0x1e0(a)perf.data [0x90]: event: 69 > . > . ... raw event: size 144 bytes > . 0000: 45 00 00 00 00 00 90 00 04 00 00 00 00 00 00 00 E............... > . 0010: 48 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 H............... > . 0020: 00 00 00 00 00 00 00 00 74 03 00 00 00 00 00 00 ........t....... > . 0030: 49 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 I............... > . 0040: 01 00 00 00 00 00 00 00 74 03 00 00 00 00 00 00 ........t....... > . 0050: 4a 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 J............... > . 0060: 00 00 00 00 00 00 00 00 74 03 00 00 00 00 00 00 ........t....... > . 0070: 4b 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 K............... > . 0080: 01 00 00 00 00 00 00 00 74 03 00 00 00 00 00 00 ........t....... > > 0 0 0x1e0 [0x90]: PERF_RECORD_ID_INDEX nr: 4 > ... id: 72 idx: 0 cpu: 0 tid: 884 > ... id: 73 idx: 1 cpu: 1 tid: 884 > ... id: 74 idx: 0 cpu: 0 tid: 884 > ... id: 75 idx: 1 cpu: 1 tid: 884 > > 0x270(a)perf.data [0xc8]: event: 70 > . > . ... raw event: size 200 bytes > . 0000: 46 00 00 00 00 00 c8 00 03 00 00 00 00 00 00 00 F............... > . 0010: 01 00 00 00 00 00 00 00 02 00 00 00 08 00 00 00 ................ > . 0020: 00 00 00 00 00 00 00 00 40 40 40 40 40 40 40 40 ........@@@@@@@@ > . 0030: 00 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................ > . 0040: 40 08 00 00 00 00 00 00 10 00 00 00 00 00 00 00 @............... > . 0050: a1 0e 00 28 00 00 00 00 01 f4 00 41 00 00 00 00 ...(.......A.... > . 0060: 88 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > . 0070: cc 00 00 00 00 00 00 00 40 40 40 40 40 40 40 40 ........@@@@@@@@ > . 0080: 01 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................ > . 0090: 40 08 00 00 00 00 00 00 12 00 00 00 00 00 00 00 @............... > . 00a0: a1 0e 00 28 00 00 00 00 01 f4 00 41 00 00 00 00 ...(.......A.... > . 00b0: 88 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > . 00c0: cc 00 00 00 00 00 00 00 ........ > > 0 0 0x270 [0xc8]: PERF_RECORD_AUXTRACE_INFO type: 3 > - log 58/58 100% > > > Thanks, > Radhey

1 year, 12 months

Re: [PATCH 1/1] arm64: dts: imx8mp: Add coresight trace components

by Suzuki K Poulose

On 05/05/2023 20:51, Frank Li wrote: > Add coresight trace components (ETM, ETF, ETB and Funnel). > > ┌───────┐ ┌───────┐ ┌───────┐ > │ CPU0 ├─►│ ETM0 ├─►│ │ > └───────┘ └───────┘ │ │ > │ │ > ┌───────┐ ┌───────┐ │ ATP │ > │ CPU1 ├─►│ ETM1 ├─►│ │ > └───────┘ └───────┘ │ │ > │ FUNNEL│ > ┌───────┐ ┌───────┐ │ │ > │ CPU2 ├─►│ ETM2 ├─►│ │ > └───────┘ └───────┘ │ │ ┌─────┐ ┌─────┐ > │ │ │ │ │ │ > ┌───────┐ ┌───────┐ │ │ │ M7 │ │ DSP │ > │ CPU3 ├─►│ ETM3 ├─►│ │ │ │ │ │ > └───────┘ └───────┘ └───┬───┘ └──┬──┘ └──┬──┘ AXI > │ │ │ ▲ > ▼ ▼ ▼ │ > ┌───────────────────────────┐ ┌─────┐ ┌─┴──┐ > │ ATP FUNNEL ├──►│ETF ├─► │ETR │ > └───────────────────────────┘ └─────┘ └────┘ > > Signed-off-by: Frank Li <Frank.Li(a)nxp.com> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 179 ++++++++++++++++++++++ > 1 file changed, 179 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > index a19224fe1a6a..0fa74477b9e1 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -304,6 +304,185 @@ soc: soc@0 { > nvmem-cells = <&imx8mp_uid>; > nvmem-cell-names = "soc_unique_id"; > > + etm0: etm@28440000 { > + compatible = "arm,coresight-etm4x", "arm,primecell"; > + reg = <0x28440000 0x10000>; > + arm,primecell-periphid = <0xbb95d>; Why is this needed (and for all the ETMs) ? Suzuki

1 year, 12 months

Re: [PATCH] dt-bindings: cleanup DTS example whitespaces

by Mike Leach

On Sun, 2 Jul 2023 at 19:23, Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> wrote: > > The DTS code coding style expects spaces around '=' sign. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > > --- > > Rob, > > Maybe this could go via your tree? Rebased on your for-next: > v6.4-rc2-45-gf0ac35049606 > --- > .../bindings/arm/arm,coresight-cti.yaml | 18 +++++++++--------- > .../bindings/arm/keystone/ti,sci.yaml | 8 ++++---- > .../devicetree/bindings/display/msm/gmu.yaml | 2 +- > .../display/panel/samsung,s6e8aa0.yaml | 2 +- > .../display/rockchip/rockchip-vop.yaml | 4 ++-- > .../bindings/iio/adc/ti,adc108s102.yaml | 2 +- > .../bindings/media/renesas,rzg2l-cru.yaml | 4 ++-- > .../devicetree/bindings/media/renesas,vin.yaml | 4 ++-- > .../devicetree/bindings/mtd/mtd-physmap.yaml | 2 +- > .../bindings/net/mediatek-dwmac.yaml | 2 +- > .../bindings/perf/amlogic,g12-ddr-pmu.yaml | 4 ++-- > .../bindings/phy/mediatek,dsi-phy.yaml | 2 +- > .../remoteproc/amlogic,meson-mx-ao-arc.yaml | 2 +- > .../devicetree/bindings/usb/mediatek,mtu3.yaml | 2 +- > .../devicetree/bindings/usb/ti,am62-usb.yaml | 2 +- > 15 files changed, 30 insertions(+), 30 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml > index 0c5b875cb654..d6c84b6e7fe6 100644 > --- a/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml > +++ b/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml > @@ -287,7 +287,7 @@ examples: > arm,trig-in-sigs = <0 1>; > arm,trig-in-types = <PE_DBGTRIGGER > PE_PMUIRQ>; > - arm,trig-out-sigs=<0 1 2 >; > + arm,trig-out-sigs = <0 1 2 >; > arm,trig-out-types = <PE_EDBGREQ > PE_DBGRESTART > PE_CTIIRQ>; > @@ -309,24 +309,24 @@ examples: > > trig-conns@0 { > reg = <0>; > - arm,trig-in-sigs=<0>; > - arm,trig-in-types=<GEN_INTREQ>; > - arm,trig-out-sigs=<0>; > - arm,trig-out-types=<GEN_HALTREQ>; > + arm,trig-in-sigs = <0>; > + arm,trig-in-types = <GEN_INTREQ>; > + arm,trig-out-sigs = <0>; > + arm,trig-out-types = <GEN_HALTREQ>; > arm,trig-conn-name = "sys_profiler"; > }; > > trig-conns@1 { > reg = <1>; > - arm,trig-out-sigs=<2 3>; > - arm,trig-out-types=<GEN_HALTREQ GEN_RESTARTREQ>; > + arm,trig-out-sigs = <2 3>; > + arm,trig-out-types = <GEN_HALTREQ GEN_RESTARTREQ>; > arm,trig-conn-name = "watchdog"; > }; > > trig-conns@2 { > reg = <2>; > - arm,trig-in-sigs=<1 6>; > - arm,trig-in-types=<GEN_HALTREQ GEN_RESTARTREQ>; > + arm,trig-in-sigs = <1 6>; > + arm,trig-in-types = <GEN_HALTREQ GEN_RESTARTREQ>; > arm,trig-conn-name = "g_counter"; > }; > }; for above CTI chagnes Acked-by: Mike Leach <mike.leach(a)linaro.org> > diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml > index 91b96065f7df..86b59de7707e 100644 > --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml > +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml > @@ -96,8 +96,8 @@ examples: > compatible = "ti,k2g-sci"; > ti,system-reboot-controller; > mbox-names = "rx", "tx"; > - mboxes= <&msgmgr 5 2>, > - <&msgmgr 0 0>; > + mboxes = <&msgmgr 5 2>, > + <&msgmgr 0 0>; > reg-names = "debug_messages"; > reg = <0x02921800 0x800>; > }; > @@ -107,8 +107,8 @@ examples: > compatible = "ti,k2g-sci"; > ti,host-id = <12>; > mbox-names = "rx", "tx"; > - mboxes= <&secure_proxy_main 11>, > - <&secure_proxy_main 13>; > + mboxes = <&secure_proxy_main 11>, > + <&secure_proxy_main 13>; > reg-names = "debug_messages"; > reg = <0x44083000 0x1000>; > > diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml > index 029d72822d8b..65b02c7a1211 100644 > --- a/Documentation/devicetree/bindings/display/msm/gmu.yaml > +++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml > @@ -225,7 +225,7 @@ examples: > #include <dt-bindings/interrupt-controller/arm-gic.h> > > gmu: gmu@506a000 { > - compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu"; > + compatible = "qcom,adreno-gmu-630.2", "qcom,adreno-gmu"; > > reg = <0x506a000 0x30000>, > <0xb280000 0x10000>, > diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e8aa0.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6e8aa0.yaml > index 1cdc91b3439f..200fbf1c74a0 100644 > --- a/Documentation/devicetree/bindings/display/panel/samsung,s6e8aa0.yaml > +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e8aa0.yaml > @@ -74,7 +74,7 @@ examples: > vdd3-supply = <&vcclcd_reg>; > vci-supply = <&vlcd_reg>; > reset-gpios = <&gpy4 5 0>; > - power-on-delay= <50>; > + power-on-delay = <50>; > reset-delay = <100>; > init-delay = <100>; > panel-width-mm = <58>; > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > index 6f43d885c9b3..df61cb5f5c54 100644 > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > @@ -121,11 +121,11 @@ examples: > #size-cells = <0>; > vopb_out_edp: endpoint@0 { > reg = <0>; > - remote-endpoint=<&edp_in_vopb>; > + remote-endpoint = <&edp_in_vopb>; > }; > vopb_out_hdmi: endpoint@1 { > reg = <1>; > - remote-endpoint=<&hdmi_in_vopb>; > + remote-endpoint = <&hdmi_in_vopb>; > }; > }; > }; > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc108s102.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc108s102.yaml > index 9b072b057f16..a60b1e100ee4 100644 > --- a/Documentation/devicetree/bindings/iio/adc/ti,adc108s102.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc108s102.yaml > @@ -35,7 +35,7 @@ unevaluatedProperties: false > examples: > - | > spi { > - #address-cells= <1>; > + #address-cells = <1>; > #size-cells = <0>; > > adc@0 { > diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml > index 7dde7967c886..1e72b8808d24 100644 > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml > @@ -137,7 +137,7 @@ examples: > > cru_parallel_in: endpoint@0 { > reg = <0>; > - remote-endpoint= <&ov5642>; > + remote-endpoint = <&ov5642>; > hsync-active = <1>; > vsync-active = <1>; > }; > @@ -150,7 +150,7 @@ examples: > > cru_csi_in: endpoint@0 { > reg = <0>; > - remote-endpoint= <&csi_cru_in>; > + remote-endpoint = <&csi_cru_in>; > }; > }; > }; > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > index 91e8f368fb52..324703bfb1bd 100644 > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > @@ -303,11 +303,11 @@ examples: > > vin0csi20: endpoint@0 { > reg = <0>; > - remote-endpoint= <&csi20vin0>; > + remote-endpoint = <&csi20vin0>; > }; > vin0csi40: endpoint@2 { > reg = <2>; > - remote-endpoint= <&csi40vin0>; > + remote-endpoint = <&csi40vin0>; > }; > }; > }; > diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.yaml b/Documentation/devicetree/bindings/mtd/mtd-physmap.yaml > index f8c976898a95..18f6733408b4 100644 > --- a/Documentation/devicetree/bindings/mtd/mtd-physmap.yaml > +++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.yaml > @@ -164,7 +164,7 @@ examples: > reg = <0 0xf80000>; > }; > firmware@f80000 { > - label ="firmware"; > + label = "firmware"; > reg = <0xf80000 0x80000>; > read-only; > }; > diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml > index 0fa2132fa4f4..400aedb58205 100644 > --- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml > @@ -156,7 +156,7 @@ examples: > reg = <0x1101c000 0x1300>; > interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>; > interrupt-names = "macirq"; > - phy-mode ="rgmii-rxid"; > + phy-mode = "rgmii-rxid"; > mac-address = [00 55 7b b5 7d f7]; > clock-names = "axi", > "apb", > diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml > index 50f46a6898b1..4adab0149108 100644 > --- a/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml > +++ b/Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml > @@ -42,8 +42,8 @@ examples: > - | > #include <dt-bindings/interrupt-controller/arm-gic.h> > pmu { > - #address-cells=<2>; > - #size-cells=<2>; > + #address-cells = <2>; > + #size-cells = <2>; > > pmu@ff638000 { > compatible = "amlogic,g12a-ddr-pmu"; > diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > index 26f2b887cfc1..b8d77165c4a1 100644 > --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > @@ -83,7 +83,7 @@ examples: > clocks = <&clk26m>; > clock-output-names = "mipi_tx0_pll"; > drive-strength-microamp = <4000>; > - nvmem-cells= <&mipi_tx_calibration>; > + nvmem-cells = <&mipi_tx_calibration>; > nvmem-cell-names = "calibration-data"; > #clock-cells = <0>; > #phy-cells = <0>; > diff --git a/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml b/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml > index 3100cb870170..76e8ca44906a 100644 > --- a/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml > @@ -75,7 +75,7 @@ additionalProperties: false > examples: > - | > remoteproc@1c { > - compatible= "amlogic,meson8-ao-arc", "amlogic,meson-mx-ao-arc"; > + compatible = "amlogic,meson8-ao-arc", "amlogic,meson-mx-ao-arc"; > reg = <0x1c 0x8>, <0x38 0x8>; > reg-names = "remap", "cpu"; > resets = <&media_cpu_reset>; > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml > index 478214ab045e..a59d91243ac8 100644 > --- a/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml > @@ -304,7 +304,7 @@ examples: > # Dual role switch with type-c > - | > usb@11201000 { > - compatible ="mediatek,mt8183-mtu3", "mediatek,mtu3"; > + compatible = "mediatek,mt8183-mtu3", "mediatek,mtu3"; > reg = <0x11201000 0x2e00>, <0x11203e00 0x0100>; > reg-names = "mac", "ippc"; > interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_LOW>; > diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml > index d25fc708e32c..fec5651f5602 100644 > --- a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml > +++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml > @@ -92,7 +92,7 @@ examples: > > usb@31100000 { > compatible = "snps,dwc3"; > - reg =<0x00 0x31100000 0x00 0x50000>; > + reg = <0x00 0x31100000 0x00 0x50000>; > interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */ > <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>; /* irq.0 */ > interrupt-names = "host", "peripheral"; > -- > 2.34.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel(a)lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 year, 12 months

Re: [PATCH 2/2] perf report: Don't add to histogram when there is no thread found

by James Clark

On 30/06/2023 22:02, Namhyung Kim wrote: > On Wed, Jun 28, 2023 at 1:06 PM Namhyung Kim <namhyung(a)kernel.org> wrote: >> >> On Wed, Jun 28, 2023 at 3:34 AM James Clark <james.clark(a)arm.com> wrote: >>> >>> >>> >>> On 27/06/2023 18:19, Ian Rogers wrote: >>>> On Tue, Jun 27, 2023 at 9:58 AM Namhyung Kim <namhyung(a)kernel.org> wrote: >>>>> >>>>> On Tue, Jun 27, 2023 at 9:43 AM Ian Rogers <irogers(a)google.com> wrote: >>>>>> >>>>>> On Mon, Jun 26, 2023 at 5:02 PM Namhyung Kim <namhyung(a)kernel.org> wrote: >>>>>>> >>>>>>> On Mon, Jun 26, 2023 at 05:10:58PM +0100, James Clark wrote: >>>>>>>> thread__find_map() chooses to exit without assigning a thread to the >>>>>>>> addr_location in some scenarios, for example when there are samples from >>>>>>>> a guest and perf_guest == false. This results in a segfault when adding >>>>>>>> to the histogram because it uses unguarded accesses to the thread member >>>>>>>> of the addr_location. >>>>>>> >>>>>>> Looking at the commit 0dd5041c9a0ea ("perf addr_location: Add >>>>>>> init/exit/copy functions") that introduced the change, I'm not sure if >>>>>>> it's the intend behavior. >>>>>>> >>>>>>> It might change maps and map, but not thread. Then I think no reason >>>>>>> to not set the al->thread at the beginning. >>>>>>> >>>>>>> How about this? Ian? >>>>>>> (I guess we can get rid of the duplicate 'al->map = NULL' part) >>>>>> >>>>>> It seemed strange that we were failing to find a map (the function's >>>>>> purpose) but then populating the address_location. The change below >>>>>> brings back that somewhat odd behavior. I'm okay with reverting to the >>>>>> old behavior, clearly there were users relying on it. We should >>>>>> probably also copy maps and not just thread, as that was the previous >>>>>> behavior. >>>>> >>>>> Probably. But it used to support samples without maps and I think >>>>> that's why it ignores the return value of thread__find_map(). So >>>>> we can expect al.map is NULL and maybe fine to leave it for now. >>>>> >>>>> As machine__resolve() returns -1 if it gets no thread, we should set >>>>> al.thread when it returns 0. >>>>> >>>>> Can I get your Acked-by? >>>> >>>> Yep: >>>> Acked-by: Ian Rogers <irogers(a)google.com> >>> >>> Looks good to me too. Should I resend the set with this change instead >>> of my one? >> >> No, I can take care of that. I'll take this as your Acked-by. :) > > This part is applied to perf-tools-next, thanks! Thanks Namhyung

1 year, 12 months

Re: [PATCH 1/1] perf: cs-etm: Fixes in instruction sample synthesis

by James Clark

On 23/06/2023 19:22, Tanmay Jagdale wrote: > The existing method of synthesizing instruction samples has the > following issues: > 1. Non-branch instructions have mnemonics of branch instructions. > 2. Branch target address is missing. > > Set the sample flags only when we reach the last instruction in > the tidq (which would be a branch instruction) to solve issue 1). > > To fix issue 2), start synthesizing the instructions from the > previous packet (tidq->prev_packet) instead of current packet > (tidq->packet). This way, it is easy to figure out the target > address of the branch instruction in tidq->prev_packet which > is the current packet's (tidq->packet) first executed instruction. > > After the switch to processing the previous packet first, we no > longer need to swap the packets during cs_etm__flush(). Hi Tanmay, I think the fix for setting the right flags and instruction type makes sense, but is it possible to do it without the change to swapping in cs_etm__flush() or some of the other changes to cs_etm__synth_instruction_sample()? I'm seeing some differences in the output related to the PID that's assigned to a sample and some of the addresses that aren't explained by the commit message. Also there is no corresponding change to cs_etm__synth_branch_sample(), which is also using prev_packet etc so I'm wondering if that's correct now without the swap? That function gets used with the default itrace options or itrace=b For example if I run 'perf script --itrace=i100ns' and diff the output before and after your change I see a difference even though branch and instruction info isn't printed, so I wouldn't expect to see any changes. This is on a systemwide recording of a system under load. Thanks James > > Signed-off-by: Tanmay Jagdale <tanmay(a)marvell.com> > --- > tools/perf/util/cs-etm.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 91299cc56bf7..446e00d98fd5 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -1418,10 +1418,26 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, > sample.stream_id = etmq->etm->instructions_id; > sample.period = period; > sample.cpu = tidq->packet->cpu; > - sample.flags = tidq->prev_packet->flags; > sample.cpumode = event->sample.header.misc; > > - cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample); > + cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet, &sample); > + > + /* Populate branch target information only when we encounter > + * branch instruction, which is at the end of tidq->prev_packet. > + */ > + if (addr == (tidq->prev_packet->end_addr - 4)) { > + /* Update the perf_sample flags using the prev_packet > + * since that is the queue we are synthesizing. > + */ > + sample.flags = tidq->prev_packet->flags; > + > + /* The last instruction of the previous queue would be a > + * branch operation. Get the target of that branch by looking > + * into the first executed instruction of the current packet > + * queue. > + */ > + sample.addr = cs_etm__first_executed_instr(tidq->packet); > + } > > if (etm->synth_opts.last_branch) > sample.branch_stack = tidq->last_branch; > @@ -1641,7 +1657,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq, > /* Get instructions remainder from previous packet */ > instrs_prev = tidq->period_instructions; > > - tidq->period_instructions += tidq->packet->instr_count; > + tidq->period_instructions += tidq->prev_packet->instr_count; > > /* > * Record a branch when the last instruction in > @@ -1721,8 +1737,11 @@ static int cs_etm__sample(struct cs_etm_queue *etmq, > * been executed, but PC has not advanced to next > * instruction) > */ > + /* Get address from prev_packet since we are synthesizing > + * that in cs_etm__synth_instruction_sample() > + */ > addr = cs_etm__instr_addr(etmq, trace_chan_id, > - tidq->packet, offset - 1); > + tidq->prev_packet, offset - 1); > ret = cs_etm__synth_instruction_sample( > etmq, tidq, addr, > etm->instructions_sample_period); > @@ -1786,7 +1805,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq, > > /* Handle start tracing packet */ > if (tidq->prev_packet->sample_type == CS_ETM_EMPTY) > - goto swap_packet; > + goto reset_last_br; > > if (etmq->etm->synth_opts.last_branch && > etmq->etm->synth_opts.instructions && > @@ -1822,8 +1841,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq, > return err; > } > > -swap_packet: > - cs_etm__packet_swap(etm, tidq); > +reset_last_br: > > /* Reset last branches after flush the trace */ > if (etm->synth_opts.last_branch)

2 years

Re: [PATCH 2/2] perf report: Don't add to histogram when there is no thread found

by James Clark

On 27/06/2023 18:19, Ian Rogers wrote: > On Tue, Jun 27, 2023 at 9:58 AM Namhyung Kim <namhyung(a)kernel.org> wrote: >> >> On Tue, Jun 27, 2023 at 9:43 AM Ian Rogers <irogers(a)google.com> wrote: >>> >>> On Mon, Jun 26, 2023 at 5:02 PM Namhyung Kim <namhyung(a)kernel.org> wrote: >>>> >>>> On Mon, Jun 26, 2023 at 05:10:58PM +0100, James Clark wrote: >>>>> thread__find_map() chooses to exit without assigning a thread to the >>>>> addr_location in some scenarios, for example when there are samples from >>>>> a guest and perf_guest == false. This results in a segfault when adding >>>>> to the histogram because it uses unguarded accesses to the thread member >>>>> of the addr_location. >>>> >>>> Looking at the commit 0dd5041c9a0ea ("perf addr_location: Add >>>> init/exit/copy functions") that introduced the change, I'm not sure if >>>> it's the intend behavior. >>>> >>>> It might change maps and map, but not thread. Then I think no reason >>>> to not set the al->thread at the beginning. >>>> >>>> How about this? Ian? >>>> (I guess we can get rid of the duplicate 'al->map = NULL' part) >>> >>> It seemed strange that we were failing to find a map (the function's >>> purpose) but then populating the address_location. The change below >>> brings back that somewhat odd behavior. I'm okay with reverting to the >>> old behavior, clearly there were users relying on it. We should >>> probably also copy maps and not just thread, as that was the previous >>> behavior. >> >> Probably. But it used to support samples without maps and I think >> that's why it ignores the return value of thread__find_map(). So >> we can expect al.map is NULL and maybe fine to leave it for now. >> >> As machine__resolve() returns -1 if it gets no thread, we should set >> al.thread when it returns 0. >> >> Can I get your Acked-by? > > Yep: > Acked-by: Ian Rogers <irogers(a)google.com> Looks good to me too. Should I resend the set with this change instead of my one? > > Thanks, > Ian > >> Thanks, >> Namhyung

2 years

Re: [PATCH v6 09/13] Add nodes for dsb edge control

by Tao Zhang

On 6/20/2023 4:49 PM, Greg Kroah-Hartman wrote: > On Tue, Jun 20, 2023 at 04:31:59PM +0800, Tao Zhang wrote: >> On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote: >>> On Tue, Jun 20, 2023 at 03:32:37PM +0800, Tao Zhang wrote: >>>> Add the nodes to set value for DSB edge control and DSB edge >>>> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR >>>> resgisters to configure edge control. DSB edge detection control >>>> 00: Rising edge detection >>>> 01: Falling edge detection >>>> 10: Rising and falling edge detection (toggle detection) >>>> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to >>>> configure mask. Eight 32 bit registers providing DSB interface >>>> edge detection mask control. >>>> >>>> Signed-off-by: Tao Zhang<quic_taozha(a)quicinc.com> >>>> --- >>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++ >>>> drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++- >>>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++ >>>> 3 files changed, 196 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>> index 2a82cd0..34189e4a 100644 >>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>> @@ -60,3 +60,35 @@ Description: >>>> Bit[3] : Set to 0 for low performance mode. >>>> Set to 1 for high performance mode. >>>> Bit[4:8] : Select byte lane for high performance mode. >>>> + >>>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge_ctrl >>>> +Date: March 2023 >>>> +KernelVersion 6.5 >>>> +Contact: Jinlong Mao (QUIC)<quic_jinlmao(a)quicinc.com>, Tao Zhang (QUIC)<quic_taozha(a)quicinc.com> >>>> +Description: >>>> + Read/Write a set of the edge control registers of the DSB >>>> + in TPDM. >>>> + >>>> + Expected format is the following: >>>> + <integer1> <integer2> <integer3> >>> sysfs is "one value", not 3. Please never have to parse a sysfs file. >> Do you mean sysfs file can only accept "one value"? > Yes. Hi Greg, I‘d like to clarify the usage of this sysfs file again. In the current design, three integers will be written to "dsb_edge_ctrl" to configure DSB edge detection. Integer #1: The start number of edge detection which needs to be configured. Integer #2: The end number of edge detection which needs to be configured. Integer #3: The type of the edge detection needs to be configured. Below is an example. echo 0x3 0x25 0x1 > dsb_edge_ctrl It will configure edge detection #3 to #37 as "falling edge detection". Since these three integers are interrelated and written to achieve the same function, can we use these three integers as "one tuple" here? Best, Tao > >> I see that more than one value are written to the sysfs file >> "trigout_attach". > Then someone missed that and it needs to be fixed. > >>>> +static ssize_t dsb_edge_ctrl_show(struct device *dev, >>>> + struct device_attribute *attr, >>>> + char *buf) >>>> +{ >>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >>>> + ssize_t size = 0; >>>> + unsigned long bytes; >>>> + int i; >>>> + >>>> + spin_lock(&drvdata->spinlock); >>>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) { >>>> + bytes = sysfs_emit_at(buf, size, >>>> + "Index:0x%x Val:0x%x\n", i, >>> Again, no, one value, no "string" needed to parse anything. >> I also see other sysfs files can be read more than one value in other >> drivers. > Again, they are broken, please send patches to fix them. > >> Is this "one value" limitation the usage rule of Linux sysfs system? > Yes. > > thanks, > > greg k-h

2 years

Re: [PATCH V5 6/6] coresight: etm4x: Add ACPI support in platform driver

by Suzuki K Poulose

On 27/06/2023 10:20, Greg Kroah-Hartman wrote: > On Tue, Jun 27, 2023 at 10:06:11AM +0100, Suzuki K Poulose wrote: >> Hi Greg, >> >> On 29/05/2023 07:25, Anshuman Khandual wrote: >>> From: Suzuki K Poulose <suzuki.poulose(a)arm.com> >>> >>> Drop ETM4X ACPI ID from the AMBA ACPI device list, and instead just move it >>> inside the new ACPI devices list detected and used via platform driver. >>> >>> Cc: "Rafael J. Wysocki" <rafael(a)kernel.org> >>> Cc: Len Brown <lenb(a)kernel.org> >>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> >>> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> >>> Cc: Mike Leach <mike.leach(a)linaro.org> >>> Cc: Leo Yan <leo.yan(a)linaro.org> >>> Cc: Sudeep Holla <sudeep.holla(a)arm.com> >>> Cc: Lorenzo Pieralisi <lpieralisi(a)kernel.org> >>> Cc: linux-acpi(a)vger.kernel.org >>> Cc: coresight(a)lists.linaro.org >>> Cc: linux-arm-kernel(a)lists.infradead.org >>> Cc: linux-kernel(a)vger.kernel.org >>> Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com> (for ACPI specific changes) >>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> >>> Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com> >>> --- >>> drivers/acpi/acpi_amba.c | 1 - >>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++++ >>> 2 files changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c >>> index f5b443ab01c2..099966cbac5a 100644 >>> --- a/drivers/acpi/acpi_amba.c >>> +++ b/drivers/acpi/acpi_amba.c >>> @@ -22,7 +22,6 @@ >>> static const struct acpi_device_id amba_id_list[] = { >>> {"ARMH0061", 0}, /* PL061 GPIO Device */ >>> {"ARMH0330", 0}, /* ARM DMA Controller DMA-330 */ >>> - {"ARMHC500", 0}, /* ARM CoreSight ETM4x */ >>> {"ARMHC501", 0}, /* ARM CoreSight ETR */ >>> {"ARMHC502", 0}, /* ARM CoreSight STM */ >>> {"ARMHC503", 0}, /* ARM CoreSight Debug */ >> >> This is a bit awkward request. >> >> I would like to get your opinion on merging this to coresight tree. >> This change is removing the coresight ETMv4 from the ACPI AMBA >> scan list and moving it to the coresight driver. This change is >> essential for >> 1) Adding ACPI support for later versions of ETMv4 that are not AMBA >> devices. >> 2) Adding power management support for AMBA ETMv4 with ACPI. >> >> The above change has been reviewed by Sudeep (Arm64 ACPI reviewer), but >> hasn't been Acked by the ACPI maintainer (Rafael) even after a month of >> follow up with at least 4 reminders [0]. >> >> Are you happy with the Reviews from Sudeep and given the minimal >> change to the drivers/acpi/acpi_amba.c file ? > > As we can't do anything now with the merge window open, please resend > after 6.5-rc1 is out and ask for the ACPI developers to ack this. Sure, will do that. Thanks Greg

2 years

[PATCH V5 0/6] coresight: etm4x: Migrate ACPI AMBA devices to platform driver

by Anshuman Khandual

CoreSight ETM4x devices could be accessed either via MMIO (handled via amba_driver) or CPU system instructions (handled via platform driver). But this has the following issues : - Each new CPU comes up with its own PID and thus we need to keep on adding the "known" PIDs to get it working with AMBA driver. While the ETM4 architecture (and CoreSight architecture) defines way to identify a device as ETM4. Thus older kernels won't be able to "discover" a newer CPU, unless we add the PIDs. - With ACPI, the ETM4x devices have the same HID to identify the device irrespective of the mode of access. This creates a problem where two different drivers (both AMBA based driver and platform driver) would hook into the "HID" and could conflict. e.g., if AMBA driver gets hold of a non-MMIO device, the probe fails. If we have single driver hooked into the given "HID", we could handle them seamlessly, irrespective of the mode of access. - CoreSight is heavily dependent on the runtime power management. With ACPI, amba_driver doesn't get us anywhere with handling the power and thus one need to always turn the power ON to use them. Moving to platform driver gives us the power management for free. Due to all of the above, we are moving ACPI MMIO based etm4x devices to be supported via tha platform driver. The series makes the existing platform driver generic to handle both type of the access modes. Although existing AMBA driver would still continue to support DT based etm4x MMIO devices. Although some problems still remain, such as manually adding PIDs for all new AMBA DT based devices. The series applies on 6.4-rc4. Changes in V5: - Updated the comment for apb clock in drvdata structure - Updated conditional check in etm4_runtime_suspend/resume() - Asserted that the APB clock is present and also enabled Changes in V4: https://lore.kernel.org/all/20230523044553.1525048-1-anshuman.khandual@arm.… - Changed in-code comment in etm4_check_arch_features() - Re-ordered pm_runtime_disable() in etm4_remove_platform_dev() - Renamed back etm4_match as etm4_sysreg_match - Moved back [PATCH 6/6] as [PATCH 5/6] Changes in V3: https://lore.kernel.org/all/20230519052149.1367814-1-anshuman.khandual@arm.… - Returned from etm4_check_arch_features() for non iomem devices - Renamed ETM_DEVTYPE_ETMv4x_ARCH as CS_DEVTYPE_PE_TRACE - Renamed is_etm4x_devtype() as is_devtype_cpu_trace() - Added a patch to ignore the absence of graph connections Changes in V2: https://lore.kernel.org/all/20230327050537.30861-1-anshuman.khandual@arm.co… - Enables ACPI etm4x device support in the existing platform driver - Dropped last two patches from the series - Dropped redundant 'devarch' checking from is_etm4x_device() - Renamed updated is_etm4x_device() as is_etm4x_devtype() - Fixed arguments in fallback stub for etm4_check_arch_features() - Tagged etm4_dev_pm_ops with etm4_platform_driver - Updated the comment for coresight_get_enable_apb_pclk() helper - Updated the comment for new 'pclk' element in struct etm4_drvdata - Dropped the clock when devm_ioremap_resource() fails - Convert IS_ERR() into a direct pointer check in etm4_remove_platform_dev() - Dropped "arm,coresight-etm4x" compatible property from etm4_match[] Changes in V1: https://lore.kernel.org/all/20230317030501.1811905-1-anshuman.khandual@arm.… Cc: Ganapatrao Kulkarni <gankulkarni(a)os.amperecomputing.com> Cc: Steve Clevenger <scclevenger(a)os.amperecomputing.com> Cc: Rob Herring <robh+dt(a)kernel.org> Cc: Frank Rowand <frowand.list(a)gmail.com> Cc: Russell King (Oracle) <linux(a)armlinux.org.uk> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael(a)kernel.org> Cc: Len Brown <lenb(a)kernel.org> Cc: Sudeep Holla <sudeep.holla(a)arm.com> Cc: Lorenzo Pieralisi <lpieralisi(a)kernel.org> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> Cc: Mike Leach <mike.leach(a)linaro.org> Cc: Leo Yan <leo.yan(a)linaro.org> Cc: devicetree(a)vger.kernel.org Cc: linux-acpi(a)vger.kernel.org Cc: coresight(a)lists.linaro.org Cc: linux-arm-kernel(a)lists.infradead.org Cc: linux-kernel(a)vger.kernel.org Anshuman Khandual (4): coresight: etm4x: Allocate and device assign 'struct etmv4_drvdata' earlier coresight: etm4x: Drop iomem 'base' argument from etm4_probe() coresight: etm4x: Drop pid argument from etm4_probe() coresight: etm4x: Change etm4_platform_driver driver for MMIO devices Suzuki K Poulose (2): coresight: platform: acpi: Ignore the absence of graph coresight: etm4x: Add ACPI support in platform driver drivers/acpi/acpi_amba.c | 1 - .../coresight/coresight-etm4x-core.c | 117 ++++++++++++++---- drivers/hwtracing/coresight/coresight-etm4x.h | 4 + .../hwtracing/coresight/coresight-platform.c | 6 +- include/linux/coresight.h | 59 +++++++++ 5 files changed, 163 insertions(+), 24 deletions(-) -- 2.25.1

2 years

[PATCH 0/2] perf cs-etm: Track exception level fixups

by James Clark

A couple of changes related to edge cases since commit 8d3031d39fe8 ("perf cs-etm: Track exception level"). I think the second one is low risk seeing as any path requiring a thread leading up to adding to the histogram would already have been crashing. Maybe the thread check could also be added to hist_entry_iter__add() although other users of it don't seem to have the same issue, and there is another use of al.thread above in builtin-report.c so it's probably ok where I've added it. Applies to perf-tools-next/perf-tools-next (929ff679b69) James Clark (2): perf cs-etm: Handle per-thread mode on EL1 host kernel case perf report: Don't add to histogram when there is no thread found tools/perf/builtin-report.c | 3 +++ tools/perf/util/cs-etm.c | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) base-commit: 929ff679b694f0f9656aec38b3a7d5c440c5ca24 -- 2.34.1

2 years

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight