[char-misc-next 0/3] mei: few more reset recursion fixes

[char-misc-next 0/3] mei: few more reset recursion fixes

[Tomas Winkler]

*** BLURB HERE ***

Alexander Usyskin (1):
  mei: cancel stall timers in mei_reset

Tomas Winkler (2):
  mei: make me client counters less error prone
  mei: bus: stop wait for read during cl state transition

 drivers/misc/mei/amthif.c  |  1 +
 drivers/misc/mei/bus.c     |  5 ++++-
 drivers/misc/mei/client.h  |  6 ++++++
 drivers/misc/mei/hbm.c     | 10 ++++++----
 drivers/misc/mei/init.c    |  3 +++
 drivers/misc/mei/main.c    | 11 ++++-------
 drivers/misc/mei/mei_dev.h |  6 +++---
 7 files changed, 27 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[char-misc-next 1/3] mei: make me client counters less error prone

[Tomas Winkler]

1. u8 counters are prone to hard to detect overflow:
 make them unsigned long to match bit_ functions argument type

2. don't check me_clients_num for negativity, it is unsigned.

3. init all the me client counters from one place

Cc:  <stable@vger.kernel.org> # 3.9+
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hbm.c     | 10 ++++++----
 drivers/misc/mei/mei_dev.h |  6 +++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index f706fe8..52afe1e 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -35,11 +35,15 @@ static void mei_hbm_me_cl_allocate(struct mei_device *dev)
 	struct mei_me_client *clients;
 	int b;
 
+	dev->me_clients_num = 0;
+	dev->me_client_presentation_num = 0;
+	dev->me_client_index = 0;
+
 	/* count how many ME clients we have */
 	for_each_set_bit(b, dev->me_clients_map, MEI_CLIENTS_MAX)
 		dev->me_clients_num++;
 
-	if (dev->me_clients_num <= 0)
+	if (dev->me_clients_num == 0)
 		return;
 
 	kfree(dev->me_clients);
@@ -221,7 +225,7 @@ static int mei_hbm_prop_req(struct mei_device *dev)
 	struct hbm_props_request *prop_req;
 	const size_t len = sizeof(struct hbm_props_request);
 	unsigned long next_client_index;
-	u8 client_num;
+	unsigned long client_num;
 
 
 	client_num = dev->me_client_presentation_num;
@@ -680,8 +684,6 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 		if (dev->dev_state == MEI_DEV_INIT_CLIENTS &&
 		    dev->hbm_state == MEI_HBM_ENUM_CLIENTS) {
 				dev->init_clients_timer = 0;
-				dev->me_client_presentation_num = 0;
-				dev->me_client_index = 0;
 				mei_hbm_me_cl_allocate(dev);
 				dev->hbm_state = MEI_HBM_CLIENT_PROPERTIES;
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 7b918b2..456b322 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -396,9 +396,9 @@ struct mei_device {
 	struct mei_me_client *me_clients; /* Note: memory has to be allocated */
 	DECLARE_BITMAP(me_clients_map, MEI_CLIENTS_MAX);
 	DECLARE_BITMAP(host_clients_map, MEI_CLIENTS_MAX);
-	u8 me_clients_num;
-	u8 me_client_presentation_num;
-	u8 me_client_index;
+	unsigned long me_clients_num;
+	unsigned long me_client_presentation_num;
+	unsigned long me_client_index;
 
 	struct mei_cl wd_cl;
 	enum mei_wd_states wd_state;
-- 
1.8.3.1

[char-misc-next 2/3] mei: bus: stop wait for read during cl state transition

[Tomas Winkler]

Bus layer omitted check for client state transition while waiting
for read completion
The client state transition may occur for example as result
of firmware initiated reset

Add mei_cl_is_transitioning wrapper to reduce the code
repetition.:

Cc:  <stable@vger.kernel.org> # 3.9+
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c    |  5 ++++-
 drivers/misc/mei/client.h |  6 ++++++
 drivers/misc/mei/main.c   | 11 ++++-------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index a150a42..f5dabb3 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -295,10 +295,13 @@ int __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 
 	if (cl->reading_state != MEI_READ_COMPLETE &&
 	    !waitqueue_active(&cl->rx_wait)) {
+
 		mutex_unlock(&dev->device_lock);
 
 		if (wait_event_interruptible(cl->rx_wait,
-				(MEI_READ_COMPLETE == cl->reading_state))) {
+				cl->reading_state == MEI_READ_COMPLETE  ||
+				mei_cl_is_transitioning(cl))) {
+
 			if (signal_pending(current))
 				return -EINTR;
 			return -ERESTARTSYS;
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 9eb031e..892cc42 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -90,6 +90,12 @@ static inline bool mei_cl_is_connected(struct mei_cl *cl)
 		cl->dev->dev_state == MEI_DEV_ENABLED &&
 		cl->state == MEI_FILE_CONNECTED);
 }
+static inline bool mei_cl_is_transitioning(struct mei_cl *cl)
+{
+	return (MEI_FILE_INITIALIZING == cl->state ||
+		MEI_FILE_DISCONNECTED == cl->state ||
+		MEI_FILE_DISCONNECTING == cl->state);
+}
 
 bool mei_cl_is_other_connecting(struct mei_cl *cl);
 int mei_cl_disconnect(struct mei_cl *cl);
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 7404584..b8f8604 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -255,19 +255,16 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 		mutex_unlock(&dev->device_lock);
 
 		if (wait_event_interruptible(cl->rx_wait,
-			(MEI_READ_COMPLETE == cl->reading_state ||
-			 MEI_FILE_INITIALIZING == cl->state ||
-			 MEI_FILE_DISCONNECTED == cl->state ||
-			 MEI_FILE_DISCONNECTING == cl->state))) {
+				MEI_READ_COMPLETE == cl->reading_state ||
+				mei_cl_is_transitioning(cl))) {
+
 			if (signal_pending(current))
 				return -EINTR;
 			return -ERESTARTSYS;
 		}
 
 		mutex_lock(&dev->device_lock);
