[PATCH 137/141] staging: unisys: Hide contents of pending_msg_hdr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

From: Don Zickus <dzickus@xxxxxxxxxx>

The pending_msg_hdr element of struct visor_device should really be
private and not public.  Currently there is no easy way to do that
without allocating memory and passing it along from the command
notifier all the way to the command responder.

I tried to minimize the change and use NULL as the equivalent to
INVALID.  I also added a check for when the pending_msg_hdr is not
NULL during the call to _epilog.  This indicates a previous command
is inflight for the device/bus.  Not good.

I also guessed on an appropriate memory failure response to return
in case the kzalloc failed to allocate memory for the message header.

Signed-off-by: Don Zickus <dzickus@xxxxxxxxxx>
Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
---
 drivers/staging/unisys/include/visorbus.h      |  5 +-
 drivers/staging/unisys/visorbus/visorchipset.c | 67 +++++++++++++++++---------
 2 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index cc0724a..17c15f5 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -37,7 +37,6 @@
 
 #include "periodic_work.h"
 #include "channel.h"
-#include "controlvmchannel.h"
 
 struct visor_driver;
 struct visor_device;
@@ -118,6 +117,8 @@ struct visor_driver {
 
 /** A device type for things "plugged" into the visorbus bus */
 
+struct controlvm_message_header;
+
 struct visor_device {
 	/** visor driver can use the visorchannel member with the functions
 	 *  defined in visorchannel.h to access the channel
@@ -154,7 +155,7 @@ struct visor_device {
 	uuid_le inst;
 	u8 *name;
 	u8 *description;
-	struct controlvm_message_header pending_msg_hdr;
+	struct controlvm_message_header *pending_msg_hdr;
 	void *vbus_hdr_info;
 	u32 switch_no;
 	u32 internal_port_no;
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index b1e059a..4a895e8 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -885,12 +885,13 @@ bus_responder(enum controlvm_id cmd_id, struct visor_device *p, int response)
 	if (cmd_id == CONTROLVM_BUS_CREATE)
 			p->state.created = 1;
 
-	if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+	if (p->pending_msg_hdr == NULL)
 		return;		/* no controlvm response needed */
-	if (p->pending_msg_hdr.id != (u32)cmd_id)
+	if (p->pending_msg_hdr->id != (u32)cmd_id)
 		return;
-	controlvm_respond(&p->pending_msg_hdr, response);
-	p->pending_msg_hdr.id = CONTROLVM_INVALID;
+	controlvm_respond(p->pending_msg_hdr, response);
+	kfree(p->pending_msg_hdr);
+	p->pending_msg_hdr = NULL;
 }
 
 static void
@@ -900,12 +901,12 @@ device_changestate_responder(enum controlvm_id cmd_id,
 {
 	struct controlvm_message outmsg;
 
-	if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+	if (p->pending_msg_hdr == NULL)
 		return;		/* no controlvm response needed */
-	if (p->pending_msg_hdr.id != cmd_id)
+	if (p->pending_msg_hdr->id != cmd_id)
 		return;
 
-	controlvm_init_response(&outmsg, &p->pending_msg_hdr, response);
+	controlvm_init_response(&outmsg, p->pending_msg_hdr, response);
 
 	outmsg.cmd.device_change_state.bus_no = p->chipset_bus_no;
 	outmsg.cmd.device_change_state.dev_no = p->chipset_dev_no;
@@ -915,7 +916,8 @@ device_changestate_responder(enum controlvm_id cmd_id,
 				       CONTROLVM_QUEUE_REQUEST, &outmsg))
 		return;
 
-	p->pending_msg_hdr.id = CONTROLVM_INVALID;
+	kfree(p->pending_msg_hdr);
+	p->pending_msg_hdr = NULL;
 }
 
 static void
@@ -926,14 +928,15 @@ device_responder(enum controlvm_id cmd_id, struct visor_device *p, int response)
 			p->state.created = 1;
 	}
 
