From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D80F0C43463 for ; Fri, 18 Sep 2020 15:35:21 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7890321582 for ; Fri, 18 Sep 2020 15:35:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="wAA4eIo5"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="fK1BaqjD" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7890321582 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=vbcqNmS8Er9K3vhulYI3xXu+6ZXYncVXKQg194i85QE=; b=wAA4eIo5ZpHQeKgPTaftExNac 1O1Q218CQfJ2syi2bSsZ9kGJMUuvK9OaLvHqEMdXX6UgUccWTLPgC1zt0Gk1FbLlHN9BFZTw+49oS tZiNQmgGUX8KtixW4XCX3bcvNE1y/qZAgE27Z1X+tM/goxab13aRZ1nt6e3e5nX+wNVuOTJJaLqKR sArbIPMZW7UhcyLLzlj5hM166RaGoZzLpGzDTjRMmFPia90ZZp/8hwNyRf4ttrkHQ6CqlhC5pW4jm UwwxvZvK+YA6s8V1EZNoQaCR4M+aWeh6r5IhOY6uQ+9R6dW5Z6WNdhHQZwRgdsIgQB6aUtZIsEx8b jaMmHK0Qw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJIOU-00052W-Dv; Fri, 18 Sep 2020 15:33:50 +0000 Received: from mail-ej1-x643.google.com ([2a00:1450:4864:20::643]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJIOQ-00051k-NY for linux-arm-kernel@lists.infradead.org; Fri, 18 Sep 2020 15:33:47 +0000 Received: by mail-ej1-x643.google.com with SMTP id nw23so8680977ejb.4 for ; Fri, 18 Sep 2020 08:33:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=P0eKtQKE4xOPafbgv4KFhZjVWHxgIBS8d1Y6fKApMA8=; b=fK1BaqjD1LZ4wHsaZMc3+HkgW8PUOXlpoeei+Ur2XzE9NEoS/HM/IasxntEwhuOQbM DcqI9FILyPk3EIHISY3r09+sU891fbS1QqTyJ0gwG+uSCX+zn157ToUtIUygD+GNnW+y nZ+dEUpznxGk2v8/QTryReqr8SjYbac5YtF2k9c4WOr8/CWFAOU/8CJ5BLhO8Np9UTcv 4Q8Acg+ffv9qhwFNHOn9kfXfa5/n/Eu/scr2w97SF5EEEpl51MY8fOl1sw74uktDYk8b 3U0+3UV7vhaviTIc4JRj3IOlRAyLSzmpPeIalfK90kK4HpX222eKlcvlhkU4eohFFI/l SFzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P0eKtQKE4xOPafbgv4KFhZjVWHxgIBS8d1Y6fKApMA8=; b=OM5GSaKSma0LePahpqiw2GbBlMM9fAcQgAZPGWl46xDpdH0rJretn+EJzFMmNsCVpo RjOTlWh0EHwJKFsxfXrMDDzRoMkG/5go3UpLLMyaClMJvgsljJUAyY9SCTlnNt4134jX m/qqvaqEIGVcZwMHGzNQb8YyT0XSx888rEUv1BDbrdIOYQpeW7RDr1vReyqoKwHI3Ed/ 5WjAzHsdaJga9+HdoB/gTvyV5UdOz+a0733Byv2jlVab00SrKDb6mSIEBS986/WbFOta 5rctR/sIrbaTwWQv61nw9kXFQQICEJROLIeSJjXkSEEfLcAUil6oM87bO+vmUD01GyVz q87Q== X-Gm-Message-State: AOAM531fYTmUfsAeJcADKi8+ElLaqcgV55dIxJO2FvsvFBNHwccb8VhX oBqpTh6xirpoUmQ4DOiGAaQfp3uxyPS0MdlAUMhZUg== X-Google-Smtp-Source: ABdhPJwaFJXGsuI3ZJOZjW9RkJjYJjQQn3lWFBvKLsTseioHcLlFLKjGG4nBPUbL8UJDh0liF94zuABPHkIdFuiMkec= X-Received: by 2002:a17:906:724b:: with SMTP id n11mr37107440ejk.328.1600443224420; Fri, 18 Sep 2020 08:33:44 -0700 (PDT) MIME-Version: 1.0 References: <20200911084119.1080694-1-suzuki.poulose@arm.com> In-Reply-To: <20200911084119.1080694-1-suzuki.poulose@arm.com> From: Mike Leach Date: Fri, 18 Sep 2020 16:33:33 +0100 Message-ID: Subject: Re: [PATCH 00/19] coresight: Support for ETMv4.4 system instructions To: Suzuki K Poulose X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200918_113346_784688_14853D52 X-CRM114-Status: GOOD ( 39.59 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Coresight ML , Anshuman.Khandual@arm.com, Mathieu Poirier , linux-arm-kernel , Leo Yan Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Suzuki, I've looked at the set and have only one real gripe - the implementation and timing of component detection on the sysreg path. I've summarised my thoughts here, but as the changes are found across multiple patches I may well have repeated myself a little in the individual places. On Fri, 11 Sep 2020 at 09:41, Suzuki K Poulose wrote: > > CoreSight ETMv4.4 introduced system instructions for accessing > the ETM. This also implies that they may not be on the amba bus. System instructions have always been an option - but we have never supported them up to now. In fact both memory access and system instructions can live side by side - but the driver really needs to choose just one! What did happen is that a PE that supports Arm Trace 8.4 mandates ETM 4.4, and ETM 4.4 mandates system instruction access for PEs with Arm Trace 8.4, and deprecates memory access. But there is nothing to stop other variants having the system instruction interface. So there is no need to describe this as a purely 4.4. onwards support - it will support any version of the ETM that has sysreg access. The spec permits aarch32 / armv7 register access via CP14 - but I assume this is omitted deliberately & not intended to be supported at this time. > Right now all the CoreSight components are accessed via memory > map. Also, we have some common routines in coresight generic > code driver (e.g, CS_LOCK, claim/disclaim), which assume the > mmio. In order to preserve the generic algorithms at a single > place and to allow dynamic switch for ETMs, this series introduces > an abstraction layer for accessing a coresight device. It is > designed such that the mmio access are fast tracked (i.e, without > an indirect function call). > > This will also help us to get rid of the driver+attribute specific > sysfs show/store routines and replace them with a single routine > to access a given register offset (which can be embedded in the > dev_ext_attribute). This is not currently implemented in the series, > but can be achieved. > > Further we switch the generic routines to work with the abstraction. > With this in place, we refactor the etm4x code a bit to allow for > supporting the system instructions with very little new code. The > changes also switch to using the system instructions by default > even when we may have an MMIO. > > We use TRCDEVARCH for the detection of the ETM component, which > is a standard register as per CoreSight architecture, rather than > the etm specific id register TRCIDR3. This is for making sure > that we are able to detect the ETM via system instructions accurately, > when the the trace unit could be anything (etm or a custom trace unit). > I'm assuming you mean TRCIDR1 here -- which in part, defines the etm architecture version. TRCIDR3 does something else entirely. Not sure I agree with this though - the driver is designed to match the ETM spec so there is no problem with using TRCIDR1 to spot functional variants according to ETM version, The etm4_init_arch_data() function is not about detecting the presence of an ETMv4 component, but about exploring the capabilities it has. We check 4 bits of the version as a sanity check, but at this point we should be pretty sure we are dealing with an ETM of some kind. TRCDEVARCH is already used for detection in the AMBA matching code - assuming the table includes the optional CoreSIght UCI. I would imagine that similar detection needs to go on for instruction access - but once we have detected an ETM, then ETM architected registers are sufficient. If the device is not an ETM then it should be detected and rejected early - and the bindings examined to determine why this driver was attached! The act of adding in a check against TRCDEVARCH as part of the etm4_init_arch_data() function adds new and hidden checks to AMBA devices where it was sufficient to have an entry in the probe match table before. Most recent additions include the UCI matching, but older ones don't. I am concerned that this changes may trip up older existing implementations which for some reason may not have TRCDEVCARCH, or have set it to not present. For this reason, I beleive that the TRCDEVARCH check for the sys reg access should occur on the sysreg specific probe - balancing what happens on the AMBA side. That way the common code remains common. Further the setup of the CSA for the device can happen immediately in the common etm_probe() function, based on *base being NULL or not, rather than as a side effect of the etm4_init_arch_data() call. Regards Mike > The series has been mildly tested on a model. I would really > appreciate any testing on real hardware. > > Applies on coresight/next > > Changes since V1: > - Flip the switch for iomem from no_iomem to io_mem in csdev_access. > - Split patches for claim/disclaim and CS_LOCK/UNLOCK conversions. > - Move device access initialisation for etm4x to the target CPU > - Cleanup secure exception level mask handling. > - Switch to use TRCDEVARCH for ETM component discovery. This > is for making > - Check the availability of OS/Software Locks before using them. > > > Suzuki K Poulose (19): > coresight: Introduce device access abstraction > coresight: tpiu: Prepare for using coresight device access abstraction > coresight: Convert coresight_timeout to use access abstraction > coresight: Convert claim/disclaim operations to use access wrappers > coresight: Use device access layer for Software lock/unlock operations > coresight: etm4x: Always read the registers on the host CPU > coresight: etm4x: Convert all register accesses > coresight: etm4x: Add commentary on the registers > coresight: etm4x: Add sysreg access helpers > coresight: etm4x: Define DEVARCH register fields > coresight: etm4x: Check for OS and Software Lock > coresight: etm4x: Cleanup secure exception level masks > coresight: etm4x: Clean up exception level masks > coresight: etm4x: Detect access early on the target CPU > coresight: etm4x: Use TRCDEVARCH for component discovery > coresight: etm4x: Detect system instructions support > coresight: etm4x: Refactor probing routine > coresight: etm4x: Add support for sysreg only devices > dts: bindings: coresight: ETMv4.4 system register access only units > > .../devicetree/bindings/arm/coresight.txt | 6 +- > drivers/hwtracing/coresight/coresight-catu.c | 24 +- > .../hwtracing/coresight/coresight-cpu-debug.c | 22 +- > .../hwtracing/coresight/coresight-cti-sysfs.c | 5 +- > drivers/hwtracing/coresight/coresight-cti.c | 34 +- > drivers/hwtracing/coresight/coresight-etb10.c | 29 +- > .../coresight/coresight-etm3x-sysfs.c | 10 +- > drivers/hwtracing/coresight/coresight-etm3x.c | 35 +- > .../coresight/coresight-etm4x-sysfs.c | 44 +- > drivers/hwtracing/coresight/coresight-etm4x.c | 716 +++++++++++------- > drivers/hwtracing/coresight/coresight-etm4x.h | 440 ++++++++++- > .../hwtracing/coresight/coresight-funnel.c | 22 +- > drivers/hwtracing/coresight/coresight-priv.h | 9 +- > .../coresight/coresight-replicator.c | 31 +- > drivers/hwtracing/coresight/coresight-stm.c | 50 +- > .../hwtracing/coresight/coresight-tmc-etf.c | 38 +- > .../hwtracing/coresight/coresight-tmc-etr.c | 20 +- > drivers/hwtracing/coresight/coresight-tmc.c | 16 +- > drivers/hwtracing/coresight/coresight-tpiu.c | 32 +- > drivers/hwtracing/coresight/coresight.c | 130 +++- > include/linux/coresight.h | 230 +++++- > 21 files changed, 1449 insertions(+), 494 deletions(-) > > -- > 2.24.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel