On Wed, Aug 14, 2019 at 11:34:15AM -0500, Scott Wood wrote:

On Wed, 2019-07-24 at 22:22 +0800, Wu Hao wrote:

quoted

On Wed, Jul 24, 2019 at 11:35:32AM +0200, Greg KH wrote:

quoted

On Tue, Jul 23, 2019 at 12:51:24PM +0800, Wu Hao wrote:

quoted

@@ -67,8 +69,43 @@
 #define PR_WAIT_TIMEOUT   8000000
 #define PR_HOST_STATUS_IDLE	0
 
+#if defined(CONFIG_X86) && defined(CONFIG_AS_AVX512)
+
+#include <linux/cpufeature.h>
+#include <asm/fpu/api.h>
+
+static inline int is_cpu_avx512_enabled(void)
+{
+	return cpu_feature_enabled(X86_FEATURE_AVX512F);
+}

That's a very arch specific function, why would a driver ever care about
this?

Yes, this is only applied to a specific FPGA solution, which FPGA
has been integrated with XEON. Hardware indicates this using register
to software. As it's cpu integrated solution, so CPU always has this
AVX512 capability. The only check we do, is make sure this is not
manually disabled by kernel.

With this hardware, software could use AVX512 to accelerate the FPGA
partial reconfiguration as mentioned in the patch commit message.
It brings performance benifits to people who uses it. This is only one
optimization (512 vs 32bit data write to hw) for a specific hardware.

I thought earlier you said that 512 bit accesses were required for this
particular integrated-only version of the device, and not just an
optimization?

yes, some optimization implemented in a specific integrated-only version
of hardware, this patch is used to support that particular hardware. This
is also the reason you see code here to check hardware revision in this
patch.

quoted

quoted

quoted

+#else
+static inline int is_cpu_avx512_enabled(void)
+{
+	return 0;
+}
+
+static inline void copy512(const void *src, void __iomem *dst)
+{
+	WARN_ON_ONCE(1);

Are you trying to get reports from syzbot?  :)

Oh.. no.. I will remove it. :)

Thank you very much!

What's wrong with this?  The driver should never call copy512() if
is_cpu_avx512_enabled() returns 0, and if syzbot can somehow make the driver
do so, then yes we do want a report.

Yes, you are right, in previous version, it doesn't have avx512 enable check
there, so it's possible to have false reporting, it should be fine after
driver does early check on this during probe. As this patch has been dropped
from main patchset, may rework it later and resubmit. Thanks for the comments.

Hao

-Scott