greybus: fix host-device buffer constraints

Host devices impose buffer-size constraints on Greybus core which are
taken into account when allocating messages.

Make sure to verify these constraints when the host device is allocated,
rather than when the first message is allocated.

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c
index 45fa4c3dfd2b768e41bc3325ccd55f983e74a7b6..e32d6c41e87e87cd17991fa03b97175238aae145 100644 (file)
--- a/drivers/staging/greybus/core.c
+++ b/drivers/staging/greybus/core.c
@@ -173,7 +173,8 @@ static void free_hd(struct kref *kref)
 }
 
 struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver,
-                                             struct device *parent)
+                                             struct device *parent,
+                                             size_t buffer_size_max)
 {
        struct greybus_host_device *hd;
 
@@ -187,6 +188,16 @@ struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver
                return NULL;
        }
 
+       /*
+        * Make sure to never allocate messages larger than what the Greybus
+        * protocol supports.
+        */
+       if (buffer_size_max > GB_OPERATION_MESSAGE_SIZE_MAX) {
+               dev_warn(parent, "limiting buffer size to %u\n",
+                        GB_OPERATION_MESSAGE_SIZE_MAX);
+               buffer_size_max = GB_OPERATION_MESSAGE_SIZE_MAX;
+       }
+
        hd = kzalloc(sizeof(*hd) + driver->hd_priv_size, GFP_KERNEL);
        if (!hd)
                return NULL;
@@ -197,6 +208,7 @@ struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver
        INIT_LIST_HEAD(&hd->interfaces);
        INIT_LIST_HEAD(&hd->connections);
        ida_init(&hd->cport_id_map);
