[v14,05/23] event/dlb: add inline functions
Checks
Commit Message
Add miscellaneous inline functions that may be called
from multiple files. These functions include inline
assembly of new x86 instructions, such as movdir64b,
since they are not available as builtin functions in
the minimum supported GCC version.
Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
Reviewed-by: Gage Eads <gage.eads@intel.com>
---
drivers/event/dlb/dlb_inline_fns.h | 40 ++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 drivers/event/dlb/dlb_inline_fns.h
Comments
On Sat, Oct 31, 2020 at 7:17 PM Timothy McDaniel
<timothy.mcdaniel@intel.com> wrote:
>
> Add miscellaneous inline functions that may be called
> from multiple files. These functions include inline
> assembly of new x86 instructions, such as movdir64b,
> since they are not available as builtin functions in
> the minimum supported GCC version.
>
> Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> Reviewed-by: Gage Eads <gage.eads@intel.com>
> ---
> drivers/event/dlb/dlb_inline_fns.h | 40 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 drivers/event/dlb/dlb_inline_fns.h
>
> diff --git a/drivers/event/dlb/dlb_inline_fns.h b/drivers/event/dlb/dlb_inline_fns.h
> new file mode 100644
> index 0000000..1ecddb7
> --- /dev/null
> +++ b/drivers/event/dlb/dlb_inline_fns.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2016-2020 Intel Corporation
> + */
> +
Missing banners, like in dlb2 header (caught by diffing with it).
#ifndef __DLB_INLINE_FNS_H__
etc...
> +#include "rte_memcpy.h"
> +#include "rte_io.h"
> +
> +/* Inline functions required in more than one source file. */
> +
> +static inline struct dlb_eventdev *
> +dlb_pmd_priv(const struct rte_eventdev *eventdev)
> +{
> + return eventdev->data->dev_private;
> +}
> +
> +static inline void
> +dlb_movntdq_single(void *dest, void *src)
> +{
> + long long *_src = (long long *)src;
> + __m128i src_data0 = (__m128i){_src[0], _src[1]};
> +
> + _mm_stream_si128(dest, src_data0);
> +}
> +
> +static inline void
> +dlb_cldemote(void *addr)
> +{
> + /* Load addr into RSI, then demote the cache line of the address
> + * contained in that register.
> + */
> + asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (addr));
> +}
Use EAL API.
> +
> +static inline void
> +dlb_movdir64b(void *dest, void *src)
> +{
> + asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
> + :
> + : "a" (dest), "d" (src));
> +}
NO!
We introduced stuff in EAL for this, please double check.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Saturday, October 31, 2020 4:54 PM
> To: McDaniel, Timothy <timothy.mcdaniel@intel.com>
> Cc: dev <dev@dpdk.org>; Carrillo, Erik G <erik.g.carrillo@intel.com>; Eads,
> Gage <gage.eads@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v14 05/23] event/dlb: add inline functions
>
> On Sat, Oct 31, 2020 at 7:17 PM Timothy McDaniel
> <timothy.mcdaniel@intel.com> wrote:
> >
> > Add miscellaneous inline functions that may be called
> > from multiple files. These functions include inline
> > assembly of new x86 instructions, such as movdir64b,
> > since they are not available as builtin functions in
> > the minimum supported GCC version.
> >
> > Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> > Reviewed-by: Gage Eads <gage.eads@intel.com>
> > ---
> > drivers/event/dlb/dlb_inline_fns.h | 40
> ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 drivers/event/dlb/dlb_inline_fns.h
> >
> > diff --git a/drivers/event/dlb/dlb_inline_fns.h
> b/drivers/event/dlb/dlb_inline_fns.h
> > new file mode 100644
> > index 0000000..1ecddb7
> > --- /dev/null
> > +++ b/drivers/event/dlb/dlb_inline_fns.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2016-2020 Intel Corporation
> > + */
> > +
>
> Missing banners, like in dlb2 header (caught by diffing with it).
>
> #ifndef __DLB_INLINE_FNS_H__
> etc...
>
> > +#include "rte_memcpy.h"
> > +#include "rte_io.h"
> > +
> > +/* Inline functions required in more than one source file. */
> > +
> > +static inline struct dlb_eventdev *
> > +dlb_pmd_priv(const struct rte_eventdev *eventdev)
> > +{
> > + return eventdev->data->dev_private;
> > +}
> > +
> > +static inline void
> > +dlb_movntdq_single(void *dest, void *src)
> > +{
> > + long long *_src = (long long *)src;
> > + __m128i src_data0 = (__m128i){_src[0], _src[1]};
> > +
> > + _mm_stream_si128(dest, src_data0);
> > +}
> > +
> > +static inline void
> > +dlb_cldemote(void *addr)
> > +{
> > + /* Load addr into RSI, then demote the cache line of the address
> > + * contained in that register.
> > + */
> > + asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (addr));
> > +}
>
> Use EAL API.
>
>
> > +
> > +static inline void
> > +dlb_movdir64b(void *dest, void *src)
> > +{
> > + asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
> > + :
> > + : "a" (dest), "d" (src));
> > +}
>
> NO!
> We introduced stuff in EAL for this, please double check.
>
>
> --
> David Marchand
I do not see a direct single instruction replacement for the following:
dlb_movntdq_single(void *dest, void *src)
{
long long *_src = (long long *)src;
__m128i src_data0 = (__m128i){_src[0], _src[1]};
_mm_stream_si128(dest, src_data0);
}
The original code used a builting for movntdq, but we switched to _mm_stream_si128
because one of the builds did not have the movntdq builtin.
Tim
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Saturday, October 31, 2020 4:54 PM
> To: McDaniel, Timothy <timothy.mcdaniel@intel.com>
> Cc: dev <dev@dpdk.org>; Carrillo, Erik G <erik.g.carrillo@intel.com>; Eads,
> Gage <gage.eads@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v14 05/23] event/dlb: add inline functions
>
> On Sat, Oct 31, 2020 at 7:17 PM Timothy McDaniel
> <timothy.mcdaniel@intel.com> wrote:
> >
> > Add miscellaneous inline functions that may be called
> > from multiple files. These functions include inline
> > assembly of new x86 instructions, such as movdir64b,
> > since they are not available as builtin functions in
> > the minimum supported GCC version.
> >
> > Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
> > Reviewed-by: Gage Eads <gage.eads@intel.com>
> > ---
> > drivers/event/dlb/dlb_inline_fns.h | 40
> ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 drivers/event/dlb/dlb_inline_fns.h
> >
> > diff --git a/drivers/event/dlb/dlb_inline_fns.h
> b/drivers/event/dlb/dlb_inline_fns.h
> > new file mode 100644
> > index 0000000..1ecddb7
> > --- /dev/null
> > +++ b/drivers/event/dlb/dlb_inline_fns.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2016-2020 Intel Corporation
> > + */
> > +
>
> Missing banners, like in dlb2 header (caught by diffing with it).
>
> #ifndef __DLB_INLINE_FNS_H__
> etc...
>
> > +#include "rte_memcpy.h"
> > +#include "rte_io.h"
> > +
> > +/* Inline functions required in more than one source file. */
> > +
> > +static inline struct dlb_eventdev *
> > +dlb_pmd_priv(const struct rte_eventdev *eventdev)
> > +{
> > + return eventdev->data->dev_private;
> > +}
> > +
> > +static inline void
> > +dlb_movntdq_single(void *dest, void *src)
> > +{
> > + long long *_src = (long long *)src;
> > + __m128i src_data0 = (__m128i){_src[0], _src[1]};
> > +
> > + _mm_stream_si128(dest, src_data0);
> > +}
> > +
> > +static inline void
> > +dlb_cldemote(void *addr)
> > +{
> > + /* Load addr into RSI, then demote the cache line of the address
> > + * contained in that register.
> > + */
> > + asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (addr));
> > +}
>
> Use EAL API.
>
>
> > +
> > +static inline void
> > +dlb_movdir64b(void *dest, void *src)
> > +{
> > + asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
> > + :
> > + : "a" (dest), "d" (src));
> > +}
>
> NO!
> We introduced stuff in EAL for this, please double check.
>
>
> --
> David Marchand
I also do not see a replacement for the new MOVDIR64B instruction in dpdk-next-eventdev or dpdk main.
> > > +
> > > +static inline void
> > > +dlb_movdir64b(void *dest, void *src)
> > > +{
> > > + asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
> > > + :
> > > + : "a" (dest), "d" (src));
> > > +}
> >
> > NO!
> > We introduced stuff in EAL for this, please double check.
> I also do not see a replacement for the new MOVDIR64B instruction in dpdk-next-eventdev or dpdk main.
Ok, what got introduced in EAL is for MOVDIRI.
So here we go with a MOVDIR64B...
Google tells me:
Availability of the MOVDIR64B instruction is indicated by the presence
of the CPUID feature flag MOVDIR64B (bit 28 of the ECX register in
leaf 07H, see “CPUID—CPU Identification” in the Intel® 64 and IA-32
Architectures Software Developer’s Manual, Volume 2A).
I understand that calling this code must be under a check for
RTE_CPUFLAG_MOVDIR64B.
Which I can't find in this patchset.
What did I miss this time?
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Sunday, November 1, 2020 12:01 PM
> To: McDaniel, Timothy <timothy.mcdaniel@intel.com>
> Cc: dev <dev@dpdk.org>; Carrillo, Erik G <erik.g.carrillo@intel.com>; Eads,
> Gage <gage.eads@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v14 05/23] event/dlb: add inline functions
>
> > > > +
> > > > +static inline void
> > > > +dlb_movdir64b(void *dest, void *src)
> > > > +{
> > > > + asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
> > > > + :
> > > > + : "a" (dest), "d" (src));
> > > > +}
> > >
> > > NO!
> > > We introduced stuff in EAL for this, please double check.
> > I also do not see a replacement for the new MOVDIR64B instruction in dpdk-
> next-eventdev or dpdk main.
>
> Ok, what got introduced in EAL is for MOVDIRI.
>
>
> So here we go with a MOVDIR64B...
> Google tells me:
> Availability of the MOVDIR64B instruction is indicated by the presence
> of the CPUID feature flag MOVDIR64B (bit 28 of the ECX register in
> leaf 07H, see “CPUID—CPU Identification” in the Intel® 64 and IA-32
> Architectures Software Developer’s Manual, Volume 2A).
>
> I understand that calling this code must be under a check for
> RTE_CPUFLAG_MOVDIR64B.
> Which I can't find in this patchset.
> What did I miss this time?
>
>
> --
> David Marchand
Fair enough question. We currently do not check for availability of MOVDIR64B since
every Intel part that includes DLB or DLB2 is guaranteed to have the MOVDIR64B
instruction.
On Sun, Nov 1, 2020 at 7:07 PM McDaniel, Timothy
<timothy.mcdaniel@intel.com> wrote:
> > Availability of the MOVDIR64B instruction is indicated by the presence
> > of the CPUID feature flag MOVDIR64B (bit 28 of the ECX register in
> > leaf 07H, see “CPUID—CPU Identification” in the Intel® 64 and IA-32
> > Architectures Software Developer’s Manual, Volume 2A).
> >
> > I understand that calling this code must be under a check for
> > RTE_CPUFLAG_MOVDIR64B.
> > Which I can't find in this patchset.
>
> Fair enough question. We currently do not check for availability of MOVDIR64B since
> every Intel part that includes DLB or DLB2 is guaranteed to have the MOVDIR64B
> instruction.
A check at probe time would not hurt, but if you are sure of it, ok.
new file mode 100644
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2016-2020 Intel Corporation
+ */
+
+#include "rte_memcpy.h"
+#include "rte_io.h"
+
+/* Inline functions required in more than one source file. */
+
+static inline struct dlb_eventdev *
+dlb_pmd_priv(const struct rte_eventdev *eventdev)
+{
+ return eventdev->data->dev_private;
+}
+
+static inline void
+dlb_movntdq_single(void *dest, void *src)
+{
+ long long *_src = (long long *)src;
+ __m128i src_data0 = (__m128i){_src[0], _src[1]};
+
+ _mm_stream_si128(dest, src_data0);
+}
+
+static inline void
+dlb_cldemote(void *addr)
+{
+ /* Load addr into RSI, then demote the cache line of the address
+ * contained in that register.
+ */
+ asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (addr));
+}
+
+static inline void
+dlb_movdir64b(void *dest, void *src)
+{
+ asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
+ :
+ : "a" (dest), "d" (src));
+}