| Message ID | 1460023087-31509-3-git-send-email-vijayak@caviumnetworks.com |
|---|---|
| State | New |
| Headers | show |
On 7 April 2016 at 10:58, <vijayak@caviumnetworks.com> wrote: > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > > utils cannot read target cpu information to > fetch cpu information to implement cpu specific > features or erratas. For this parse /proc/cpuinfo > and fetch cpu information. > > For now this helper only fetches cpu information > for arm architectures. As I understand it /proc/cpuinfo is intended only for humans to read. Please don't write code to parse it; find a different way to get this information instead if you really need it. (I'm not really happy about such specific-to-a-particular-vendor patches in QEMU anyway; we should have migration code that works acceptably for any implementation.) thanks -- PMM
On Thu, Apr 7, 2016 at 3:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 7 April 2016 at 10:58, <vijayak@caviumnetworks.com> wrote: >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >> >> utils cannot read target cpu information to >> fetch cpu information to implement cpu specific >> features or erratas. For this parse /proc/cpuinfo >> and fetch cpu information. >> >> For now this helper only fetches cpu information >> for arm architectures. > > As I understand it /proc/cpuinfo is intended only for > humans to read. Please don't write code to parse it; > find a different way to get this information instead > if you really need it. The utils code does not accept any dependency with target specific code. The libqemuutil.a is compiled and linked before target specific code is compiled. Also, utils functions neither have any cpu object to fetch cpu identification information (ex: midr in case of arm) to identify the cpu information nor utils cannot make any ioctl to read cpu information from qemu. Also unlike x86 there is no cpuid.h where we can get cpu identification information for arm64. So, I think userspace process can rely on /proc/cpuinfo for fetching cpu information. > > (I'm not really happy about such specific-to-a-particular-vendor > patches in QEMU anyway; we should have migration code that > works acceptably for any implementation.) > > thanks > -- PMM
On 7 April 2016 at 11:56, Vijay Kilari <vijay.kilari@gmail.com> wrote: > On Thu, Apr 7, 2016 at 3:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 7 April 2016 at 10:58, <vijayak@caviumnetworks.com> wrote: >>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >>> >>> utils cannot read target cpu information to >>> fetch cpu information to implement cpu specific >>> features or erratas. For this parse /proc/cpuinfo >>> and fetch cpu information. >>> >>> For now this helper only fetches cpu information >>> for arm architectures. >> >> As I understand it /proc/cpuinfo is intended only for >> humans to read. Please don't write code to parse it; >> find a different way to get this information instead >> if you really need it. > Also unlike x86 there is no cpuid.h where we can get cpu identification > information for arm64. I'm told there are kernel patches in progress to get this sort of information in a maintainable way to userspace, which are currently somewhat stalled due to lack of anybody who wants to consume it. If you have a use case then you should probably flag it up with the kernel devs. That said, I think we should probably hold off on this discussion until we have clearer benchmarking info that demonstrates that doing these prefetches really does make a significant difference. I would much prefer to have a single aarch64 routine that works for everybody, rather than a thunderx-only special case. thanks -- PMM
Hi Peter, On Thu, Apr 7, 2016 at 5:15 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 7 April 2016 at 11:56, Vijay Kilari <vijay.kilari@gmail.com> wrote: >> On Thu, Apr 7, 2016 at 3:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 7 April 2016 at 10:58, <vijayak@caviumnetworks.com> wrote: >>>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >>>> >>>> utils cannot read target cpu information to >>>> fetch cpu information to implement cpu specific >>>> features or erratas. For this parse /proc/cpuinfo >>>> and fetch cpu information. >>>> >>>> For now this helper only fetches cpu information >>>> for arm architectures. >>> >>> As I understand it /proc/cpuinfo is intended only for >>> humans to read. Please don't write code to parse it; >>> find a different way to get this information instead >>> if you really need it. > >> Also unlike x86 there is no cpuid.h where we can get cpu identification >> information for arm64. > > I'm told there are kernel patches in progress to get this sort > of information in a maintainable way to userspace, which are > currently somewhat stalled due to lack of anybody who wants to > consume it. If you have a use case then you should probably > flag it up with the kernel devs. Can you please give references to those patches/discussion? > > That said, I think we should probably hold off on this > discussion until we have clearer benchmarking info that > demonstrates that doing these prefetches really does make > a significant difference. I would much prefer to have a Thunderx pass2 board does not have hardware prefetch. So explicit sw prefetch instructions is required for this platform. Here is the benchmarking result with and without prefetch. of an idle VM with 4 VCPUS, 8GB RAM. Without prefech, total migration time is 8.2 seconds With prefetch total migration time is 2.7 seconds. Without prefetch: ------------------------ (qemu) info migrate capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off Migration status: completed total time: 8217 milliseconds downtime: 86 milliseconds setup: 4 milliseconds transferred ram: 212624 kbytes throughput: 212.08 mbps remaining ram: 0 kbytes total ram: 8520128 kbytes duplicate: 2085805 pages skipped: 0 pages normal: 48478 pages normal bytes: 193912 kbytes dirty sync count: 3 With prefetch: -------------------- (qemu) info migrate capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off Migration status: completed total time: 2744 milliseconds downtime: 48 milliseconds setup: 5 milliseconds transferred ram: 213526 kbytes throughput: 637.76 mbps remaining ram: 0 kbytes total ram: 8520128 kbytes duplicate: 2085014 pages skipped: 0 pages normal: 48705 pages normal bytes: 194820 kbytes dirty sync count: 3 > single aarch64 routine that works for everybody, rather > than a thunderx-only special case. Now, I found that the generic existings function by name buffer_find_nonzero_offset_inner() can be made to work with neon. So no need of special function by name buffer_find_nonzero_offset_neon() for arm64 creating in this patch series. However, adding prefetch code needs to be added for performance reason.
On 8 April 2016 at 07:21, Vijay Kilari <vijay.kilari@gmail.com> wrote: > On Thu, Apr 7, 2016 at 5:15 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> I'm told there are kernel patches in progress to get this sort >> of information in a maintainable way to userspace, which are >> currently somewhat stalled due to lack of anybody who wants to >> consume it. If you have a use case then you should probably >> flag it up with the kernel devs. > > Can you please give references to those patches/discussion? I'm told the most recent thread is https://lkml.org/lkml/2015/10/5/517 (and that most of the patches in that series have gone in, except for the last 4 or 5 which implement the ABI). thanks -- PMM
Adding Suzuki Poulose. Hi Suzuki, On Fri, Apr 8, 2016 at 3:13 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 8 April 2016 at 07:21, Vijay Kilari <vijay.kilari@gmail.com> wrote: >> On Thu, Apr 7, 2016 at 5:15 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> I'm told there are kernel patches in progress to get this sort >>> of information in a maintainable way to userspace, which are >>> currently somewhat stalled due to lack of anybody who wants to >>> consume it. If you have a use case then you should probably >>> flag it up with the kernel devs. >> >> Can you please give references to those patches/discussion? > > I'm told the most recent thread is https://lkml.org/lkml/2015/10/5/517 > (and that most of the patches in that series have gone in, except > for the last 4 or 5 which implement the ABI). Can you please throw some light on what is the status of ABI to read cpu information in user space. I wanted to know cpu implementer, part number in QEMU utils to add prefetches to speed up live migration for Thunderx platform.
On 11/04/16 07:52, Vijay Kilari wrote: > Adding Suzuki Poulose. > > Hi Suzuki, > > On Fri, Apr 8, 2016 at 3:13 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 8 April 2016 at 07:21, Vijay Kilari <vijay.kilari@gmail.com> wrote: >>> On Thu, Apr 7, 2016 at 5:15 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> I'm told there are kernel patches in progress to get this sort >>>> of information in a maintainable way to userspace, which are >>>> currently somewhat stalled due to lack of anybody who wants to >>>> consume it. If you have a use case then you should probably >>>> flag it up with the kernel devs. >>> >>> Can you please give references to those patches/discussion? >> >> I'm told the most recent thread is https://lkml.org/lkml/2015/10/5/517 >> (and that most of the patches in that series have gone in, except >> for the last 4 or 5 which implement the ABI). > > Can you please throw some light on what is the status of ABI to > read cpu information in user space. > I wanted to know cpu implementer, part number in QEMU utils > to add prefetches to speed up live migration for Thunderx platform. > As for the patch series, except for that last 5 patches (which actually implements the ABI), the infrastructure patches have been merged in v4.4. We are awaiting feedback from possible consumers like toolchain (gcc, glibc). If you think this will be suitable for you, thats good to know. There is documentation available in the last patch in the above series. Could you please try the series (on v4.4, which would be easier, by simply picking up the last 5 patches) and let us know if that works for you ? Cheers Suzuki
On Mon, Apr 11, 2016 at 3:07 PM, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > On 11/04/16 07:52, Vijay Kilari wrote: >> >> Adding Suzuki Poulose. >> >> Hi Suzuki, >> >> On Fri, Apr 8, 2016 at 3:13 PM, Peter Maydell <peter.maydell@linaro.org> >> wrote: >>> >>> On 8 April 2016 at 07:21, Vijay Kilari <vijay.kilari@gmail.com> wrote: >>>> >>>> On Thu, Apr 7, 2016 at 5:15 PM, Peter Maydell <peter.maydell@linaro.org> >>>> wrote: >>>>> >>>>> I'm told there are kernel patches in progress to get this sort >>>>> of information in a maintainable way to userspace, which are >>>>> currently somewhat stalled due to lack of anybody who wants to >>>>> consume it. If you have a use case then you should probably >>>>> flag it up with the kernel devs Hi Peter, Looks like getting Suzuki's patches merged might take some time. I propose to use /proc/cpuinfo for now and later I can move to using Suzuki's way. >>>> >>>> >>>> Can you please give references to those patches/discussion? >>> >>> >>> I'm told the most recent thread is https://lkml.org/lkml/2015/10/5/517 >>> (and that most of the patches in that series have gone in, except >>> for the last 4 or 5 which implement the ABI). >> >> >> Can you please throw some light on what is the status of ABI to >> read cpu information in user space. >> I wanted to know cpu implementer, part number in QEMU utils >> to add prefetches to speed up live migration for Thunderx platform. >> > > As for the patch series, except for that last 5 patches (which actually > implements > the ABI), the infrastructure patches have been merged in v4.4. > > We are awaiting feedback from possible consumers like toolchain (gcc, > glibc). > If you think this will be suitable for you, thats good to know. There is > documentation available in the last patch in the above series. Could you > please > try the series (on v4.4, which would be easier, by simply picking up the > last > 5 patches) and let us know if that works for you ? Hi Suzuki, The last 5 patches are not compiling on v4.4. Looks like your patch series is not merged completely. Can you please rebase your patches and let me know.
On 13/04/16 10:54, Vijay Kilari wrote: > On Mon, Apr 11, 2016 at 3:07 PM, Suzuki K Poulose > <Suzuki.Poulose@arm.com> wrote: >> On 11/04/16 07:52, Vijay Kilari wrote: > > Hi Suzuki, > > The last 5 patches are not compiling on v4.4. Looks like your patch > series is not merged completely. Can you please > rebase your patches and let me know. > Could you please give the tree below a try ? git://linux-arm.org/linux-skp.git cpu-ftr/v3-4.3-rc4 Cheers Suzuki
Hi Suzuki/Peter, On Wed, Apr 13, 2016 at 5:59 PM, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > On 13/04/16 10:54, Vijay Kilari wrote: >> >> On Mon, Apr 11, 2016 at 3:07 PM, Suzuki K Poulose >> <Suzuki.Poulose@arm.com> wrote: >>> >>> On 11/04/16 07:52, Vijay Kilari wrote: > > >> >> Hi Suzuki, >> >> The last 5 patches are not compiling on v4.4. Looks like your patch >> series is not merged completely. Can you please >> rebase your patches and let me know. >> > > Could you please give the tree below a try ? > > git://linux-arm.org/linux-skp.git cpu-ftr/v3-4.3-rc4 This works. Now the question is, Are your patches getting merged anytime soon?. If not, I prefer to go with /proc/cpuinfo. Another solution is look for /sys/devices/system/cpu/cpu$ID/identification/midr if not available then fall back on /proc/cpuinfo. Regards Vijay
On 09/05/16 04:30, Vijay Kilari wrote: >>> Hi Suzuki, >>> >>> The last 5 patches are not compiling on v4.4. Looks like your patch >>> series is not merged completely. Can you please >>> rebase your patches and let me know. >>> >> >> Could you please give the tree below a try ? >> >> git://linux-arm.org/linux-skp.git cpu-ftr/v3-4.3-rc4 > > This works. > Now the question is, Are your patches getting merged anytime soon?. Well, we have been waiting for a use case, like this, before we merge the series. Will, Catalin, Now that we have some real users of the infrastructure, what do you think ? I can post an updated/rebased series, if you would like. Suzuki > If not, I prefer to go with /proc/cpuinfo. > > Another solution is look for /sys/devices/system/cpu/cpu$ID/identification/midr > if not available then fall back on /proc/cpuinfo. > > Regards > Vijay >
On 9 May 2016 at 11:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > Well, we have been waiting for a use case, like this, before we merge > the series. This isn't a great strategy for moving people away from things you'd like them to avoid like parsing /proc/cpuinfo, because typically userspace app writers are not very interested in coding to facilities which don't exist yet, and will prefer to make do with what's actually present in the kernel today... You need to provide the improved API, and then it needs to get out into kernel versions in distros and otherwise, and only then are you likely to get app developers who will start to say "this is useful". thanks -- PMM
On Mon, May 09, 2016 at 12:21:08PM +0100, Peter Maydell wrote: > On 9 May 2016 at 11:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > > Well, we have been waiting for a use case, like this, before we merge > > the series. > > This isn't a great strategy for moving people away from things > you'd like them to avoid like parsing /proc/cpuinfo, because typically > userspace app writers are not very interested in coding to facilities > which don't exist yet, and will prefer to make do with what's actually > present in the kernel today... You need to provide the improved API, > and then it needs to get out into kernel versions in distros and > otherwise, and only then are you likely to get app developers who > will start to say "this is useful". The problem is that the way kernel people think the API may be improved does not always match the use-cases required by app writers. One example here is exposing MIDR via MRS emulation, we know there are problems with big.LITTLE and the only clear answer I got so far is that we ignore such configurations. We don't even have a way to tell user space that this is a heterogeneous CPU configuration, unless we add another HWCAP bit specifically for this (or the opposite: HWCAP_HOMOGENEOUS_CPUS). That said, I'm perfectly fine with exposing: /sys/devices/system/cpu/cpu$ID/identification/ \- midr \- revidr I had the wrong impression that we already merged this part but Suzuki just pointed out to me that it's not. I think our 4.7-rc1 tree is pretty much frozen to new features now, though the sysfs patch is relatively small (I'll let Will comment): https://patches.linaro.org/patch/54502/ The MRS emulation, we should restart the discussion around big.LITTLE implications and make a decision one way or another by the 4.8 merging window.
On Mon, May 09, 2016 at 01:44:11PM +0000, Catalin Marinas wrote: > On Mon, May 09, 2016 at 12:21:08PM +0100, Peter Maydell wrote: > > On 9 May 2016 at 11:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > > > Well, we have been waiting for a use case, like this, before we merge > > > the series. > > > > This isn't a great strategy for moving people away from things > > you'd like them to avoid like parsing /proc/cpuinfo, because typically > > userspace app writers are not very interested in coding to facilities > > which don't exist yet, and will prefer to make do with what's actually > > present in the kernel today... You need to provide the improved API, > > and then it needs to get out into kernel versions in distros and > > otherwise, and only then are you likely to get app developers who > > will start to say "this is useful". > > The problem is that the way kernel people think the API may be improved > does not always match the use-cases required by app writers. One example > here is exposing MIDR via MRS emulation, we know there are problems with > big.LITTLE and the only clear answer I got so far is that we ignore such > configurations. We don't even have a way to tell user space that this is > a heterogeneous CPU configuration, unless we add another HWCAP bit > specifically for this (or the opposite: HWCAP_HOMOGENEOUS_CPUS). Personally, I think we should expose big.LITTLE as-is to userspace. That is, if you execute an mrs instruction you'll get whichever core the emulation happens to run on. This might even be useful to things like pinned threadpools w/ userspace schedulers sitting on top. > That said, I'm perfectly fine with exposing: > > /sys/devices/system/cpu/cpu$ID/identification/ > \- midr > \- revidr > > I had the wrong impression that we already merged this part but Suzuki > just pointed out to me that it's not. Yes, there are use-cases for this interface as well. I don't think it's a choice between one or the other and I firmly believe we need both (the sysfs and mrs code). > I think our 4.7-rc1 tree is pretty much frozen to new features now, > though the sysfs patch is relatively small (I'll let Will comment): The merge window opens in less than a week, so it's fixes only atm. Will
Cc'ing Ramana to get some input from the toolchain side. On Tue, May 10, 2016 at 11:24:04AM +0100, Will Deacon wrote: > On Mon, May 09, 2016 at 01:44:11PM +0000, Catalin Marinas wrote: > > On Mon, May 09, 2016 at 12:21:08PM +0100, Peter Maydell wrote: > > > On 9 May 2016 at 11:59, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > > > > Well, we have been waiting for a use case, like this, before we merge > > > > the series. > > > > > > This isn't a great strategy for moving people away from things > > > you'd like them to avoid like parsing /proc/cpuinfo, because typically > > > userspace app writers are not very interested in coding to facilities > > > which don't exist yet, and will prefer to make do with what's actually > > > present in the kernel today... You need to provide the improved API, > > > and then it needs to get out into kernel versions in distros and > > > otherwise, and only then are you likely to get app developers who > > > will start to say "this is useful". > > > > The problem is that the way kernel people think the API may be improved > > does not always match the use-cases required by app writers. One example > > here is exposing MIDR via MRS emulation, we know there are problems with > > big.LITTLE and the only clear answer I got so far is that we ignore such > > configurations. We don't even have a way to tell user space that this is > > a heterogeneous CPU configuration, unless we add another HWCAP bit > > specifically for this (or the opposite: HWCAP_HOMOGENEOUS_CPUS). > > Personally, I think we should expose big.LITTLE as-is to userspace. That > is, if you execute an mrs instruction you'll get whichever core the > emulation happens to run on. This might even be useful to things like > pinned threadpools w/ userspace schedulers sitting on top. That's the point I try to make. We "think" there may be use-cases but there are no concrete examples yet. IIRC, the only request for mrs handling came from the tools guys for the ifunc support. However, they don't seem to have a solution for big.LITTLE either and they may simply ignore this feature. OTOH, we have to maintain it in the kernel on the long run because it became ABI (that said, I would be fine if this was complemented by another HWCAP bit for heterogeneous systems). The CPU feature registers wouldn't be affected by the big.LITTLE configurations as we provide a sanitised version anyway. But, again, do we have concrete use-cases? > > That said, I'm perfectly fine with exposing: > > > > /sys/devices/system/cpu/cpu$ID/identification/ > > \- midr > > \- revidr > > > > I had the wrong impression that we already merged this part but Suzuki > > just pointed out to me that it's not. > > Yes, there are use-cases for this interface as well. I don't think it's > a choice between one or the other and I firmly believe we need both (the > sysfs and mrs code). At least for this one we have a clear use-case: JVM and errata workarounds. > > I think our 4.7-rc1 tree is pretty much frozen to new features now, > > though the sysfs patch is relatively small (I'll let Will comment): > > The merge window opens in less than a week, so it's fixes only atm. We have more time to debate ;)
diff --git a/include/qemu-common.h b/include/qemu-common.h index 163bcbb..364aa0a 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -170,4 +170,15 @@ void page_size_init(void); * returned. */ bool dump_in_progress(void); +/* + * cpu info structure read from /proc/cpuinfo + */ + +struct cpu_info { + uint32_t imp; + uint32_t arch; + uint32_t part; +}; + +void qemu_read_cpu_info(struct cpu_info *cinf); #endif diff --git a/util/Makefile.objs b/util/Makefile.objs index a8a777e..59cde64 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -32,3 +32,4 @@ util-obj-y += buffer.o util-obj-y += timed-average.o util-obj-y += base64.o util-obj-y += log.o +util-obj-y += cpuinfo.o diff --git a/util/cpuinfo.c b/util/cpuinfo.c new file mode 100644 index 0000000..e049672 --- /dev/null +++ b/util/cpuinfo.c @@ -0,0 +1,94 @@ +/* + * Dealing with /proc/cpuinfo + * + * Copyright (C) 2016 Cavium, Inc. + * + * Authors: + * Vijaya Kumar K <vijayak@caviumnetworks.com> + * + * 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 <string.h> + +#if defined(__arm__) || defined(__aarch64__) +static uint32_t read_arm_cpu_implementer(char *str) +{ + char *match; + uint32_t imp = 0; + + match = strstr(str, "CPU implementer"); + if (match != NULL) { + sscanf(match, "CPU implementer : 0x%x", &imp); + } + + return imp; +} + +static uint32_t read_arm_cpu_architecture(char *str) +{ + char *match; + uint32_t arch = 0; + + match = strstr(str, "CPU architecture"); + if (match != NULL) { + sscanf(match, "CPU architecture: %d", &arch); + } + + return arch; +} + +static uint32_t read_arm_cpu_part(char *str) +{ + char *match; + uint32_t part = 0; + + match = strstr(str, "CPU part"); + if (match != NULL) { + sscanf(match, "CPU part : 0x%x", &part); + } + + return part; +} +#endif + +void qemu_read_cpu_info(struct cpu_info *cinf) +{ + FILE *fp; + char *buf; +#define BUF_SIZE 1024 + size_t bytes_read; + + cinf->imp = cinf->arch = cinf->part = 0; + fp = fopen("/proc/cpuinfo", "r"); + if (!fp) { + return; + } + + buf = g_malloc0(BUF_SIZE); + if (!buf) { + fclose(fp); + return; + } + + /* Read the contents of /proc/cpuinfo into the buffer. */ + bytes_read = fread(buf, 1, BUF_SIZE, fp); + fclose(fp); + + if (bytes_read == 0) { + g_free(buf); + return; + } + + buf[bytes_read] = '\0'; + +#if defined(__arm__) || defined(__aarch64__) + cinf->imp = read_arm_cpu_implementer(buf); + cinf->arch = read_arm_cpu_architecture(buf); + cinf->part = read_arm_cpu_part(buf); +#endif + g_free(buf); +}