On Wed, Jun 26, 2019 at 4:08 PM Jakub Kicinski
[off-list ref] wrote:

On Wed, 26 Jun 2019 11:52:48 -0700, Catherine Sullivan wrote:

quoted

Add a driver framework for the Compute Engine Virtual NIC that will be
available in the future.

At this point the only functionality is loading the driver.

Signed-off-by: Catherine Sullivan <redacted>
Signed-off-by: Sagi Shahar <redacted>
Signed-off-by: Jon Olson <redacted>
Acked-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Luigi Rizzo <redacted>
---

quoted

+if NET_VENDOR_GOOGLE
+
+config GVE
+     tristate "Google Virtual NIC (gVNIC) support"
+     depends on (PCI_MSI && X86)

We usually prefer for drivers not to depend on the platform unless
really necessary, but IDK how that applies to the curious new world
of NICs nobody can buy :)

This is the only platform it will ever need to run on so we would really
prefer to not have to support others :)

quoted

+     help
+       This driver supports Google Virtual NIC (gVNIC)"
+
+       To compile this driver as a module, choose M here.
+       The module will be called gve.
+
+endif #NET_VENDOR_GOOGLE

quoted

+void gve_adminq_release(struct gve_priv *priv)
+{
+     int i;
+
+     /* Tell the device the adminq is leaving */
+     writel(0x0, &priv->reg_bar0->adminq_pfn);
+     for (i = 0; i < GVE_MAX_ADMINQ_RELEASE_CHECK; i++) {
+             if (!readl(&priv->reg_bar0->adminq_pfn)) {
+                     gve_clear_device_rings_ok(priv);
+                     gve_clear_device_resources_ok(priv);
+                     gve_clear_admin_queue_ok(priv);
+                     return;
+             }
+             msleep(GVE_ADMINQ_SLEEP_LEN);
+     }
+     /* If this is reached the device is unrecoverable and still holding
+      * memory. Anything other than a BUG risks memory corruption.
+      */
+     WARN(1, "Unrecoverable platform error!");
+     BUG();

Please don't add BUG()s to the kernel.  You're probably better off
spinning for ever in the loop above.  Also if there is an IOMMU in
the way the device won't be able to mess with the memory.

Ack, it will be switched to a loop that won't end in v2.

quoted

+}
+

quoted

diff --git a/drivers/net/ethernet/google/gve/gve_size_assert.h b/drivers/net/ethernet/google/gve/gve_size_assert.h
new file mode 100644
index 000000000000..a58422d4f16e
--- /dev/null
+++ b/drivers/net/ethernet/google/gve/gve_size_assert.h

@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ * Google virtual Ethernet (gve) driver
+ *
+ * Copyright (C) 2015-2019 Google, Inc.
+ */
+
+#ifndef _GVE_ASSERT_H_
+#define _GVE_ASSERT_H_
+#define GVE_ASSERT_SIZE(tag, type, size) \
+     static void gve_assert_size_ ## type(void) __attribute__((used)); \
+     static inline void gve_assert_size_ ## type(void) \
+     { \
+             BUILD_BUG_ON(sizeof(tag type) != (size)); \
+     }
+#endif /* _GVE_ASSERT_H_ */

Please use static_assert() directly in your struct size checks.

Thanks - that is much cleaner. Fixed in v2.