Re: [PATCH 03/17] fpga: dfl: fme: support 512bit data width PR

On Tue, 2019-03-26 at 14:33 -0500, Alan Tull wrote:

On Mon, Mar 25, 2019 at 5:58 PM Scott Wood [off-list ref] wrote:

Hi Scott,

quoted

On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:

quoted

On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:

quoted

+#if defined(CONFIG_X86) && defined(CONFIG_AS_AVX512)
+
+#include <asm/fpu/api.h>
+
+static inline void copy512(void *src, void __iomem *dst)
+{
+   kernel_fpu_begin();
+
+   asm volatile("vmovdqu64 (%0), %%zmm0;"
+                "vmovntdq %%zmm0, (%1);"
+                :
+                : "r"(src), "r"(dst));
+
+   kernel_fpu_end();
+}

Shouldn't there be some sort of check that AVX512 is actually
supported
on the running system?

Also, src should be const, and the asm statement should have a memory
clobber.

quoted

+#else
+static inline void copy512(void *src, void __iomem *dst)
+{
+   WARN_ON_ONCE(1);
+}
+#endif

Likewise, this will be called if a revision 2 device is used on non-
x86
(or on x86 with an old binutils).  The driver should fall back to 32-
bit
in such cases.

Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case?  Seems worthwhile to check
for
AVX512 support anyway.  And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running
on a
kernel where avx512 was disabled via a boot option.

The code checks for CONFIG_AS_AVX512 above.

That just indicates that binutils supports it.  Plus, the code does not
check for CONFIG_AS_AVX512 when deciding whether to set pr_datawidth to 64
(and thus call copy512), so you'll get a WARN_ON rather than falling back to
32-bit.

What boot option are you referring to?

clearcpuid=304

-Scott