[PATCH] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect codegen.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/identification/midr
/sys/devices/system/cpu/cpu$ID/identification/revidr

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>
[ Return error for access to !present CPU registers ]
Signed-off-by: Suzuki K. Poulose <suzuki....@arm.com>
---
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---
arch/arm64/include/asm/cpu.h | 1 +
arch/arm64/kernel/cpuinfo.c | 69 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 13a6103..116a382 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -29,6 +29,7 @@ struct cpuinfo_arm64 {
u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+ u32 reg_revidr;

u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index c173d32..c2d0c42 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -212,6 +212,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);
@@ -264,3 +265,71 @@ void __init cpuinfo_store_boot_cpu(void)
boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);
}
+
+#define CPUINFO_ATTR_RO(_name) \
+ static ssize_t show_##_name (struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
+ if (!cpu_present(dev->id)) \
+ return -ENODEV; \
+ \
+ if (info->reg_midr) \
+ return sprintf(buf, "0x%016x\n", info->reg_##_name); \
+ else \
+ return 0; \
+ } \
+ static DEVICE_ATTR(_name, 0444, show_##_name, NULL)
+
+CPUINFO_ATTR_RO(midr);
+CPUINFO_ATTR_RO(revidr);
+
+static struct attribute *cpuregs_attrs[] = {
+ &dev_attr_midr.attr,
+ &dev_attr_revidr.attr,
+ NULL
+};
+
+static struct attribute_group cpuregs_attr_group = {
+ .attrs = cpuregs_attrs,
+ .name = "identification"
+};
+
+static int __init cpuinfo_regs_init(void)
+{
+ int cpu, finalcpu, ret;
+ struct device *dev;
+
+ for_each_present_cpu(cpu) {
+ dev = get_cpu_device(cpu);
+
+ if (!dev) {
+ ret = -ENODEV;
+ break;
+ }
+
+ ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
+ if (ret)
+ break;
+ }
+
+ if (!ret)
+ return 0;
+ /*
+ * We were unable to put down sysfs groups for all the CPUs, revert
+ * all the groups we have placed down s.t. none are visible.
+ * Otherwise we could give a misleading picture of what's present.
+ */
+ finalcpu = cpu;
+ for_each_present_cpu(cpu) {
+ if (cpu == finalcpu)
+ break;
+ dev = get_cpu_device(cpu);
+ if (dev)
+ sysfs_remove_group(&dev->kobj, &cpuregs_attr_group);
+ }
+
+ return ret;
+}
+
+device_initcall(cpuinfo_regs_init);
--
1.9.1

[Will Deacon]

Should this be 0x%08x, as these are 32-bit registers?

Can CPUs be removed from underneath us using unregister_cpu? If so, I
don't think we should assume that get_cpu_device will succeed in the
same places for both the loops.

Will

[Mark Rutland]

That's a difficult question to answer. Per C5.1.1, "Principles of the
System instruction class encoding" in ARM DDI 0487A.i, when a system
register is escribed as 32-bit, this only means that the upper 32 bits
are RES0, not that they will never be made use of.

CLIDR_EL1 (previously described as a 32-bit register) is now a 64-bit
register, so clearly extension is possible.

I imagine that otehr registers (e.g. MIDR_EL1) may also get extended in
future, and I think we need to treat "32-bit registers" as 64-bit, to
account for future allocation of the RES0 bits.

On that note, we should probably rework the sanity checks code to read
all registers as 64 bit.

Thanks,
Mark.

[Suzuki K Poulose]

On 10/06/16 18:02, Will Deacon wrote:
> On Fri, Jun 10, 2016 at 04:19:44PM +0100, Suzuki K Poulose wrote:
>> From: Steve Capper <steve....@linaro.org>
>>
>> It can be useful for JIT software to be aware of MIDR_EL1 and
>> REVIDR_EL1 to ascertain the presence of any core errata that could
>> affect codegen.
>>
>> This patch exposes these registers through sysfs:
>>
>> /sys/devices/system/cpu/cpu$ID/identification/midr
>> /sys/devices/system/cpu/cpu$ID/identification/revidr

>> +
>> +#define CPUINFO_ATTR_RO(_name) \
>> + static ssize_t show_##_name (struct device *dev, \
>> + struct device_attribute *attr, char *buf) \
>> + { \
>> + struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
>> + if (!cpu_present(dev->id)) \
>> + return -ENODEV; \
>> + \
>> + if (info->reg_midr) \
>> + return sprintf(buf, "0x%016x\n", info->reg_##_name); \
>
> Should this be 0x%08x, as these are 32-bit registers?

Yes. Will change it. As per Mark's comments, I can change them to 64bit in a separate
patch.

Yes. Good point. Though this is done at early boot, nobody prevents
an unregister_cpu(). The safer way would be to wrap the code in
cpu_hotplug_disable()...enable().

I will respin it.

> don't think we should assume that get_cpu_device will succeed in the
> same places for both the loops.

Thanks
Suzuki

[Will Deacon]

On Mon, Jun 13, 2016 at 01:02:36PM +0100, Suzuki K Poulose wrote:
> On 10/06/16 18:02, Will Deacon wrote:
> >On Fri, Jun 10, 2016 at 04:19:44PM +0100, Suzuki K Poulose wrote:
> >>From: Steve Capper <steve....@linaro.org>
> >>
> >>It can be useful for JIT software to be aware of MIDR_EL1 and
> >>REVIDR_EL1 to ascertain the presence of any core errata that could
> >>affect codegen.
> >>
> >>This patch exposes these registers through sysfs:
> >>
> >>/sys/devices/system/cpu/cpu$ID/identification/midr
> >>/sys/devices/system/cpu/cpu$ID/identification/revidr
>
>
> >>+
> >>+#define CPUINFO_ATTR_RO(_name) \
> >>+ static ssize_t show_##_name (struct device *dev, \
> >>+ struct device_attribute *attr, char *buf) \
> >>+ { \
> >>+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
> >>+ if (!cpu_present(dev->id)) \
> >>+ return -ENODEV; \
> >>+ \
> >>+ if (info->reg_midr) \
> >>+ return sprintf(buf, "0x%016x\n", info->reg_##_name); \
> >
> >Should this be 0x%08x, as these are 32-bit registers?
>
> Yes. Will change it. As per Mark's comments, I can change them to 64bit in
> a separate patch

No -- this is a sysfs ABI and I think we should be consistent from the
beginning. I'm fine with having them 64-bit, since Mark's comments make
sense, but a comment justifying that would be a good idea.

Will

[Suzuki K Poulose]

OK. Will add a comment then.

Thanks
Suzuki

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect codegen.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/identification/midr
/sys/devices/system/cpu/cpu$ID/identification/revidr

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>

[ Return error for access to !present CPU registers, ABI documentation
updates, Protection from hotplug ]
Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>
---
Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu

Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---

Documentation/ABI/testing/sysfs-devices-system-cpu | 13 ++++
arch/arm64/include/asm/cpu.h | 1 +
arch/arm64/kernel/cpuinfo.c | 80 ++++++++++++++++++++++
3 files changed, 94 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 1650133..8c4607d 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -340,3 +340,16 @@ Description: POWERNV CPUFreq driver's frequency throttle stats directory and
'policyX/throttle_stats' directory and all the attributes are same as
the /sys/devices/system/cpu/cpuX/cpufreq/throttle_stats directory and
attributes which give the frequency throttle information of the chip.
+
+What: /sys/devices/system/cpu/cpuX/identification/
+ /sys/devices/system/cpu/cpuX/identification/midr
+ /sys/devices/system/cpu/cpuX/identification/revidr
+Date: June 2016
+Contact: Linux ARM Kernel Mailing list <linux-ar...@lists.infradead.org>
+ Linux Kernel mailing list <linux-...@vger.kernel.org>
+Description: ARM64 CPU identification registers
+ 'identification' directory exposes the CPU ID registers for
+ identifying model and revision of the CPU.
+ - midr : This file gives contents of Main ID Register (MIDR_EL1).
+ - revidr : This file gives contents of the Revision ID register
+ (REVIDR_EL1).

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 13a6103..116a382 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -29,6 +29,7 @@ struct cpuinfo_arm64 {
u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+ u32 reg_revidr;

u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c

index c173d32..c30da19 100644

--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -212,6 +212,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +265,82 @@ void __init cpuinfo_store_boot_cpu(void)

boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);
}
+

+/*
+ * Both MIDR_EL1 and REVIDR_EL1 are 32bit registers. However, per C5.1.1,
+ * "Principles of the System instruction class encoding" in ARM DDI 0487A.i,
+ * when a system register is escribed as 32-bit, this only means that the
+ * upper 32 bits are RES0, not that they will never be made use of. To avoid
+ * changing the ABI for the future, the values are exported as 64bit values.
+ */

+#define CPUINFO_ATTR_RO(_name) \
+ static ssize_t show_##_name (struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
+ if (!cpu_present(dev->id)) \
+ return -ENODEV; \
+ \
+ if (info->reg_midr) \
+ return sprintf(buf, "0x%016x\n", info->reg_##_name); \

+ else \
+ return 0; \
+ } \
+ static DEVICE_ATTR(_name, 0444, show_##_name, NULL)
+
+CPUINFO_ATTR_RO(midr);
+CPUINFO_ATTR_RO(revidr);
+
+static struct attribute *cpuregs_attrs[] = {
+ &dev_attr_midr.attr,
+ &dev_attr_revidr.attr,
+ NULL
+};
+
+static struct attribute_group cpuregs_attr_group = {
+ .attrs = cpuregs_attrs,
+ .name = "identification"
+};

+
+static int __init cpuinfo_regs_init(void)
+{
+ int cpu, finalcpu, ret;
+ struct device *dev;
+

+ cpu_hotplug_disable();

+
+ for_each_present_cpu(cpu) {
+ dev = get_cpu_device(cpu);
+
+ if (!dev) {
+ ret = -ENODEV;
+ break;
+ }
+
+ ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
+ if (ret)
+ break;
+ }
+
+ if (!ret)

+ goto out;

+ /*
+ * We were unable to put down sysfs groups for all the CPUs, revert
+ * all the groups we have placed down s.t. none are visible.
+ * Otherwise we could give a misleading picture of what's present.
+ */
+ finalcpu = cpu;
+ for_each_present_cpu(cpu) {
+ if (cpu == finalcpu)
+ break;
+ dev = get_cpu_device(cpu);
+ if (dev)
+ sysfs_remove_group(&dev->kobj, &cpuregs_attr_group);
+ }

+
+out:
+ cpu_hotplug_enable();

[Russell King - ARM Linux]

On Mon, Jun 13, 2016 at 06:08:09PM +0100, Suzuki K Poulose wrote:
> From: Steve Capper <steve....@linaro.org>
>
> It can be useful for JIT software to be aware of MIDR_EL1 and
> REVIDR_EL1 to ascertain the presence of any core errata that could
> affect codegen.
>
> This patch exposes these registers through sysfs:
>
> /sys/devices/system/cpu/cpu$ID/identification/midr
> /sys/devices/system/cpu/cpu$ID/identification/revidr
>
> where $ID is the cpu number. For big.LITTLE systems, one can have a
> mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> to be enumerated.
>
> If the kernel does not have valid information to populate these entries
> with, an empty string is returned to userspace.

So there's no confusion, I've historically said no on 32-bit ARM to
exposing the MIDR to userspace, and my position on that for 32-bit
ARM has not changed.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[Mark Rutland]

On Mon, Jun 13, 2016 at 06:08:09PM +0100, Suzuki K Poulose wrote:

> +/*
> + * Both MIDR_EL1 and REVIDR_EL1 are 32bit registers. However, per C5.1.1,
> + * "Principles of the System instruction class encoding" in ARM DDI 0487A.i,
> + * when a system register is escribed as 32-bit, this only means that the
> + * upper 32 bits are RES0, not that they will never be made use of. To avoid
> + * changing the ABI for the future, the values are exported as 64bit values.
> + */

I see this is a direct copy+paste of my earlier message, typo and all.

I'd prefer something like the below:

/*
* The ARM ARM uses the phrase "32-bit register" to describe a register
* whose upper 32 bits are RES0 (per C5.1.1, ARM DDI 0487A.i), however
* no statement is made as to whether the upper 32 bits will or will not
* be made use of in future, and between ARM DDI 0487A.c and ARM DDI
* 0487A.d CLIDR_EL1 was expanded from 32-bit to 64-bit.
*
* Thus, while both MIDR_EL1 and REVIDR_EL1 are described as 32-bit
* registers, we expose them both as 64 bit values to cater for possible
* future expansion without an ABI break.
*/

Thanks,
Mark.

[Suzuki K Poulose]

Sorry about that, will fix it.

[Catalin Marinas]

On Mon, Jun 13, 2016 at 06:21:40PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 13, 2016 at 06:08:09PM +0100, Suzuki K Poulose wrote:
> > From: Steve Capper <steve....@linaro.org>
> >
> > It can be useful for JIT software to be aware of MIDR_EL1 and
> > REVIDR_EL1 to ascertain the presence of any core errata that could
> > affect codegen.
> >
> > This patch exposes these registers through sysfs:
> >
> > /sys/devices/system/cpu/cpu$ID/identification/midr
> > /sys/devices/system/cpu/cpu$ID/identification/revidr
> >
> > where $ID is the cpu number. For big.LITTLE systems, one can have a
> > mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> > to be enumerated.
> >
> > If the kernel does not have valid information to populate these entries
> > with, an empty string is returned to userspace.
>
> So there's no confusion, I've historically said no on 32-bit ARM to
> exposing the MIDR to userspace, and my position on that for 32-bit
> ARM has not changed.

It depends on what you mean by "exposing". You can already get this
information in two ways: /proc/cpuinfo parsing or KVM ioctls. Of course,
this information can be abused in ways the kernel people did not intend
but it's late to hide something already exposed.

What we don't have is REVIDR and that's something useful to a JIT in
deciding whether it needs to work around certain CPU errata (unless we
come up with another mechanism to inform user about hardware bugs).

--
Catalin

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect codegen.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/identification/midr
/sys/devices/system/cpu/cpu$ID/identification/revidr

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>
[ Return error for access to !present CPU registers, ABI documentation
updates, Protection from hotplug ]

Signed-off-by: Suzuki K. Poulose <suzuki....@arm.com>
---
Changes since V4:
- Update the comment about exporting 64bit value

Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 13 ++++
arch/arm64/include/asm/cpu.h | 1 +

arch/arm64/kernel/cpuinfo.c | 84 ++++++++++++++++++++++
3 files changed, 98 insertions(+)

index c173d32..dd3168c 100644

--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -212,6 +212,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +265,86 @@ void __init cpuinfo_store_boot_cpu(void)

boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);
}
+
+/*

+ * The ARM ARM uses the phrase "32-bit register" to describe a register
+ * whose upper 32 bits are RES0 (per C5.1.1, ARM DDI 0487A.i), however
+ * no statement is made as to whether the upper 32 bits will or will not
+ * be made use of in future, and between ARM DDI 0487A.c and ARM DDI
+ * 0487A.d CLIDR_EL1 was expanded from 32-bit to 64-bit.
+ *
+ * Thus, while both MIDR_EL1 and REVIDR_EL1 are described as 32-bit
+ * registers, we expose them both as 64 bit values to cater for possible
+ * future expansion without an ABI break.

[Will Deacon]

Given that you're built the sysfs entries out of the present mask, don't
you need a hotplug notifier to keep things up to date?

Will

[Suzuki K Poulose]

We already check if the CPU is present at runtime before we give out the data.

+#define CPUINFO_ATTR_RO(_name) \
+ static ssize_t show_##_name (struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
+ if (!cpu_present(dev->id)) \
+ return -ENODEV; \
+ \

Did you mean to check online/offline ?

Cheers
Suzuki

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could

affect code generation.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/identification/midr
/sys/devices/system/cpu/cpu$ID/identification/revidr

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>

[ ABI documentation updates, hotplug notifiers ]
Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>
---
Changes since V5:
- Add hotplug notifier to {add/remove} the attributes when the CPU is brought
{online/offline}.
- Replace cpu_hotplug_{disable,enable} => cpu_notifier_register_{begin/done}
- Remove redundant check for cpu present, as the sysfs infrastructure does
check already returning -ENODEV, if the CPU goes offline between open() and
read().
Changes since V4:
- Update comment as suggested by Mark Rutland

Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 13 +++

arch/arm64/include/asm/cpu.h | 1 +
arch/arm64/kernel/cpuinfo.c | 106 +++++++++++++++++++++
3 files changed, 120 insertions(+)

index c173d32..44ec263 100644

--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -212,6 +212,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +265,108 @@ void __init cpuinfo_store_boot_cpu(void)

boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);
}
+
+/*
+ * The ARM ARM uses the phrase "32-bit register" to describe a register
+ * whose upper 32 bits are RES0 (per C5.1.1, ARM DDI 0487A.i), however
+ * no statement is made as to whether the upper 32 bits will or will not
+ * be made use of in future, and between ARM DDI 0487A.c and ARM DDI
+ * 0487A.d CLIDR_EL1 was expanded from 32-bit to 64-bit.
+ *
+ * Thus, while both MIDR_EL1 and REVIDR_EL1 are described as 32-bit
+ * registers, we expose them both as 64 bit values to cater for possible
+ * future expansion without an ABI break.
+ */

+#define CPUINFO_ATTR_RO(_name) \
+ static ssize_t show_##_name(struct device *dev, \

+ struct device_attribute *attr, char *buf) \
+ { \
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \

+ \
+ if (info->reg_midr) \
+ return sprintf(buf, "0x%016x\n", info->reg_##_name); \
+ else \
+ return 0; \
+ } \
+ static DEVICE_ATTR(_name, 0444, show_##_name, NULL)
+
+CPUINFO_ATTR_RO(midr);
+CPUINFO_ATTR_RO(revidr);
+
+static struct attribute *cpuregs_attrs[] = {
+ &dev_attr_midr.attr,
+ &dev_attr_revidr.attr,
+ NULL
+};
+
+static struct attribute_group cpuregs_attr_group = {
+ .attrs = cpuregs_attrs,
+ .name = "identification"
+};
+

+static int cpuid_callback(struct notifier_block *nb,
+ unsigned long action, void *hcpu)
+{
+ int rc = 0;
+ unsigned long cpu = (unsigned long)hcpu;
+ struct device *dev = get_cpu_device(cpu);
+
+ if (dev) {
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ rc = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
+ break;
+ case CPU_DEAD:
+ sysfs_remove_group(&dev->kobj, &cpuregs_attr_group);
+ break;
+ }
+ } else {
+ rc = -ENODEV;
+ }
+
+ return notifier_from_errno(rc);
+}
+

+static int __init cpuinfo_regs_init(void)
+{
+ int cpu, finalcpu, ret;
+ struct device *dev;
+

+ cpu_notifier_register_begin();
+
+ for_each_online_cpu(cpu) {

+ dev = get_cpu_device(cpu);
+

+ if (dev)

+ ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);

+ else
+ ret = -ENODEV;

+ if (ret)
+ break;
+ }
+
+ if (!ret) {

+ __hotcpu_notifier(cpuid_callback, 0);
+ goto out;
+ }
+

+ /*
+ * We were unable to put down sysfs groups for all the CPUs, revert
+ * all the groups we have placed down s.t. none are visible.
+ * Otherwise we could give a misleading picture of what's present.
+ */
+ finalcpu = cpu;

+ for_each_online_cpu(cpu) {

+ if (cpu == finalcpu)
+ break;
+ dev = get_cpu_device(cpu);
+ if (dev)
+ sysfs_remove_group(&dev->kobj, &cpuregs_attr_group);
+ }
+
+out:

+ cpu_notifier_register_done();

+ return ret;
+}
+
+device_initcall(cpuinfo_regs_init);

--
1.9.1

[Will Deacon]

On Tue, Jun 21, 2016 at 12:12:36PM +0100, Suzuki K Poulose wrote:
> From: Steve Capper <steve....@linaro.org>
>
> It can be useful for JIT software to be aware of MIDR_EL1 and
> REVIDR_EL1 to ascertain the presence of any core errata that could
> affect code generation.
>
> This patch exposes these registers through sysfs:
>
> /sys/devices/system/cpu/cpu$ID/identification/midr
> /sys/devices/system/cpu/cpu$ID/identification/revidr
>
> where $ID is the cpu number. For big.LITTLE systems, one can have a
> mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> to be enumerated.
>
> If the kernel does not have valid information to populate these entries
> with, an empty string is returned to userspace.
>
> Cc: Catalin Marinas <catalin...@arm.com>
> Cc: Will Deacon <will....@arm.com>
> Cc: Mark Rutland <mark.r...@arm.com>
> Signed-off-by: Steve Capper <steve....@linaro.org>
> [ ABI documentation updates, hotplug notifiers ]
> Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>

Looks good to me, but one minor comment below.

> +static struct attribute_group cpuregs_attr_group = {
> + .attrs = cpuregs_attrs,
> + .name = "identification"
> +};

This makes sense because MIDR/REVIDR belong to the "Identification"
functional group of registers, however I wonder if we should put this
under a directory called "regs" or similar, so that we don't have a
confusing top-level directory where "identification" lives alongside
things like "hotplug" and "cpuidle".

Either way:

Reviewed-by: Will Deacon <will....@arm.com>

Will

[Catalin Marinas]

IMO, there are too many subdirectories and I don't think we would expose
any other registers than the ID ones.

--
Catalin

[Catalin Marinas]

On Tue, Jun 21, 2016 at 12:12:36PM +0100, Suzuki K. Poulose wrote:
> +#define CPUINFO_ATTR_RO(_name) \
> + static ssize_t show_##_name(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> + { \
> + struct cpuinfo_arm64 *info = &per_cpu(cpu_data, dev->id); \
> + \
> + if (info->reg_midr) \
> + return sprintf(buf, "0x%016x\n", info->reg_##_name); \
> + else \
> + return 0; \
> + } \
> + static DEVICE_ATTR(_name, 0444, show_##_name, NULL)
> +
> +CPUINFO_ATTR_RO(midr);
> +CPUINFO_ATTR_RO(revidr);

Since exposing these values is aimed at JIT code (and not human
readable), wouldn't it make more sense to present the binary value
instead of the ascii transformation?

--
Catalin

[Suzuki K Poulose]

I am fine with either.

Edward,

Do you have any preference ?

Suzuki

[Mark Rutland]

Per Documentation/filesystems/sysfs.txt, attributes should be ASCII text
files, with one value per file. I think they should stay as they are.

Thanks,
Mark.

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect code generation.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
/sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>

[ ABI documentation updates, hotplug notifiers, kobject changes ]

Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>

---
Changes since V6:
- Introduce regs/identification hierarchy(using kobject for the added level)
- Use the register names as in ARM ARM (i.e, midr => midr_el1)

Changes since V5:
- Add hotplug notifier to {add/remove} the attributes when the CPU is brought
{online/offline}.
- Replace cpu_hotplug_{disable,enable} => cpu_notifier_register_{begin/done}
- Remove redundant check for cpu present, as the sysfs infrastructure does
check already returning -ENODEV, if the CPU goes offline between open() and
read().
Changes since V4:
- Update comment as suggested by Mark Rutland
Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---

Documentation/ABI/testing/sysfs-devices-system-cpu | 14 +++
arch/arm64/include/asm/cpu.h | 2 +
arch/arm64/kernel/cpuinfo.c | 137 +++++++++++++++++++++
3 files changed, 153 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 1650133..31dee60 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -340,3 +340,17 @@ Description: POWERNV CPUFreq driver's frequency throttle stats directory and

'policyX/throttle_stats' directory and all the attributes are same as
the /sys/devices/system/cpu/cpuX/cpufreq/throttle_stats directory and
attributes which give the frequency throttle information of the chip.
+

+What: /sys/devices/system/cpu/cpuX/regs/
+ /sys/devices/system/cpu/cpuX/regs/identification/
+ /sys/devices/system/cpu/cpuX/regs/identification/midr_el1
+ /sys/devices/system/cpu/cpuX/regs/identification/revidr_el1

+Date: June 2016
+Contact: Linux ARM Kernel Mailing list <linux-ar...@lists.infradead.org>
+ Linux Kernel mailing list <linux-...@vger.kernel.org>
+Description: ARM64 CPU identification registers
+ 'identification' directory exposes the CPU ID registers for
+ identifying model and revision of the CPU.

+ - midr_el1 : This file gives contents of Main ID Register (MIDR_EL1).
+ - revidr_el1 : This file gives contents of the Revision ID register

+ (REVIDR_EL1).
diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h

index 13a6103..889226b 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -25,10 +25,12 @@
*/
struct cpuinfo_arm64 {
struct cpu cpu;
+ struct kobject kobj;
u32 reg_ctr;

u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+ u32 reg_revidr;

u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c

index c173d32..59d3076 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -183,6 +183,140 @@ const struct seq_operations cpuinfo_op = {
.show = c_show
};

+
+static struct kobj_type cpuregs_kobj_type = {
+ .sysfs_ops = &kobj_sysfs_ops,
+};

+
+/*
+ * The ARM ARM uses the phrase "32-bit register" to describe a register
+ * whose upper 32 bits are RES0 (per C5.1.1, ARM DDI 0487A.i), however
+ * no statement is made as to whether the upper 32 bits will or will not
+ * be made use of in future, and between ARM DDI 0487A.c and ARM DDI
+ * 0487A.d CLIDR_EL1 was expanded from 32-bit to 64-bit.
+ *
+ * Thus, while both MIDR_EL1 and REVIDR_EL1 are described as 32-bit
+ * registers, we expose them both as 64 bit values to cater for possible
+ * future expansion without an ABI break.
+ */

+#define kobj_to_cpuinfo(kobj) container_of(kobj, struct cpuinfo_arm64, kobj)
+#define CPUREGS_ATTR_RO(_name, _field) \
+ static ssize_t _name##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, char *buf) \
+ { \
+ struct cpuinfo_arm64 *info = kobj_to_cpuinfo(kobj); \

+ \
+ if (info->reg_midr) \

+ return sprintf(buf, "0x%016x\n", info->reg_##_field); \

+ else \
+ return 0; \
+ } \

+ static struct kobj_attribute cpuregs_attr_##_name = __ATTR_RO(_name)
+
+CPUREGS_ATTR_RO(midr_el1, midr);
+CPUREGS_ATTR_RO(revidr_el1, revidr);
+
+static struct attribute *cpuregs_id_attrs[] = {
+ &cpuregs_attr_midr_el1.attr,
+ &cpuregs_attr_revidr_el1.attr,
+ NULL
+};
+

+static struct attribute_group cpuregs_attr_group = {

+ .attrs = cpuregs_id_attrs,

+ .name = "identification"
+};

+
+static int cpuid_add_regs(int cpu)
+{
+ int rc;
+ struct device *dev;
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+

+ dev = get_cpu_device(cpu);
+ if (dev) {

+ rc = kobject_add(&info->kobj, &dev->kobj, "regs");
+ if (!rc)
+ rc = sysfs_create_group(&info->kobj, &cpuregs_attr_group);
+ } else {
+ return -ENODEV;
+ }
+
+ return rc;
+}
+
+static int cpuid_remove_regs(int cpu)

+{
+ int rc = 0;

+ struct device *dev;
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+

+ dev = get_cpu_device(cpu);
+ if (dev) {

+ sysfs_remove_group(&info->kobj, &cpuregs_attr_group);
+ kobject_del(&info->kobj);

+ } else {
+ rc = -ENODEV;
+ }
+

+ return rc;

+}
+
+static int cpuid_callback(struct notifier_block *nb,
+ unsigned long action, void *hcpu)
+{
+ int rc = 0;
+ unsigned long cpu = (unsigned long)hcpu;
+

+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:

+ rc = cpuid_add_regs(cpu);

+ break;
+ case CPU_DEAD:

+ rc = cpuid_remove_regs(cpu);
+ break;
+ }
+

+ return notifier_from_errno(rc);
+}
+
+static int __init cpuinfo_regs_init(void)
+{
+ int cpu, finalcpu, ret;
+

+ cpu_notifier_register_begin();
+
+ for_each_possible_cpu(cpu) {
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+
+ kobject_init(&info->kobj, &cpuregs_kobj_type);
+ if (cpu_online(cpu)) {
+ ret = cpuid_add_regs(cpu);

+ if (ret)
+ break;
+ }
+ }
+

+
+ /*
+ * We were unable to put down sysfs groups for all the CPUs, revert
+ * all the groups we have placed down s.t. none are visible.
+ * Otherwise we could give a misleading picture of what's present.
+ */

+ if (ret) {

+ finalcpu = cpu;
+ for_each_online_cpu(cpu) {
+ if (cpu == finalcpu)
+ break;

+ cpuid_remove_regs(cpu);
+ }
+ } else {
+ __hotcpu_notifier(cpuid_callback, 0);
+ }
+

+ cpu_notifier_register_done();
+ return ret;
+}

static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
{
unsigned int cpu = smp_processor_id();
@@ -212,6 +346,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)

info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +399,5 @@ void __init cpuinfo_store_boot_cpu(void)

boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);
}
+

+device_initcall(cpuinfo_regs_init);
--
2.7.4

[Catalin Marinas]

s/ARM64/AArch64/.

> + 'identification' directory exposes the CPU ID registers for
> + identifying model and revision of the CPU.
> + - midr_el1 : This file gives contents of Main ID Register (MIDR_EL1).
> + - revidr_el1 : This file gives contents of the Revision ID register
> + (REVIDR_EL1).

[...]

What is the failure scenario here and do we expect it to fail in the
middle of a for_each_possible_cpu()? You don't do any such clean-up if
this fails during CPU_ONLINE, so I think we should either ignore this
altogether or have a different clean-up function that handles the
CPU_ONLINE cases. I prefer the former.

--
Catalin

[Suzuki K Poulose]

On 01/07/16 14:01, Catalin Marinas wrote:
> On Thu, Jun 30, 2016 at 06:36:44PM +0100, Suzuki K. Poulose wrote:
>> From: Steve Capper <steve....@linaro.org>
>>
>> It can be useful for JIT software to be aware of MIDR_EL1 and
>> REVIDR_EL1 to ascertain the presence of any core errata that could
>> affect code generation.
>>
>> This patch exposes these registers through sysfs:
>>
>> /sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
>> /sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1

>> +What: /sys/devices/system/cpu/cpuX/regs/
>> + /sys/devices/system/cpu/cpuX/regs/identification/
>> + /sys/devices/system/cpu/cpuX/regs/identification/midr_el1
>> + /sys/devices/system/cpu/cpuX/regs/identification/revidr_el1
>> +Date: June 2016
>> +Contact: Linux ARM Kernel Mailing list <linux-ar...@lists.infradead.org>
>> + Linux Kernel mailing list <linux-...@vger.kernel.org>
>> +Description: ARM64 CPU identification registers
>
> s/ARM64/AArch64/.

Ok.

>
>> + 'identification' directory exposes the CPU ID registers for
>> + identifying model and revision of the CPU.
>> + - midr_el1 : This file gives contents of Main ID Register (MIDR_EL1).
>> + - revidr_el1 : This file gives contents of the Revision ID register
>> + (REVIDR_EL1).
> [...]

I will remove those superfluous explanation.

This was done to decide whether we should report the registers at all at
init time. i.e, if we failed to setup the sysfs attributes at init time
for at least one CPU, we don't enable these attributes (and we skip the
notifiers.) for any.

