linux-kernel - [UPDATE PATCH v2 1/2] Revert "ACPI / EC: Add support to disallow QR_EC to be issued before completing previous QR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 29 Oct 2014 12:37:23 +0800
From:	Lv Zheng <lv.zheng@...el.com>
To:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Len Brown <len.brown@...el.com>
Cc:	Lv Zheng <lv.zheng@...el.com>, Lv Zheng <zetalog@...il.com>,
	<linux-kernel@...r.kernel.org>, linux-acpi@...r.kernel.org
Subject: [UPDATE PATCH v2 1/2] Revert "ACPI / EC: Add support to disallow QR_EC to be issued before completing previous QR_EC"

[update due to wrong utf-8 character]

It is reported that the following commit breaks Samsung hardware:
 Commit: 558e4736f2e1b0e6323adf7a5e4df77ed6cfc1a4.
 Subject: ACPI / EC: Add support to disallow QR_EC to be issued before
          completing previous QR_EC

Which means the Samsung behavior conflicts with the Acer behavior.

1. Samsung may behave like:
   [ +event 1 ] SCI_EVT set
   [ +event 2 ] SCI_EVT set
                              write QR_EC
                              read event
   [ -event 1 ] SCI_EVT clear
   Without the above commit, Samsung can work:
   [ +event 1 ] SCI_EVT set
   [ +event 2 ] SCI_EVT set
                              write QR_EC
                              CAN prepare next QR_EC as SCI_EVT=1
                              read event
   [ -event 1 ] SCI_EVT clear
                              write QR_EC
                              read event
   [ -event 2 ] SCI_EVT clear
   With the above commit, Samsung cannot work:
   [ +event 1 ] SCI_EVT set
   [ +event 2 ] SCI_EVT set
                              write QR_EC
                              read event
   [ -event 1 ] SCI_EVT clear
                              CANNOT prepare next QR_EC as SCI_EVT=0
2. Acer may behave like:
   [ +event 1 ] SCI_EVT set
   [ +event 2 ]
                              write QR_EC
                              read event
   [ -event 1 ] SCI_EVT clear
   [ +event 2 ] SCI_EVT set
   Without the above commit, Acer cannot work when there is only 1 event:
   [ +event 1 ] SCI_EVT set
                              write QR_EC
                              can prepared next QR_EC as SCI_EVT=1
                              read event
   [ -event 1 ] SCI_EVT clear
                              CANNOT write QR_EC as SCI_EVT=0
   With the above commit, Acer can work:
   [ +event 1 ] SCI_EVT set
   [ +event 2 ]
                              write QR_EC
                              read event
   [ -event 1 ] SCI_EVT set
                              can prepare next QR_EC because SCI_EVT=0
                              CAN write QR_EC as SCI_EVT=1

Since Acer can also work with only the following commit applied:
 Commit: 3afcf2ece453e1a8c2c6de19cdf06da3772a1b08
 Subject: ACPI / EC: Add support to disallow QR_EC to be issued when
          SCI_EVT isn't set
We can revert the above commit.

This reverts commit 558e4736f2e1b0e6323adf7a5e4df77ed6cfc1a4.

Conflicts:

	drivers/acpi/ec.c

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=44161
Reported-and-tested-by: Ortwin Glück <odi@....ch> 
Signed-off-by: Lv Zheng <lv.zheng@...el.com>
---
 drivers/acpi/ec.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 3d304ff..31b699f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -334,13 +334,13 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 	pr_debug("***** Command(%s) started *****\n",
 		 acpi_ec_cmd_string(t->command));
 	start_transaction(ec);
-	spin_unlock_irqrestore(&ec->lock, tmp);
-	ret = ec_poll(ec);
-	spin_lock_irqsave(&ec->lock, tmp);
 	if (ec->curr->command == ACPI_EC_COMMAND_QUERY) {
 		clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags);
 		pr_debug("***** Event stopped *****\n");
 	}
+	spin_unlock_irqrestore(&ec->lock, tmp);
+	ret = ec_poll(ec);
+	spin_lock_irqsave(&ec->lock, tmp);
 	pr_debug("***** Command(%s) stopped *****\n",
 		 acpi_ec_cmd_string(t->command));
 	ec->curr = NULL;
-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/