| Message ID | 20170926222324.17409-4-ahs3@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/26/2017 03:23 PM, Al Stone wrote: > While it is very useful to know what CPU is being used, it is also > useful to know who made the platform being used. On servers, this > can point to the right person to contact when a server is having > trouble. > > Go get the product info -- manufacturer, product name and version -- > from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more > like other server platforms, include the CPU type and frequency when > displaying the product info, too. > > Signed-off-by: Al Stone <ahs3@redhat.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > --- [snip] > +/* Look up the baseboard info in DMI */ > +static void get_dmi_product_info(void) > +{ > + if (!dmi_product_info) { > + dmi_product_info = kcalloc(DMI_MAX_STRLEN, > + sizeof(char), GFP_KERNEL); > + if (!dmi_product_info) > + return; > + } > + > + dmi_walk(find_dmi_product_info, dmi_product_info); > +} Don't you need all of these DMI-related functions you defined to be also enclosed within an #if IS_ENABLED(CONFIG_DMI) otherwise chances are that we are going to get defined but unused warnings? > impl = (u8) MIDR_IMPLEMENTOR(midr); > for (j = 0; hw_implementer[j].id != 0; j++) { > if (hw_implementer[j].id == impl) { > @@ -208,7 +341,7 @@ static int c_show(struct seq_file *m, void *v) > part = (u16) MIDR_PARTNUM(midr); > for (j = 0; parts[j].id != (-1); j++) { > if (parts[j].id == part) { > - seq_printf(m, "%s\n", parts[j].name); > + seq_printf(m, "%s ", parts[j].name); > break; > } Should this hunk be part of patch 3?
On 26/09/17 23:23, Al Stone wrote: > While it is very useful to know what CPU is being used, it is also > useful to know who made the platform being used. On servers, this > can point to the right person to contact when a server is having > trouble. > > Go get the product info -- manufacturer, product name and version -- > from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more > like other server platforms, include the CPU type and frequency when > displaying the product info, too. > > Signed-off-by: Al Stone <ahs3@redhat.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > --- [...] > @@ -190,6 +298,31 @@ static int c_show(struct seq_file *m, void *v) > seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", > MIDR_REVISION(midr), COMPAT_ELF_PLATFORM); > > + if (IS_ENABLED(CONFIG_DMI)) { > + seq_puts(m, "product name\t: "); > + > + if (!dmi_product_info) > + get_dmi_product_info(); > + if (dmi_product_info) > + seq_printf(m, "%s", dmi_product_info); > + else > + seq_puts(m, "<unknown>"); > + > + seq_printf(m, ", ARM 8.%d (r%dp%d) CPU", > + MIDR_VARIANT(midr), > + MIDR_VARIANT(midr), > + MIDR_REVISION(midr)); What is "ARM 8.1" meant to infer for, say, a typical Cortex-A57? Robin. > + > + if (!dmi_max_mhz) > + dmi_max_mhz = get_dmi_max_mhz(); > + if (dmi_max_mhz) > + seq_printf(m, " @ %d.%02dGHz\n", > + (int)(dmi_max_mhz / 1000), > + (int)(dmi_max_mhz % 1000)); > + else > + seq_puts(m, " @ <unknown>GHz\n"); > + } > + > impl = (u8) MIDR_IMPLEMENTOR(midr); > for (j = 0; hw_implementer[j].id != 0; j++) { > if (hw_implementer[j].id == impl) {
Hi Al, On Tue, Sep 26, 2017 at 04:23:24PM -0600, Al Stone wrote: > While it is very useful to know what CPU is being used, it is also > useful to know who made the platform being used. On servers, this > can point to the right person to contact when a server is having > trouble. > > Go get the product info -- manufacturer, product name and version -- > from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more > like other server platforms, include the CPU type and frequency when > displaying the product info, too. As mentioned on the cover letter, NAK to modifiying /proc/cpuinfo. I note this is all DMI information, which I thought we already exposed under sysfs (in an architecture-neutral format). Is that not the case? ... or do existing tools not pick that up today? Thanks, Mark. > > Signed-off-by: Al Stone <ahs3@redhat.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > --- > arch/arm64/kernel/cpuinfo.c | 135 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 134 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c > index 0b4261884862..6a9dbad5ee3f 100644 > --- a/arch/arm64/kernel/cpuinfo.c > +++ b/arch/arm64/kernel/cpuinfo.c > @@ -19,10 +19,12 @@ > #include <asm/cpu.h> > #include <asm/cputype.h> > #include <asm/cpufeature.h> > +#include <asm/unaligned.h> > > #include <linux/bitops.h> > #include <linux/bug.h> > #include <linux/compat.h> > +#include <linux/dmi.h> > #include <linux/elf.h> > #include <linux/init.h> > #include <linux/kernel.h> > @@ -31,6 +33,7 @@ > #include <linux/printk.h> > #include <linux/seq_file.h> > #include <linux/sched.h> > +#include <linux/slab.h> > #include <linux/smp.h> > #include <linux/delay.h> > #include <linux/cpuinfo.h> > @@ -167,6 +170,111 @@ static const char *const compat_hwcap2_str[] = { > }; > #endif /* CONFIG_COMPAT */ > > +/* Details needed when extracting fields from DMI info */ > +#define DMI_ENTRY_BASEBOARD_MIN_LENGTH 8 > +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH 48 > + > +#define DMI_BASEBOARD_MANUFACTURER 0x04 > +#define DMI_BASEBOARD_PRODUCT 0x05 > +#define DMI_BASEBOARD_VERSION 0x06 > +#define DMI_PROCESSOR_MAX_SPEED 0x14 > + > +#define DMI_MAX_STRLEN 80 > + > +/* Values captured from DMI info */ > +static u64 dmi_max_mhz; > +static char *dmi_product_info; > + > +/* Callback function used to retrieve the max frequency from DMI */ > +static void find_dmi_mhz(const struct dmi_header *dm, void *private) > +{ > + const u8 *dmi_data = (const u8 *)dm; > + u16 *mhz = (u16 *)private; > + > + if (dm->type == DMI_ENTRY_PROCESSOR && > + dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) { > + u16 val = (u16)get_unaligned((const u16 *) > + (dmi_data + DMI_PROCESSOR_MAX_SPEED)); > + *mhz = val > *mhz ? val : *mhz; > + } > +} > + > +/* Look up the max frequency in DMI */ > +static u64 get_dmi_max_mhz(void) > +{ > + u16 mhz = 0; > + > + dmi_walk(find_dmi_mhz, &mhz); > + > + /* > + * Real stupid fallback value, just in case there is no > + * actual value set. > + */ > + mhz = mhz ? mhz : 1; > + > + return (u64)mhz; > +} > + > +/* Helper function for the product info callback */ > +static char *copy_string_n(char *dst, char *table, int idx) > +{ > + char *d = dst; > + char *ptr = table; > + int ii; > + > + /* skip the first idx-1 strings */ > + for (ii = 1; ii < idx; ii++) { > + while (*ptr) > + ptr++; > + ptr++; > + } > + > + /* copy in the string we need */ > + while (*ptr && (d - dst) < (DMI_MAX_STRLEN - 2)) > + *d++ = *ptr++; > + > + return d; > +} > + > +/* Callback function used to retrieve the product info DMI */ > +static void find_dmi_product_info(const struct dmi_header *dm, void *private) > +{ > + const u8 *dmi_data = (const u8 *)dm; > + char *ptr = (char *)private; > + > + if (dm->type == DMI_ENTRY_BASEBOARD && > + dm->length >= DMI_ENTRY_BASEBOARD_MIN_LENGTH) { > + int idx; > + > + idx = (int)get_unaligned((const u8 *) > + (dmi_data + DMI_BASEBOARD_MANUFACTURER)); > + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); > + *ptr++ = ' '; > + > + idx = (int)get_unaligned((const u8 *) > + (dmi_data + DMI_BASEBOARD_PRODUCT)); > + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); > + *ptr++ = ' '; > + > + idx = (int)get_unaligned((const u8 *) > + (dmi_data + DMI_BASEBOARD_VERSION)); > + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); > + } > +} > + > +/* Look up the baseboard info in DMI */ > +static void get_dmi_product_info(void) > +{ > + if (!dmi_product_info) { > + dmi_product_info = kcalloc(DMI_MAX_STRLEN, > + sizeof(char), GFP_KERNEL); > + if (!dmi_product_info) > + return; > + } > + > + dmi_walk(find_dmi_product_info, dmi_product_info); > +} > + > static int c_show(struct seq_file *m, void *v) > { > int i, j; > @@ -190,6 +298,31 @@ static int c_show(struct seq_file *m, void *v) > seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", > MIDR_REVISION(midr), COMPAT_ELF_PLATFORM); > > + if (IS_ENABLED(CONFIG_DMI)) { > + seq_puts(m, "product name\t: "); > + > + if (!dmi_product_info) > + get_dmi_product_info(); > + if (dmi_product_info) > + seq_printf(m, "%s", dmi_product_info); > + else > + seq_puts(m, "<unknown>"); > + > + seq_printf(m, ", ARM 8.%d (r%dp%d) CPU", > + MIDR_VARIANT(midr), > + MIDR_VARIANT(midr), > + MIDR_REVISION(midr)); > + > + if (!dmi_max_mhz) > + dmi_max_mhz = get_dmi_max_mhz(); > + if (dmi_max_mhz) > + seq_printf(m, " @ %d.%02dGHz\n", > + (int)(dmi_max_mhz / 1000), > + (int)(dmi_max_mhz % 1000)); > + else > + seq_puts(m, " @ <unknown>GHz\n"); > + } > + > impl = (u8) MIDR_IMPLEMENTOR(midr); > for (j = 0; hw_implementer[j].id != 0; j++) { > if (hw_implementer[j].id == impl) { > @@ -208,7 +341,7 @@ static int c_show(struct seq_file *m, void *v) > part = (u16) MIDR_PARTNUM(midr); > for (j = 0; parts[j].id != (-1); j++) { > if (parts[j].id == part) { > - seq_printf(m, "%s\n", parts[j].name); > + seq_printf(m, "%s ", parts[j].name); > break; > } > } > -- > 2.13.5 >
Hi, On Wed, Sep 27, 2017 at 11:42:07AM +0100, Robin Murphy wrote: > On 26/09/17 23:23, Al Stone wrote: > > + seq_printf(m, ", ARM 8.%d (r%dp%d) CPU", > > + MIDR_VARIANT(midr), > > + MIDR_VARIANT(midr), > > + MIDR_REVISION(midr)); > > What is "ARM 8.1" meant to infer for, say, a typical Cortex-A57? Just to make Robin's point a little clearer, MIDR_EL1.Variant is IMPLEMENTATION DEFINED, and doesn't describe the ARMv8.x architecture revision. For example, on Cortex A57 is contains the major revision number of the CPU, and is 1 for any r1pY Cortex-A57 (e.g. those on Juno R1). For better or worse, the architecture provides us no mechanism to determine the architecture revision. Thanks, Mark.
On Tue, Sep 26, 2017 at 5:23 PM, Al Stone <ahs3@redhat.com> wrote: > + if (IS_ENABLED(CONFIG_DMI)) { > + seq_puts(m, "product name\t: "); > + > + if (!dmi_product_info) > + get_dmi_product_info(); > + if (dmi_product_info) > + seq_printf(m, "%s", dmi_product_info); This line prints out like this: product name : QUALCOMM BASEBOARD ASSEMBLY, AMBERWIN 20-P7989-H1S , ARM 8.0 (r0p0) CPU @ 2.500GHz Is this a bug in our DMI table? 'dmidecode' reports this: Handle 0x0004, DMI type 2, 17 bytes Base Board Information Manufacturer: QUALCOMM Product Name: BASEBOARD ASSEMBLY, AMBERWIN.. Version: 20-P7989-H1S I'm guessing that the actual string has a CR/LF at the end of "AMBERWIN"
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index 0b4261884862..6a9dbad5ee3f 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -19,10 +19,12 @@ #include <asm/cpu.h> #include <asm/cputype.h> #include <asm/cpufeature.h> +#include <asm/unaligned.h> #include <linux/bitops.h> #include <linux/bug.h> #include <linux/compat.h> +#include <linux/dmi.h> #include <linux/elf.h> #include <linux/init.h> #include <linux/kernel.h> @@ -31,6 +33,7 @@ #include <linux/printk.h> #include <linux/seq_file.h> #include <linux/sched.h> +#include <linux/slab.h> #include <linux/smp.h> #include <linux/delay.h> #include <linux/cpuinfo.h> @@ -167,6 +170,111 @@ static const char *const compat_hwcap2_str[] = { }; #endif /* CONFIG_COMPAT */ +/* Details needed when extracting fields from DMI info */ +#define DMI_ENTRY_BASEBOARD_MIN_LENGTH 8 +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH 48 + +#define DMI_BASEBOARD_MANUFACTURER 0x04 +#define DMI_BASEBOARD_PRODUCT 0x05 +#define DMI_BASEBOARD_VERSION 0x06 +#define DMI_PROCESSOR_MAX_SPEED 0x14 + +#define DMI_MAX_STRLEN 80 + +/* Values captured from DMI info */ +static u64 dmi_max_mhz; +static char *dmi_product_info; + +/* Callback function used to retrieve the max frequency from DMI */ +static void find_dmi_mhz(const struct dmi_header *dm, void *private) +{ + const u8 *dmi_data = (const u8 *)dm; + u16 *mhz = (u16 *)private; + + if (dm->type == DMI_ENTRY_PROCESSOR && + dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) { + u16 val = (u16)get_unaligned((const u16 *) + (dmi_data + DMI_PROCESSOR_MAX_SPEED)); + *mhz = val > *mhz ? val : *mhz; + } +} + +/* Look up the max frequency in DMI */ +static u64 get_dmi_max_mhz(void) +{ + u16 mhz = 0; + + dmi_walk(find_dmi_mhz, &mhz); + + /* + * Real stupid fallback value, just in case there is no + * actual value set. + */ + mhz = mhz ? mhz : 1; + + return (u64)mhz; +} + +/* Helper function for the product info callback */ +static char *copy_string_n(char *dst, char *table, int idx) +{ + char *d = dst; + char *ptr = table; + int ii; + + /* skip the first idx-1 strings */ + for (ii = 1; ii < idx; ii++) { + while (*ptr) + ptr++; + ptr++; + } + + /* copy in the string we need */ + while (*ptr && (d - dst) < (DMI_MAX_STRLEN - 2)) + *d++ = *ptr++; + + return d; +} + +/* Callback function used to retrieve the product info DMI */ +static void find_dmi_product_info(const struct dmi_header *dm, void *private) +{ + const u8 *dmi_data = (const u8 *)dm; + char *ptr = (char *)private; + + if (dm->type == DMI_ENTRY_BASEBOARD && + dm->length >= DMI_ENTRY_BASEBOARD_MIN_LENGTH) { + int idx; + + idx = (int)get_unaligned((const u8 *) + (dmi_data + DMI_BASEBOARD_MANUFACTURER)); + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); + *ptr++ = ' '; + + idx = (int)get_unaligned((const u8 *) + (dmi_data + DMI_BASEBOARD_PRODUCT)); + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); + *ptr++ = ' '; + + idx = (int)get_unaligned((const u8 *) + (dmi_data + DMI_BASEBOARD_VERSION)); + ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx); + } +} + +/* Look up the baseboard info in DMI */ +static void get_dmi_product_info(void) +{ + if (!dmi_product_info) { + dmi_product_info = kcalloc(DMI_MAX_STRLEN, + sizeof(char), GFP_KERNEL); + if (!dmi_product_info) + return; + } + + dmi_walk(find_dmi_product_info, dmi_product_info); +} + static int c_show(struct seq_file *m, void *v) { int i, j; @@ -190,6 +298,31 @@ static int c_show(struct seq_file *m, void *v) seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", MIDR_REVISION(midr), COMPAT_ELF_PLATFORM); + if (IS_ENABLED(CONFIG_DMI)) { + seq_puts(m, "product name\t: "); + + if (!dmi_product_info) + get_dmi_product_info(); + if (dmi_product_info) + seq_printf(m, "%s", dmi_product_info); + else + seq_puts(m, "<unknown>"); + + seq_printf(m, ", ARM 8.%d (r%dp%d) CPU", + MIDR_VARIANT(midr), + MIDR_VARIANT(midr), + MIDR_REVISION(midr)); + + if (!dmi_max_mhz) + dmi_max_mhz = get_dmi_max_mhz(); + if (dmi_max_mhz) + seq_printf(m, " @ %d.%02dGHz\n", + (int)(dmi_max_mhz / 1000), + (int)(dmi_max_mhz % 1000)); + else + seq_puts(m, " @ <unknown>GHz\n"); + } + impl = (u8) MIDR_IMPLEMENTOR(midr); for (j = 0; hw_implementer[j].id != 0; j++) { if (hw_implementer[j].id == impl) { @@ -208,7 +341,7 @@ static int c_show(struct seq_file *m, void *v) part = (u16) MIDR_PARTNUM(midr); for (j = 0; parts[j].id != (-1); j++) { if (parts[j].id == part) { - seq_printf(m, "%s\n", parts[j].name); + seq_printf(m, "%s ", parts[j].name); break; } }
While it is very useful to know what CPU is being used, it is also useful to know who made the platform being used. On servers, this can point to the right person to contact when a server is having trouble. Go get the product info -- manufacturer, product name and version -- from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more like other server platforms, include the CPU type and frequency when displaying the product info, too. Signed-off-by: Al Stone <ahs3@redhat.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/kernel/cpuinfo.c | 135 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 1 deletion(-)