-		if (MEI_FILE_INITIALIZING == cl->state ||
-		    MEI_FILE_DISCONNECTED == cl->state ||
-		    MEI_FILE_DISCONNECTING == cl->state) {
+		if (mei_cl_is_transitioning(cl)) {
 			rets = -EBUSY;
 			goto out;
 		}
-- 
1.8.3.1

[char-misc-next 3/3] mei: cancel stall timers in mei_reset

[Tomas Winkler]

From: Alexander Usyskin <alexander.usyskin@intel.com>

Unset init_clients_timer and amthif_stall_timers
in mei_reset in order to cancel timer ticking and hence
avoid recursive reset calls.

Cc:  <stable@vger.kernel.org> # 3.9+
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c | 1 +
 drivers/misc/mei/init.c   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index d0fdc13..f6ff711 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -57,6 +57,7 @@ void mei_amthif_reset_params(struct mei_device *dev)
 	dev->iamthif_ioctl = false;
 	dev->iamthif_state = MEI_IAMTHIF_IDLE;
 	dev->iamthif_timer = 0;
+	dev->iamthif_stall_timer = 0;
 }
 
 /**
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 92c7311..6197018 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -175,6 +175,9 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 		memset(&dev->wr_ext_msg, 0, sizeof(dev->wr_ext_msg));
 	}
 
+	/* we're already in reset, cancel the init timer */
+	dev->init_clients_timer = 0;
+
 	dev->me_clients_num = 0;
 	dev->rd_msg_hdr = 0;
 	dev->wd_pending = false;
-- 
1.8.3.1

Re: [char-misc-next 0/3] mei: few more reset recursion fixes

[Greg KH]

On Mon, Sep 02, 2013 at 01:29:44PM +0300, Tomas Winkler wrote:
> *** BLURB HERE ***

I think you need to read your emails a bit better :)

> Alexander Usyskin (1):
>   mei: cancel stall timers in mei_reset
> 
> Tomas Winkler (2):
>   mei: make me client counters less error prone
>   mei: bus: stop wait for read during cl state transition

You tagged all of these for the stable tree, yet didn't ask for them to
be included in 3.12-final, which is a bit confusing.  If these are
really issues that people are hitting, don't you want these merged to
3.12-final?

thanks,

greg k-h

RE: [char-misc-next 0/3] mei: few more reset recursion fixes

[Winkler, Tomas]

> > *** BLURB HERE ***
> 
> I think you need to read your emails a bit better :)

Oops.
 
> > Alexander Usyskin (1):
> >   mei: cancel stall timers in mei_reset
> >
> > Tomas Winkler (2):
> >   mei: make me client counters less error prone
> >   mei: bus: stop wait for read during cl state transition
> 
> You tagged all of these for the stable tree, yet didn't ask for them to
> be included in 3.12-final, which is a bit confusing.  If these are
> really issues that people are hitting, don't you want these merged to
> 3.12-final?

Yes it should go to 3.12 as well, I thought this is understood. 
I will refresh myself on the process. 

Thanks
Tomas