* [PATCH v6 10/13] coresight: etm3x: change drvdata->spinlock type to raw_spin_lock_t
From: Yeoreum Yun @ 2026-04-22 13:22 UTC (permalink / raw)
To: coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan, jie.gan, Yeoreum Yun
In-Reply-To: <20260422132203.977549-1-yeoreum.yun@arm.com>
etm_starting_cpu()/etm_dying_cpu() are called in not sleepable context.
This poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable.
To address this, change etm3's drvdata->spinlock type to raw_spin_lock_t.
This will be good to align with etm4x.
Reviewed-by: Jie Gan <jie.gan@oss.qualcomm.com>
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
drivers/hwtracing/coresight/coresight-etm.h | 2 +-
.../coresight/coresight-etm3x-core.c | 18 +--
.../coresight/coresight-etm3x-sysfs.c | 130 +++++++++---------
3 files changed, 75 insertions(+), 75 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index 1d753cca2943..40f20daded4f 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -232,7 +232,7 @@ struct etm_drvdata {
struct csdev_access csa;
struct clk *atclk;
struct coresight_device *csdev;
- spinlock_t spinlock;
+ raw_spinlock_t spinlock;
int cpu;
int port_size;
u8 arch;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index a547a6d2e0bd..4a702b515733 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -511,7 +511,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
struct etm_enable_arg arg = { };
int ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
drvdata->traceid = path->trace_id;
@@ -534,7 +534,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
if (ret)
etm_release_trace_id(drvdata);
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
if (!ret)
dev_dbg(&csdev->dev, "ETM tracing enabled\n");
@@ -634,7 +634,7 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
* DYING hotplug callback is serviced by the ETM driver.
*/
cpus_read_lock();
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
/*
* Executing etm_disable_hw on the cpu whose ETM is being disabled
@@ -643,7 +643,7 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
smp_call_function_single(drvdata->cpu, etm_disable_sysfs_smp_call,
drvdata, 1);
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
cpus_read_unlock();
/*
@@ -709,7 +709,7 @@ static int etm_starting_cpu(unsigned int cpu)
if (!etmdrvdata[cpu])
return 0;
- spin_lock(&etmdrvdata[cpu]->spinlock);
+ raw_spin_lock(&etmdrvdata[cpu]->spinlock);
if (!etmdrvdata[cpu]->os_unlock) {
etm_os_unlock(etmdrvdata[cpu]);
etmdrvdata[cpu]->os_unlock = true;
@@ -717,7 +717,7 @@ static int etm_starting_cpu(unsigned int cpu)
if (coresight_get_mode(etmdrvdata[cpu]->csdev))
etm_enable_hw(etmdrvdata[cpu]);
- spin_unlock(&etmdrvdata[cpu]->spinlock);
+ raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
return 0;
}
@@ -726,10 +726,10 @@ static int etm_dying_cpu(unsigned int cpu)
if (!etmdrvdata[cpu])
return 0;
- spin_lock(&etmdrvdata[cpu]->spinlock);
+ raw_spin_lock(&etmdrvdata[cpu]->spinlock);
if (coresight_get_mode(etmdrvdata[cpu]->csdev))
etm_disable_hw(etmdrvdata[cpu]);
- spin_unlock(&etmdrvdata[cpu]->spinlock);
+ raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
return 0;
}
@@ -856,7 +856,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
desc.access = drvdata->csa = CSDEV_ACCESS_IOMEM(base);
- spin_lock_init(&drvdata->spinlock);
+ raw_spin_lock_init(&drvdata->spinlock);
drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
if (IS_ERR(drvdata->atclk))
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 762109307b86..42b12c33516b 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -49,13 +49,13 @@ static ssize_t etmsr_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
pm_runtime_get_sync(dev->parent);
- spin_lock_irqsave(&drvdata->spinlock, flags);
+ raw_spin_lock_irqsave(&drvdata->spinlock, flags);
CS_UNLOCK(drvdata->csa.base);
val = etm_readl(drvdata, ETMSR);
CS_LOCK(drvdata->csa.base);
- spin_unlock_irqrestore(&drvdata->spinlock, flags);
+ raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
pm_runtime_put(dev->parent);
return sprintf(buf, "%#lx\n", val);
@@ -76,7 +76,7 @@ static ssize_t reset_store(struct device *dev,
return ret;
if (val) {
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
memset(config, 0, sizeof(struct etm_config));
config->mode = ETM_MODE_EXCLUDE;
config->trigger_event = ETM_DEFAULT_EVENT_VAL;
@@ -86,7 +86,7 @@ static ssize_t reset_store(struct device *dev,
etm_set_default(config);
etm_release_trace_id(drvdata);
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
}
return size;
@@ -117,7 +117,7 @@ static ssize_t mode_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->mode = val & ETM_MODE_ALL;
if (config->mode & ETM_MODE_EXCLUDE)
@@ -168,12 +168,12 @@ static ssize_t mode_store(struct device *dev,
if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
etm_config_trace_mode(config);
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
err_unlock:
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return ret;
}
static DEVICE_ATTR_RW(mode);
@@ -299,9 +299,9 @@ static ssize_t addr_idx_store(struct device *dev,
* Use spinlock to ensure index doesn't change while it gets
* dereferenced multiple times within a spinlock block elsewhere.
*/
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->addr_idx = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -315,16 +315,16 @@ static ssize_t addr_single_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EINVAL;
}
val = config->addr_val[idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -343,17 +343,17 @@ static ssize_t addr_single_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EINVAL;
}
config->addr_val[idx] = val;
config->addr_type[idx] = ETM_ADDR_TYPE_SINGLE;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -367,23 +367,23 @@ static ssize_t addr_range_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (idx % 2 != 0) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
if (!((config->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
(config->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
val1 = config->addr_val[idx];
val2 = config->addr_val[idx + 1];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx %#lx\n", val1, val2);
}
@@ -403,17 +403,17 @@ static ssize_t addr_range_store(struct device *dev,
if (val1 > val2)
return -EINVAL;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (idx % 2 != 0) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
if (!((config->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
(config->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
@@ -422,7 +422,7 @@ static ssize_t addr_range_store(struct device *dev,
config->addr_val[idx + 1] = val2;
config->addr_type[idx + 1] = ETM_ADDR_TYPE_RANGE;
config->enable_ctrl1 |= (1 << (idx/2));
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -436,16 +436,16 @@ static ssize_t addr_start_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_START)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
val = config->addr_val[idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -464,11 +464,11 @@ static ssize_t addr_start_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_START)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
@@ -476,7 +476,7 @@ static ssize_t addr_start_store(struct device *dev,
config->addr_type[idx] = ETM_ADDR_TYPE_START;
config->startstop_ctrl |= (1 << idx);
config->enable_ctrl1 |= ETMTECR1_START_STOP;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -490,16 +490,16 @@ static ssize_t addr_stop_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
val = config->addr_val[idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -518,11 +518,11 @@ static ssize_t addr_stop_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
idx = config->addr_idx;
if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return -EPERM;
}
@@ -530,7 +530,7 @@ static ssize_t addr_stop_store(struct device *dev,
config->addr_type[idx] = ETM_ADDR_TYPE_STOP;
config->startstop_ctrl |= (1 << (idx + 16));
config->enable_ctrl1 |= ETMTECR1_START_STOP;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -543,9 +543,9 @@ static ssize_t addr_acctype_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
val = config->addr_acctype[config->addr_idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -563,9 +563,9 @@ static ssize_t addr_acctype_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->addr_acctype[config->addr_idx] = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -601,9 +601,9 @@ static ssize_t cntr_idx_store(struct device *dev,
* Use spinlock to ensure index doesn't change while it gets
* dereferenced multiple times within a spinlock block elsewhere.
*/
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->cntr_idx = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -616,9 +616,9 @@ static ssize_t cntr_rld_val_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
val = config->cntr_rld_val[config->cntr_idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -636,9 +636,9 @@ static ssize_t cntr_rld_val_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->cntr_rld_val[config->cntr_idx] = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -651,9 +651,9 @@ static ssize_t cntr_event_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
val = config->cntr_event[config->cntr_idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -671,9 +671,9 @@ static ssize_t cntr_event_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->cntr_event[config->cntr_idx] = val & ETM_EVENT_MASK;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -686,9 +686,9 @@ static ssize_t cntr_rld_event_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
val = config->cntr_rld_event[config->cntr_idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -706,9 +706,9 @@ static ssize_t cntr_rld_event_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->cntr_rld_event[config->cntr_idx] = val & ETM_EVENT_MASK;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -723,11 +723,11 @@ static ssize_t cntr_val_show(struct device *dev,
struct etm_config *config = &drvdata->config;
if (!coresight_get_mode(drvdata->csdev)) {
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
for (i = 0; i < drvdata->nr_cntr; i++)
ret += sprintf(buf, "counter %d: %x\n",
i, config->cntr_val[i]);
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return ret;
}
@@ -752,9 +752,9 @@ static ssize_t cntr_val_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->cntr_val[config->cntr_idx] = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -947,13 +947,13 @@ static ssize_t seq_curr_state_show(struct device *dev,
}
pm_runtime_get_sync(dev->parent);
- spin_lock_irqsave(&drvdata->spinlock, flags);
+ raw_spin_lock_irqsave(&drvdata->spinlock, flags);
CS_UNLOCK(drvdata->csa.base);
val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
CS_LOCK(drvdata->csa.base);
- spin_unlock_irqrestore(&drvdata->spinlock, flags);
+ raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
pm_runtime_put(dev->parent);
out:
return sprintf(buf, "%#lx\n", val);
@@ -1012,9 +1012,9 @@ static ssize_t ctxid_idx_store(struct device *dev,
* Use spinlock to ensure index doesn't change while it gets
* dereferenced multiple times within a spinlock block elsewhere.
*/
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->ctxid_idx = val;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
@@ -1034,9 +1034,9 @@ static ssize_t ctxid_pid_show(struct device *dev,
if (task_active_pid_ns(current) != &init_pid_ns)
return -EINVAL;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
val = config->ctxid_pid[config->ctxid_idx];
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
}
@@ -1066,9 +1066,9 @@ static ssize_t ctxid_pid_store(struct device *dev,
if (ret)
return ret;
- spin_lock(&drvdata->spinlock);
+ raw_spin_lock(&drvdata->spinlock);
config->ctxid_pid[config->ctxid_idx] = pid;
- spin_unlock(&drvdata->spinlock);
+ raw_spin_unlock(&drvdata->spinlock);
return size;
}
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related
* [PATCH v6 11/13] coresight: etm3x: introduce struct etm_caps
From: Yeoreum Yun @ 2026-04-22 13:22 UTC (permalink / raw)
To: coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan, jie.gan, Yeoreum Yun
In-Reply-To: <20260422132203.977549-1-yeoreum.yun@arm.com>
Introduce struct etm_caps to describe ETMv3 capabilities
and move capabilities information into it.
Since drvdata->etmccr and drvdata->etmccer are used to check
whether it supports fifofull logic and timestamping,
remove etmccr and etmccer field from drvdata and add relevant fields
in etm_caps structure.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
drivers/hwtracing/coresight/coresight-etm.h | 42 ++++++++++++-------
.../coresight/coresight-etm3x-core.c | 39 ++++++++++-------
.../coresight/coresight-etm3x-sysfs.c | 29 ++++++++-----
3 files changed, 67 insertions(+), 43 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index 40f20daded4f..932bec82fb47 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -140,6 +140,30 @@
ETM_ADD_COMP_0 | \
ETM_EVENT_NOT_A)
+/**
+ * struct etm_caps - specifics ETM capabilities
+ * @port_size: port size as reported by ETMCR bit 4-6 and 21.
+ * @nr_addr_cmp:Number of pairs of address comparators as found in ETMCCR.
+ * @nr_cntr: Number of counters as found in ETMCCR bit 13-15.
+ * @nr_ext_inp: Number of external input as found in ETMCCR bit 17-19.
+ * @nr_ext_out: Number of external output as found in ETMCCR bit 20-22.
+ * @nr_ctxid_cmp: Number of contextID comparators as found in ETMCCR bit 24-25.
+ * @fifofull: FIFOFULL logic is present.
+ * @timestamp: Timestamping is implemented.
+ * @retstack: Return stack is implemented.
+ */
+struct etm_caps {
+ int port_size;
+ u8 nr_addr_cmp;
+ u8 nr_cntr;
+ u8 nr_ext_inp;
+ u8 nr_ext_out;
+ u8 nr_ctxid_cmp;
+ bool fifofull : 1;
+ bool timestamp : 1;
+ bool retstack : 1;
+};
+
/**
* struct etm_config - configuration information related to an ETM
* @mode: controls various modes supported by this ETM/PTM.
@@ -212,19 +236,12 @@ struct etm_config {
* @csdev: component vitals needed by the framework.
* @spinlock: only one at a time pls.
* @cpu: the cpu this component is affined to.
- * @port_size: port size as reported by ETMCR bit 4-6 and 21.
* @arch: ETM/PTM version number.
+ * @caps: ETM capabilities.
* @use_cpu14: true if management registers need to be accessed via CP14.
* @sticky_enable: true if ETM base configuration has been done.
* @boot_enable:true if we should start tracing at boot time.
* @os_unlock: true if access to management registers is allowed.
- * @nr_addr_cmp:Number of pairs of address comparators as found in ETMCCR.
- * @nr_cntr: Number of counters as found in ETMCCR bit 13-15.
- * @nr_ext_inp: Number of external input as found in ETMCCR bit 17-19.
- * @nr_ext_out: Number of external output as found in ETMCCR bit 20-22.
- * @nr_ctxid_cmp: Number of contextID comparators as found in ETMCCR bit 24-25.
- * @etmccr: value of register ETMCCR.
- * @etmccer: value of register ETMCCER.
* @traceid: value of the current ID for this component.
* @config: structure holding configuration parameters.
*/
@@ -234,19 +251,12 @@ struct etm_drvdata {
struct coresight_device *csdev;
raw_spinlock_t spinlock;
int cpu;
- int port_size;
u8 arch;
+ struct etm_caps caps;
bool use_cp14;
bool sticky_enable;
bool boot_enable;
bool os_unlock;
- u8 nr_addr_cmp;
- u8 nr_cntr;
- u8 nr_ext_inp;
- u8 nr_ext_out;
- u8 nr_ctxid_cmp;
- u32 etmccr;
- u32 etmccer;
u32 traceid;
struct etm_config config;
};
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 4a702b515733..e42ca346da91 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -308,6 +308,7 @@ void etm_config_trace_mode(struct etm_config *config)
static int etm_parse_event_config(struct etm_drvdata *drvdata,
struct perf_event *event)
{
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
struct perf_event_attr *attr = &event->attr;
u8 ts_level;
@@ -356,8 +357,7 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
* has ret stack) on the same SoC. So only enable when it can be honored
* - trace will still continue normally otherwise.
*/
- if (ATTR_CFG_GET_FLD(attr, retstack) &&
- (drvdata->etmccer & ETMCCER_RETSTACK))
+ if (ATTR_CFG_GET_FLD(attr, retstack) && (caps->retstack))
config->ctrl |= ETMCR_RETURN_STACK;
return 0;
@@ -367,6 +367,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
{
int i, rc;
u32 etmcr;
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
struct coresight_device *csdev = drvdata->csdev;
@@ -388,7 +389,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
etmcr = etm_readl(drvdata, ETMCR);
/* Clear setting from a previous run if need be */
etmcr &= ~ETM3X_SUPPORTED_OPTIONS;
- etmcr |= drvdata->port_size;
+ etmcr |= caps->port_size;
etmcr |= ETMCR_ETM_EN;
etm_writel(drvdata, config->ctrl | etmcr, ETMCR);
etm_writel(drvdata, config->trigger_event, ETMTRIGGER);
@@ -396,11 +397,11 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
etm_writel(drvdata, config->enable_event, ETMTEEVR);
etm_writel(drvdata, config->enable_ctrl1, ETMTECR1);
etm_writel(drvdata, config->fifofull_level, ETMFFLR);
- for (i = 0; i < drvdata->nr_addr_cmp; i++) {
+ for (i = 0; i < caps->nr_addr_cmp; i++) {
etm_writel(drvdata, config->addr_val[i], ETMACVRn(i));
etm_writel(drvdata, config->addr_acctype[i], ETMACTRn(i));
}
- for (i = 0; i < drvdata->nr_cntr; i++) {
+ for (i = 0; i < caps->nr_cntr; i++) {
etm_writel(drvdata, config->cntr_rld_val[i], ETMCNTRLDVRn(i));
etm_writel(drvdata, config->cntr_event[i], ETMCNTENRn(i));
etm_writel(drvdata, config->cntr_rld_event[i],
@@ -414,9 +415,9 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
etm_writel(drvdata, config->seq_32_event, ETMSQ32EVR);
etm_writel(drvdata, config->seq_13_event, ETMSQ13EVR);
etm_writel(drvdata, config->seq_curr_state, ETMSQR);
- for (i = 0; i < drvdata->nr_ext_out; i++)
+ for (i = 0; i < caps->nr_ext_out; i++)
etm_writel(drvdata, ETM_DEFAULT_EVENT_VAL, ETMEXTOUTEVRn(i));
- for (i = 0; i < drvdata->nr_ctxid_cmp; i++)
+ for (i = 0; i < caps->nr_ctxid_cmp; i++)
etm_writel(drvdata, config->ctxid_pid[i], ETMCIDCVRn(i));
etm_writel(drvdata, config->ctxid_mask, ETMCIDCMR);
etm_writel(drvdata, config->sync_freq, ETMSYNCFR);
@@ -563,6 +564,7 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
static void etm_disable_hw(struct etm_drvdata *drvdata)
{
int i;
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
struct coresight_device *csdev = drvdata->csdev;
@@ -572,7 +574,7 @@ static void etm_disable_hw(struct etm_drvdata *drvdata)
/* Read back sequencer and counters for post trace analysis */
config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
- for (i = 0; i < drvdata->nr_cntr; i++)
+ for (i = 0; i < caps->nr_cntr; i++)
config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
etm_set_pwrdwn(drvdata);
@@ -754,7 +756,9 @@ static void etm_init_arch_data(void *info)
{
u32 etmidr;
u32 etmccr;
+ u32 etmccer;
struct etm_drvdata *drvdata = info;
+ struct etm_caps *caps = &drvdata->caps;
/* Make sure all registers are accessible */
etm_os_unlock(drvdata);
@@ -779,16 +783,19 @@ static void etm_init_arch_data(void *info)
/* Find all capabilities */
etmidr = etm_readl(drvdata, ETMIDR);
drvdata->arch = BMVAL(etmidr, 4, 11);
- drvdata->port_size = etm_readl(drvdata, ETMCR) & PORT_SIZE_MASK;
+ caps->port_size = etm_readl(drvdata, ETMCR) & PORT_SIZE_MASK;
+
+ etmccer = etm_readl(drvdata, ETMCCER);
+ caps->timestamp = !!(etmccer & ETMCCER_TIMESTAMP);
+ caps->retstack = !!(etmccer & ETMCCER_RETSTACK);
- drvdata->etmccer = etm_readl(drvdata, ETMCCER);
etmccr = etm_readl(drvdata, ETMCCR);
- drvdata->etmccr = etmccr;
- drvdata->nr_addr_cmp = BMVAL(etmccr, 0, 3) * 2;
- drvdata->nr_cntr = BMVAL(etmccr, 13, 15);
- drvdata->nr_ext_inp = BMVAL(etmccr, 17, 19);
- drvdata->nr_ext_out = BMVAL(etmccr, 20, 22);
- drvdata->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
+ caps->fifofull = !!(etmccr & ETMCCR_FIFOFULL);
+ caps->nr_addr_cmp = BMVAL(etmccr, 0, 3) * 2;
+ caps->nr_cntr = BMVAL(etmccr, 13, 15);
+ caps->nr_ext_inp = BMVAL(etmccr, 17, 19);
+ caps->nr_ext_out = BMVAL(etmccr, 20, 22);
+ caps->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
coresight_clear_self_claim_tag_unlocked(&drvdata->csa);
etm_set_pwrdwn(drvdata);
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 42b12c33516b..f7330d830e21 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -15,8 +15,9 @@ static ssize_t nr_addr_cmp_show(struct device *dev,
{
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
- val = drvdata->nr_addr_cmp;
+ val = caps->nr_addr_cmp;
return sprintf(buf, "%#lx\n", val);
}
static DEVICE_ATTR_RO(nr_addr_cmp);
@@ -25,8 +26,9 @@ static ssize_t nr_cntr_show(struct device *dev,
struct device_attribute *attr, char *buf)
{ unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
- val = drvdata->nr_cntr;
+ val = caps->nr_cntr;
return sprintf(buf, "%#lx\n", val);
}
static DEVICE_ATTR_RO(nr_cntr);
@@ -37,7 +39,7 @@ static ssize_t nr_ctxid_cmp_show(struct device *dev,
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
- val = drvdata->nr_ctxid_cmp;
+ val = drvdata->caps.nr_ctxid_cmp;
return sprintf(buf, "%#lx\n", val);
}
static DEVICE_ATTR_RO(nr_ctxid_cmp);
@@ -80,7 +82,7 @@ static ssize_t reset_store(struct device *dev,
memset(config, 0, sizeof(struct etm_config));
config->mode = ETM_MODE_EXCLUDE;
config->trigger_event = ETM_DEFAULT_EVENT_VAL;
- for (i = 0; i < drvdata->nr_addr_cmp; i++) {
+ for (i = 0; i < drvdata->caps.nr_addr_cmp; i++) {
config->addr_type[i] = ETM_ADDR_TYPE_NONE;
}
@@ -111,6 +113,7 @@ static ssize_t mode_store(struct device *dev,
int ret;
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
ret = kstrtoul(buf, 16, &val);
@@ -131,7 +134,7 @@ static ssize_t mode_store(struct device *dev,
config->ctrl &= ~ETMCR_CYC_ACC;
if (config->mode & ETM_MODE_STALL) {
- if (!(drvdata->etmccr & ETMCCR_FIFOFULL)) {
+ if (!caps->fifofull) {
dev_warn(dev, "stall mode not supported\n");
ret = -EINVAL;
goto err_unlock;
@@ -141,7 +144,7 @@ static ssize_t mode_store(struct device *dev,
config->ctrl &= ~ETMCR_STALL_MODE;
if (config->mode & ETM_MODE_TIMESTAMP) {
- if (!(drvdata->etmccer & ETMCCER_TIMESTAMP)) {
+ if (!caps->timestamp) {
dev_warn(dev, "timestamp not supported\n");
ret = -EINVAL;
goto err_unlock;
@@ -286,13 +289,14 @@ static ssize_t addr_idx_store(struct device *dev,
int ret;
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
ret = kstrtoul(buf, 16, &val);
if (ret)
return ret;
- if (val >= drvdata->nr_addr_cmp)
+ if (val >= caps->nr_addr_cmp)
return -EINVAL;
/*
@@ -589,13 +593,14 @@ static ssize_t cntr_idx_store(struct device *dev,
int ret;
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
ret = kstrtoul(buf, 16, &val);
if (ret)
return ret;
- if (val >= drvdata->nr_cntr)
+ if (val >= caps->nr_cntr)
return -EINVAL;
/*
* Use spinlock to ensure index doesn't change while it gets
@@ -720,18 +725,19 @@ static ssize_t cntr_val_show(struct device *dev,
int i, ret = 0;
u32 val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
if (!coresight_get_mode(drvdata->csdev)) {
raw_spin_lock(&drvdata->spinlock);
- for (i = 0; i < drvdata->nr_cntr; i++)
+ for (i = 0; i < caps->nr_cntr; i++)
ret += sprintf(buf, "counter %d: %x\n",
i, config->cntr_val[i]);
raw_spin_unlock(&drvdata->spinlock);
return ret;
}
- for (i = 0; i < drvdata->nr_cntr; i++) {
+ for (i = 0; i < caps->nr_cntr; i++) {
val = etm_readl(drvdata, ETMCNTVRn(i));
ret += sprintf(buf, "counter %d: %x\n", i, val);
}
@@ -999,13 +1005,14 @@ static ssize_t ctxid_idx_store(struct device *dev,
int ret;
unsigned long val;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ const struct etm_caps *caps = &drvdata->caps;
struct etm_config *config = &drvdata->config;
ret = kstrtoul(buf, 16, &val);
if (ret)
return ret;
- if (val >= drvdata->nr_ctxid_cmp)
+ if (val >= caps->nr_ctxid_cmp)
return -EINVAL;
/*
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related
* [PATCH v6 12/13] coresight: etm3x: fix inconsistencies with sysfs configuration
From: Yeoreum Yun @ 2026-04-22 13:22 UTC (permalink / raw)
To: coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan, jie.gan, Yeoreum Yun
In-Reply-To: <20260422132203.977549-1-yeoreum.yun@arm.com>
The current ETM3x configuration via sysfs can lead to the following
inconsistencies:
- If a configuration is modified via sysfs while a perf session is
active, the running configuration may differ between before
a sched-out and after a subsequent sched-in.
To resolve these issues, separate the configuration into:
- active_config: the configuration applied to the current session
- config: the configuration set via sysfs
Additionally:
- Since active_config and related fields are accessed only by the local CPU
in etm_enable/disable_sysfs_smp_call() (similar to perf enable/disable),
remove the lock/unlock from the sysfs enable/disable path and
starting/dying_cpu path except when to access config fields only.
Fixes: 1925a470ce69 ("coresight: etm3x: splitting struct etm_drvdata")
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
drivers/hwtracing/coresight/coresight-etm.h | 2 +
.../coresight/coresight-etm3x-core.c | 47 +++++++++----------
2 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index 932bec82fb47..01f1a7f2559c 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -243,6 +243,7 @@ struct etm_config {
* @boot_enable:true if we should start tracing at boot time.
* @os_unlock: true if access to management registers is allowed.
* @traceid: value of the current ID for this component.
+ * @active_config: structure holding current running configuration parameters.
* @config: structure holding configuration parameters.
*/
struct etm_drvdata {
@@ -258,6 +259,7 @@ struct etm_drvdata {
bool boot_enable;
bool os_unlock;
u32 traceid;
+ struct etm_config active_config;
struct etm_config config;
};
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index e42ca346da91..85778e6349f4 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -309,7 +309,7 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
struct perf_event *event)
{
const struct etm_caps *caps = &drvdata->caps;
- struct etm_config *config = &drvdata->config;
+ struct etm_config *config = &drvdata->active_config;
struct perf_event_attr *attr = &event->attr;
u8 ts_level;
@@ -368,7 +368,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
int i, rc;
u32 etmcr;
const struct etm_caps *caps = &drvdata->caps;
- struct etm_config *config = &drvdata->config;
+ struct etm_config *config = &drvdata->active_config;
struct coresight_device *csdev = drvdata->csdev;
CS_UNLOCK(drvdata->csa.base);
@@ -442,29 +442,38 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
struct etm_enable_arg {
struct etm_drvdata *drvdata;
+ const struct coresight_path *path;
+ struct etm_config config;
int rc;
};
static void etm_enable_sysfs_smp_call(void *info)
{
struct etm_enable_arg *arg = info;
+ struct etm_drvdata *drvdata;
struct coresight_device *csdev;
if (WARN_ON(!arg))
return;
- csdev = arg->drvdata->csdev;
+ drvdata = arg->drvdata;
+ csdev = drvdata->csdev;
if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
/* Someone is already using the tracer */
arg->rc = -EBUSY;
return;
}
+ drvdata->active_config = arg->config;
+ drvdata->traceid = arg->path->trace_id;
+
arg->rc = etm_enable_hw(arg->drvdata);
/* The tracer didn't start */
if (arg->rc)
coresight_set_mode(csdev, CS_MODE_DISABLED);
+ else
+ drvdata->sticky_enable = true;
}
static int etm_cpu_id(struct coresight_device *csdev)
@@ -512,33 +521,31 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
struct etm_enable_arg arg = { };
int ret;
- raw_spin_lock(&drvdata->spinlock);
-
- drvdata->traceid = path->trace_id;
-
/*
* Configure the ETM only if the CPU is online. If it isn't online
* hw configuration will take place on the local CPU during bring up.
*/
if (cpu_online(drvdata->cpu)) {
arg.drvdata = drvdata;
+ arg.path = path;
+
+ raw_spin_lock(&drvdata->spinlock);
+ arg.config = drvdata->config;
+ raw_spin_unlock(&drvdata->spinlock);
+
ret = smp_call_function_single(drvdata->cpu,
etm_enable_sysfs_smp_call, &arg, 1);
if (!ret)
ret = arg.rc;
- if (!ret)
- drvdata->sticky_enable = true;
} else {
ret = -ENODEV;
}
- if (ret)
- etm_release_trace_id(drvdata);
-
- raw_spin_unlock(&drvdata->spinlock);
-
if (!ret)
dev_dbg(&csdev->dev, "ETM tracing enabled\n");
+ else
+ etm_release_trace_id(drvdata);
+
return ret;
}
@@ -565,7 +572,7 @@ static void etm_disable_hw(struct etm_drvdata *drvdata)
{
int i;
const struct etm_caps *caps = &drvdata->caps;
- struct etm_config *config = &drvdata->config;
+ struct etm_config *config = &drvdata->active_config;
struct coresight_device *csdev = drvdata->csdev;
CS_UNLOCK(drvdata->csa.base);
@@ -636,7 +643,6 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
* DYING hotplug callback is serviced by the ETM driver.
*/
cpus_read_lock();
- raw_spin_lock(&drvdata->spinlock);
/*
* Executing etm_disable_hw on the cpu whose ETM is being disabled
@@ -645,7 +651,6 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
smp_call_function_single(drvdata->cpu, etm_disable_sysfs_smp_call,
drvdata, 1);
- raw_spin_unlock(&drvdata->spinlock);
cpus_read_unlock();
/*
@@ -711,15 +716,11 @@ static int etm_starting_cpu(unsigned int cpu)
if (!etmdrvdata[cpu])
return 0;
- raw_spin_lock(&etmdrvdata[cpu]->spinlock);
- if (!etmdrvdata[cpu]->os_unlock) {
+ if (!etmdrvdata[cpu]->os_unlock)
etm_os_unlock(etmdrvdata[cpu]);
- etmdrvdata[cpu]->os_unlock = true;
- }
if (coresight_get_mode(etmdrvdata[cpu]->csdev))
etm_enable_hw(etmdrvdata[cpu]);
- raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
return 0;
}
@@ -728,10 +729,8 @@ static int etm_dying_cpu(unsigned int cpu)
if (!etmdrvdata[cpu])
return 0;
- raw_spin_lock(&etmdrvdata[cpu]->spinlock);
if (coresight_get_mode(etmdrvdata[cpu]->csdev))
etm_disable_hw(etmdrvdata[cpu]);
- raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
return 0;
}
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related
* [PATCH v6 13/13] coresight: etm3x: remove redundant call etm_enable_hw() with hotplug
From: Yeoreum Yun @ 2026-04-22 13:22 UTC (permalink / raw)
To: coresight, linux-arm-kernel, linux-kernel
Cc: suzuki.poulose, mike.leach, james.clark, alexander.shishkin,
leo.yan, jie.gan, Yeoreum Yun
In-Reply-To: <20260422132203.977549-1-yeoreum.yun@arm.com>
The cpu_online_mask is set at the CPUHP_BRINGUP_CPU step.
In other words, if etm4_enable_sysfs() is called between
CPUHP_BRINGUP_CPU and CPUHP_AP_ARM_CORESIGHT_STARTING,
etm_enable_hw() may be invoked in etm_enable_sysfs_smp_call()
and then executed again in etm_starting_cpu().
To remove this redundant call, take the hotplug lock before executing
etm_enable_sysfs_smp_call().
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
.../coresight/coresight-etm3x-core.c | 29 ++++++++++---------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 85778e6349f4..72219bbbf616 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -521,25 +521,26 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
struct etm_enable_arg arg = { };
int ret;
+ arg.drvdata = drvdata;
+ arg.path = path;
+
+ raw_spin_lock(&drvdata->spinlock);
+ arg.config = drvdata->config;
+ raw_spin_unlock(&drvdata->spinlock);
+
/*
* Configure the ETM only if the CPU is online. If it isn't online
* hw configuration will take place on the local CPU during bring up.
*/
- if (cpu_online(drvdata->cpu)) {
- arg.drvdata = drvdata;
- arg.path = path;
-
- raw_spin_lock(&drvdata->spinlock);
- arg.config = drvdata->config;
- raw_spin_unlock(&drvdata->spinlock);
-
- ret = smp_call_function_single(drvdata->cpu,
- etm_enable_sysfs_smp_call, &arg, 1);
- if (!ret)
- ret = arg.rc;
- } else {
+ cpus_read_lock();
+ ret = smp_call_function_single(drvdata->cpu,
+ etm_enable_sysfs_smp_call, &arg, 1);
+ cpus_read_unlock();
+
+ if (!ret)
+ ret = arg.rc;
+ else
ret = -ENODEV;
- }
if (!ret)
dev_dbg(&csdev->dev, "ETM tracing enabled\n");
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related
* Re: [PATCH v2 1/4] KVM: arm64: timer: Repaint kvm_timer_{should,irq_can}_fire() to kvm_timer_{pending,enabled}()
From: Joey Gouly @ 2026-04-22 13:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, Deepanshu Kartikey, Suzuki K Poulose,
Oliver Upton, Zenghui Yu
In-Reply-To: <20260422100210.3008156-2-maz@kernel.org>
On Wed, Apr 22, 2026 at 11:02:07AM +0100, Marc Zyngier wrote:
> kvm_timer_should_fire() seems to date back to a time where the author
> of the timer code didn't seem to have made the word "pending" part of
> their vocabulary.
>
> Having since slightly improved on that front, let's rename this predicate
> to kvm_timer_pending(), which clearly indicates whether the timer
> interrupt is pending or not.
>
> Similarly, kvm_timer_irq_can_fire() is renamed to kvm_timer_enabled().
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
Minor comment below.
Reviewed-by: Joey Gouly <joey.gouly@arm.com>
> ---
> arch/arm64/kvm/arch_timer.c | 49 ++++++++++++++++++-------------------
> 1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index cbea4d9ee9552..22e79ecb34bc4 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -39,10 +39,9 @@ static const u8 default_ppi[] = {
> [TIMER_HVTIMER] = 28,
> };
>
> -static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx);
> static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
> struct arch_timer_context *timer_ctx);
> -static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx);
> +static bool kvm_timer_pending(struct arch_timer_context *timer_ctx);
> static void kvm_arm_timer_write(struct kvm_vcpu *vcpu,
> struct arch_timer_context *timer,
> enum kvm_arch_timer_regs treg,
> @@ -224,7 +223,7 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
> else
> ctx = map.direct_ptimer;
>
> - if (kvm_timer_should_fire(ctx))
> + if (kvm_timer_pending(ctx))
> kvm_timer_update_irq(vcpu, true, ctx);
>
> if (userspace_irqchip(vcpu->kvm) &&
> @@ -257,7 +256,7 @@ static u64 kvm_timer_compute_delta(struct arch_timer_context *timer_ctx)
> return kvm_counter_compute_delta(timer_ctx, timer_get_cval(timer_ctx));
> }
>
> -static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
> +static bool kvm_timer_enabled(struct arch_timer_context *timer_ctx)
> {
> WARN_ON(timer_ctx && timer_ctx->loaded);
> return timer_ctx &&
> @@ -294,7 +293,7 @@ static u64 kvm_timer_earliest_exp(struct kvm_vcpu *vcpu)
> struct arch_timer_context *ctx = &vcpu->arch.timer_cpu.timers[i];
>
> WARN(ctx->loaded, "timer %d loaded\n", i);
> - if (kvm_timer_irq_can_fire(ctx))
> + if (kvm_timer_enabled(ctx))
> min_delta = min(min_delta, kvm_timer_compute_delta(ctx));
> }
>
> @@ -358,7 +357,7 @@ static enum hrtimer_restart kvm_hrtimer_expire(struct hrtimer *hrt)
> return HRTIMER_NORESTART;
> }
>
> -static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
> +static bool kvm_timer_pending(struct arch_timer_context *timer_ctx)
> {
> enum kvm_arch_timers index;
> u64 cval, now;
> @@ -391,7 +390,7 @@ static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
> !(cnt_ctl & ARCH_TIMER_CTRL_IT_MASK);
> }
>
> - if (!kvm_timer_irq_can_fire(timer_ctx))
> + if (!kvm_timer_enabled(timer_ctx))
> return false;
>
> cval = timer_get_cval(timer_ctx);
> @@ -417,9 +416,9 @@ void kvm_timer_update_run(struct kvm_vcpu *vcpu)
> /* Populate the device bitmap with the timer states */
> regs->device_irq_level &= ~(KVM_ARM_DEV_EL1_VTIMER |
> KVM_ARM_DEV_EL1_PTIMER);
> - if (kvm_timer_should_fire(vtimer))
> + if (kvm_timer_pending(vtimer))
> regs->device_irq_level |= KVM_ARM_DEV_EL1_VTIMER;
> - if (kvm_timer_should_fire(ptimer))
> + if (kvm_timer_pending(ptimer))
> regs->device_irq_level |= KVM_ARM_DEV_EL1_PTIMER;
> }
>
> @@ -473,21 +472,21 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
> /* Only called for a fully emulated timer */
> static void timer_emulate(struct arch_timer_context *ctx)
> {
> - bool should_fire = kvm_timer_should_fire(ctx);
> + bool pending = kvm_timer_pending(ctx);
>
> - trace_kvm_timer_emulate(ctx, should_fire);
> + trace_kvm_timer_emulate(ctx, pending);
>
> - if (should_fire != ctx->irq.level)
> - kvm_timer_update_irq(timer_context_to_vcpu(ctx), should_fire, ctx);
> + if (pending != ctx->irq.level)
> + kvm_timer_update_irq(timer_context_to_vcpu(ctx), pending, ctx);
>
> - kvm_timer_update_status(ctx, should_fire);
> + kvm_timer_update_status(ctx, pending);
>
> /*
> * If the timer can fire now, we don't need to have a soft timer
> * scheduled for the future. If the timer cannot fire at all,
> * then we also don't need a soft timer.
> */
Could update this comment to use pending/enabled now?
> - if (should_fire || !kvm_timer_irq_can_fire(ctx))
> + if (pending || !kvm_timer_enabled(ctx))
> return;
>
> soft_timer_start(&ctx->hrtimer, kvm_timer_compute_delta(ctx));
> @@ -594,10 +593,10 @@ static void kvm_timer_blocking(struct kvm_vcpu *vcpu)
> * If no timers are capable of raising interrupts (disabled or
> * masked), then there's no more work for us to do.
> */
> - if (!kvm_timer_irq_can_fire(map.direct_vtimer) &&
> - !kvm_timer_irq_can_fire(map.direct_ptimer) &&
> - !kvm_timer_irq_can_fire(map.emul_vtimer) &&
> - !kvm_timer_irq_can_fire(map.emul_ptimer) &&
> + if (!kvm_timer_enabled(map.direct_vtimer) &&
> + !kvm_timer_enabled(map.direct_ptimer) &&
> + !kvm_timer_enabled(map.emul_vtimer) &&
> + !kvm_timer_enabled(map.emul_ptimer) &&
> !vcpu_has_wfit_active(vcpu))
> return;
>
> @@ -685,7 +684,7 @@ static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)
> * this point and the register restoration, we'll take the
> * interrupt anyway.
> */
> - kvm_timer_update_irq(vcpu, kvm_timer_should_fire(ctx), ctx);
> + kvm_timer_update_irq(vcpu, kvm_timer_pending(ctx), ctx);
>
> if (irqchip_in_kernel(vcpu->kvm))
> phys_active = kvm_vgic_map_is_active(vcpu, timer_irq(ctx));
> @@ -706,7 +705,7 @@ static void kvm_timer_vcpu_load_nogic(struct kvm_vcpu *vcpu)
> * this point and the register restoration, we'll take the
> * interrupt anyway.
> */
> - kvm_timer_update_irq(vcpu, kvm_timer_should_fire(vtimer), vtimer);
> + kvm_timer_update_irq(vcpu, kvm_timer_pending(vtimer), vtimer);
>
> /*
> * When using a userspace irqchip with the architected timers and a
> @@ -917,8 +916,8 @@ bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu)
> vlevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_VTIMER;
> plevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_PTIMER;
>
> - return kvm_timer_should_fire(vtimer) != vlevel ||
> - kvm_timer_should_fire(ptimer) != plevel;
> + return kvm_timer_pending(vtimer) != vlevel ||
> + kvm_timer_pending(ptimer) != plevel;
> }
>
> void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
> @@ -1006,7 +1005,7 @@ static void unmask_vtimer_irq_user(struct kvm_vcpu *vcpu)
> {
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>
> - if (!kvm_timer_should_fire(vtimer)) {
> + if (!kvm_timer_pending(vtimer)) {
> kvm_timer_update_irq(vcpu, false, vtimer);
> if (static_branch_likely(&has_gic_active_state))
> set_timer_irq_phys_active(vtimer, false);
> @@ -1579,7 +1578,7 @@ static bool kvm_arch_timer_get_input_level(int vintid)
>
> ctx = vcpu_get_timer(vcpu, i);
> if (timer_irq(ctx) == vintid)
> - return kvm_timer_should_fire(ctx);
> + return kvm_timer_pending(ctx);
> }
>
> /* A timer IRQ has fired, but no matching timer was found? */
> --
> 2.47.3
>
^ permalink raw reply
* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-22 13:32 UTC (permalink / raw)
To: Sudeep Holla
Cc: Marc Zyngier, linux-security-module, linux-kernel,
linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, zohar,
roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, oupton,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
sebastianene
In-Reply-To: <aec/YSxYO1bOhXhn@e129823.arm.com>
Hi All,
> > On Tue, Apr 21, 2026 at 07:57:43AM +0100, Yeoreum Yun wrote:
> >
> > [...]
> >
> > >
> > > Also, the FF-A initialization is not driven by a device probe, but rather
> > > happens as part of the bus registration itself,
> > > so it does not fit well with a device_link or probe deferral based approach.
> > >
> > > Instead, perhaps we could go with the idea I mentioned previously:
> > > either introduce a notifier, or create a pseudo ffa_device
> > > once pKVM initialization has completed, and
> > > then let the ffa driver perform the additional initialization from there.
> > >
> > > Am I missing something?
> > >
> >
> > In order to handle/cleanup some ugliness in interrupt management in the
> > FF-A driver, we may introduce DT node eventually. But it will take sometime.
>
> Unfortunately, I think this DT node wouldn't be helpful to solve
> this situation for dependency with the kvm misc device...
>
> IMHO, current situation, the notifier seems to good option. unless
> we make the initcall to recongise this dependency.
>
I think the best approach for now is to introduce a notifier to handle this situation.
If there are no further suggestions, I’ll send a v2 based on:
- https://lore.kernel.org/all/aeS4rAeVQ0yJIPYw@e129823.arm.com/
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH] KVM: arm64: Validate the FF-A memory access descriptor placement
From: Sebastian Ene @ 2026-04-22 13:35 UTC (permalink / raw)
To: Marc Zyngier
Cc: oupton, will, ayrton, catalin.marinas, joey.gouly, korneld,
kvmarm, linux-arm-kernel, linux-kernel, android-kvm,
mrigendra.chaubey, perlarsen, suzuki.poulose, yuzenghui, stable
In-Reply-To: <86bjfb18v1.wl-maz@kernel.org>
On Wed, Apr 22, 2026 at 01:24:02PM +0100, Marc Zyngier wrote:
> On Wed, 22 Apr 2026 11:25:40 +0100,
> Sebastian Ene <sebastianene@google.com> wrote:
> >
> > Prevent the pKVM hypervisor from making assumptions that the
> > endpoint memory access descriptor (EMAD) comes right after the
> > FF-A memory region header and enforce a strict placement for it
> > when validating an FF-A memory lend/share transaction.
Hello Marc,
>
> As I read this, you want to remove a bad assumption...
>
> >
> > Prior to FF-A version 1.1 the header of the memory region
> > didn't contain an offset to the endpoint memory access descriptor.
> > The layout of a memory transaction looks like this:
> >
> > Field name | Offset
> > -- 0
> > [ Header (ffa_mem_region) |__ ep_mem_offset
> > EMAD 1 (ffa_mem_region_attributes) |
> > ]
> >
> > Reject the host from specifying a memory access descriptor offset
> > that is different than the size of the memory region header.
>
> And yet you decide that you want to enforce this assumption. I don't
> understand how you arrive to this conclusion.
>
> Looking at the spec, it appears that the offset is *designed* to allow
> a gap between the header and the EMAD. Refusing to handle a it seems to be a
> violation of the spec.
>
> What am I missing?
While the spec allows the gap to be variable (since version 1.1), the
arm ff-a driver places it at a fixed position in:
ffa_mem_region_additional_setup()
https://elixir.bootlin.com/linux/v7.0/source/drivers/firmware/arm_ffa/driver.c#L671
and makes use of the same assumption in: ffa_mem_desc_offset().
https://elixir.bootlin.com/linux/v7.0/source/include/linux/arm_ffa.h#L448
The later one seems wrong IMO. because we should compute the offset
based on the value stored in ep_mem_offset and not adding it up with
sizeof(struct ffa_mem_region).
Maybe this should be the fix instead and not the one in pKVM ? What do
you think ?
The current implementation in pKVM makes use of the
ffa_mem_desc_offset() to validate the first EMAD. If a compromised host
places an EMAD at a different offset than sizeof(struct ffa_mem_region),
then pKVM will not validate that EMAD.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Thanks,
Sebastian
^ permalink raw reply
* Re: [PATCH 0/4] Add hstimer support for H616 and T113-S3
From: Andre Przywara @ 2026-04-22 13:38 UTC (permalink / raw)
To: Michal Piekos
Cc: Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Maxime Ripard, linux-kernel, devicetree, linux-arm-kernel,
linux-sunxi
In-Reply-To: <h4b3xluygfptee3ej4gbr3hctgjk6q4oytkisk66saii4uyvl3@lw4qdtfuyoz5>
Hi Michal,
On 4/21/26 16:05, Michal Piekos wrote:
> On Mon, Apr 20, 2026 at 04:14:44PM +0200, Andre Przywara wrote:
>> Hi Michal,
>>
>> On 4/20/26 13:27, Michal Piekos wrote:
>>> On Sun, Apr 19, 2026 at 10:55:39PM +0200, Andre Przywara wrote:
>>>> On Sun, 19 Apr 2026 14:46:06 +0200
>>>> Michal Piekos <michal.piekos@mmpsystems.pl> wrote:
>>>>
....
>>>>
>>> I took the table from https://linux-sunxi.org/Linux_mainlining_effort as
>>> a todo list and wanted to help with it. I do not have own use case for
>>> this timer. If it is not needed then I will spin v2 to include your
>>> comments and abandon it.
>>
>> Ah, that's good to know, and thanks for picking things from that list! I
>> don't think there is a particular need to abandon your work, we could as
>> well upstream it. At least the DT changes should be added, so that other DT
>> users could make use of the timers - after all it's a Linux implementation
>> choice to utilise just one timer. But please go ahead and post a complete
>> v2, I don't think it hurts to have HSTIMER support in the kernel.
>> And while you are at it: can you figure out what the need is for using two
>> timers? One is a clock source, the other is for clock events? And why do we
>> limit the counters and timers to 32 bit? Even the A13 manual lists them as
>> 56 bits, and a wraparound time of roughly 21 seconds (with 32 bit counters)
>> does not sound very long to me.
>>
> Yes. Channel 0 is clockevent and channel 1 is a clocksource and sync
> reference for channel 0 disable timing.
>
> 32 bit counters seems like implementation choice rather than limitation
> but that would need to be implemented and tested. Would you suggest to
> extend it to 56 bit in the following patch?
Well, yes, I would assume we want as long an overflow period as
possible. The tricky/interesting part is that the interface is still
32-bit MMIO reads, so we need to find out how the consistency works. The
manual recommends to read LO first, but not sure that means its latching
HI upon the LO read. Otherwise we should read HI, LO, and HI again and
compare both HI's. Probably needs some testing.
>> Not sure what your primary motivation for fixing Allwinner support is, but
>> we could probably find more worthwhile targets. Do you have Allwinner boards
>> other than the OrangePi Zero 3? There are not many low hanging fruits on the
>> H616 left (MBUS and LDOs(?) maybe), but the A523 has quite some missing
>> drivers still, some of them probably more on the easy side.
>>
> I have boards with A733, A527, T113-S3, H616, H6, H3 and I
> think some older stuff too. My motivation is mostly fun and learning.
That's great, and what I was hoping for! ;-)
Feel free to reach out on IRC if you have any questions or comments.
> I also use those boards in custom projects.
>
> I will take up GPADC on A527 after finishing this as I worked with ADC's
> a lot on MCU's. Unless other suggestions?
Yes, LRADC and GPADC are good devices to start with. Also crypto comes
to mind, the most useful there being the TRNG device, which helps the
kernel to start its own RNG much quicker. Chances are those things are
close to the existing SoCs, so there might be not too much to do here.
Cheers,
Andre
>
> Thank you for comments.
> Michal
>
>> If you are stuck with the OpiZero3, then you could just look and check the
>> existing devices, and verify their operation. For instance I think USB-OTG
>> is still broken - across most Allwinner SoCs actually, so it's a sunxi
>> driver issue.
>>
>> Thanks,
>> Andre
>>
>>>
>>> Michal
>>>
>>>>>
>>>>> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
>>>>> ---
>>>>> Michal Piekos (4):
>>>>> dt-bindings: timer: allwinner,sun5i-a13-hstimer: add H616 and T113-S3
>>>>> clocksource/drivers/sun5i: add H616 hstimer support
>>>>> arm64: dts: allwinner: h616: add hstimer node
>>>>> arm: dts: allwinner: t113s: add hstimer node
>>>>>
>>>>> .../timer/allwinner,sun5i-a13-hstimer.yaml | 8 +++-
>>>>> arch/arm/boot/dts/allwinner/sun8i-t113s.dtsi | 12 +++++
>>>>> arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 ++++
>>>>> drivers/clocksource/timer-sun5i.c | 56 +++++++++++++++++++---
>>>>> 4 files changed, 78 insertions(+), 7 deletions(-)
>>>>> ---
>>>>> base-commit: faeab166167f5787719eb8683661fd41a3bb1514
>>>>> change-id: 20260413-h616-t113s-hstimer-62939948f91c
>>>>>
>>>>> Best regards,
>>>>
>>>>
>>
>>
^ permalink raw reply
* Re: [PATCH RFC 2/2] clk: scmi: Add support for two #clock-cells to pass rate rounding mode
From: Peng Fan @ 2026-04-22 14:00 UTC (permalink / raw)
To: Sudeep Holla
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Cristian Marussi, linux-kernel, linux-clk,
devicetree, arm-scmi, linux-arm-kernel, Peng Fan
In-Reply-To: <20260422-huge-kiwi-of-tornado-4fce89@sudeepholla>
Hi Sudeep,
Thanks for giving a look.
On Wed, Apr 22, 2026 at 02:14:56PM +0100, Sudeep Holla wrote:
>On Fri, Mar 06, 2026 at 02:20:13PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> SCMI CLOCK_RATE_SET allows the caller to specify the rounding behaviour
>> when setting a clock rate. The previously added dt-bindings header
>> defines three modes:
>>
>> ROUND_DOWN / ROUND_UP / ROUND_AUTO
>>
>> To enable device tree clients to select a rounding mode, extend the
>> SCMI clock provider to support "#clock-cells = <2>", where the second
>> cell encodes the desired rounding mode. The default remains
>> ROUND_DOWN for backwards compatibility with existing device trees.
>>
>
>Where is the binding update documented ? It's not in 1/2.
This was missed in this patchset, I will fix in new version, if this
patchset does not have big design flaw.
>
>Also if it can be static in the device tree, why can't it be
>autonomously handled in the platform firmware ? I think I know the
Linux passes ROUND_DOWN, SCMI firmware uses round down for clk calculation.
>answer for this but I want to make sure it is a valid use-case and
>gets documented here as part of binding updates.
Per info from our video software team.
We have some video modes where the best pixel clock rate is slightly above the
nominal rate, and the default round down rule (CLOCK_ROUND_RULE_CEILING in SM
firmware) can cause the resulting clock rate to be much lower than expected.
disp1pix = 96200000 Hz (desired pixel clock rate)
The MIPI DPHY cannot hit the exact frequency of 288600000 Hz needed for this
pixel clock rate, so the next best DPHY PLL frequency is 289000000 Hz. This
corresponds to a pixel clock frequency of 96333333 Hz, which is slightly higher
than the nominal rate of 96200000 Hz the video mode specifies.
Setting the VIDEOPLL (disp1pix parent) to 289000000 Hz should divide down to
the adjusted disp1pix frequency of 96333333 Hz, but here is what happens in the
SM firmware:
quotient = 289000000 / (96200000 + 1) = 3.004 => 3 (notice that the SM always
receives the nominal clock rate, not the adjusted rate)
If the rounding rule is round down (CLOCK_ROUND_RULE_CEILING),
quotient = quotient + 1. Therefore, quotient becomes 4.
disp1pix = 289000000 / 4 = 72250000, which is nowhere close to the target of
96333333.
However, if we can use ROUND_AUTO the SM firmware would select a quotient of 3
in this case, and disp1pix would match our target: 289000000 / 3 = 96333333.
Thanks,
Peng
>
>--
>Regards,
>Sudeep
^ permalink raw reply
* Re: [PATCH v4 1/8] sframe: Allow kernelspace sframe sections
From: Jens Remus @ 2026-04-22 14:08 UTC (permalink / raw)
To: Dylan Hatch, Roman Gushchin, Weinan Liu, Will Deacon,
Josh Poimboeuf, Indu Bhagat, Peter Zijlstra, Steven Rostedt,
Catalin Marinas, Jiri Kosina
Cc: Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan, Song Liu,
joe.lawrence, linux-toolchains, linux-kernel, live-patching,
linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <20260421225200.1198447-2-dylanbhatch@google.com>
On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> Generalize the sframe lookup code to support kernelspace sections. This
> is done by defining a SFRAME_LOOKUP option that can be activated
> separate from HAVE_UNWIND_USER_SFRAME, as there will be other client to
> this library than just userspace unwind.
>
> Sframe section location is now tracked in a separate sec_type field to
> determine whether user-access functions are necessary to read the sframe
> data. Relevant type delarations are moved and renamed to reflect the
> non-user sframe support.
>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
With return -EFAULT changed to goto label in DATA_COPY() and DATA_GET():
Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> ---
> MAINTAINERS | 2 +-
> arch/Kconfig | 4 +
> .../{unwind_user_sframe.h => unwind_sframe.h} | 6 +-
> arch/x86/include/asm/unwind_user.h | 12 +-
> include/linux/sframe.h | 48 ++--
> include/linux/unwind_types.h | 46 +++
> include/linux/unwind_user_types.h | 41 ---
> kernel/unwind/Makefile | 2 +-
> kernel/unwind/sframe.c | 270 ++++++++++++------
> kernel/unwind/user.c | 41 +--
> 10 files changed, 293 insertions(+), 179 deletions(-)
> rename arch/x86/include/asm/{unwind_user_sframe.h => unwind_sframe.h} (50%)
> create mode 100644 include/linux/unwind_types.h
> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
> +enum sframe_sec_type {
> + SFRAME_KERNEL,
> + SFRAME_USER,
> +};
> struct sframe_section {
> - struct rcu_head rcu;
> + struct rcu_head rcu;
> #ifdef CONFIG_DYNAMIC_DEBUG
> - const char *filename;
> + const char *filename;
> #endif
> - unsigned long sframe_start;
> - unsigned long sframe_end;
> - unsigned long text_start;
> - unsigned long text_end;
> -
> - unsigned long fdes_start;
> - unsigned long fres_start;
> - unsigned long fres_end;
> - unsigned int num_fdes;
> -
> - signed char ra_off;
> - signed char fp_off;
> + enum sframe_sec_type sec_type;
> + unsigned long sframe_start;
> + unsigned long sframe_end;
> + unsigned long text_start;
> + unsigned long text_end;
> +
> + unsigned long fdes_start;
> + unsigned long fres_start;
> + unsigned long fres_end;
> + unsigned int num_fdes;
> +
> + signed char ra_off;
> + signed char fp_off;
> };
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> +#define DATA_COPY(sec, to, from, size, label) \
> +({ \
> + switch (sec->sec_type) { \
> + case SFRAME_KERNEL: \
> + KERNEL_COPY(to, from, size, label); \
> + break; \
> + case SFRAME_USER: \
> + UNSAFE_USER_COPY(to, from, size, label); \
> + break; \
I wonder whether it would be worthwhile to come up with an approach
where this would get evaluated at compile time instead at run time?
Or is this overengineering? Of course such improvement could be
made later on, so no need to solve that now.
Options that came into my mind:
A) Introduce and pass through a "bool user" parameter, whose value is
specified in sframe_find_user() and sframe_find_kernel(). Due to
inlining I would expect that to get any conditions based on that
to get evaluated at compile time. See below. Downside is the
ugly additional parameter.
B) Introduce lightweight .c wrappers, e.g. sframe_kernel.c and
sframe_user.c, that define DATA_GET() and DATA_COPY() and include
sframe.c. All HAVE_UNWIND_KERNEL_SFRAME code would be moved into
sframe_kernel.c and likewise all HAVE_UNWIND_USER_SFRAME code into
sframe_user.c.
> + default: \
> + return -EFAULT; \
goto label; \
Users of DATA_COPY() do expect the macro to branch to the label in case
of an error and therefore do not evaluate any return value. The
wrapping then needs also be changed from "({ .. })" to
"do { ... } while (0)".
> + } \
> +})
> +
> +#define DATA_GET(sec, to, from, type, label) \
> +({ \
> + switch (sec->sec_type) { \
> + case SFRAME_KERNEL: \
> + KERNEL_GET(to, from, type, label); \
> + break; \
> + case SFRAME_USER: \
> + UNSAFE_USER_GET(to, from, type, label); \
> + break; \
> + default: \
> + return -EFAULT; \
Likewise.
> + } \
> +})
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +
> +int sframe_find_user(unsigned long ip, struct unwind_frame *frame)
> +{
> + struct mm_struct *mm = current->mm;
> + struct sframe_section *sec;
> + int ret;
> +
> + if (!mm)
> + return -EINVAL;
> +
> + guard(srcu)(&sframe_srcu);
> +
> + sec = mtree_load(&mm->sframe_mt, ip);
> + if (!sec)
> + return -EINVAL;
> +
> + if (!user_read_access_begin((void __user *)sec->sframe_start,
> + sec->sframe_end - sec->sframe_start))
> + return -EFAULT;
> +
> + ret = __sframe_find(sec, ip, frame);
In sframe_find_user() sec->sec_type must be SFRAME_USER. Likewise in
sframe_find_kernel() it must be SFRAME_KERNEL. So instead of
introducing sec_type, we could add a parameter
__sframe_find(..., bool user) and do:
ret = __sframe_find(sec, ip, frame, true);
The downside is that this then requires to pass that flag through
everywhere... (see below).
> +
> + user_read_access_end();
> +
> + if (ret == -EFAULT) {
> + dbg_sec("removing bad .sframe section\n");
> + WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
> + }
> +
> + return ret;
> +}
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v4 7/8] sframe: Introduce in-kernel SFRAME_VALIDATION
From: Jens Remus @ 2026-04-22 14:11 UTC (permalink / raw)
To: Dylan Hatch, Roman Gushchin, Weinan Liu, Will Deacon,
Josh Poimboeuf, Indu Bhagat, Peter Zijlstra, Steven Rostedt,
Catalin Marinas, Jiri Kosina
Cc: Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan, Song Liu,
joe.lawrence, linux-toolchains, linux-kernel, live-patching,
linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <20260421225200.1198447-8-dylanbhatch@google.com>
On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> Generalize the __safe* helpers to support a non-user-access code path.
>
> This requires arch-specific function address validation. This is because
> arm64 vmlinux has an .rodata.text section which lies outside the bounds
> of the normal .text. It contains code that is never executed by the
> kernel mapping, but for which the toolchain nonetheless generates sframe
> data, and needs to be considered valid for a PC lookup.
>
> This arch-specific address validation logic is only necessary to support
> SFRAME_VALIDATION for the vmlinux .sframe, since these .rodata.text
> functions would never be encountered during normal unwinding.
>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> Suggested-by: Jens Remus <jremus@linux.ibm.com>
With the minor nit below fixed:
Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> ---
> arch/Kconfig | 2 +-
> arch/arm64/include/asm/sections.h | 1 +
> arch/arm64/include/asm/unwind_sframe.h | 21 +++++++++++++++++++++
> arch/arm64/kernel/vmlinux.lds.S | 2 ++
> include/linux/sframe.h | 2 ++
> kernel/unwind/sframe.c | 25 +++++++++++++++++++++++--
> 6 files changed, 50 insertions(+), 3 deletions(-)
> diff --git a/arch/Kconfig b/arch/Kconfig
> @@ -503,7 +503,7 @@ config HAVE_UNWIND_USER_SFRAME
>
> config SFRAME_VALIDATION
> bool "Enable .sframe section debugging"
> - depends on HAVE_UNWIND_USER_SFRAME
> + depends on SFRAME_LOOKUP
depends on UNWIND_SFRAME__LOOKUP
> depends on DYNAMIC_DEBUG
> help
> When adding an .sframe section for a task, validate the entire
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH] net/stmmac: Fix typos: 'tx_undeflow_irq' -> 'tx_underflow_irq'
From: Jakub Raczynski @ 2026-04-22 14:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, kuba, davem, andrew+netdev, kernel-janitors,
linux-arm-kernel, linux-stm32
In-Reply-To: <f1d51362-ca8f-481a-b9c1-400ab6422686@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]
On Wed, Apr 22, 2026 at 02:47:38PM +0200, Andrew Lunn wrote:
> > I don't see anything wrong with it?
> > - naming is correct, same as stmmac_extra_stats from common.h, as it
> > wouldn't compile otherwise
> > - string length is ok, as max name length is ETH_GSTRING_LEN=32 and it is
> > not close
> > - ethtool just polls data from driver and in my tests it is ok
> > - all instances of 'undeflow' are changed
> > - 'underflow' semantic is ok, 'undeflow' is just not correct
> >
> > Please correct me if I am wrong, but imo no issues with this patch.
>
> ABI
>
> This name is published as part of the kAPI. You are changing its
> name. User space could be looking for this name, even thought it has a
> typo in it.
>
> Andrew
>
I don't think it is? This part of extra stats (struct stmmac_extra_stats) and
is not part of standard ABI from
Documentation/ABI/testing/sysfs-class-net-statistics
nor is mentioned in
Documentation/networking/device_drivers/ethernet/stmicro/stmmac.rst
These extra stats are specific to stmmac driver and most of these are more
than standard
https://www.kernel.org/doc/html/v7.0/networking/statistics.html#c.rtnl_link_stats64
This name does not exist outside stmmac driver, so while some application may
expect this (stmmac specific app), question is should this typo stick?
This type of typo is even mentioned in scripts/spelling.txt.
Regards
Jakub Raczynski
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/8] arm64, unwind: build kernel with sframe V3 info
From: Jens Remus @ 2026-04-22 14:15 UTC (permalink / raw)
To: Dylan Hatch, Roman Gushchin, Weinan Liu, Will Deacon,
Josh Poimboeuf, Indu Bhagat, Peter Zijlstra, Steven Rostedt,
Catalin Marinas, Jiri Kosina
Cc: Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan, Song Liu,
joe.lawrence, linux-toolchains, linux-kernel, live-patching,
linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <20260421225200.1198447-3-dylanbhatch@google.com>
On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> Build with -Wa,--gsframe-3 flags to generate a .sframe section. This
> will be used for in-kernel reliable stacktrace in cases where the frame
> pointer alone is insufficient.
>
> Currently, the sframe format only supports arm64, x86_64 and s390x
> architectures.
>
> Signed-off-by: Weinan Liu <wnliu@google.com>
> Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> ---
> MAINTAINERS | 1 +
> Makefile | 8 ++++++++
> arch/Kconfig | 21 +++++++++++++++++++++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/unwind_sframe.h | 8 ++++++++
> arch/arm64/kernel/vdso/Makefile | 2 +-
> include/asm-generic/sections.h | 4 ++++
> include/asm-generic/vmlinux.lds.h | 15 +++++++++++++++
> 8 files changed, 59 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/unwind_sframe.h
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v4 3/8] arm64: entry: add unwind info for various kernel entries
From: Jens Remus @ 2026-04-22 14:18 UTC (permalink / raw)
To: Dylan Hatch, Roman Gushchin, Weinan Liu, Will Deacon,
Josh Poimboeuf, Indu Bhagat, Peter Zijlstra, Steven Rostedt,
Catalin Marinas, Jiri Kosina
Cc: Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan, Song Liu,
joe.lawrence, linux-toolchains, linux-kernel, live-patching,
linux-arm-kernel, Randy Dunlap
In-Reply-To: <20260421225200.1198447-4-dylanbhatch@google.com>
On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> From: Weinan Liu <wnliu@google.com>
>
> DWARF CFI (Call Frame Information) specifies how to recover the return
> address and callee-saved registers at each PC in a given function.
> Compilers are able to generate the CFI annotations when they compile
> the code to assembly language. For handcrafted assembly, we need to
> annotate them by hand.
>
> Annotate minimal CFI to enable stacktracing using SFrame for kernel
> exception entries through el1*_64_*() paths and irq entries through
> call_on_irq_stack()
>
> Signed-off-by: Weinan Liu <wnliu@google.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> Suggested-by: Jens Remus <jremus@linux.ibm.com>
Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> ---
> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v4 8/8] unwind: arm64: Use sframe to unwind interrupt frames
From: Jens Remus @ 2026-04-22 14:25 UTC (permalink / raw)
To: Dylan Hatch, Roman Gushchin, Weinan Liu, Will Deacon,
Josh Poimboeuf, Indu Bhagat, Peter Zijlstra, Steven Rostedt,
Catalin Marinas, Jiri Kosina
Cc: Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan, Song Liu,
joe.lawrence, linux-toolchains, linux-kernel, live-patching,
linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <20260421225200.1198447-9-dylanbhatch@google.com>
On 4/22/2026 12:52 AM, Dylan Hatch wrote:
> Add unwind_next_frame_sframe() function to unwind by sframe info if
> present. Use this method at exception boundaries, falling back to
> frame-pointer unwind only on failure. In such failure cases, the
> stacktrace is considered unreliable.
>
> During normal unwind, prefer frame pointer unwind (for better
> performance) with sframe as a backup.
>
> This change restores the LR behavior originally introduced in commit
> c2c6b27b5aa14fa2 ("arm64: stacktrace: unwind exception boundaries"),
> But later removed in commit 32ed1205682e ("arm64: stacktrace: Skip
> reporting LR at exception boundaries")
>
> This can be done because the sframe data can be used to determine
> whether the LR is current for the PC value recovered from pt_regs at the
> exception boundary.
>
> Signed-off-by: Weinan Liu <wnliu@google.com>
> Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> ---
> arch/arm64/include/asm/stacktrace/common.h | 6 +
> arch/arm64/kernel/stacktrace.c | 246 +++++++++++++++++++--
> 2 files changed, 232 insertions(+), 20 deletions(-)
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH] dt-bindings: arm-smmu: qcom:: Fix Hawi compatible placement
From: Mukesh Ojha @ 2026-04-22 14:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Will Deacon, Joerg Roedel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robin Murphy, linux-arm-kernel, iommu, devicetree,
linux-kernel
In-Reply-To: <d1d24380-06e8-4833-b893-631c063d77ff@oss.qualcomm.com>
On Wed, Apr 22, 2026 at 11:04:09AM +0200, Krzysztof Kozlowski wrote:
> On 22/04/2026 10:33, Mukesh Ojha wrote:
> > qcom,hawi-smmu-500 was placed in the wrong enum block of GPU. Move it to
> > the correct location alongside other Qualcomm SMMU-500 compatibles for
> > CPU.
>
> How could it pass dtbs_check?
It was bad on my part.
> >
> > Fixes: 5e8323c3d528 ("dt-bindings: arm-smmu: qcom: Add compatible for Hawi SoC")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > index 06fb5c8e7547..ba9ad1f5a8ff 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > @@ -37,6 +37,7 @@ properties:
> > - enum:
> > - qcom,eliza-smmu-500
> > - qcom,glymur-smmu-500
> > + - qcom,hawi-smmu-500
> > - qcom,kaanapali-smmu-500
> > - qcom,milos-smmu-500
> > - qcom,qcm2290-smmu-500
> > @@ -93,7 +94,6 @@ properties:
> > items:
> > - enum:
> > - qcom,glymur-smmu-500
> > - - qcom,hawi-smmu-500
>
> No, why? That's for GPU. Why are you moving GPU compatible to non-GPU place?
Earlier commit was meant for CPUs and not for GPUs, hence moving it Or,
Are you saying do not do the movement and add one for CPUs ?
>
>
> Best regards,
> Krzysztof
--
-Mukesh Ojha
^ permalink raw reply
* Re: [PATCH] dt-bindings: arm-smmu: qcom:: Fix Hawi compatible placement
From: Krzysztof Kozlowski @ 2026-04-22 14:37 UTC (permalink / raw)
To: Mukesh Ojha, Krzysztof Kozlowski
Cc: Will Deacon, Joerg Roedel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robin Murphy, linux-arm-kernel, iommu, devicetree,
linux-kernel
In-Reply-To: <20260422142710.2f6f2oijbsjojbmi@hu-mojha-hyd.qualcomm.com>
On 22/04/2026 16:27, Mukesh Ojha wrote:
>>> - enum:
>>> - qcom,eliza-smmu-500
>>> - qcom,glymur-smmu-500
>>> + - qcom,hawi-smmu-500
>>> - qcom,kaanapali-smmu-500
>>> - qcom,milos-smmu-500
>>> - qcom,qcm2290-smmu-500
>>> @@ -93,7 +94,6 @@ properties:
>>> items:
>>> - enum:
>>> - qcom,glymur-smmu-500
>>> - - qcom,hawi-smmu-500
>>
>> No, why? That's for GPU. Why are you moving GPU compatible to non-GPU place?
>
> Earlier commit was meant for CPUs and not for GPUs, hence moving it Or,
OK, but:
> Are you saying do not do the movement and add one for CPUs ?
yeah, because you might need it in this place for the GPU. Therefore I
would propose to check what compatible you expect for the GPU part and
just add it.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm64: traps: Add a macro to simplify the condition codes check
From: Vladimir Murzin @ 2026-04-22 14:43 UTC (permalink / raw)
To: Jinjie Ruan, catalin.marinas, will, mark.rutland, kees, maz,
ada.coupriediaz, smostafa, leitao, mrigendra.chaubey,
linux-arm-kernel, linux-kernel
In-Reply-To: <dd2f076b-976c-480d-8414-e6685cad40dd@huawei.com>
Hi Jinjie,
On 4/22/26 04:06, Jinjie Ruan wrote:
>
> On 3/20/2026 4:28 PM, Jinjie Ruan wrote:
>> Add DEFINE_COND_CHECK macro to define the simple __check_* functions
>> to simplify the condition codes check.
>>
>> No functional changes.
> Gentle ping.
>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> arch/arm64/kernel/traps.c | 59 ++++++++++-----------------------------
>> 1 file changed, 15 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index 914282016069..6216fe9e8e42 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -49,45 +49,21 @@
>> #include <asm/system_misc.h>
>> #include <asm/sysreg.h>
>>
>> -static bool __kprobes __check_eq(unsigned long pstate)
>> -{
>> - return (pstate & PSR_Z_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_ne(unsigned long pstate)
>> -{
>> - return (pstate & PSR_Z_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_cs(unsigned long pstate)
>> -{
>> - return (pstate & PSR_C_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_cc(unsigned long pstate)
>> -{
>> - return (pstate & PSR_C_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_mi(unsigned long pstate)
>> -{
>> - return (pstate & PSR_N_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_pl(unsigned long pstate)
>> -{
>> - return (pstate & PSR_N_BIT) == 0;
>> -}
>> -
>> -static bool __kprobes __check_vs(unsigned long pstate)
>> -{
>> - return (pstate & PSR_V_BIT) != 0;
>> -}
>> -
>> -static bool __kprobes __check_vc(unsigned long pstate)
>> -{
>> - return (pstate & PSR_V_BIT) == 0;
>> -}
>> +#define DEFINE_COND_CHECK(name, flag, expected) \
>> +static bool __kprobes __check_##name(unsigned long pstate) \
>> +{ \
>> + return ((pstate & (flag)) != 0) == (expected); \
>> +}
>> +
>> +DEFINE_COND_CHECK(eq, PSR_Z_BIT, true)
>> +DEFINE_COND_CHECK(ne, PSR_Z_BIT, false)
>> +DEFINE_COND_CHECK(cs, PSR_C_BIT, true)
>> +DEFINE_COND_CHECK(cc, PSR_C_BIT, false)
>> +DEFINE_COND_CHECK(mi, PSR_N_BIT, true)
>> +DEFINE_COND_CHECK(pl, PSR_N_BIT, false)
>> +DEFINE_COND_CHECK(vs, PSR_V_BIT, true)
>> +DEFINE_COND_CHECK(vc, PSR_V_BIT, false)
>> +DEFINE_COND_CHECK(al, 0, false) /* Always true */
>>
>> static bool __kprobes __check_hi(unsigned long pstate)
>> {
>> @@ -131,11 +107,6 @@ static bool __kprobes __check_le(unsigned long pstate)
>> return (temp & PSR_N_BIT) != 0;
>> }
>>
>> -static bool __kprobes __check_al(unsigned long pstate)
>> -{
>> - return true;
>> -}
>> -
>> /*
>> * Note that the ARMv8 ARM calls condition code 0b1111 "nv", but states that
>> * it behaves identically to 0b1110 ("al").
>
It looks like we now have a mix of checks implemented via macros
and others written out explicitly. The existing approach has the
advantage of being consistent and easy to follow, whereas
introducing macros here, even if it reduces some duplication,
adds a bit of cognitive overhead when reading the code.
This may come down to preference, but I think sticking to a
single, consistent style would make the code easier to scan and
maintain.
Just my 2p.
Cheers
Vladimir
^ permalink raw reply
* Re: [PATCH 1/4] arm64: signal: Preserve POR_EL0 if poe_context is missing
From: Kevin Brodsky @ 2026-04-22 14:55 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, linux-kernel, Catalin Marinas, Joey Gouly,
Mark Brown, Shuah Khan, linux-kselftest
In-Reply-To: <aei87MmXaEdg8YOf@willie-the-truck>
On 22/04/2026 14:19, Will Deacon wrote:
>> @@ -74,8 +76,12 @@ struct rt_sigframe_user_layout {
>> * This state needs to be carefully managed to ensure that it doesn't cause
>> * uaccess to fail when setting up the signal frame, and the signal handler
>> * itself also expects a well-defined state when entered.
>> + *
>> + * The valid_fields member is a bitfield (see UA_STATE_HAS_*), specifying which
>> + * of the remaining fields is valid (has been set to a value).
>> */
>> struct user_access_state {
>> + unsigned int valid_fields;
>> u64 por_el0;
>> };
> Do you think it would be worth adding some accessors to make it easier
> to keep the flags in sync? For example:
>
> /* Stores por_el0 into uas->por_el0 and sets UA_STATE_HAS_POR_EL0 */
> void set_ua_state_por_el0(struct user_access_state *uas, u64 por_el0);
>
> /*
> * If UA_STATE_HAS_POR_EL0, *por_el0 = uas->por_el0 and return 0.
> * Otherwise, return -ENOENT.
> */
> int get_ua_state_por_el0(struct user_access_state *uas, u64 *por_el0);
>
> WDYT?
I did get a feeling having helpers would be a good idea. I wonder if
getters/setters aren't a bit overkill though, as they make accesses to
the struct more cumbersome and we'd need a pair for every member (unless
we use some macro magic). Maybe it would be sufficient to have say
ua_state_has_field(POR_EL0) to check if the bit is set, and
ua_state_set_field_valid(POR_EL0) to set the bit?
>> @@ -1095,7 +1104,7 @@ SYSCALL_DEFINE0(rt_sigreturn)
>> {
>> struct pt_regs *regs = current_pt_regs();
>> struct rt_sigframe __user *frame;
>> - struct user_access_state ua_state;
>> + struct user_access_state ua_state = {0};
> nit: {} should do (no need for the '0'). Same in setup_rt_frame().
Will change it, I have some vague recollection that GCC and Clang
disagreed about the meaning of {}... but that's probably fixed nowadays.
Thanks for the review!
- Kevin
^ permalink raw reply
* Re: [PATCH] spi: sun6i: Set SPI mode in prepare_message
From: Mark Brown @ 2026-04-22 14:57 UTC (permalink / raw)
To: Kevin Mehall
Cc: Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Mirko Vogt,
Ralf Schlatterbeck, linux-spi, linux-arm-kernel, linux-sunxi,
linux-kernel
In-Reply-To: <20260420164755.1131645-1-km@kevinmehall.net>
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On Mon, Apr 20, 2026 at 10:46:04AM -0600, Kevin Mehall wrote:
> With a GPIO chip select, CS is asserted before entering transfer_one.
> The spi-sun6i driver previously configured the SPI mode (including clock
> polarity) and enabled the bus in transfer_one, which can cause an
> extraneous SCK transition with CS asserted, corrupting the transferred
> data.
> This patch moves the SPI mode configuration and bus enable to the
> spi_prepare_message callback, ensuring that SCK is driven to the correct
> level prior to asserting CS.
> + /* We want to control the chip select manually */
> + reg |= SUN6I_TFR_CTL_CS_MANUAL;
> +
> + sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
Might this cause the native chip select to get asserted, we didn't set
up values so it'll have defaults if it wasn't previously configured?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
From: Vishnu Pajjuri @ 2026-04-22 14:57 UTC (permalink / raw)
To: Marc Zyngier
Cc: Fuad Tabba, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Christoffer Dall, Mark Brown, kvm, linux-arm-kernel,
kvmarm, Darren Hart
In-Reply-To: <86eck71o1v.wl-maz@kernel.org>
Hi Marc,
On 22-04-2026 12:25, Marc Zyngier wrote:
> [+ Darren]
>
> On Tue, 31 Mar 2026 10:42:57 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 31 Mar 2026 07:31:54 +0100,
>> Vishnu Pajjuri <vishnu@os.amperecomputing.com> wrote:
>>>
>>>>> LOG:
>>>>> [ 164.647367] Call trace:
>>>>> [ 164.647368] smp_call_function_many_cond+0x334/0x7a0 (P)
>>>>> [ 164.647372] smp_call_function_many+0x20/0x40
>>>>> [ 164.647374] kvm_make_all_cpus_request+0xec/0x1b8
>>>>> [ 164.647377] vgic_queue_irq_unlock+0x1c8/0x2c8
>>>>> [ 164.647380] kvm_vgic_inject_irq+0x194/0x1e0
>>>>> [ 164.647381] kvm_vm_ioctl_irq_line+0x170/0x400
>>>>> [ 164.647386] kvm_vm_ioctl+0x7b8/0xc88
>>>>> [ 164.647389] __arm64_sys_ioctl+0xb4/0x118
>>>>> [ 164.647393] invoke_syscall+0x6c/0x100
>>>>> [ 164.647397] el0_svc_common.constprop.0+0x48/0xf0
>>>>> [ 164.647398] do_el0_svc+0x24/0x38
>>>>> [ 164.647400] el0_svc+0x3c/0x170
>>>>> [ 164.647403] el0t_64_sync_handler+0xa0/0xe8
>>>>> [ 164.647405] el0t_64_sync+0x1b0/0x1b8
>>>>
>>>> This trace is about interrupt injection from userspace, not
>>>> deactivation of a HW interrupt.
>>>> None of that makes much sense.
>>>
>>> Although this behavior is puzzling, it matches the trace I typically
>>> observe on L0. After reverting the patch, I was able to boot L2 guests
>>> successfully.
>>
>> Well, this patch fixes real bugs, so it isn't going anywhere.
>>
>> The patch you are reverting addresses the deactivation of a HW
>> interrupt, which is likely to be a timer (that's the only one we
>> support). The stacktrace points to the userspace injection of an SPI.
>>
>> If we need to broadcast IPI, that's because there is no other SPI
>> currently in flight. But if a CPU is not responding to the IPI, what
>> is it doing? How does this interact with the patch you are reverting?
>>
>> Given that I don't know what you're running, how you are running it,
>> that I don't have access to whatever HW you are using, and that you
>> are providing no useful information that'd help me debug this, I will
>> leave it up to you to debug it and come back with a detailed analysis
>> of the problem.
>
> Have you made progress on this? I can't reproduce it at all despite my
> best effort. I'm perfectly happy to help, but you need to give me
> *something* to go on.
Thanks for your support!!
The issue is triggered as soon as the timer interrupt (IRQ 27) is
deactivated. Preventing the deactivation of IRQ 27 during nested VGIC
state transitions prevents the failure from reproducing.
I am currently tracing execution paths and inspecting VGIC state to
determine how disabling this interrupt leads to the observed behavior.
Regards,
-Vishnu.
> Thanks,
>
> M.
>
^ permalink raw reply
* Re: [PATCH v7 1/5] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
From: Mark Brown @ 2026-04-22 15:12 UTC (permalink / raw)
To: Linus Torvalds, Stephen Boyd
Cc: Maíra Canal, Michael Turquette, Stephen Boyd,
Nicolas Saenz Julienne, Florian Fainelli, Stefan Wahren,
Maxime Ripard, Melissa Wen, Iago Toral Quiroga, Chema Casanova,
Dave Stevenson, Philipp Zabel, linux-clk, dri-devel,
linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev
In-Reply-To: <5f0bec08-f458-4fba-8bf3-06817a100c4c@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 982 bytes --]
On Tue, Mar 31, 2026 at 01:49:24PM +0100, Mark Brown wrote:
> On Thu, Mar 12, 2026 at 06:34:23PM -0300, Maíra Canal wrote:
> > On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> > actually power off the clock. To achieve meaningful power savings, the
> > clock rate must be set to the minimum before disabling. This might be
> > fixed in future firmware releases.
>
> I'm seeing boot regressions in -next with NFS root on Raspberry Pi 3B+
> which bisect to this commit. We get a likely unrelated oops from the
> firmware interface and the boot grinds to a halt some time later since
> the ethernet never comes up:
This boot regression, which was originally reported against -next
several weeks ago, is now present in mainline. Maíra quickly provided a
fix:
https://lore.kernel.org/r/20260401111416.562279-2-mcanal@igalia.com
which I have confirmed works but I've not seen any other response to
either that fix or the original thread.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/8] sframe: Allow kernelspace sframe sections
From: Dylan Hatch @ 2026-04-22 15:15 UTC (permalink / raw)
To: Jens Remus
Cc: Roman Gushchin, Weinan Liu, Will Deacon, Josh Poimboeuf,
Indu Bhagat, Peter Zijlstra, Steven Rostedt, Catalin Marinas,
Jiri Kosina, Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan,
Song Liu, joe.lawrence, linux-toolchains, linux-kernel,
live-patching, linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <38c9d976-6205-409c-8874-4c9757b25fd6@linux.ibm.com>
On Wed, Apr 22, 2026 at 7:08 AM Jens Remus <jremus@linux.ibm.com> wrote:
>
> On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> > Generalize the sframe lookup code to support kernelspace sections. This
> > is done by defining a SFRAME_LOOKUP option that can be activated
> > separate from HAVE_UNWIND_USER_SFRAME, as there will be other client to
> > this library than just userspace unwind.
> >
> > Sframe section location is now tracked in a separate sec_type field to
> > determine whether user-access functions are necessary to read the sframe
> > data. Relevant type delarations are moved and renamed to reflect the
> > non-user sframe support.
> >
> > Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
>
> With return -EFAULT changed to goto label in DATA_COPY() and DATA_GET():
>
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
>
> > ---
> > MAINTAINERS | 2 +-
> > arch/Kconfig | 4 +
> > .../{unwind_user_sframe.h => unwind_sframe.h} | 6 +-
> > arch/x86/include/asm/unwind_user.h | 12 +-
> > include/linux/sframe.h | 48 ++--
> > include/linux/unwind_types.h | 46 +++
> > include/linux/unwind_user_types.h | 41 ---
> > kernel/unwind/Makefile | 2 +-
> > kernel/unwind/sframe.c | 270 ++++++++++++------
> > kernel/unwind/user.c | 41 +--
> > 10 files changed, 293 insertions(+), 179 deletions(-)
> > rename arch/x86/include/asm/{unwind_user_sframe.h => unwind_sframe.h} (50%)
> > create mode 100644 include/linux/unwind_types.h
>
> > diff --git a/include/linux/sframe.h b/include/linux/sframe.h
>
> > +enum sframe_sec_type {
> > + SFRAME_KERNEL,
> > + SFRAME_USER,
> > +};
>
> > struct sframe_section {
> > - struct rcu_head rcu;
> > + struct rcu_head rcu;
> > #ifdef CONFIG_DYNAMIC_DEBUG
> > - const char *filename;
> > + const char *filename;
> > #endif
> > - unsigned long sframe_start;
> > - unsigned long sframe_end;
> > - unsigned long text_start;
> > - unsigned long text_end;
> > -
> > - unsigned long fdes_start;
> > - unsigned long fres_start;
> > - unsigned long fres_end;
> > - unsigned int num_fdes;
> > -
> > - signed char ra_off;
> > - signed char fp_off;
> > + enum sframe_sec_type sec_type;
> > + unsigned long sframe_start;
> > + unsigned long sframe_end;
> > + unsigned long text_start;
> > + unsigned long text_end;
> > +
> > + unsigned long fdes_start;
> > + unsigned long fres_start;
> > + unsigned long fres_end;
> > + unsigned int num_fdes;
> > +
> > + signed char ra_off;
> > + signed char fp_off;
> > };
>
> > diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>
> > +#define DATA_COPY(sec, to, from, size, label) \
> > +({ \
> > + switch (sec->sec_type) { \
> > + case SFRAME_KERNEL: \
> > + KERNEL_COPY(to, from, size, label); \
> > + break; \
> > + case SFRAME_USER: \
> > + UNSAFE_USER_COPY(to, from, size, label); \
> > + break; \
>
> I wonder whether it would be worthwhile to come up with an approach
> where this would get evaluated at compile time instead at run time?
> Or is this overengineering? Of course such improvement could be
> made later on, so no need to solve that now.
I had a similar thought when I was writing this patch, but I ended up
deciding to avoid premature optimization before getting feedback. I'd
definitely be interested in improving upon this later on.
>
> Options that came into my mind:
> A) Introduce and pass through a "bool user" parameter, whose value is
> specified in sframe_find_user() and sframe_find_kernel(). Due to
> inlining I would expect that to get any conditions based on that
> to get evaluated at compile time. See below. Downside is the
> ugly additional parameter.
>
> B) Introduce lightweight .c wrappers, e.g. sframe_kernel.c and
> sframe_user.c, that define DATA_GET() and DATA_COPY() and include
> sframe.c. All HAVE_UNWIND_KERNEL_SFRAME code would be moved into
> sframe_kernel.c and likewise all HAVE_UNWIND_USER_SFRAME code into
> sframe_user.c.
(A) definitely sounds simpler to implement. For (B) it seems uncommon
for .c files to include one another. Style-wise, is this something
that is typically allowed (e.g. by checkpatch.pl)?
>
> > + default: \
> > + return -EFAULT; \
>
> goto label; \
>
> Users of DATA_COPY() do expect the macro to branch to the label in case
> of an error and therefore do not evaluate any return value. The
> wrapping then needs also be changed from "({ .. })" to
> "do { ... } while (0)".
>
> > + } \
> > +})
> > +
> > +#define DATA_GET(sec, to, from, type, label) \
> > +({ \
> > + switch (sec->sec_type) { \
> > + case SFRAME_KERNEL: \
> > + KERNEL_GET(to, from, type, label); \
> > + break; \
> > + case SFRAME_USER: \
> > + UNSAFE_USER_GET(to, from, type, label); \
> > + break; \
> > + default: \
> > + return -EFAULT; \
>
> Likewise.
>
> > + } \
> > +})
>
> > +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> > +
> > +int sframe_find_user(unsigned long ip, struct unwind_frame *frame)
> > +{
> > + struct mm_struct *mm = current->mm;
> > + struct sframe_section *sec;
> > + int ret;
> > +
> > + if (!mm)
> > + return -EINVAL;
> > +
> > + guard(srcu)(&sframe_srcu);
> > +
> > + sec = mtree_load(&mm->sframe_mt, ip);
> > + if (!sec)
> > + return -EINVAL;
> > +
> > + if (!user_read_access_begin((void __user *)sec->sframe_start,
> > + sec->sframe_end - sec->sframe_start))
> > + return -EFAULT;
> > +
> > + ret = __sframe_find(sec, ip, frame);
>
> In sframe_find_user() sec->sec_type must be SFRAME_USER. Likewise in
> sframe_find_kernel() it must be SFRAME_KERNEL. So instead of
> introducing sec_type, we could add a parameter
> __sframe_find(..., bool user) and do:
>
> ret = __sframe_find(sec, ip, frame, true);
>
> The downside is that this then requires to pass that flag through
> everywhere... (see below).
>
> > +
> > + user_read_access_end();
> > +
> > + if (ret == -EFAULT) {
> > + dbg_sec("removing bad .sframe section\n");
> > + WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
> > + }
> > +
> > + return ret;
> > +}
> Regards,
> Jens
> --
> Jens Remus
> Linux on Z Development (D3303)
> jremus@de.ibm.com / jremus@linux.ibm.com
>
> IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
> IBM Data Privacy Statement: https://www.ibm.com/privacy/
>
^ permalink raw reply
* Re: [PATCH] arm64: traps: Add a macro to simplify the condition codes check
From: Mark Rutland @ 2026-04-22 15:16 UTC (permalink / raw)
To: Vladimir Murzin
Cc: smostafa, kees, catalin.marinas, Jinjie Ruan, linux-kernel,
mrigendra.chaubey, maz, leitao, will, linux-arm-kernel
In-Reply-To: <fbd9268f-ace7-42d8-b920-8b6f9b97cf6d@arm.com>
On Wed, Apr 22, 2026 at 03:43:39PM +0100, Vladimir Murzin wrote:
> Hi Jinjie,
>
> On 4/22/26 04:06, Jinjie Ruan wrote:
> >
> > On 3/20/2026 4:28 PM, Jinjie Ruan wrote:
> >> Add DEFINE_COND_CHECK macro to define the simple __check_* functions
> >> to simplify the condition codes check.
> >>
> >> No functional changes.
> > Gentle ping.
> >
> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> >> ---
> >> arch/arm64/kernel/traps.c | 59 ++++++++++-----------------------------
> >> 1 file changed, 15 insertions(+), 44 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> >> index 914282016069..6216fe9e8e42 100644
> >> --- a/arch/arm64/kernel/traps.c
> >> +++ b/arch/arm64/kernel/traps.c
> >> @@ -49,45 +49,21 @@
> >> #include <asm/system_misc.h>
> >> #include <asm/sysreg.h>
> >>
> >> -static bool __kprobes __check_eq(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_Z_BIT) != 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_ne(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_Z_BIT) == 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_cs(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_C_BIT) != 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_cc(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_C_BIT) == 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_mi(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_N_BIT) != 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_pl(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_N_BIT) == 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_vs(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_V_BIT) != 0;
> >> -}
> >> -
> >> -static bool __kprobes __check_vc(unsigned long pstate)
> >> -{
> >> - return (pstate & PSR_V_BIT) == 0;
> >> -}
> >> +#define DEFINE_COND_CHECK(name, flag, expected) \
> >> +static bool __kprobes __check_##name(unsigned long pstate) \
> >> +{ \
> >> + return ((pstate & (flag)) != 0) == (expected); \
> >> +}
> >> +
> >> +DEFINE_COND_CHECK(eq, PSR_Z_BIT, true)
> >> +DEFINE_COND_CHECK(ne, PSR_Z_BIT, false)
> >> +DEFINE_COND_CHECK(cs, PSR_C_BIT, true)
> >> +DEFINE_COND_CHECK(cc, PSR_C_BIT, false)
> >> +DEFINE_COND_CHECK(mi, PSR_N_BIT, true)
> >> +DEFINE_COND_CHECK(pl, PSR_N_BIT, false)
> >> +DEFINE_COND_CHECK(vs, PSR_V_BIT, true)
> >> +DEFINE_COND_CHECK(vc, PSR_V_BIT, false)
> >> +DEFINE_COND_CHECK(al, 0, false) /* Always true */
> >>
> >> static bool __kprobes __check_hi(unsigned long pstate)
> >> {
> >> @@ -131,11 +107,6 @@ static bool __kprobes __check_le(unsigned long pstate)
> >> return (temp & PSR_N_BIT) != 0;
> >> }
> >>
> >> -static bool __kprobes __check_al(unsigned long pstate)
> >> -{
> >> - return true;
> >> -}
> >> -
> >> /*
> >> * Note that the ARMv8 ARM calls condition code 0b1111 "nv", but states that
> >> * it behaves identically to 0b1110 ("al").
> >
>
> It looks like we now have a mix of checks implemented via macros
> and others written out explicitly. The existing approach has the
> advantage of being consistent and easy to follow, whereas
> introducing macros here, even if it reduces some duplication,
> adds a bit of cognitive overhead when reading the code.
>
> This may come down to preference, but I think sticking to a
> single, consistent style would make the code easier to scan and
> maintain.
FWIW, I agree. I think it'd be better to leave this as-is for now.
Mark.
^ permalink raw reply
* Re: [PATCH v13 00/48] arm64: Support for Arm CCA in KVM
From: Steven Price @ 2026-04-22 15:38 UTC (permalink / raw)
To: Jiahao zheng
Cc: alexandru.elisei, alpergun, aneesh.kumar, catalin.marinas,
christoffer.dall, fj0570is, gankulkarni, gshan, james.morse,
joey.gouly, kvm, kvmarm, linux-arm-kernel, linux-coco,
linux-kernel, maz, oliver.upton, sdonthineni, suzuki.poulose,
tabba, vannapurve, will, yuzenghui
In-Reply-To: <20260421135145.14789-1-jahao.zheng@gmail.com_quarantine>
On 21/04/2026 14:51, Jiahao zheng wrote:
> Hi Steven,
>
> I've been testing CCA patch series and noticed Realm VM cannot boot successfully when the host is forced to run in nVHE mode (e.g., via `kvm-arm.mode=nvhe`). The kvmtool debug information will be truncated in set_guest_bank_private_gpa.
>
> Currently, in `kvm_ioctl_vcpu_run()`, running a Realm VM (REC) bypasses the standard nVHE EL2 stub. `kvm_rec_enter()` directly executes the SMC instruction to transition to the RMM. Upon returning to the EL1 host, the code falls back to `kvm_vgic_sync_hwstate()`, where the VGIC save operation is explicitly skipped for nVHE. Since the EL2 stub was bypassed, `__vgic_v3_save_state()` is never executed, and `ICH_*_EL2` states are lost.
>
> To resolve this, I have a couple of thoughts:
> 1. If Host nVHE mode is not intended to be supported for Realms:
> Since RME implies ARMv9 which mandates VHE, running a Realm with an nVHE host might just be an unsupported edge case. If so, we should explicitly reject RME initialization or REC creation when `!is_kernel_in_hyp_mode()`. This would cleanly prevent the undefined behavior.
> 2. If Host nVHE mode is intended to be supported:
> Since RMM should remain agnostic to the Non-Secure VGIC states, the burden of saving these states falls strictly on KVM. However, the EL1 host cannot access `ICH_*_EL2`. Therefore, KVM needs to add specific logic for this scenario. We would likely need to route the REC exit through a dedicated nVHE EL2 stub to invoke `__vgic_v3_save_state()` before dropping back to EL1, rather than jumping straight back to `kvm_ioctl_vcpu_run()`.
>
> I might have missed some documentation or comments regarding nVHE restrictions for CCA. If this is an oversight, it would be great to see a check added in the next iteration of the series.
Thanks for the testing. Yes indeed this is an oversight. For now option
1 is what I'm going to go for. There's nothing stopping nVHE mode being
supported but as you note any platform with RME will have VHE so it's
not an immediate priority to support.
One interesting case of nVHE is of course pKVM and for that there needs
to be some significant work to ensure that the EL2 hypervisor
understands the RMM communication and prevent any confused-deputy style
attacks. E.g. the host must not be able to map a pVM's private memory
into a realm guest.
I don't have any immediate plans to work on nVHE - my focus is getting
the basic support merged. But I know there was some interest to ensure
that pKVM and CCA would be able to co-exist on a platform so I expect it
will come in some form or another.
Thanks,
Steve
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox