On Fri, Sep 22, 2023 at 11:47 AM Bruce Richardson
<bruce.richard...@intel.com> wrote:
>
> On Fri, Sep 22, 2023 at 11:37:20AM +0200, David Marchand wrote:
> > In some really specific cases, it may be needed to get a detailed
> > information on the processor running a DPDK application for drivers to
> > achieve better performance, or for matters that concern only them.
> >
> > Those information are highly arch-specific and require a specific API.
> >
> > Introduce a set of functions to get brand, family and model of a x86
> > processor.
> > Those functions do not make sense on other arches and a
> > driver must first check rte_cpu_is_x86() before anything else.
> >
> > Signed-off-by: David Marchand <david.march...@redhat.com>
> > ---
> >  MAINTAINERS                     |   1 +
> >  app/test/meson.build            |   1 +
> >  app/test/test_cpu.c             |  37 +++++++++
> >  lib/eal/common/eal_common_cpu.c | 141 ++++++++++++++++++++++++++++++++
> >  lib/eal/common/eal_cpu.h        |  77 +++++++++++++++++
> >  lib/eal/common/meson.build      |   1 +
> >  lib/eal/version.map             |   6 ++
> >  7 files changed, 264 insertions(+)
> >  create mode 100644 app/test/test_cpu.c
> >  create mode 100644 lib/eal/common/eal_common_cpu.c
> >  create mode 100644 lib/eal/common/eal_cpu.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 698608cdb2..b87d47a1e4 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -158,6 +158,7 @@ F: app/test/test_barrier.c
> >  F: app/test/test_bitcount.c
> >  F: app/test/test_byteorder.c
> >  F: app/test/test_common.c
> > +F: app/test/test_cpu.c
> >  F: app/test/test_cpuflags.c
> >  F: app/test/test_cycles.c
> >  F: app/test/test_debug.c
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 05bae9216d..4b37ad02fa 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -44,6 +44,7 @@ source_file_deps = {
> >      'test_cmdline_string.c': [],
> >      'test_common.c': [],
> >      'test_compressdev.c': ['compressdev'],
> > +    'test_cpu.c': [],
> >      'test_cpuflags.c': [],
> >      'test_crc.c': ['net'],
> >      'test_cryptodev.c': test_cryptodev_deps,
> > diff --git a/app/test/test_cpu.c b/app/test/test_cpu.c
> > new file mode 100644
> > index 0000000000..40d8bd94eb
> > --- /dev/null
> > +++ b/app/test/test_cpu.c
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2023 Red Hat, Inc.
> > + */
> > +
> > +#include <stdio.h>
> > +#include <inttypes.h>
> > +
> > +#include "eal_cpu.h"
> > +
> > +#include "test.h"
> > +
> > +static int
> > +test_cpu(void)
> > +{
> > +#ifndef RTE_ARCH_X86
> > +     RTE_TEST_ASSERT(!rte_cpu_is_x86(), "rte_cpu_is_x86() returned true on 
> > " RTE_STR(RTE_ARCH));
> > +#else
> > +     const char *vendor;
> > +
> > +     RTE_TEST_ASSERT(rte_cpu_is_x86(), "rte_cpu_is_x86() returned false");
> > +
> > +     if (rte_cpu_x86_is_amd())
> > +             vendor = "AMD";
> > +     else if (rte_cpu_x86_is_intel())
> > +             vendor = "Intel";
> > +     else
> > +             vendor = "unknown";
> > +
> > +     printf("The processor running this program is a x86 %s processor, 
> > brand=0x%"
> > +             PRIx8", family=0x%"PRIx8", model=0x%"PRIx8"\n", vendor, 
> > rte_cpu_x86_brand(),
> > +             rte_cpu_x86_family(), rte_cpu_x86_model());
> > +#endif
> > +
> > +     return TEST_SUCCESS;
> > +}
> > +
> > +REGISTER_FAST_TEST(cpu_autotest, true, true, test_cpu);
> > diff --git a/lib/eal/common/eal_common_cpu.c 
> > b/lib/eal/common/eal_common_cpu.c
> > new file mode 100644
> > index 0000000000..18cdb27f75
> > --- /dev/null
> > +++ b/lib/eal/common/eal_common_cpu.c
> > @@ -0,0 +1,141 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2023 Red Hat, Inc.
> > + */
> > +
> > +#include <rte_debug.h>
> > +
> > +#include "eal_cpu.h"
> > +
> > +#ifdef RTE_ARCH_X86
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > +#include <cpuid.h>
> > +#endif
> > +
> > +static void
> > +x86_cpuid(uint32_t leaf, uint32_t subleaf, uint32_t *eax, uint32_t *ebx,
> > +     uint32_t *ecx, uint32_t *edx)
> > +{
> > +     uint32_t regs[4] = { 0 };
> > +
> > +#ifdef RTE_TOOLCHAIN_MSVC
> > +     __cpuidex(regs, leaf, subleaf);
> > +#else
> > +     __cpuid_count(leaf, subleaf, regs[0], regs[1], regs[2], regs[3]);
> > +#endif
> > +
> > +     *eax = regs[0];
> > +     *ebx = regs[1];
> > +     *ecx = regs[2];
> > +     *edx = regs[3];
> > +}
> > +#endif /* RTE_ARCH_X86 */
>
> From a readability perspective, I think it would be better to expand the
> scope of this ifdef and have two copies of each function in the file,
> rather than a single copy of each with #ifdefs. WDYT?

I can have a try later, but if you want to take over the series, I
don't mind :-).


-- 
David Marchand

Reply via email to