From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 3A9387CA9 for ; Mon, 4 Sep 2017 11:38:13 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2017 02:38:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,474,1498546800"; d="scan'208";a="125362166" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by orsmga004.jf.intel.com with ESMTP; 04 Sep 2017 02:38:10 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by IRSMSX101.ger.corp.intel.com ([169.254.1.22]) with mapi id 14.03.0319.002; Mon, 4 Sep 2017 10:38:09 +0100 From: "Van Haaren, Harry" To: "Gonzalez Monroy, Sergio" , "dev@dpdk.org" CC: "Ananyev, Konstantin" , "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH] eal/x86: implement x86 specific tsc hz Thread-Index: AQHTHCCmmxMhyd0i80yPxGkivDo7caKkh0+w Date: Mon, 4 Sep 2017 09:38:08 +0000 Message-ID: References: <20170823150027.70565-1-sergio.gonzalez.monroy@intel.com> In-Reply-To: <20170823150027.70565-1-sergio.gonzalez.monroy@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjMzYWM3YzYtYWIwNy00YThhLTlkZWUtZGU0NjU0NWEyODM5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Iis1QVwvY0dEUlZQc0xxcHRnZEZXN1FNMjk5Qjlia3VOS2pJMDBFcXZnRzJrPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] eal/x86: implement x86 specific tsc hz X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Sep 2017 09:38:13 -0000 > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez Monr= oy > Sent: Wednesday, August 23, 2017 4:00 PM > To: dev@dpdk.org > Cc: Ananyev, Konstantin ; Richardson, Bruce > > Subject: [dpdk-dev] [PATCH] eal/x86: implement x86 specific tsc hz >=20 > First, try to use CPUID Time Stamp Counter and Nominal Core Crystal > Clock Information Leaf to determine the tsc hz on platforms that > supports it (does not require priviledge user). Checkpatch notifies me that "privilege" is spelled wrong (once above, once = below). > If the CPUID leaf is not available, then try to determine the tsc hz by > reading the MSR 0xCE (requires priviledge user). >=20 > Default to the tsc hz estimation if both methods fail. >=20 > Signed-off-by: Sergio Gonzalez Monroy > --- >=20 > DEPENDS on Jerin's patch: > http://dpdk.org/dev/patchwork/patch/27526/ >=20 > lib/librte_eal/common/arch/x86/rte_cycles.c | 142 +++++++++++++++= ++++++ > .../common/include/arch/x86/rte_cycles.h | 7 +- > lib/librte_eal/linuxapp/eal/Makefile | 1 + > 3 files changed, 145 insertions(+), 5 deletions(-) > create mode 100644 lib/librte_eal/common/arch/x86/rte_cycles.c >=20 > diff --git a/lib/librte_eal/common/arch/x86/rte_cycles.c > b/lib/librte_eal/common/arch/x86/rte_cycles.c > new file mode 100644 > index 0000000..9336947 > --- /dev/null > +++ b/lib/librte_eal/common/arch/x86/rte_cycles.c > @@ -0,0 +1,142 @@ > +/*- > + * BSD LICENSE > + * > + * Copyright(c) 2017 Intel Corporation. All rights reserved. > + * All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * * Redistributions in binary form must reproduce the above copyrig= ht > + * notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the > + * distribution. > + * * Neither the name of Intel Corporation nor the names of its > + * contributors may be used to endorse or promote products derived > + * from this software without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS F= OR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGH= T > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTA= L, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US= E, > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON A= NY > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE U= SE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE= . > + */ > + > +#include > +#include > +#include > +#include > + > +static unsigned int > +rte_cpu_get_model(uint32_t fam_mod_step) > +{ > + uint32_t family, model, ext_model; > + > + family =3D (fam_mod_step >> 8) & 0xf; > + model =3D (fam_mod_step >> 4) & 0xf; > + > + if (family =3D=3D 6 || family =3D=3D 15) { > + ext_model =3D (fam_mod_step >> 16) & 0xf; > + model +=3D (ext_model << 4); > + } > + > + return model; > +} > + > +static int32_t > +rdmsr(int msr, uint64_t *val) > +{ > + int fd; > + int ret =3D 0; Initialization of ret will always be overwritten by pread() below, so no ne= ed to initialize. > + > + fd =3D open("/dev/cpu/0/msr", O_RDONLY); > + if (fd < 0) > + return fd; > + > + ret =3D pread(fd, val, sizeof(uint64_t), msr); > + > + close(fd); > + > + return ret; > +} > + > +static uint32_t > +check_model_wsm_nhm(uint8_t model) > +{ > + switch (model) { > + /* Westmere */ > + case 0x25: > + case 0x2C: > + case 0x2F: > + /* Nehalem */ > + case 0x1E: > + case 0x1F: > + case 0x1A: > + case 0x2E: > + return 1; > + } DPDK coding standards say /* fallthrough */ comments required when falling = through cases. In this case I feel it would reduce readability, more than it improves it, = but I recall some recent gcc/clang prints warnings if no /* fallthrough */ comments exis= t.. opinions? Same for switch() below. > + > + return 0; > +} > + > +static uint32_t > +check_model_gdm_dnv(uint8_t model) > +{ > + switch (model) { > + /* Goldmont */ > + case 0x5C: > + /* Denverton */ > + case 0x5F: > + return 1; > + } > + > + return 0; > +} > + > +uint64_t > +rte_rdtsc_arch_hz(void) > +{ > + uint64_t tsc_hz =3D 0; > + uint32_t a, b, c, d, maxleaf; > + uint8_t mult, model; > + int32_t ret; > + > + /* > + * Time Stamp Counter and Nominal Core Crystal Clock > + * Information Leaf > + */ > + maxleaf =3D __get_cpuid_max(0, NULL); > + > + if (maxleaf >=3D 0x15) { > + __cpuid(0x15, a, b, c, d); > + > + /* EBX : TSC/Crystal ratio, ECX : Crystal Hz */ > + if (b && c) > + return c * (b / a); > + } > + > + __cpuid(0x1, a, b, c, d); > + model =3D rte_cpu_get_model(a); > + > + if (check_model_wsm_nhm(model)) > + mult =3D 133; > + else if ((c & bit_AVX) || check_model_gdm_dnv(model)) > + mult =3D 100; > + else > + return 0; > + > + ret =3D rdmsr(0xCE, &tsc_hz); > + if (!(ret < 0)) > + return ((tsc_hz >> 8) & 0xff) * mult * 1E6; > + > + return 0; if (!(ret < 0)) feels a little awkward, end of the function could be rewo= rked to, which reads a little cleaner to me? + if (ret < 0) + return 0; + return ((tsc_hz >> 8) & 0xff) * mult * 1E6; > +} > diff --git a/lib/librte_eal/common/include/arch/x86/rte_cycles.h > b/lib/librte_eal/common/include/arch/x86/rte_cycles.h > index e2661e2..0db89dc 100644 > --- a/lib/librte_eal/common/include/arch/x86/rte_cycles.h > +++ b/lib/librte_eal/common/include/arch/x86/rte_cycles.h > @@ -84,11 +84,8 @@ rte_rdtsc(void) > * The number of rdtsc cycles in one second. Return zero if the archit= ecture > * support is not available. > */ > -static inline uint64_t > -rte_rdtsc_arch_hz(void) > -{ > - return 0; > -} > +uint64_t > +rte_rdtsc_arch_hz(void); >=20 > static inline uint64_t > rte_rdtsc_precise(void) > diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxa= pp/eal/Makefile > index 90bca4d..9d44828 100644 > --- a/lib/librte_eal/linuxapp/eal/Makefile > +++ b/lib/librte_eal/linuxapp/eal/Makefile > @@ -104,6 +104,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) +=3D rte_service= .c > # from arch dir > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) +=3D rte_cpuflags.c > SRCS-$(CONFIG_RTE_ARCH_X86) +=3D rte_spinlock.c > +SRCS-$(CONFIG_RTE_ARCH_X86) +=3D rte_cycles.c >=20 > CFLAGS_eal_common_cpuflags.o :=3D $(CPUFLAGS_LIST) >=20 > -- > 2.9.5 With above changes, and optionally /* fallthrough */ comments; Acked-by: Harry van Haaren