On Wed, Dec 02, 2020 at 08:18:03AM +0000, Marc Zyngier wrote:
> On 2020-12-01 16:57, Will Deacon wrote:
> > On Fri, Nov 27, 2020 at 06:16:35PM +0000, Marc Zyngier wrote:
> > > On 2020-11-27 17:24, Quentin Perret wrote:
> > > > On Friday 27 Nov 2020 at 17:14:11 (+0000), Marc Zyngier wrote:
> > >
> > > [...]
> > >
> > > > > Yeah, the sanitized read feels better, if only because that is
> > > > > what we are going to read in all the valid cases, unfortunately.
> > > > > read_sanitised_ftr_reg() is sadly not designed to be called on
> > > > > a fast path, meaning that 32bit guests will do a bsearch() on
> > > > > the ID-regs every time they exit...
> > > > >
> > > > > I guess we will have to evaluate how much we loose with this.
> > > >
> > > > Could we use the trick we have for arm64_ftr_reg_ctrel0 to speed this
> > > > up?
> > >
> > > Maybe. I want to first verify whether this has any measurable impact.
> > > Another possibility would be to cache the last
> > > read_sanitised_ftr_reg()
> > > access, just to see if that helps. There shouldn't be that many code
> > > paths hammering it.
> >
> > We don't have huge numbers of ID registers, so the bsearch shouldn't be
> > too expensive. However, I'd like to remind myself why we can't index
> > into
> > the feature register array directly as we _should_ know all of this
> > stuff
> > at compile time, right?
>
> Simply because it's not indexed by ID reg. It's just an ordered collection,
> similar to the for sys_reg emulation in KVM. You can compute the index
> ahead of time, but just not at compile time. At least not with the
> way the arm64_ftr_regs array is built.
FWIW, if your testing shows that the bsearch() is costing us, I've hacked
up an interface to access the ID registers directly (see below) which I
can include with this series.
Will
--->8
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index da250e4741bd..23766104d756 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -599,7 +599,49 @@ static inline bool id_aa64pfr0_sve(u64 pfr0)
void __init setup_cpu_features(void);
void check_local_cpu_capabilities(void);
+#define ARM64_FTR_REG2IDX(id) id ## _IDX
+enum arm64_ftr_reg_idx {
+ ARM64_FTR_REG2IDX(SYS_ID_PFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_PFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_DFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR2_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR3_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR2_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR3_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR4_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR5_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR4_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_ISAR6_EL1),
+ ARM64_FTR_REG2IDX(SYS_MVFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_MVFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_MVFR2_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_PFR2_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_DFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_MMFR5_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64PFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64PFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64ZFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64DFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64DFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64ISAR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64ISAR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64MMFR0_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64MMFR1_EL1),
+ ARM64_FTR_REG2IDX(SYS_ID_AA64MMFR2_EL1),
+ ARM64_FTR_REG2IDX(SYS_ZCR_EL1),
+ ARM64_FTR_REG2IDX(SYS_CTR_EL0),
+ ARM64_FTR_REG2IDX(SYS_DCZID_EL0),
+ ARM64_FTR_REG2IDX(SYS_CNTFRQ_EL0),
+
+ ARM64_FTR_REG_IDX_MAX,
+};
+
u64 read_sanitised_ftr_reg(u32 id);
+u64 read_sanitised_ftr_reg_by_idx(enum arm64_ftr_reg_idx idx);
static inline bool cpu_supports_mixed_endian_el0(void)
{
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6f36c4f62f69..05223352db5d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -546,16 +546,18 @@ static const struct arm64_ftr_bits ftr_raz[] = {
ARM64_FTR_END,
};
-#define ARM64_FTR_REG(id, table) { \
- .sys_id = id, \
- .reg = &(struct arm64_ftr_reg){ \
- .name = #id, \
- .ftr_bits = &((table)[0]), \
- }}
+#define ARM64_FTR_REG(id, table) \
+ [id ## _IDX] = { \
+ .sys_id = id, \
+ .reg = &(struct arm64_ftr_reg) { \
+ .name = #id, \
+ .ftr_bits = &((table)[0]), \
+ } \
+ }
static const struct __ftr_reg_entry {
u32 sys_id;
- struct arm64_ftr_reg *reg;
+ struct arm64_ftr_reg *reg;
} arm64_ftr_regs[] = {
/* Op1 = 0, CRn = 0, CRm = 1 */
@@ -607,7 +609,7 @@ static const struct __ftr_reg_entry {
ARM64_FTR_REG(SYS_ZCR_EL1, ftr_zcr),
/* Op1 = 3, CRn = 0, CRm = 0 */
- { SYS_CTR_EL0, &arm64_ftr_reg_ctrel0 },
+ [ARM64_FTR_REG2IDX(SYS_CTR_EL0)] = { SYS_CTR_EL0, &arm64_ftr_reg_ctrel0 },
ARM64_FTR_REG(SYS_DCZID_EL0, ftr_dczid),
/* Op1 = 3, CRn = 14, CRm = 0 */
@@ -1116,6 +1118,18 @@ u64 read_sanitised_ftr_reg(u32 id)
}
EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);
+u64 read_sanitised_ftr_reg_by_idx(enum arm64_ftr_reg_idx idx)
+{
+ struct arm64_ftr_reg *regp;
+
+ if (WARN_ON((unsigned)idx >= ARM64_FTR_REG_IDX_MAX))
+ return 0;
+
+ regp = arm64_ftr_regs[idx].reg;
+ return regp->sys_val;
+}
+EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg_by_idx);
+
#define read_sysreg_case(r) \
case r: return read_sysreg_s(r)
--
2.29.2.576.ga3fc446d84-goog
