Applied, thanks!
dnie...@gmail.com, le lun. 27 janv. 2025 01:17:33 +0000, a ecrit:
> From: Diego Nieto Cid <dnie...@gmail.com>
>
> Hello,
>
> On Sun, Jan 26, 2025 at 10:18:03PM +0100, Samuel Thibault wrote:
> >
> > dnie...@gmail.com, le ven. 10 janv. 2025 23:52:28 +0000, a ecrit:
> > > --- a/procfs/rootdir.c
> > > +
> > > + m = open_memstream(contents, (size_t *) contents_len);
> >
> > Please mind the GNU coding style which wants a space before opening
> > parentheses, here and in other places (function calls, sizeof, etc.).
> >
>
> I hope this time I got it better :)
>
> > > +
> > > + memcpy(vendor + 0 * sizeof(unsigned int), (char *) &ebx,
> > > sizeof(unsigned int));
> >
> > There is no need to cast &ebx, memcpy takes a void*, so the cast is
> > implicit in C.
> >
>
> Ok, I removed the casts.
>
> >
> > > +
> > > + fprintf(m,
> > > + "processor : 0\n"
> > > + "vendor_id : %s\n"
> > > + "cpu family : %d\n"
> > > + "model : %d\n"
> > > + "model name : %s\n"
> > > + "stepping : %d\n",
> > > + vendor, family, model, model_name, stepping);
> > > +
> > > + fprintf(m, "flags :");v
> >
> > Linux uses tab characters before ':', better use that too for
> > compatibility.
> >
>
> Ok, I switched to tabs. Though, I had to use a mix of one and two tabs.
>
> > > +
> > > + m = open_memstream(contents, (size_t *) contents_len);
> > > + if (m == NULL)
> > > + return ENOMEM;
> >
> > Better return errno like on x86.
> >
>
> Done so.
>
> > > +
> > > + err = aarch64_get_hwcaps(mach_host_self(), &caps, &mdir, &revdir);
> > > + if (err)
> > > + goto out;
> > > +
> > > + implementer = (mdir & 0xff000000) >> 24;
> > > + variant = (mdir & 0x00f00000) >> 20;
> > > + architecture = (mdir & 0x000f0000) >> 16;
> > > + part_num = (mdir & 0x0000fff0) >> 4;
> > > + revision = (mdir & 0x0000000f) >> 0;
> >
> > Better align mdir too to make it look nicer.
> >
>
> I aligned the = sign, the variable and the semi-colon.
> Now it should look better.
>
> > > +
> > > + fprintf(m, "processor : 0\n");
> > > + fprintf(m, "BogoMIPS : 0\n");
> >
> > I'd rather not print BogoMIPS at all rather than 0.
> >
>
> Ok, I just removed the BogoMIPS entry until we figure out how
> to compute it properly.
>
> >
> > Also use tabs as well for compatibility.
> >
>
> Done
>
> > > +out:
> > > + flcose(m);
> >
> > You didn't try to compile it ;)
> >
>
> Oops, it didn't cross my mind I could do that :-)
>
> --
>
> On Mon, Jan 27, 2025 at 12:25:15AM +0300, Sergey Bugaev wrote:
> > On Mon, Jan 27, 2025 at 12:18 AM Samuel Thibault
> > <samuel.thiba...@gnu.org> wrote:
> > >
> > > Better align mdir too to make it look nicer.
> >
> > Speaking of which, it's midr (main identification register) and revidr
> > (revision identification register), not *dir.
> >
>
> Thanks, I renamed the variables. revidr is unused though, the docs
> said it was implementation defined so I didn't know what to do with it.
>
> --
>
> On Sun, Jan 26, 2025 at 10:27:57PM +0100, Samuel Thibault wrote:
> > Sergey Bugaev, le lun. 27 janv. 2025 00:25:15 +0300, a ecrit:
> > > > > +out:
> > > > > + flcose(m);
> > > >
> > > > You didn't try to compile it ;)
> > >
> > > I suppose that's not easy for Diego to do, given the state of aarch64-gnu
> > > :)
> >
> > Sure, but he can comment the very few really-arm64-specific lines and
> > try to build the rest :)
> >
>
> It didn't occur to me about testing that way. I fiddled with the defined
> macros and replaced the RPC with some harcoded data, it should work now.
>
> Regards,
> Diego
>
> -- >8 -- >8 -- >8 --
>
> * procfs/rootdir.c: (rootdir_gc_cpuinfo) new function
> (rootdir_entries) add entry for cpuinfo file
> (cpuinfo_x86, cpuinfo_aarch64) implementations for x86 and
> aarch64 respectively.
> ---
> procfs/rootdir.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 219 insertions(+)
>
> diff --git a/procfs/rootdir.c b/procfs/rootdir.c
> index b8a8c4d1..05304015 100644
> --- a/procfs/rootdir.c
> +++ b/procfs/rootdir.c
> @@ -38,6 +38,12 @@
> #include "procfs_dir.h"
> #include "main.h"
> #include <net/route.h>
> +#if defined (__x86_64__) || defined (__i486__) || defined (__i586__) ||
> defined (__i686__)
> +#include <cpuid.h>
> +#elif defined (__aarch64__)
> +#warning Aarch64 port of cpuinfo is untested
> +#include <mach/machine/mach_aarch64.h>
> +#endif
>
> #include "mach_debug_U.h"
> #include "pfinet_U.h"
> @@ -696,6 +702,212 @@ out_fclose:
> fclose (m);
> return err;
> }
> +
> +#if defined (__x86_64__) || defined (__i486__) || defined (__i586__) ||
> defined (__i686__)
> +static char *cpu_features_edx[] =
> + {
> + "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce", "cx8", "apic",
> + NULL, "sep", "mtrr", "pge", "mca", "cmov", "pat", "pse36", "pn",
> "clfush",
> + NULL, "dts", "acpi", "mmx", "fxsr", "sse", "sse2", "ss", "ht", "tm",
> + "ia64", "pbe"
> + };
> +
> +static char *cpu_features_ecx[] =
> + {
> + "sse3", "pclmulqdq", "dtes64", "monitor", "ds_cpl", "vmx", "smx", "est",
> + "tm2", "ssse3", "cid", "sdbg", "fma", "cx16", "xtpr", "pdcm",
> + NULL, "pcid", "dca", "sse4_1", "sse4_2", "x2apic", "movbe", "popcnt",
> + "tsc_deadline_timer", "aes", "xsave", "osxsave", "avx", "f16c",
> "rdrand", "hypervisor"
> + };
> +
> +#define VENDOR_ID_LEN 12
> +#define MODEL_NAME_LEN 48
> +
> +static error_t
> +cpuinfo_x86 (void* hook, char **contents, ssize_t *contents_len)
> +{
> + error_t err = 0;
> + FILE* m;
> + int ret, index, stepping, model, family, extended_model, extended_family;
> + unsigned int eax, ebx, ecx, edx;
> + unsigned int feature_edx, feature_ecx;
> + char vendor[VENDOR_ID_LEN + 1] = { 0 };
> + char model_name[MODEL_NAME_LEN + 1] = { 0 };
> +
> + m = open_memstream (contents, (size_t *) contents_len);
> + if (m == NULL)
> + return errno;
> +
> + ret = __get_cpuid (0, &eax, &ebx, &ecx, &edx);
> + if (ret != 1)
> + {
> + err = EIO;
> + goto out;
> + }
> +
> + memcpy (vendor + 0 * sizeof (unsigned int), &ebx, sizeof (unsigned int));
> + memcpy (vendor + 1 * sizeof (unsigned int), &edx, sizeof (unsigned int));
> + memcpy (vendor + 2 * sizeof (unsigned int), &ecx, sizeof (unsigned int));
> +
> + ret = __get_cpuid (1, &eax, &ebx, &ecx, &edx);
> + if (ret != 1)
> + {
> + err = EIO;
> + goto out;
> + }
> +
> + feature_edx = edx;
> + feature_ecx = ecx;
> + stepping = eax & 0x0F;
> + model = (eax & 0xF0) >> 4;
> + family = (eax & 0xF00) >> 8;
> + extended_model = (eax & 0xF0000) >> 16;
> + extended_family = (eax &0xFF00000) >> 20;
> +
> + if (family == 6 || family == 15)
> + model += (extended_model << 4);
> + if (family == 15)
> + family += extended_family;
> +
> + __get_cpuid (0x80000000, &eax, &ebx, &ecx, &edx);
> + if (eax >= 0x80000004)
> + {
> + __get_cpuid (0x80000002, &eax, &ebx, &ecx, &edx);
> + memcpy (model_name + 0 * sizeof (unsigned int), &eax, sizeof (unsigned
> int));
> + memcpy (model_name + 1 * sizeof (unsigned int), &ebx, sizeof (unsigned
> int));
> + memcpy (model_name + 2 * sizeof (unsigned int), &ecx, sizeof (unsigned
> int));
> + memcpy (model_name + 3 * sizeof (unsigned int), &edx, sizeof (unsigned
> int));
> +
> + __get_cpuid (0x80000003, &eax, &ebx, &ecx, &edx);
> + memcpy (model_name + 4 * sizeof (unsigned int), &eax, sizeof (unsigned
> int));
> + memcpy (model_name + 5 * sizeof (unsigned int), &ebx, sizeof (unsigned
> int));
> + memcpy (model_name + 6 * sizeof (unsigned int), &ecx, sizeof (unsigned
> int));
> + memcpy (model_name + 7 * sizeof (unsigned int), &edx, sizeof (unsigned
> int));
> +
> + __get_cpuid (0x80000004, &eax, &ebx, &ecx, &edx);
> + memcpy (model_name + 8 * sizeof (unsigned int), &eax, sizeof (unsigned
> int));
> + memcpy (model_name + 9 * sizeof (unsigned int), &ebx, sizeof (unsigned
> int));
> + memcpy (model_name + 10 * sizeof (unsigned int), &ecx, sizeof
> (unsigned int));
> + memcpy (model_name + 11 * sizeof (unsigned int), &edx, sizeof
> (unsigned int));
> + }
> +
> + fprintf (m,
> + "processor\t: 0\n"
> + "vendor_id\t: %s\n"
> + "cpu family\t: %d\n"
> + "model\t\t: %d\n"
> + "model name\t: %s\n"
> + "stepping\t: %d\n",
> + vendor, family, model, model_name, stepping);
> +
> + fprintf (m, "flags\t\t:");
> + for (index = 0; index < (sizeof(cpu_features_edx)/sizeof(char*)); index++)
> + {
> + if (cpu_features_edx[index] == NULL)
> + continue;
> + if (feature_edx & (1ul << index))
> + fprintf(m, " %s", cpu_features_edx[index]);
> + }
> + for (index = 0; index < (sizeof(cpu_features_ecx)/sizeof(char*)); index++)
> + {
> + if (cpu_features_ecx[index] == NULL)
> + continue;
> + if (feature_ecx & (1ul << index))
> + fprintf(m, " %s", cpu_features_ecx[index]);
> + }
> +
> + fprintf(m, "\n\n");
> +
> +out:
> + fclose(m);
> + return err;
> +}
> +#endif
> +
> +#if defined (__aarch64__)
> +
> +static char *cpu_features_1[] =
> + {
> + "fp", "asimd", "evtstrm", "aes", "pmul", "sha1", "sha2", "crc32",
> + "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm", "jscvt", "fcma",
> "lrcpc",
> + "dcpop", "sha3", "sm3", "sm4", "asimddp", "sha512", "sve", "asimdfhm",
> + "dit", "uscat", "ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"
> + };
> +
> +static char *cpu_features_2[] =
> + {
> + "dcpodp", "sve2", "sveaes", "svepmull", "svebitperm", "svesha3",
> "svesm4", "flagm2",
> + "frint", "svei8mm", "svef32mm", "svef64mm", "svebf16", "i8mm", "bf16",
> "dgh",
> + "rng", "bti", "mie", "ecv", "afp", "rpres", "mte3", "sme",
> + "sme_i16i64", "sme_f64f64", "sme_i8i32", "sme_f16f32", "sme_b16f32",
> "sme_f32f32", "sme_fa64", "wfxt",
> + "ebf16", "sve_ebf16", "cssc", "rprfm", "sve2p1", "sme2", "sme2p1",
> "sme_i15i32",
> + "sme_bi32i32", "sme_b16b16", "sme_f16f16", "mops", "hbc", "sve_b16b16",
> "lrcpc3", "lse123"
> + };
> +
> +static error_t
> +cpuinfo_aarch64 (void *hook, char **contents, ssize_t *contents_len)
> +{
> + error_t err = 0;
> + hwcaps_t caps;
> + uint64_t midr, revidr;
> + int index;
> + unsigned int implementer, variant, architecture, part_num, revision;
> + FILE *m;
> +
> + m = open_memstream (contents, (size_t *) contents_len);
> + if (m == NULL)
> + return errno;
> +
> + err = aarch64_get_hwcaps (mach_host_self(), &caps, &midr, &revidr);
> + if (err)
> + goto out;
> +
> + implementer = (midr & 0xff000000) >> 24;
> + variant = (midr & 0x00f00000) >> 20;
> + architecture = (midr & 0x000f0000) >> 16;
> + part_num = (midr & 0x0000fff0) >> 4;
> + revision = (midr & 0x0000000f) >> 0;
> +
> + fprintf(m, "processor\t\t: 0\n");
> + fprintf(m, "Features\t\t:");
> + for (index = 0; index < (sizeof (cpu_features_1) / sizeof (char *));
> index++)
> + {
> + if (cpu_features_1[index] == NULL)
> + continue;
> + if (caps[0] & (1ul << index))
> + fprintf(m, " %s", cpu_features_1[index]);
> + }
> + for (index = 0; index < (sizeof(cpu_features_2) / sizeof(char *)); index++)
> + {
> + if (cpu_features_2[index] == NULL)
> + continue;
> + if (caps[1] & (1ul << index))
> + fprintf(m, " %s", cpu_features_2[index]);
> + }
> + fprintf(m, "\n");
> + fprintf(m, "CPU implementer\t\t: 0x%x\n", implementer);
> + fprintf(m, "CPU architecture\t: %d\n", architecture);
> + fprintf(m, "CPU variant\t\t: 0x%x\n", variant);
> + fprintf(m, "CPU part\t\t: 0x%x\n", part_num);
> + fprintf(m, "CPU revision\t\t: %d\n", revision);
> + fprintf(m, "\n");
> +out:
> + fclose(m);
> + return err;
> +}
> +#endif
> +
> +static error_t
> +rootdir_gc_cpuinfo (void *hook, char **contents, ssize_t *contents_len)
> +{
> +#if defined (__x86_64__) || defined (__i486__) || defined (__i586__) ||
> defined (__i686__)
> + return cpuinfo_x86(hook, contents, contents_len);
> +#elif defined (__aarch64__)
> + return cpuinfo_aarch64(hook, contents, contents_len);
> +#else
> + return ENOTSUP;
> +#endif
> +}
>
> /* Glue logic and entries table */
>
> @@ -888,6 +1100,13 @@ static const struct procfs_dir_entry rootdir_entries[]
> = {
> .cleanup_contents = procfs_cleanup_contents_with_free,
> },
> },
> + {
> + .name = "cpuinfo",
> + .hook = & (struct procfs_node_ops) {
> + .get_contents = rootdir_gc_cpuinfo,
> + .cleanup_contents = procfs_cleanup_contents_with_free,
> + },
> + },
> #ifdef PROFILE
> /* In order to get a usable gmon.out file, we must apparently use exit().
> */
> {
> --
> 2.47.1
>
>
--
Samuel
War doesn't prove who's right, just who's left.