But you're right. I think we could remove this check altogether.
It was initially put in place when we didn't have the protection against
hotplug.

> altogether or have a different clean-up function that handles the
> CPU_ONLINE cases. I prefer the former.

Thanks
Suzuki

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect code generation.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
/sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>
[ ABI documentation updates, hotplug notifiers, kobject changes ]
Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>
---

Changes since V7:
- Remove unnecessary clean up cpuinfo_regs_init

Changes since V6:
- Introduce regs/identification hierarchy(using kobject for the added level)
- Use the register names as in ARM ARM (i.e, midr => midr_el1)
Changes since V5:
- Add hotplug notifier to {add/remove} the attributes when the CPU is brought
{online/offline}.
- Replace cpu_hotplug_{disable,enable} => cpu_notifier_register_{begin/done}
- Remove redundant check for cpu present, as the sysfs infrastructure does
check already returning -ENODEV, if the CPU goes offline between open() and
read().
Changes since V4:
- Update comment as suggested by Mark Rutland
Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---

Documentation/ABI/testing/sysfs-devices-system-cpu | 11 ++
arch/arm64/include/asm/cpu.h | 2 +
arch/arm64/kernel/cpuinfo.c | 118 +++++++++++++++++++++
3 files changed, 131 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 1650133..5062dc1 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -340,3 +340,14 @@ Description: POWERNV CPUFreq driver's frequency throttle stats directory and

