Re: [PATCH] lscpu: get the processor information by DMI

[Huang Shijie <shijie@os.amperecomputing.com>]

On Mon, Jun 14, 2021 at 12:41:26PM +0200, Karel Zak wrote:
> 
> CC: Masayoshi and Jeffrey,
> 
> On Mon, Jun 14, 2021 at 09:48:45AM +0000, Huang Shijie wrote:
> > The patch :367c85c47286 ("lscpu: use SMBIOS tables on ARM for lscpu")
> > relies on the existence of "/sys/firmware/dmi/entries/4-0/raw",
> > which may not exist in standard linux kernel.
> > 
> > But "/sys/firmware/dmi/tables/DMI" should exist and can provide the required
> > processor information.
> 
> Good idea to add a fallback solution.
> 
> > This patch uses "/sys/firmware/dmi/tables/DMI"
> > to get the processor information:
> >   1.) Use the DMI to provide more accurate "Model name" information.
> 
> We've had a long discussion about data from DMI  and we had a few
> attempts to implement it ;-) The conclusion is to differentiate
> between information decoded from IDs and information from BIOS, so now
> we have two fields ct->bios_modelname and ct->modelname (and
> ct->bios_vendor).
Okay. 

I will the ct->bios_modelname/ct->modelname /ct->bios_vendor in  next version.

> 
> The reason is that in some cases the strings from DMI do not provide
> well-known CPU names and info by user.
> 
>         Vendor ID:           ARM
>         BIOS Vendor ID:      https://www.mellanox.com
>         Model:               0
>         Model name:          Cortex-A72
>         BIOS Model name:     Mellanox BlueField-2 [A0] A72(D08) r1p0
> 
> "Cortex-A72" is pretty well-known, Mellanox BlueField is some
> marketing name, another example:
> 
>         Vendor ID:           Cavium
>         BIOS Vendor ID:      CN8890-2000BG2601-AAP-PR-Y-G
>         Model:               0
>         Model name:          ThunderX 88XX
>         BIOS Model name:     2.0
> 
> > After this patch, we can get the lscpu output
> > in Ampere Altra platform:
> >    ---------------------------------------------
> > 	Architecture:                    aarch64
> > 	CPU op-mode(s):                  32-bit, 64-bit
> > 	Byte Order:                      Little Endian
> > 	CPU(s):                          160
> > 	On-line CPU(s) list:             0-159
> > 	Vendor ID:                       ARM
> > 	Model name:                      Ampere(R) Altra(R) Processor Q00-00 CPU @ 3.0GHz
> 
> Should be 
> 
>     Model name:          Neoverse-N1
okay..				      
>     BIOS Model name:     Ampere(R) Altra(R) Processor Q00-00 CPU @ 3.0GHz
> 
> >  static void arm_decode(struct lscpu_cxt *cxt, struct lscpu_cputype *ct)
> >  {
> > +	/* dmi_decode_cputype may get more accurate information later */
> > +	arm_ids_decode(ct);
> > +
> >  	/* use SMBIOS Type 4 data if available */
> >  	if (!cxt->noalive && access(_PATH_SYS_DMI_TYPE4, R_OK) == 0)
> >  		arm_smbios_decode(ct);
> > +	else if (!cxt->noalive && access(_PATH_SYS_DMI, R_OK) == 0)
> > +		dmi_decode_cputype(ct);
> >  
> > -	arm_ids_decode(ct);
> 
> Please, do not move arm_ids_decode().
no problem.
> 
> > +int dmi_decode_cputype(struct lscpu_cputype *ct)
> > +{
> > +	static char const sys_fw_dmi_tables[] = _PATH_SYS_DMI;
> > +	struct dmi_info di = { };
> > +	struct stat st;
> > +	uint8_t *data;
> > +	int rc = 0;
> > +	char buf[100] = { };
> > +
> > +	if (stat(sys_fw_dmi_tables, &st))
> > +		return rc;
> > +
> > +	data = get_mem_chunk(0, st.st_size, sys_fw_dmi_tables);
> > +	if (!data)
> > +		return rc;
> > +
> > +	rc = parse_dmi_table(st.st_size, st.st_size/4, data, &di);
> > +	if (rc < 0) {
> > +		free(data);
> > +		return rc;
> > +	}
> > +
> > +	/* Get module name */
> > +	sprintf(buf, "%s %s CPU @ %d.%dGHz", di.processor_version, di.part_num,
> > +			di.current_speed/1000, (di.current_speed % 1000) / 100);
> 
> So, it's not string from DMI, but it's composed from more information
> and it seems compatible to "model name:" from (x86) /proc/cpuinfo. I'm
> fine with it.
Thanks.

> 
> > +	free(ct->modelname);
> > +	ct->modelname = xstrdup(buf);
> 
> Please:
> 
>   ct->bios_modelname = xstrdup(buf);
okay.
> 
> 
> > +	/* Get CPU family */
> > +	memset(buf, 0, sizeof(buf));
> > +	sprintf(buf, "%d", di.processor_family);
> > +	free(ct->family);
> > +	ct->family = xstrdup(buf);
> 
> is there any difference between "cpu family" from /proc/cpuinfo and
> this DMI field? Do we need a new field ct->bios_family or overwrite 
> the ct->family good enough? I don't know ;-)
> 
In the arm platform, it seems it does not show the "cpu family" info in lscpu.
And I did not find the "cpu family" in the /proc/cpuinfo, only find "cpu architecture: 8".
Unfortunately, the "cpu architecture :8 " is hardcode, it will be changed for armv9 in future.

In the DMI, the "cpu family" shows 0x101 which means "arm v8".

Thanks
Huang Shijie