Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
> > Third, it's probably a bad idea to call this function from generic code, so > > make it static and add the detection function from patch 2/2 already here. > > By making it static, it's also possible to define it only if CONFIG_LINUX > > is defined; the ThunderX detection will then return false if !CONFIG_LINUX. > > > > You mean to say, move contents of this patch to util/cutils.c and make it > static and define under __aarch64__ and CONFIG_LINUX?. I don't think util/cutils.c is the right file. It should be a new file, something like util/aarch64-cpuid.c. If CONFIG_LINUX is not defined, the ThunderX detection function should return zero. If __aarch64__ is not defined, the function should not be defined at all. Paolo
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
Hi Paolo, On Tue, Aug 2, 2016 at 4:18 PM, Paolo Bonzini wrote: > - Original Message - >> From: "vijay kilari" >> To: qemu-...@nongnu.org, "peter maydell" , >> pbonz...@redhat.com >> Cc: qemu-devel@nongnu.org, "Prasun Kapoor" , >> "vijay kilari" , >> "Vijaya Kumar K" >> Sent: Tuesday, August 2, 2016 12:20:15 PM >> Subject: [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register >> >> From: Vijaya Kumar K >> >> Add helper API to read MIDR_EL1 registers to fetch >> cpu identification information. This helps in >> adding errata's and architecture specific features. >> >> This is implemented only for arm architecture. >> >> Signed-off-by: Vijaya Kumar K >> --- >> include/qemu-common.h |1 + >> util/Makefile.objs|1 + >> util/cpuinfo.c| 52 >> + >> 3 files changed, 54 insertions(+) >> >> diff --git a/include/qemu-common.h b/include/qemu-common.h >> index 1f2cb94..62ad674 100644 >> --- a/include/qemu-common.h >> +++ b/include/qemu-common.h >> @@ -134,4 +134,5 @@ void page_size_init(void); >> * returned. */ >> bool dump_in_progress(void); >> >> +long int qemu_read_cpuid_info(void); > > First, please avoid adding to include/qemu-common.h (it really should > go away). > > Second, this is too generic a name. Please call it something like > qemu_read_aarch64_midr_el1. OK > > Third, it's probably a bad idea to call this function from generic code, so > make it static and add the detection function from patch 2/2 already here. > By making it static, it's also possible to define it only if CONFIG_LINUX > is defined; the ThunderX detection will then return false if !CONFIG_LINUX. > You mean to say, move contents of this patch to util/cutils.c and make it static and define under __aarch64__ and CONFIG_LINUX?. > Thanks, > > Paolo > >> #endif >> diff --git a/util/Makefile.objs b/util/Makefile.objs >> index 96cb1e0..9d25a72 100644 >> --- a/util/Makefile.objs >> +++ b/util/Makefile.objs >> @@ -35,3 +35,4 @@ util-obj-y += log.o >> util-obj-y += qdist.o >> util-obj-y += qht.o >> util-obj-y += range.o >> +util-obj-y += cpuinfo.o >> diff --git a/util/cpuinfo.c b/util/cpuinfo.c >> new file mode 100644 >> index 000..3ba7194 >> --- /dev/null >> +++ b/util/cpuinfo.c >> @@ -0,0 +1,52 @@ >> +/* >> + * Dealing with arm cpu identification information. >> + * >> + * Copyright (C) 2016 Cavium, Inc. >> + * >> + * Authors: >> + * Vijaya Kumar K >> + * >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 >> + * or later. See the COPYING.LIB file in the top-level directory. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> +#include "qemu/cutils.h" >> + >> +#if defined(__aarch64__) >> + >> +long int qemu_read_cpuid_info(void) >> +{ >> +FILE *fp; >> +char *buf; >> +long int midr = 0; >> +#define BUF_SIZE 32 >> + >> +fp = fopen("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1", >> + "r"); >> +if (!fp) { >> +return 0; >> +} >> + >> +buf = g_malloc0(BUF_SIZE); >> +if (!buf) { >> +fclose(fp); >> +return 0; >> +} >> + >> +if (buf != fgets(buf, BUF_SIZE - 1, fp)) { >> +goto out; >> +} >> + >> +if (qemu_strtol(buf, NULL, 0, &midr) < 0) { >> +goto out; >> +} >> + >> +out: >> +g_free(buf); >> +fclose(fp); >> + >> +return midr; >> +} >> +#endif >> -- >> 1.7.9.5 >> >>
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
On 2 August 2016 at 11:20, wrote: > +long int qemu_read_cpuid_info(void) Don't use "long" here, it might be 32 or 64 bits. The kernel ABI for the /sys/ file we're reading says it is a 64-bit value, so uint64_t is what you want. > +{ > +FILE *fp; > +char *buf; > +long int midr = 0; > +#define BUF_SIZE 32 > + > +fp = fopen("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1", > + "r"); > +if (!fp) { > +return 0; > +} > + > +buf = g_malloc0(BUF_SIZE); > +if (!buf) { > +fclose(fp); > +return 0; > +} > + > +if (buf != fgets(buf, BUF_SIZE - 1, fp)) { > +goto out; > +} g_file_get_contents() is probably easier than manually opening the file and reading it into an allocated buffer. > + > +if (qemu_strtol(buf, NULL, 0, &midr) < 0) { qemu_strtoull(). > +goto out; > +} > + > +out: > +g_free(buf); > +fclose(fp); > + > +return midr; > +} > +#endif > -- > 1.7.9.5 > thanks -- PMM
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
- Original Message - > From: "vijay kilari" > To: qemu-...@nongnu.org, "peter maydell" , > pbonz...@redhat.com > Cc: qemu-devel@nongnu.org, "Prasun Kapoor" , "vijay > kilari" , > "Vijaya Kumar K" > Sent: Tuesday, August 2, 2016 12:20:15 PM > Subject: [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register > > From: Vijaya Kumar K > > Add helper API to read MIDR_EL1 registers to fetch > cpu identification information. This helps in > adding errata's and architecture specific features. > > This is implemented only for arm architecture. > > Signed-off-by: Vijaya Kumar K > --- > include/qemu-common.h |1 + > util/Makefile.objs|1 + > util/cpuinfo.c| 52 > + > 3 files changed, 54 insertions(+) > > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 1f2cb94..62ad674 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -134,4 +134,5 @@ void page_size_init(void); > * returned. */ > bool dump_in_progress(void); > > +long int qemu_read_cpuid_info(void); First, please avoid adding to include/qemu-common.h (it really should go away). Second, this is too generic a name. Please call it something like qemu_read_aarch64_midr_el1. Third, it's probably a bad idea to call this function from generic code, so make it static and add the detection function from patch 2/2 already here. By making it static, it's also possible to define it only if CONFIG_LINUX is defined; the ThunderX detection will then return false if !CONFIG_LINUX. Thanks, Paolo > #endif > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 96cb1e0..9d25a72 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -35,3 +35,4 @@ util-obj-y += log.o > util-obj-y += qdist.o > util-obj-y += qht.o > util-obj-y += range.o > +util-obj-y += cpuinfo.o > diff --git a/util/cpuinfo.c b/util/cpuinfo.c > new file mode 100644 > index 000..3ba7194 > --- /dev/null > +++ b/util/cpuinfo.c > @@ -0,0 +1,52 @@ > +/* > + * Dealing with arm cpu identification information. > + * > + * Copyright (C) 2016 Cavium, Inc. > + * > + * Authors: > + * Vijaya Kumar K > + * > + * This work is licensed under the terms of the GNU LGPL, version 2.1 > + * or later. See the COPYING.LIB file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "qemu/cutils.h" > + > +#if defined(__aarch64__) > + > +long int qemu_read_cpuid_info(void) > +{ > +FILE *fp; > +char *buf; > +long int midr = 0; > +#define BUF_SIZE 32 > + > +fp = fopen("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1", > + "r"); > +if (!fp) { > +return 0; > +} > + > +buf = g_malloc0(BUF_SIZE); > +if (!buf) { > +fclose(fp); > +return 0; > +} > + > +if (buf != fgets(buf, BUF_SIZE - 1, fp)) { > +goto out; > +} > + > +if (qemu_strtol(buf, NULL, 0, &midr) < 0) { > +goto out; > +} > + > +out: > +g_free(buf); > +fclose(fp); > + > +return midr; > +} > +#endif > -- > 1.7.9.5 > >
[Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
From: Vijaya Kumar K Add helper API to read MIDR_EL1 registers to fetch cpu identification information. This helps in adding errata's and architecture specific features. This is implemented only for arm architecture. Signed-off-by: Vijaya Kumar K --- include/qemu-common.h |1 + util/Makefile.objs|1 + util/cpuinfo.c| 52 + 3 files changed, 54 insertions(+) diff --git a/include/qemu-common.h b/include/qemu-common.h index 1f2cb94..62ad674 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -134,4 +134,5 @@ void page_size_init(void); * returned. */ bool dump_in_progress(void); +long int qemu_read_cpuid_info(void); #endif diff --git a/util/Makefile.objs b/util/Makefile.objs index 96cb1e0..9d25a72 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -35,3 +35,4 @@ util-obj-y += log.o util-obj-y += qdist.o util-obj-y += qht.o util-obj-y += range.o +util-obj-y += cpuinfo.o diff --git a/util/cpuinfo.c b/util/cpuinfo.c new file mode 100644 index 000..3ba7194 --- /dev/null +++ b/util/cpuinfo.c @@ -0,0 +1,52 @@ +/* + * Dealing with arm cpu identification information. + * + * Copyright (C) 2016 Cavium, Inc. + * + * Authors: + * Vijaya Kumar K + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 + * or later. See the COPYING.LIB file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/cutils.h" + +#if defined(__aarch64__) + +long int qemu_read_cpuid_info(void) +{ +FILE *fp; +char *buf; +long int midr = 0; +#define BUF_SIZE 32 + +fp = fopen("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1", + "r"); +if (!fp) { +return 0; +} + +buf = g_malloc0(BUF_SIZE); +if (!buf) { +fclose(fp); +return 0; +} + +if (buf != fgets(buf, BUF_SIZE - 1, fp)) { +goto out; +} + +if (qemu_strtol(buf, NULL, 0, &midr) < 0) { +goto out; +} + +out: +g_free(buf); +fclose(fp); + +return midr; +} +#endif -- 1.7.9.5