+       hd->buffer_size_max = buffer_size_max;
 
        hd->endo = gb_endo_create(hd);
        if (!hd->endo) {

diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c
index 4bba5d558e84d1c99d095a598ebea6fc9d59d232..e0fae26d8ba3584558d47430b5d05013d063ddb1 100644 (file)
--- a/drivers/staging/greybus/es1.c
+++ b/drivers/staging/greybus/es1.c
@@ -98,22 +98,6 @@ static void cport_out_callback(struct urb *urb);
 static void usb_log_enable(struct es1_ap_dev *es1);
 static void usb_log_disable(struct es1_ap_dev *es1);
 
-/*
- * Buffer constraints for the host driver.
- *
- * A "buffer" is used to hold data to be transferred for Greybus by
- * the host driver.  A buffer is represented by a "buffer pointer",
- * which defines a region of memory used by the host driver for
- * transferring the data.  When Greybus allocates a buffer, it must
- * do so subject to the constraints associated with the host driver.
- *
- *  size_max:  The maximum size of a buffer
- */
-static void hd_buffer_constraints(struct greybus_host_device *hd)
-{
-       hd->buffer_size_max = ES1_GBUF_MSG_SIZE_MAX;
-}
-
 #define ES1_TIMEOUT    500     /* 500 ms for the SVC to do something */
 static int submit_svc(struct svc_msg *svc_msg, struct greybus_host_device *hd)
 {
@@ -571,15 +555,12 @@ static int ap_probe(struct usb_interface *interface,
 
        udev = usb_get_dev(interface_to_usbdev(interface));
 
-       hd = greybus_create_hd(&es1_driver, &udev->dev);
+       hd = greybus_create_hd(&es1_driver, &udev->dev, ES1_GBUF_MSG_SIZE_MAX);
        if (!hd) {
                usb_put_dev(udev);
                return -ENOMEM;
        }
 
-       /* Fill in the buffer allocation constraints */
-       hd_buffer_constraints(hd);
-
        es1 = hd_to_es1(hd);
        es1->hd = hd;
        es1->usb_intf = interface;

diff --git a/drivers/staging/greybus/es2.c b/drivers/staging/greybus/es2.c
index cc73fbd9b77b7d6403081a2ebcebe208fb9e6e1d..05aac3d7686ba169dcb37347738fa82381d1f842 100644 (file)
--- a/drivers/staging/greybus/es2.c
+++ b/drivers/staging/greybus/es2.c
@@ -98,22 +98,6 @@ static void cport_out_callback(struct urb *urb);
 static void usb_log_enable(struct es1_ap_dev *es1);
 static void usb_log_disable(struct es1_ap_dev *es1);
 
-/*
- * Buffer constraints for the host driver.
- *
- * A "buffer" is used to hold data to be transferred for Greybus by
- * the host driver.  A buffer is represented by a "buffer pointer",
- * which defines a region of memory used by the host driver for
- * transferring the data.  When Greybus allocates a buffer, it must
- * do so subject to the constraints associated with the host driver.
- *
- *  size_max:  The maximum size of a buffer
- */
-static void hd_buffer_constraints(struct greybus_host_device *hd)
-{
-       hd->buffer_size_max = ES1_GBUF_MSG_SIZE_MAX;
-}
-
 #define ES1_TIMEOUT    500     /* 500 ms for the SVC to do something */
 static int submit_svc(struct svc_msg *svc_msg, struct greybus_host_device *hd)
 {
@@ -571,15 +555,12 @@ static int ap_probe(struct usb_interface *interface,
 
        udev = usb_get_dev(interface_to_usbdev(interface));
 
-       hd = greybus_create_hd(&es1_driver, &udev->dev);
+       hd = greybus_create_hd(&es1_driver, &udev->dev, ES1_GBUF_MSG_SIZE_MAX);
        if (!hd) {
                usb_put_dev(udev);
                return -ENOMEM;
        }
 
-       /* Fill in the buffer allocation constraints */
-       hd_buffer_constraints(hd);
-
        es1 = hd_to_es1(hd);
        es1->hd = hd;
        es1->usb_intf = interface;

diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h
index 0a25d0c45e37457876e3b69c1ed45d7bf7b7af9e..dbb4e78cf4c94e30f9bcb37448a3dcabb4d23733 100644 (file)
--- a/drivers/staging/greybus/greybus.h
+++ b/drivers/staging/greybus/greybus.h
@@ -105,7 +105,8 @@ struct greybus_host_device {
 };
 
 struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *hd,
-                                             struct device *parent);
+                                             struct device *parent,
+                                             size_t buffer_size_max);
 void greybus_remove_hd(struct greybus_host_device *hd);
 
 struct greybus_driver {

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 1ec930c5cba9d18ea58088bb20b2b15571b5ae0e..c78ccc09d6674c6f801ba41127150e9610c5c951 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -17,12 +17,6 @@
 /* The default amount of time a request is given to complete */
 #define OPERATION_TIMEOUT_DEFAULT      1000    /* milliseconds */
 
-/*
- * XXX This needs to be coordinated with host driver parameters
- * XXX May need to reduce to allow for message header within a page
- */
-#define GB_OPERATION_MESSAGE_SIZE_MAX  4096
-
 static struct kmem_cache *gb_operation_cache;
 static struct kmem_cache *gb_message_cache;
 
@@ -282,12 +276,6 @@ gb_operation_message_alloc(struct greybus_host_device *hd, u8 type,
        struct gb_operation_msg_hdr *header;
        size_t message_size = payload_size + sizeof(*header);
 
-       if (hd->buffer_size_max > GB_OPERATION_MESSAGE_SIZE_MAX) {
-               pr_warn("limiting buffer size to %u\n",
-                       GB_OPERATION_MESSAGE_SIZE_MAX);
-               hd->buffer_size_max = GB_OPERATION_MESSAGE_SIZE_MAX;
-       }
-
        if (message_size > hd->buffer_size_max) {
                pr_warn("requested message size too big (%zu > %zu)\n",
                                message_size, hd->buffer_size_max);
@@ -936,9 +924,6 @@ EXPORT_SYMBOL_GPL(gb_operation_sync);
 
 int gb_operation_init(void)
 {
-       BUILD_BUG_ON(GB_OPERATION_MESSAGE_SIZE_MAX >
-                       U16_MAX - sizeof(struct gb_operation_msg_hdr));
-
        gb_message_cache = kmem_cache_create("gb_message_cache",
                                sizeof(struct gb_message), 0, 0, NULL);
        if (!gb_message_cache)

diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h
index 82b8fe57f8a00510a98725575c0a8bac72a1e3ee..3b02db5cd08bdaea3546cf78a306625865b828fb 100644 (file)
--- a/drivers/staging/greybus/operation.h
+++ b/drivers/staging/greybus/operation.h
@@ -69,6 +69,8 @@ struct gb_operation_msg_hdr {
        __u8    pad[2];         /* must be zero (ignore when read) */
 } __aligned(sizeof(u64));
 
+#define GB_OPERATION_MESSAGE_SIZE_MAX  4096
+
 /*
  * Protocol code should only examine the payload and payload_size
  * fields.  All other fields are intended to be private to the