Hi Steve,
On Fri, 20 Jan 2023 at 00:52, Steve Clevenger scclevenger@os.amperecomputing.com wrote:
Add Ampere early clear of ETM TRCOSLAR.OSLK prior to TRCIDR1 access. Ampere Computing erratum AC03_DEBUG_06 describes an Ampere Computing design decision MMIO reads are considered the same as an external debug access. If TRCOSLAR.OSLK is set, the TRCIDR1 access results in a bus fault followed by a kernel panic. A TRCIDR1 read is valid regardless of TRCOSLAR.OSLK provided MMIO access (now deprecated) is supported. AC03_DEBUG_06 is described in the AmpereOne Developer Errata: https://solutions.amperecomputing.com/customer-connect/products/AmpereOne-de...
Add Ampere ETM PID required for Coresight ETM driver support.
Signed-off-by: Steve Clevenger scclevenger@os.amperecomputing.com
.../coresight/coresight-etm4x-core.c | 36 +++++++++++++++---- drivers/hwtracing/coresight/coresight-etm4x.h | 2 ++ 2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 1cc052979e01..533be1928a09 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1091,19 +1091,34 @@ static void etm4_init_arch_data(void *info) drvdata = dev_get_drvdata(init_arg->dev); csa = init_arg->csa;
As far as I can tell there appears to be an initialisation issue here. etm_probe() ... struct csdev_access access = { 0 }; ... init_arg.csa = &access
::call=> etm4_init_arch_data(init_arg)
Thus csa is uninitialised?
/* Detect the support for OS Lock before we actually use it */
etm_detect_os_lock(drvdata, csa);
/*
* For ETM implementations that consider MMIO an external access
* clear TRCOSLAR.OSLK early.
*/
if (drvdata->mmio_external)
etm4_os_unlock_csa(drvdata, csa);
/* * If we are unable to detect the access mechanism, * or unable to detect the trace unit type, fail
* early.
* early. Reset TRCOSLAR.OSLK if cleared. */
if (!etm4_init_csdev_access(drvdata, csa))
if (!etm4_init_csdev_access(drvdata, csa)) {
This call initialises csa according to sysreg / iomem access requirements
if (drvdata->mmio_external)
etm4_os_lock(drvdata); return;
}
/* Detect the support for OS Lock before we actually use it */
etm_detect_os_lock(drvdata, csa);
/*
* Make sure all registers are accessible
* TRCOSLAR.OSLK may already be clear
*/
if (!drvdata->mmio_external)
etm4_os_unlock_csa(drvdata, csa);
/* Make sure all registers are accessible */
etm4_os_unlock_csa(drvdata, csa); etm4_cs_unlock(drvdata, csa); etm4_check_arch_features(drvdata, init_arg->pid);
@@ -2027,6 +2042,14 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) init_arg.csa = &access; init_arg.pid = etm_pid;
/*
* Ampere ETM v4.6 considers MMIO access as external. This mask
* isolates the manufacturer JEP106 ID in the PID.
* TRCPIDR2 (JEDC|DES_1) << 16 | TRCPIDR1 (DES_0) << 8)
*/
if ((init_arg.pid & 0x000FF000) == 0x00096000)
drvdata->mmio_external = true;
/* * Serialize against CPUHP callbacks to avoid race condition * between the smp call and saving the delayed probe.
@@ -2192,6 +2215,7 @@ static const struct amba_id etm4_ids[] = { CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */ CS_AMBA_ID(0x000bb95a), /* Cortex-A72 */ CS_AMBA_ID(0x000bb959), /* Cortex-A73 */
CS_AMBA_UCI_ID(0x00096000, uci_id_etm4),/* Ampere ARMv8 */ CS_AMBA_UCI_ID(0x000bb9da, uci_id_etm4),/* Cortex-A35 */ CS_AMBA_UCI_ID(0x000bbd05, uci_id_etm4),/* Cortex-A55 */ CS_AMBA_UCI_ID(0x000bbd0a, uci_id_etm4),/* Cortex-A75 */
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 4b21bb79f168..cf4f9f2e1807 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -1015,6 +1015,7 @@ struct etmv4_save_state {
- @skip_power_up: Indicates if an implementation can skip powering up
the trace unit.
- @arch_features: Bitmap of arch features of etmv4 devices.
*/
- @mmio_external: True if ETM considers MMIO an external access.
struct etmv4_drvdata { void __iomem *base; @@ -1067,6 +1068,7 @@ struct etmv4_drvdata { bool state_needs_restore; bool skip_power_up; DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
Rather than continue to add bools - is it not worthwhile adding to the bitmap above and extending the arch features API to allow a "has_feature" call?
bool mmio_external;
};
/* Address comparator access types */
2.25.1
Regards
Mike