[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_E
From: |
Vijay Kilari |
Subject: |
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register |
Date: |
Thu, 4 Aug 2016 14:46:37 +0530 |
Hi Paolo,
On Tue, Aug 2, 2016 at 4:18 PM, Paolo Bonzini <address@hidden> wrote:
> ----- Original Message -----
>> From: "vijay kilari" <address@hidden>
>> To: address@hidden, "peter maydell" <address@hidden>, address@hidden
>> Cc: address@hidden, "Prasun Kapoor" <address@hidden>, "vijay kilari"
>> <address@hidden>,
>> "Vijaya Kumar K" <address@hidden>
>> 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 <address@hidden>
>>
>> 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 <address@hidden>
>> ---
>> 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 0000000..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 <address@hidden>
>> + *
>> + * 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
>>
>>