Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1239751 > unrolled thread

[PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs "Suzuki K. Poulose" <suzuki.poulose@arm.com> - 2015-10-05 19:10 +0200
    Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1  to sysfs Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-06 11:20 +0200
      Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1  to sysfs Steve Capper <steve.capper@linaro.org> - 2015-10-06 12:20 +0200
        Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1  to sysfs Mark Rutland <mark.rutland@arm.com> - 2015-10-06 12:30 +0200
          Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1  to sysfs Steve Capper <steve.capper@linaro.org> - 2015-10-06 12:30 +0200
        Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1  to sysfs Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-06 21:20 +0200

#1239751 — [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

From: Steve Capper <steve.capper@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.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/cpu.h |    1 +
 arch/arm64/kernel/cpuinfo.c  |   48 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index b5e9cee..bb1b0cf 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 52331ff..93e0488 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -199,6 +199,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);
@@ -247,3 +248,50 @@ 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 (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, ret;
+
+	for_each_present_cpu(cpu) {
+		struct device *dev = get_cpu_device(cpu);
+
+		if (!dev)
+			return -1;
+
+		ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+device_initcall(cpuinfo_regs_init);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1240290 — Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

On Mon, Oct 05, 2015 at 06:02:10PM +0100, Suzuki K. Poulose wrote:
> +static int __init cpuinfo_regs_init(void)
> +{
> +	int cpu, ret;
> +
> +	for_each_present_cpu(cpu) {
> +		struct device *dev = get_cpu_device(cpu);
> +
> +		if (!dev)
> +			return -1;

NAK.  Go figure out why, I'm too lazy to tell you.

> +
> +		ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +device_initcall(cpuinfo_regs_init);
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240342 — Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

On 6 October 2015 at 10:09, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 05, 2015 at 06:02:10PM +0100, Suzuki K. Poulose wrote:
>> +static int __init cpuinfo_regs_init(void)
>> +{
>> +     int cpu, ret;
>> +
>> +     for_each_present_cpu(cpu) {
>> +             struct device *dev = get_cpu_device(cpu);
>> +
>> +             if (!dev)
>> +                     return -1;
>
> NAK.  Go figure out why, I'm too lazy to tell you.

I will correct the return code to be -ENODEV.
Was that the reasoning behind the NAK?

>
>> +
>> +             ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +device_initcall(cpuinfo_regs_init);
>> --
>> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240345 — Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

On Tue, Oct 06, 2015 at 11:18:42AM +0100, Steve Capper wrote:
> On 6 October 2015 at 10:09, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Oct 05, 2015 at 06:02:10PM +0100, Suzuki K. Poulose wrote:
> >> +static int __init cpuinfo_regs_init(void)
> >> +{
> >> +     int cpu, ret;
> >> +
> >> +     for_each_present_cpu(cpu) {
> >> +             struct device *dev = get_cpu_device(cpu);
> >> +
> >> +             if (!dev)
> >> +                     return -1;
> >
> > NAK.  Go figure out why, I'm too lazy to tell you.
> 
> I will correct the return code to be -ENODEV.
> Was that the reasoning behind the NAK?

I suspect the half-initialised sysfs groups also have something to do
with it...

Mark.

> 
> >
> >> +
> >> +             ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
> >> +             if (ret)
> >> +                     return ret;
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +device_initcall(cpuinfo_regs_init);
> >> --
> >> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240348 — Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

On 6 October 2015 at 11:25, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Oct 06, 2015 at 11:18:42AM +0100, Steve Capper wrote:
>> On 6 October 2015 at 10:09, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Oct 05, 2015 at 06:02:10PM +0100, Suzuki K. Poulose wrote:
>> >> +static int __init cpuinfo_regs_init(void)
>> >> +{
>> >> +     int cpu, ret;
>> >> +
>> >> +     for_each_present_cpu(cpu) {
>> >> +             struct device *dev = get_cpu_device(cpu);
>> >> +
>> >> +             if (!dev)
>> >> +                     return -1;
>> >
>> > NAK.  Go figure out why, I'm too lazy to tell you.
>>
>> I will correct the return code to be -ENODEV.
>> Was that the reasoning behind the NAK?
>
> I suspect the half-initialised sysfs groups also have something to do
> with it...

Okay, cheers Mark, I see what you mean.

>
> Mark.
>
>>
>> >
>> >> +
>> >> +             ret = sysfs_create_group(&dev->kobj, &cpuregs_attr_group);
>> >> +             if (ret)
>> >> +                     return ret;
>> >> +     }
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >> +device_initcall(cpuinfo_regs_init);
>> >> --
>> >> 1.7.9.5
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240867 — Re: [PATCH v2 21/22] arm64: cpuinfo: Expose MIDR_EL1 and REVIDR_EL1 to sysfs

On Tue, Oct 06, 2015 at 11:18:42AM +0100, Steve Capper wrote:
> On 6 October 2015 at 10:09, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Oct 05, 2015 at 06:02:10PM +0100, Suzuki K. Poulose wrote:
> >> +static int __init cpuinfo_regs_init(void)
> >> +{
> >> +     int cpu, ret;
> >> +
> >> +     for_each_present_cpu(cpu) {
> >> +             struct device *dev = get_cpu_device(cpu);
> >> +
> >> +             if (!dev)
> >> +                     return -1;
> >
> > NAK.  Go figure out why, I'm too lazy to tell you.
> 
> I will correct the return code to be -ENODEV.
> Was that the reasoning behind the NAK?

The reason behind the NAK was indeed returning -1 from a function where
a negative return code is a negative errno value.  I'm fed up with
telling people about this, and I put it down to laziness.  So, I've
decided I'm going to NAK such patches and let the author have the hard
work of working out why - hopefully learning something by doing so,
and hopefully remembering for next time. :)

-1 corresponds to -EPERM, which means "operation not permitted"
(and, if this gets returned to userspace, the user program executing
the syscall will most probably tell the user "operation not permitted"
as the reason for failure - which is rather daft when you think about
it.

I have my mail client setup to highlight with a red background on a
"\+.*return -1" regexp, which nicely lets me catch every one of those
in any patch I skim through.

This is about quality of implementation issue - and, tbh, if someone
can't spend the time to check what happens to the return value, and
then can't spend the time to look up a proper errno value, then what's
the chance the rest of the patch (or patch set) has had sufficient care
and attention paid to it?

I'm getting to the point where if I see a "return -1" I just stop
reviewing the rest of the patch set, especially if it's a large patch
set.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web