'policyX/throttle_stats' directory and all the attributes are same as
the /sys/devices/system/cpu/cpuX/cpufreq/throttle_stats directory and
attributes which give the frequency throttle information of the chip.
+

+What: /sys/devices/system/cpu/cpuX/regs/
+ /sys/devices/system/cpu/cpuX/regs/identification/
+ /sys/devices/system/cpu/cpuX/regs/identification/midr_el1
+ /sys/devices/system/cpu/cpuX/regs/identification/revidr_el1
+Date: June 2016
+Contact: Linux ARM Kernel Mailing list <linux-ar...@lists.infradead.org>
+ Linux Kernel mailing list <linux-...@vger.kernel.org>

+Description: AArch64 CPU identification registers

+ 'identification' directory exposes the CPU ID registers for
+ identifying model and revision of the CPU.

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 13a6103..889226b 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -25,10 +25,12 @@
*/
struct cpuinfo_arm64 {
struct cpu cpu;
+ struct kobject kobj;
u32 reg_ctr;
u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+ u32 reg_revidr;

u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c

index c173d32..135c329 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -183,6 +183,121 @@ const struct seq_operations cpuinfo_op = {

+ int cpu;

+
+ cpu_notifier_register_begin();
+
+ for_each_possible_cpu(cpu) {
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+
+ kobject_init(&info->kobj, &cpuregs_kobj_type);
+ if (cpu_online(cpu))

+ cpuid_add_regs(cpu);
+ }
+ __hotcpu_notifier(cpuid_callback, 0);
+
+ cpu_notifier_register_done();
+ return 0;

+}
static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
{
unsigned int cpu = smp_processor_id();

@@ -212,6 +327,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)

info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +380,5 @@ void __init cpuinfo_store_boot_cpu(void)

[Will Deacon]

On Mon, Jul 04, 2016 at 10:20:20AM +0100, Suzuki K Poulose wrote:
> From: Steve Capper <steve....@linaro.org>
>
> It can be useful for JIT software to be aware of MIDR_EL1 and
> REVIDR_EL1 to ascertain the presence of any core errata that could
> affect code generation.
>
> This patch exposes these registers through sysfs:
>
> /sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
> /sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1
>
> where $ID is the cpu number. For big.LITTLE systems, one can have a
> mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> to be enumerated.
>
> If the kernel does not have valid information to populate these entries
> with, an empty string is returned to userspace.

[...]

> +static int cpuid_add_regs(int cpu)
> +{
> + int rc;
> + struct device *dev;
> + struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
> +
> + dev = get_cpu_device(cpu);
> + if (dev) {
> + rc = kobject_add(&info->kobj, &dev->kobj, "regs");
> + if (!rc)
> + rc = sysfs_create_group(&info->kobj, &cpuregs_attr_group);

If this call fails...

> + } else {
> + return -ENODEV;
> + }
> +
> + return rc;
> +}
> +
> +static int cpuid_remove_regs(int cpu)
> +{
> + int rc = 0;
> + struct device *dev;
> + struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
> +
> + dev = get_cpu_device(cpu);
> + if (dev) {
> + sysfs_remove_group(&info->kobj, &cpuregs_attr_group);

.. then we still call sysfs_remove_group on the CPU_DEAD path. I think
that just results in a WARN, but it would be good to double-check this
(perhaps by forcing the failure path).

Will

[Suzuki K Poulose]

From: Steve Capper <steve....@linaro.org>

It can be useful for JIT software to be aware of MIDR_EL1 and
REVIDR_EL1 to ascertain the presence of any core errata that could
affect code generation.

This patch exposes these registers through sysfs:

/sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
/sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1

where $ID is the cpu number. For big.LITTLE systems, one can have a
mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
to be enumerated.

If the kernel does not have valid information to populate these entries
with, an empty string is returned to userspace.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Mark Rutland <mark.r...@arm.com>
Signed-off-by: Steve Capper <steve....@linaro.org>
[ ABI documentation updates, hotplug notifiers, kobject changes ]
Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>
---

Changes since V8:
- Handle sysfs group addition/removal gracefully.

Changes since V7:
- Remove unnecessary clean up cpuinfo_regs_init
Changes since V6:
- Introduce regs/identification hierarchy(using kobject for the added level)
- Use the register names as in ARM ARM (i.e, midr => midr_el1)
Changes since V5:
- Add hotplug notifier to {add/remove} the attributes when the CPU is brought
{online/offline}.
- Replace cpu_hotplug_{disable,enable} => cpu_notifier_register_{begin/done}
- Remove redundant check for cpu present, as the sysfs infrastructure does
check already returning -ENODEV, if the CPU goes offline between open() and
read().
Changes since V4:
- Update comment as suggested by Mark Rutland
Changes since V3:
- Disable cpu hotplug while we initialise
- Added a comment to explain why expose 64bit value
- Update Document/ABI/testing/sysfs-devices-system-cpu
Changes since V2:
- Fix errno for failures (Spotted-by: Russell King)
- Roll back, if we encounter a missing cpu device
- Return error for access to registers of CPUs not present.
---

Documentation/ABI/testing/sysfs-devices-system-cpu | 10 ++
arch/arm64/include/asm/cpu.h | 2 +
arch/arm64/kernel/cpuinfo.c | 120 +++++++++++++++++++++
3 files changed, 132 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 1650133..4987417 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -340,3 +340,13 @@ Description: POWERNV CPUFreq driver's frequency throttle stats directory and

'policyX/throttle_stats' directory and all the attributes are same as
the /sys/devices/system/cpu/cpuX/cpufreq/throttle_stats directory and
attributes which give the frequency throttle information of the chip.
+
+What: /sys/devices/system/cpu/cpuX/regs/
+ /sys/devices/system/cpu/cpuX/regs/identification/
+ /sys/devices/system/cpu/cpuX/regs/identification/midr_el1
+ /sys/devices/system/cpu/cpuX/regs/identification/revidr_el1
+Date: June 2016
+Contact: Linux ARM Kernel Mailing list <linux-ar...@lists.infradead.org>

+Description: AArch64 CPU registers

+ 'identification' directory exposes the CPU ID registers for
+ identifying model and revision of the CPU.
diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 13a6103..889226b 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -25,10 +25,12 @@
*/
struct cpuinfo_arm64 {
struct cpu cpu;
+ struct kobject kobj;
u32 reg_ctr;
u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+ u32 reg_revidr;

u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c

index c173d32..ed1b84f 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -183,6 +183,123 @@ const struct seq_operations cpuinfo_op = {

+static int cpuid_add_regs(int cpu)
+{
+ int rc;
+ struct device *dev;
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+
+ dev = get_cpu_device(cpu);

+ if (!dev) {
+ rc = -ENODEV;
+ goto out;
+ }

+ rc = kobject_add(&info->kobj, &dev->kobj, "regs");

+ if (rc)
+ goto out;

+ rc = sysfs_create_group(&info->kobj, &cpuregs_attr_group);

+ if (rc)
+ kobject_del(&info->kobj);
+out:

+ return rc;
+}
+
+static int cpuid_remove_regs(int cpu)
+{

+ struct device *dev;
+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+
+ dev = get_cpu_device(cpu);

+ if (!dev)
+ return -ENODEV;
+ if (info->kobj.parent) {

+ sysfs_remove_group(&info->kobj, &cpuregs_attr_group);
+ kobject_del(&info->kobj);
+ }

+

+ return 0;
+}
+

+static int cpuid_callback(struct notifier_block *nb,
+ unsigned long action, void *hcpu)

+{
+ int rc = 0;

+ unsigned long cpu = (unsigned long)hcpu;
+
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ rc = cpuid_add_regs(cpu);
+ break;
+ case CPU_DEAD:
+ rc = cpuid_remove_regs(cpu);
+ break;
+ }
+
+ return notifier_from_errno(rc);
+}
+
+static int __init cpuinfo_regs_init(void)
+{
+ int cpu;
+
+ cpu_notifier_register_begin();
+
+ for_each_possible_cpu(cpu) {

+ struct cpuinfo_arm64 *info = &per_cpu(cpu_data, cpu);
+

+ kobject_init(&info->kobj, &cpuregs_kobj_type);
+ if (cpu_online(cpu))
+ cpuid_add_regs(cpu);
+ }
+ __hotcpu_notifier(cpuid_callback, 0);
+
+ cpu_notifier_register_done();
+ return 0;
+}
static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
{
unsigned int cpu = smp_processor_id();

@@ -212,6 +329,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)

info->reg_ctr = read_cpuid_cachetype();
info->reg_dczid = read_cpuid(DCZID_EL0);
info->reg_midr = read_cpuid_id();
+ info->reg_revidr = read_cpuid(REVIDR_EL1);

info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);

@@ -264,3 +382,5 @@ void __init cpuinfo_store_boot_cpu(void)

[Will Deacon]

On Fri, Jul 08, 2016 at 04:01:13PM +0100, Suzuki K Poulose wrote:
> From: Steve Capper <steve....@linaro.org>
>
> It can be useful for JIT software to be aware of MIDR_EL1 and
> REVIDR_EL1 to ascertain the presence of any core errata that could
> affect code generation.
>
> This patch exposes these registers through sysfs:
>
> /sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
> /sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1
>
> where $ID is the cpu number. For big.LITTLE systems, one can have a
> mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> to be enumerated.
>
> If the kernel does not have valid information to populate these entries
> with, an empty string is returned to userspace.
>
> Cc: Catalin Marinas <catalin...@arm.com>
> Cc: Will Deacon <will....@arm.com>
> Cc: Mark Rutland <mark.r...@arm.com>
> Signed-off-by: Steve Capper <steve....@linaro.org>
> [ ABI documentation updates, hotplug notifiers, kobject changes ]
> Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>

Looks good to me, thanks for sticking with it:

[Catalin Marinas]

On Fri, Jul 08, 2016 at 04:01:13PM +0100, Suzuki K. Poulose wrote:
> From: Steve Capper <steve....@linaro.org>
>
> It can be useful for JIT software to be aware of MIDR_EL1 and
> REVIDR_EL1 to ascertain the presence of any core errata that could
> affect code generation.
>
> This patch exposes these registers through sysfs:
>
> /sys/devices/system/cpu/cpu$ID/regs/identification/midr_el1
> /sys/devices/system/cpu/cpu$ID/regs/identification/revidr_el1
>
> where $ID is the cpu number. For big.LITTLE systems, one can have a
> mixture of cores (e.g. Cortex A53 and Cortex A57), thus all CPUs need
> to be enumerated.
>
> If the kernel does not have valid information to populate these entries
> with, an empty string is returned to userspace.
>
> Cc: Catalin Marinas <catalin...@arm.com>
> Cc: Will Deacon <will....@arm.com>
> Cc: Mark Rutland <mark.r...@arm.com>
> Signed-off-by: Steve Capper <steve....@linaro.org>
> [ ABI documentation updates, hotplug notifiers, kobject changes ]
> Signed-off-by: Suzuki K Poulose <suzuki....@arm.com>

Applied. Thanks.

--
Catalin