-	if (p->pending_msg_hdr.id == CONTROLVM_INVALID)
+	if (p->pending_msg_hdr == NULL)
 		return;		/* no controlvm response needed */
 
-	if (p->pending_msg_hdr.id != (u32)cmd_id)
+	if (p->pending_msg_hdr->id != (u32)cmd_id)
 		return;
 
-	controlvm_respond(&p->pending_msg_hdr, response);
-	p->pending_msg_hdr.id = CONTROLVM_INVALID;
+	controlvm_respond(p->pending_msg_hdr, response);
+	kfree(p->pending_msg_hdr);
+	p->pending_msg_hdr = NULL;
 }
 
 static void
@@ -942,12 +945,21 @@ bus_epilog(struct visor_device *bus_info,
 	   int response, bool need_response)
 {
 	bool notified = false;
-
-	if (need_response) {
-		memcpy(&bus_info->pending_msg_hdr, msg_hdr,
+	struct controlvm_message_header *pmsg_hdr;
+
+	if (bus_info->pending_msg_hdr) {
+		/* only non-NULL if bus is still waiting on a response */
+		response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
+		goto away;
+	} else if (need_response) {
+		pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
+		if (!pmsg_hdr) {
+			response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
+			goto away;
+		}
+		memcpy(pmsg_hdr, msg_hdr,
 		       sizeof(struct controlvm_message_header));
-	} else {
-		bus_info->pending_msg_hdr.id = CONTROLVM_INVALID;
+		bus_info->pending_msg_hdr = pmsg_hdr;
 	}
 
 	down(&notifier_lock);
@@ -967,6 +979,7 @@ bus_epilog(struct visor_device *bus_info,
 			break;
 		}
 	}
+away:
 	if (notified)
 		/* The callback function just called above is responsible
 		 * for calling the appropriate visorchipset_busdev_responders
@@ -987,6 +1000,7 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
 	bool notified = false;
 	u32 bus_no = dev_info->chipset_bus_no;
 	u32 dev_no = dev_info->chipset_dev_no;
+	struct controlvm_message_header *pmsg_hdr;
 
 	char *envp[] = {
 		"SPARSP_DIAGPOOL_PAUSED_STATE = 1",
@@ -995,11 +1009,19 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
 
 	notifiers = &busdev_notifiers;
 
-	if (need_response) {
-		memcpy(&dev_info->pending_msg_hdr, msg_hdr,
+	if (dev_info->pending_msg_hdr) {
+		/* only non-NULL if dev is still waiting on a response */
+		response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
+		goto away;
+	} else if (need_response) {
+		pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
+		if (!pmsg_hdr) {
+			response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
+			goto away;
+		}
+		memcpy(pmsg_hdr, msg_hdr,
 		       sizeof(struct controlvm_message_header));
-	} else {
-		dev_info->pending_msg_hdr.id = CONTROLVM_INVALID;
+		dev_info->pending_msg_hdr = pmsg_hdr;
 	}
 
 	down(&notifier_lock);
@@ -1057,6 +1079,7 @@ device_epilog(struct visor_device *dev_info, struct spar_segment_state state,
 			break;
 		}
 	}
+away:
 	if (notified)
 		/* The callback function just called above is responsible
 		 * for calling the appropriate visorchipset_busdev_responders
@@ -1156,7 +1179,7 @@ bus_configure(struct controlvm_message *inmsg,
 		POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
 				 POSTCODE_SEVERITY_ERR);
 		rc = -CONTROLVM_RESP_ERROR_BUS_INVALID;
-	} else if (bus_info->pending_msg_hdr.id != CONTROLVM_INVALID) {
+	} else if (bus_info->pending_msg_hdr != NULL) {
 		POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
 				 POSTCODE_SEVERITY_ERR);
 		rc = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
-- 
2.1.4

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux