Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-14 Thread Andy Lutomirski 
On Wed, Dec 14, 2016 at 12:37 AM, David Howells <dhowe...@redhat.com> wrote:
> Andy Lutomirski <l...@amacapital.net> wrote:
>
>> > -   sg_set_buf(_out[1], pad, sizeof pad);
>> > +   sg_set_buf(_out[1], empty_zero_page, 16);
>>
>> My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
>> exactly is the code trying to do?  The old code makes no sense.  It's
>> setting the *output* buffer to zeroed padding.
>
> Padding goes into the encrypt function and is going to come out of the decrypt
> function.  Possibly derived_key_decrypt() should be checking that the padding
> that comes out is actually a bunch of zeros.  Maybe we don't actually need to
> get the padding out, but I'm not sure whether the crypto layer will
> malfunction if we don't give it a buffer for the padding.

It was the memset that threw me for a loop.

David, are these encrypted keys ever exported anywhere?  If not, could
the code use a mode that doesn't need padding?

--Andy

>
> David



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread Andy Lutomirski 
On Tue, Dec 13, 2016 at 6:48 PM, Andy Lutomirski <l...@kernel.org> wrote:
> The driver put a constant buffer of all zeros on the stack and
> pointed a scatterlist entry at it in two places.  This doesn't work
> with virtual stacks.  Use ZERO_PAGE instead.

Wait a second...

> -   sg_set_buf(_out[1], pad, sizeof pad);
> +   sg_set_buf(_out[1], empty_zero_page, 16);

My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
exactly is the code trying to do?  The old code makes no sense.  It's
setting the *output* buffer to zeroed padding.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-13 Thread Andy Lutomirski 
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it.  This doesn't work with virtual
stacks.  Use ZERO_PAGE instead.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---
 drivers/usb/wusbcore/crypto.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 79451f7ef1b7..062c205f0046 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -216,7 +216,6 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
-   const u8 bzero[16] = { 0 };
u8 iv[crypto_skcipher_ivsize(tfm_cbc)];
size_t zero_padding;
 
@@ -261,7 +260,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
sg_set_buf([1], >b1, sizeof(scratch->b1));
sg_set_buf([2], b, blen);
/* 0 if well behaved :) */
-   sg_set_buf([3], bzero, zero_padding);
+   sg_set_page([3], ZERO_PAGE(0), zero_padding, 0);
sg_init_one(_dst, dst_buf, dst_size);
 
skcipher_request_set_tfm(req, tfm_cbc);
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread Andy Lutomirski 
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it in two places.  This doesn't work
with virtual stacks.  Use ZERO_PAGE instead.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---
 security/keys/encrypted-keys/encrypted.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..e963160ed26a 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -481,7 +481,6 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
unsigned int padlen;
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -493,11 +492,10 @@ static int derived_key_encrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_decrypted_data(epayload);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 2);
sg_set_buf(_in[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(_in[1], pad, padlen);
+   sg_set_page(_in[1], ZERO_PAGE(0), padlen, 0);
 
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
@@ -584,7 +582,6 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
struct skcipher_request *req;
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -594,13 +591,12 @@ static int derived_key_decrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_encrypted_data(epayload, encrypted_datalen);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 1);
sg_init_table(sg_out, 2);
sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen);
sg_set_buf(_out[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(_out[1], pad, sizeof pad);
+   sg_set_buf(_out[1], empty_zero_page, 16);
 
memcpy(iv, epayload->iv, sizeof(iv));
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread Andy Lutomirski 
On Tue, Dec 13, 2016 at 8:45 AM, David Howells <dhowe...@redhat.com> wrote:
> Andy Lutomirski <l...@amacapital.net> wrote:
>
>> After all, rodata is ordinary memory, is backed by struct page, etc.
>
> Is that actually true?  I thought some arches excluded the kernel image from
> the page struct array to make the array consume less memory.

I don't know whether you're right, but that sounds a bit silly to me.
This is a *tiny* amount of memory.

But there's yet another snag.  Alpha doesn't have empty_zero_page --
it only has ZERO_PAGE.  I could do page_address(ZERO_PAGE(0))...

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations

2016-12-13 Thread Andy Lutomirski 
On Tue, Dec 13, 2016 at 3:35 AM, Kalle Valo <kv...@codeaurora.org> wrote:
> Andy Lutomirski <l...@kernel.org> writes:
>
>> Eric Biggers pointed out that the orinoco driver pointed scatterlists
>> at the stack.
>>
>> Fix it by switching from ahash to shash.  The result should be
>> simpler, faster, and more correct.
>>
>> Cc: sta...@vger.kernel.org # 4.9 only
>> Reported-by: Eric Biggers <ebigge...@gmail.com>
>> Signed-off-by: Andy Lutomirski <l...@kernel.org>
>
> "more correct"? Does this fix a real user visible bug or what? And why
> just stable 4.9, does this maybe have something to do with
> CONFIG_VMAP_STACK?

Whoops, I had that text in some other patches but forgot to put it in
this one.  It'll blow up with CONFIG_VMAP_STACK=y if a debug option
like CONFIG_DEBUG_VIRTUAL=y is set.  It may work by accident if
debugging is off.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread Andy Lutomirski 
[add some people who might know]

On Tue, Dec 13, 2016 at 4:20 AM, David Laight <david.lai...@aculab.com> wrote:
> From: Andy Lutomirski
>> Sent: 12 December 2016 20:53
>> The driver put a constant buffer of all zeros on the stack and
>> pointed a scatterlist entry at it in two places.  This doesn't work
>> with virtual stacks.  Use a static 16-byte buffer of zeros instead.
> ...
>
> I didn't think you could dma from static data either.

According to lib/dma-debug.c, you can't dma to or from kernel text or
rodata, but you can dma to or from kernel bss or data.  So
empty_zero_page should be okay, because it's not rodata right now.

But I think this is rather silly.  Joerg, Linus, etc: would it be okay
to change lib/dma-debug.c to allow DMA *from* rodata?  After all,
rodata is ordinary memory, is backed by struct page, etc.  And DMA
from the zero page had better be okay because I think it happens if
you mmap some zeros, don't write to them, and then direct I/O them to
a device.  Then I could also move empty_zero_page to rodata.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread Andy Lutomirski 
On Mon, Dec 12, 2016 at 2:28 PM, David Howells <dhowe...@redhat.com> wrote:
> Andy Lutomirski <l...@kernel.org> wrote:
>
>> +static const char zero_pad[16] = {0};
>
> Isn't there a global page of zeros or something that we can share?  Also, you
> shouldn't explicitly initialise it so that it stays in .bss.

This is a double-edged sword.  It seems that omitting the
initialization causes it to go in .bss, which isn't read only.  I have
no idea why initializing make a difference at all -- the IMO sensible
behavior would be to put it in .rodata as NOBITS either way.

But I'll use empty_zero_page.

>
>> - sg_set_buf(_out[1], pad, sizeof pad);
>> + sg_set_buf(_out[1], zero_pad, sizeof zero_pad);
>
> Can you put brackets on the sizeof?

Will do for v2.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-12 Thread Andy Lutomirski 
On Mon, Dec 12, 2016 at 1:44 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Mon, Dec 12, 2016 at 12:52:45PM -0800, Andy Lutomirski wrote:
>> The driver put a constant buffer of all zeros on the stack and
>> pointed a scatterlist entry at it.  This doesn't work with virtual
>> stacks.  Make the buffer static to fix it.
>>
>> Cc: sta...@vger.kernel.org # 4.9 only
>> Reported-by: Eric Biggers <ebigge...@gmail.com>
>> Signed-off-by: Andy Lutomirski <l...@kernel.org>
>> ---
>>  drivers/usb/wusbcore/crypto.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
>> index 79451f7ef1b7..a7e007a0cd49 100644
>> --- a/drivers/usb/wusbcore/crypto.c
>> +++ b/drivers/usb/wusbcore/crypto.c
>> @@ -216,7 +216,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
>>   struct scatterlist sg[4], sg_dst;
>>   void *dst_buf;
>>   size_t dst_size;
>> - const u8 bzero[16] = { 0 };
>> + static const u8 bzero[16] = { 0 };
>
> Hm, can static memory handle DMA?  That's a requirement of the USB
> stack, does this data later end up being sent down to a USB host
> controller?

I think it doesn't, but I'll switch it to use empty_zero_page instead.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] orinoco: Use shash instead of ahash for MIC calculations

2016-12-12 Thread Andy Lutomirski 
Eric Biggers pointed out that the orinoco driver pointed scatterlists
at the stack.

Fix it by switching from ahash to shash.  The result should be
simpler, faster, and more correct.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---

Compile-tested only.

drivers/net/wireless/intersil/orinoco/mic.c | 44 +++--
 drivers/net/wireless/intersil/orinoco/mic.h |  3 +-
 drivers/net/wireless/intersil/orinoco/orinoco.h |  4 +--
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/mic.c 
b/drivers/net/wireless/intersil/orinoco/mic.c
index bc7397d709d3..08bc7822f820 100644
--- a/drivers/net/wireless/intersil/orinoco/mic.c
+++ b/drivers/net/wireless/intersil/orinoco/mic.c
@@ -16,7 +16,7 @@
 //
 int orinoco_mic_init(struct orinoco_private *priv)
 {
-   priv->tx_tfm_mic = crypto_alloc_ahash("michael_mic", 0,
+   priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
  CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->tx_tfm_mic)) {
printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
@@ -25,7 +25,7 @@ int orinoco_mic_init(struct orinoco_private *priv)
return -ENOMEM;
}
 
-   priv->rx_tfm_mic = crypto_alloc_ahash("michael_mic", 0,
+   priv->rx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
  CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->rx_tfm_mic)) {
printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
@@ -40,17 +40,16 @@ int orinoco_mic_init(struct orinoco_private *priv)
 void orinoco_mic_free(struct orinoco_private *priv)
 {
if (priv->tx_tfm_mic)
-   crypto_free_ahash(priv->tx_tfm_mic);
+   crypto_free_shash(priv->tx_tfm_mic);
if (priv->rx_tfm_mic)
-   crypto_free_ahash(priv->rx_tfm_mic);
+   crypto_free_shash(priv->rx_tfm_mic);
 }
 
-int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
+int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
u8 *da, u8 *sa, u8 priority,
u8 *data, size_t data_len, u8 *mic)
 {
-   AHASH_REQUEST_ON_STACK(req, tfm_michael);
-   struct scatterlist sg[2];
+   SHASH_DESC_ON_STACK(desc, tfm_michael);
u8 hdr[ETH_HLEN + 2]; /* size of header + padding */
int err;
 
@@ -67,18 +66,27 @@ int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
hdr[ETH_ALEN * 2 + 2] = 0;
hdr[ETH_ALEN * 2 + 3] = 0;
 
-   /* Use scatter gather to MIC header and data in one go */
-   sg_init_table(sg, 2);
-   sg_set_buf([0], hdr, sizeof(hdr));
-   sg_set_buf([1], data, data_len);
+   desc->tfm = tfm_michael;
+   desc->flags = 0;
 
-   if (crypto_ahash_setkey(tfm_michael, key, MIC_KEYLEN))
-   return -1;
+   err = crypto_shash_setkey(tfm_michael, key, MIC_KEYLEN);
+   if (err)
+   return err;
+
+   err = crypto_shash_init(desc);
+   if (err)
+   return err;
+
+   err = crypto_shash_update(desc, hdr, sizeof(hdr));
+   if (err)
+   return err;
+
+   err = crypto_shash_update(desc, data, data_len);
+   if (err)
+   return err;
+
+   err = crypto_shash_final(desc, mic);
+   shash_desc_zero(desc);
 
-   ahash_request_set_tfm(req, tfm_michael);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, mic, data_len + sizeof(hdr));
-   err = crypto_ahash_digest(req);
-   ahash_request_zero(req);
return err;
 }
diff --git a/drivers/net/wireless/intersil/orinoco/mic.h 
b/drivers/net/wireless/intersil/orinoco/mic.h
index ce731d05cc98..e8724e889219 100644
--- a/drivers/net/wireless/intersil/orinoco/mic.h
+++ b/drivers/net/wireless/intersil/orinoco/mic.h
@@ -6,6 +6,7 @@
 #define _ORINOCO_MIC_H_
 
 #include 
+#include 
 
 #define MICHAEL_MIC_LEN 8
 
@@ -15,7 +16,7 @@ struct crypto_ahash;
 
 int orinoco_mic_init(struct orinoco_private *priv);
 void orinoco_mic_free(struct orinoco_private *priv);
-int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
+int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
u8 *da, u8 *sa, u8 priority,
u8 *data, size_t data_len, u8 *mic);
 
diff --git a/drivers/net/wireless/intersil/orinoco/orinoco.h 
b/drivers/net/wireless/intersil/orinoco/orinoco.h
index 2f0c84b1c440..5fa1c3e3713f 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco.h
+++ b/drivers/net/wireless/intersil/orinoco/orinoco.h
@@ -152,8 +152,8 @@ struct orinoco_private {
u8 *wpa_ie;
int wpa_ie_len;
 
-   struct crypto_ahash *rx_tfm_

[PATCH] crypto: Make a few drivers depend on !VMAP_STACK

2016-12-12 Thread Andy Lutomirski 
Eric Biggers found several crypto drivers that point scatterlists at
the stack.  These drivers should never load on x86, but, for future
safety, make them depend on !VMAP_STACK.

No -stable backport should be needed as no released kernel
configuration should be affected.

Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---
 drivers/crypto/Kconfig | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f2b223..481e67e54ffd 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -245,7 +245,7 @@ config CRYPTO_DEV_TALITOS
select CRYPTO_BLKCIPHER
select CRYPTO_HASH
select HW_RANDOM
-   depends on FSL_SOC
+   depends on FSL_SOC && !VMAP_STACK
help
  Say 'Y' here to use the Freescale Security Engine (SEC)
  to offload cryptographic algorithm computation.
@@ -357,7 +357,7 @@ config CRYPTO_DEV_PICOXCELL
 
 config CRYPTO_DEV_SAHARA
tristate "Support for SAHARA crypto accelerator"
-   depends on ARCH_MXC && OF
+   depends on ARCH_MXC && OF && !VMAP_STACK
select CRYPTO_BLKCIPHER
select CRYPTO_AES
select CRYPTO_ECB
@@ -410,7 +410,7 @@ endif # if CRYPTO_DEV_UX500
 
 config CRYPTO_DEV_BFIN_CRC
tristate "Support for Blackfin CRC hardware"
-   depends on BF60x
+   depends on BF60x && !VMAP_STACK
help
  Newer Blackfin processors have CRC hardware. Select this if you
  want to use the Blackfin CRC module.
@@ -487,7 +487,7 @@ source "drivers/crypto/qat/Kconfig"
 
 config CRYPTO_DEV_QCE
tristate "Qualcomm crypto engine accelerator"
-   depends on (ARCH_QCOM || COMPILE_TEST) && HAS_DMA && HAS_IOMEM
+   depends on (ARCH_QCOM || COMPILE_TEST) && HAS_DMA && HAS_IOMEM && 
!VMAP_STACK
select CRYPTO_AES
select CRYPTO_DES
select CRYPTO_ECB
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack

2016-12-12 Thread Andy Lutomirski 
smbencrypt() points a scatterlist to the stack, which is breaks if
CONFIG_VMAP_STACK=y.

Fix it by switching to crypto_cipher_encrypt_one().  The new code
should be considerably faster as an added benefit.

This code is nearly identical to some code that Eric Biggers
suggested.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---

Compile-tested only.

fs/cifs/smbencrypt.c | 40 
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c
index 699b7868108f..c12bffefa3c9 100644
--- a/fs/cifs/smbencrypt.c
+++ b/fs/cifs/smbencrypt.c
@@ -23,7 +23,7 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -69,46 +69,22 @@ str_to_key(unsigned char *str, unsigned char *key)
 static int
 smbhash(unsigned char *out, const unsigned char *in, unsigned char *key)
 {
-   int rc;
unsigned char key2[8];
-   struct crypto_skcipher *tfm_des;
-   struct scatterlist sgin, sgout;
-   struct skcipher_request *req;
+   struct crypto_cipher *tfm_des;
 
str_to_key(key, key2);
 
-   tfm_des = crypto_alloc_skcipher("ecb(des)", 0, CRYPTO_ALG_ASYNC);
+   tfm_des = crypto_alloc_cipher("des", 0, 0);
if (IS_ERR(tfm_des)) {
-   rc = PTR_ERR(tfm_des);
-   cifs_dbg(VFS, "could not allocate des crypto API\n");
-   goto smbhash_err;
-   }
-
-   req = skcipher_request_alloc(tfm_des, GFP_KERNEL);
-   if (!req) {
-   rc = -ENOMEM;
cifs_dbg(VFS, "could not allocate des crypto API\n");
-   goto smbhash_free_skcipher;
+   return PTR_ERR(tfm_des);
}
 
-   crypto_skcipher_setkey(tfm_des, key2, 8);
-
-   sg_init_one(, in, 8);
-   sg_init_one(, out, 8);
+   crypto_cipher_setkey(tfm_des, key2, 8);
+   crypto_cipher_encrypt_one(tfm_des, out, in);
+   crypto_free_cipher(tfm_des);
 
-   skcipher_request_set_callback(req, 0, NULL, NULL);
-   skcipher_request_set_crypt(req, , , 8, NULL);
-
-   rc = crypto_skcipher_encrypt(req);
-   if (rc)
-   cifs_dbg(VFS, "could not encrypt crypt key rc: %d\n", rc);
-
-   skcipher_request_free(req);
-
-smbhash_free_skcipher:
-   crypto_free_skcipher(tfm_des);
-smbhash_err:
-   return rc;
+   return 0;
 }
 
 static int
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread Andy Lutomirski 
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it in two places.  This doesn't work
with virtual stacks.  Use a static 16-byte buffer of zeros instead.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---
 security/keys/encrypted-keys/encrypted.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..fab2fb864002 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -46,6 +46,7 @@ static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
 static unsigned int ivsize;
 static int blksize;
+static const char zero_pad[16] = {0};
 
 #define KEY_TRUSTED_PREFIX_LEN (sizeof (KEY_TRUSTED_PREFIX) - 1)
 #define KEY_USER_PREFIX_LEN (sizeof (KEY_USER_PREFIX) - 1)
@@ -481,7 +482,6 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
unsigned int padlen;
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -493,11 +493,10 @@ static int derived_key_encrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_decrypted_data(epayload);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 2);
sg_set_buf(_in[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(_in[1], pad, padlen);
+   sg_set_buf(_in[1], zero_pad, padlen);
 
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
@@ -584,7 +583,6 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
struct skcipher_request *req;
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -594,13 +592,12 @@ static int derived_key_decrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_encrypted_data(epayload, encrypted_datalen);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 1);
sg_init_table(sg_out, 2);
sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen);
sg_set_buf(_out[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(_out[1], pad, sizeof pad);
+   sg_set_buf(_out[1], zero_pad, sizeof zero_pad);
 
memcpy(iv, epayload->iv, sizeof(iv));
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-12 Thread Andy Lutomirski 
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it.  This doesn't work with virtual
stacks.  Make the buffer static to fix it.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers <ebigge...@gmail.com>
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---
 drivers/usb/wusbcore/crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 79451f7ef1b7..a7e007a0cd49 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -216,7 +216,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
-   const u8 bzero[16] = { 0 };
+   static const u8 bzero[16] = { 0 };
u8 iv[crypto_skcipher_ivsize(tfm_cbc)];
size_t zero_padding;
 
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wusb: Stop using the stack for sg crypto scratch space

2016-10-16 Thread Andy Lutomirski 
On Thu, Oct 6, 2016 at 10:25 AM, Andy Lutomirski <l...@kernel.org> wrote:
> Pointing an sg list at the stack is verboten and, with
> CONFIG_VMAP_STACK=y, will malfunction.  Use kmalloc for the wusb
> crypto stack space instead.
>
> Untested -- I'm not entirely convinced that this hardware exists in
> the wild.

Greg, could you queue this up for 4.9?  I can't guarantee that it
works, but I can pretty much guarantee that the driver is busted
without it.

>
> Signed-off-by: Andy Lutomirski <l...@kernel.org>
> ---
>
> This is needed for 4.9 for wusb to have a chance of working on default x86
> configurations.
>
>  drivers/usb/wusbcore/crypto.c | 59 
> +++
>  1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
> index 33acd1599e99..2db79d64e66c 100644
> --- a/drivers/usb/wusbcore/crypto.c
> +++ b/drivers/usb/wusbcore/crypto.c
> @@ -133,6 +133,13 @@ static void bytewise_xor(void *_bo, const void *_bi1, 
> const void *_bi2,
> bo[itr] = bi1[itr] ^ bi2[itr];
>  }
>
> +/* Scratch space for MAC calculations. */
> +struct wusb_mac_scratch {
> +   struct aes_ccm_b0 b0;
> +   struct aes_ccm_b1 b1;
> +   struct aes_ccm_a ax;
> +};
> +
>  /*
>   * CC-MAC function WUSB1.0[6.5]
>   *
> @@ -197,16 +204,15 @@ static void bytewise_xor(void *_bo, const void *_bi1, 
> const void *_bi2,
>   *   what sg[4] is for. Maybe there is a smarter way to do this.
>   */
>  static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> -   struct crypto_cipher *tfm_aes, void *mic,
> +   struct crypto_cipher *tfm_aes,
> +   struct wusb_mac_scratch *scratch,
> +   void *mic,
> const struct aes_ccm_nonce *n,
> const struct aes_ccm_label *a, const void *b,
> size_t blen)
>  {
> int result = 0;
> SKCIPHER_REQUEST_ON_STACK(req, tfm_cbc);
> -   struct aes_ccm_b0 b0;
> -   struct aes_ccm_b1 b1;
> -   struct aes_ccm_a ax;
> struct scatterlist sg[4], sg_dst;
> void *dst_buf;
> size_t dst_size;
> @@ -218,16 +224,17 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
>  * These checks should be compile time optimized out
>  * ensure @a fills b1's mac_header and following fields
>  */
> -   WARN_ON(sizeof(*a) != sizeof(b1) - sizeof(b1.la));
> -   WARN_ON(sizeof(b0) != sizeof(struct aes_ccm_block));
> -   WARN_ON(sizeof(b1) != sizeof(struct aes_ccm_block));
> -   WARN_ON(sizeof(ax) != sizeof(struct aes_ccm_block));
> +   WARN_ON(sizeof(*a) != sizeof(scratch->b1) - sizeof(scratch->b1.la));
> +   WARN_ON(sizeof(scratch->b0) != sizeof(struct aes_ccm_block));
> +   WARN_ON(sizeof(scratch->b1) != sizeof(struct aes_ccm_block));
> +   WARN_ON(sizeof(scratch->ax) != sizeof(struct aes_ccm_block));
>
> result = -ENOMEM;
> zero_padding = blen % sizeof(struct aes_ccm_block);
> if (zero_padding)
> zero_padding = sizeof(struct aes_ccm_block) - zero_padding;
> -   dst_size = blen + sizeof(b0) + sizeof(b1) + zero_padding;
> +   dst_size = blen + sizeof(scratch->b0) + sizeof(scratch->b1) +
> +   zero_padding;
> dst_buf = kzalloc(dst_size, GFP_KERNEL);
> if (dst_buf == NULL) {
> printk(KERN_ERR "E: can't alloc destination buffer\n");
> @@ -237,9 +244,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> memset(iv, 0, sizeof(iv));
>
> /* Setup B0 */
> -   b0.flags = 0x59;/* Format B0 */
> -   b0.ccm_nonce = *n;
> -   b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
> +   scratch->b0.flags = 0x59;   /* Format B0 */
> +   scratch->b0.ccm_nonce = *n;
> +   scratch->b0.lm = cpu_to_be16(0);/* WUSB1.0[6.5] sez l(m) is 0 
> */
>
> /* Setup B1
>  *
> @@ -248,12 +255,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
>  * 14'--after clarification, it means to use A's contents
>  * for MAC Header, EO, sec reserved and padding.
>  */
> -   b1.la = cpu_to_be16(blen + 14);
> -   memcpy(_header, a, sizeof(*a));
> +   scratch->b1.la = cpu_to_be16(blen + 14);
> +   memcpy(>b1.mac_header, a, sizeof(*a));
>
> sg_init_table(sg, ARRAY_SIZE(sg));
> -   sg_set_buf([0], , sizeof(b0));
> -   sg_set_buf([1], , sizeof(b1));
> +   sg_set_buf([0], >b0, sizeof(sc

[PATCH] wusb: Stop using the stack for sg crypto scratch space

2016-10-06 Thread Andy Lutomirski 
Pointing an sg list at the stack is verboten and, with
CONFIG_VMAP_STACK=y, will malfunction.  Use kmalloc for the wusb
crypto stack space instead.

Untested -- I'm not entirely convinced that this hardware exists in
the wild.

Signed-off-by: Andy Lutomirski <l...@kernel.org>
---

This is needed for 4.9 for wusb to have a chance of working on default x86
configurations.

 drivers/usb/wusbcore/crypto.c | 59 +++
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 33acd1599e99..2db79d64e66c 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -133,6 +133,13 @@ static void bytewise_xor(void *_bo, const void *_bi1, 
const void *_bi2,
bo[itr] = bi1[itr] ^ bi2[itr];
 }
 
+/* Scratch space for MAC calculations. */
+struct wusb_mac_scratch {
+   struct aes_ccm_b0 b0;
+   struct aes_ccm_b1 b1;
+   struct aes_ccm_a ax;
+};
+
 /*
  * CC-MAC function WUSB1.0[6.5]
  *
@@ -197,16 +204,15 @@ static void bytewise_xor(void *_bo, const void *_bi1, 
const void *_bi2,
  *   what sg[4] is for. Maybe there is a smarter way to do this.
  */
 static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
-   struct crypto_cipher *tfm_aes, void *mic,
+   struct crypto_cipher *tfm_aes,
+   struct wusb_mac_scratch *scratch,
+   void *mic,
const struct aes_ccm_nonce *n,
const struct aes_ccm_label *a, const void *b,
size_t blen)
 {
int result = 0;
SKCIPHER_REQUEST_ON_STACK(req, tfm_cbc);
-   struct aes_ccm_b0 b0;
-   struct aes_ccm_b1 b1;
-   struct aes_ccm_a ax;
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
@@ -218,16 +224,17 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
 * These checks should be compile time optimized out
 * ensure @a fills b1's mac_header and following fields
 */
-   WARN_ON(sizeof(*a) != sizeof(b1) - sizeof(b1.la));
-   WARN_ON(sizeof(b0) != sizeof(struct aes_ccm_block));
-   WARN_ON(sizeof(b1) != sizeof(struct aes_ccm_block));
-   WARN_ON(sizeof(ax) != sizeof(struct aes_ccm_block));
+   WARN_ON(sizeof(*a) != sizeof(scratch->b1) - sizeof(scratch->b1.la));
+   WARN_ON(sizeof(scratch->b0) != sizeof(struct aes_ccm_block));
+   WARN_ON(sizeof(scratch->b1) != sizeof(struct aes_ccm_block));
+   WARN_ON(sizeof(scratch->ax) != sizeof(struct aes_ccm_block));
 
result = -ENOMEM;
zero_padding = blen % sizeof(struct aes_ccm_block);
if (zero_padding)
zero_padding = sizeof(struct aes_ccm_block) - zero_padding;
-   dst_size = blen + sizeof(b0) + sizeof(b1) + zero_padding;
+   dst_size = blen + sizeof(scratch->b0) + sizeof(scratch->b1) +
+   zero_padding;
dst_buf = kzalloc(dst_size, GFP_KERNEL);
if (dst_buf == NULL) {
printk(KERN_ERR "E: can't alloc destination buffer\n");
@@ -237,9 +244,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
memset(iv, 0, sizeof(iv));
 
/* Setup B0 */
-   b0.flags = 0x59;/* Format B0 */
-   b0.ccm_nonce = *n;
-   b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
+   scratch->b0.flags = 0x59;   /* Format B0 */
+   scratch->b0.ccm_nonce = *n;
+   scratch->b0.lm = cpu_to_be16(0);/* WUSB1.0[6.5] sez l(m) is 0 */
 
/* Setup B1
 *
@@ -248,12 +255,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
 * 14'--after clarification, it means to use A's contents
 * for MAC Header, EO, sec reserved and padding.
 */
-   b1.la = cpu_to_be16(blen + 14);
-   memcpy(_header, a, sizeof(*a));
+   scratch->b1.la = cpu_to_be16(blen + 14);
+   memcpy(>b1.mac_header, a, sizeof(*a));
 
sg_init_table(sg, ARRAY_SIZE(sg));
-   sg_set_buf([0], , sizeof(b0));
-   sg_set_buf([1], , sizeof(b1));
+   sg_set_buf([0], >b0, sizeof(scratch->b0));
+   sg_set_buf([1], >b1, sizeof(scratch->b1));
sg_set_buf([2], b, blen);
/* 0 if well behaved :) */
sg_set_buf([3], bzero, zero_padding);
@@ -278,11 +285,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
 * POS Crypto API: size is assumed to be AES's block size.
 * Thanks for documenting it -- tip taken from airo.c
 */
-   ax.flags = 0x01;/* as per WUSB 1.0 spec */
-   ax.ccm_nonce = *n;
-   ax.counter = 0;
-   crypto_cipher_encrypt_one(tfm_aes, (void *), (void *));
-   bytewise_xor(mic, , iv, 8);
+   scratch->ax.flags = 0x01;   /* as per WUSB 1.0 spec */
+   scratch->ax.ccm_nonce = *n;
+

wusbcore crypto stack sg

2016-08-10 Thread Andy Lutomirski 
Hi Herbert, etc-

drivers/usb/wusbcore/crypto.c is another sg-pointing-to-the-stack
user.  Want to fix it?

(Does wusb hardware even exist in the wild?)

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-22 Thread Andy Lutomirski 
On Wed, Jun 22, 2016 at 4:35 PM, D. Jared Dominguez
<jared_doming...@dell.com> wrote:
> On Wed, Jun 22, 2016 at 04:21:58PM -0700, Andy Lutomirski wrote:
>>
>> On Wed, Jun 22, 2016 at 3:29 PM,  <mario_limoncie...@dell.com> wrote:
>>>>
>>>> >
>>>> > I've only got WD15 and TB15's.
>>>>
>>>> Do you have a link to the product page for these?  I'll probably buy
>>>> one in the next couple of weeks, but I can only find the WD15, not the
>>>> TB15.  From the FAQ
>>>> (http://www.dell.com/support/article/us/en/04/SLN301105), it looks
>>>> like the TB15 supports 2x DP 4k 60Hz, which is a feature I want, but I
>>>> don't see how to buy one.  Are they available anywhere?
>>>>
>>>
>>> That's getting into a question can't answer other than "not currently
>>> for sale".  Read into that what you will, but I can't say more and
>>> hopefully in a few weeks it's back up.
>>>
>>>> Also, the DA200 bizarrely doesn't list itself as being compatible with
>>>> the XPS 13 on dell.com, and it doesn't show up in the list of XPS13
>>>> accessories.  This is sad, because it's a nice little travel adapter.
>>>
>>>
>>> That's really surprising.  Was it just listed as not compatible on the
>>> Ubuntu version or both Ubuntu and Windows didn't have it?
>>
>>
>> The website is very confused.
>>
>> This DA200 comes up as the first Google hit.  It's $121:
>>
>>
>> http://accessories.ap.dell.com/sna/productdetail.aspx?c=sg=en=bsd=sgbsd1=470-abnl
>
>
> That one is for the Singapore site.
>
>> It claims to be compatible with:
>>
>> Compatibility
>> This product is compatible with the following systems:
>> Latitude 11 (5175)
>> Latitude 11 (5179)
>> Precision 15 3000 Series (3510)
>> Precision 15 5000 Series (5510)
>> Precision 15 7000 Series (7510)
>> Precision 17 7000 Series (7710)
>>
>> This one comes up if I search dell.com directly or if I constrain the
>> Google search to site:accessories.dell.com:
>>
>>
>> http://accessories.dell.com/sna/productdetail.aspx?c=us=en=dhs=19=470-ABQN
>>
>> It only claims to be compatible with Latitude 7275.
>
>
> I've learned not to entirely trust the compatibility list when looking at
> our software and peripherals site.
>
>> I couldn't find the DA200 linked from the XPS13 accessories page (my
>> service tag, and mine is a Signature Edition) at all.
>
>
> If you look on the dell.com order configuration page, the DA200 should come
> up.
>
> Anyway, it's supported on the Skylake XPS 13 (9350). We don't support it
> with Linux _yet_ though.

Indeed. :-/

I plugged it in and plugged an HDMI display into it, and it very
loudly failed to train the DP link.  I assume it's referring to the
internal DP alternate mode link to the dongle.  Maybe it's finally
time for me to boot windows and upgrade the Alpine Ridge firmware...

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-22 Thread Andy Lutomirski 
On Wed, Jun 22, 2016 at 3:29 PM,   wrote:
>> >
>> > I've only got WD15 and TB15's.
>>
>> Do you have a link to the product page for these?  I'll probably buy
>> one in the next couple of weeks, but I can only find the WD15, not the
>> TB15.  From the FAQ
>> (http://www.dell.com/support/article/us/en/04/SLN301105), it looks
>> like the TB15 supports 2x DP 4k 60Hz, which is a feature I want, but I
>> don't see how to buy one.  Are they available anywhere?
>>
>
> That's getting into a question can't answer other than "not currently
> for sale".  Read into that what you will, but I can't say more and
> hopefully in a few weeks it's back up.
>
>> Also, the DA200 bizarrely doesn't list itself as being compatible with
>> the XPS 13 on dell.com, and it doesn't show up in the list of XPS13
>> accessories.  This is sad, because it's a nice little travel adapter.
>
> That's really surprising.  Was it just listed as not compatible on the
> Ubuntu version or both Ubuntu and Windows didn't have it?

The website is very confused.

This DA200 comes up as the first Google hit.  It's $121:

http://accessories.ap.dell.com/sna/productdetail.aspx?c=sg=en=bsd=sgbsd1=470-abnl

It claims to be compatible with:

Compatibility
This product is compatible with the following systems:
Latitude 11 (5175)
Latitude 11 (5179)
Precision 15 3000 Series (3510)
Precision 15 5000 Series (5510)
Precision 15 7000 Series (7510)
Precision 17 7000 Series (7710)

This one comes up if I search dell.com directly or if I constrain the
Google search to site:accessories.dell.com:

http://accessories.dell.com/sna/productdetail.aspx?c=us=en=dhs=19=470-ABQN

It only claims to be compatible with Latitude 7275.

I couldn't find the DA200 linked from the XPS13 accessories page (my
service tag, and mine is a Signature Edition) at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-22 Thread Andy Lutomirski 
On Wed, Jun 22, 2016 at 3:06 PM,  <mario_limoncie...@dell.com> wrote:
>> -Original Message-----
>> From: Andy Lutomirski [mailto:l...@amacapital.net]
>> Sent: Wednesday, June 22, 2016 3:44 PM
>> To: Limonciello, Mario <mario_limoncie...@dell.com>
>> Cc: Greg KH <gre...@linuxfoundation.org>; Andrew Lutomirski
>> <l...@kernel.org>; Mathias Nyman <mathias.ny...@linux.intel.com>; USB
>> list <linux-usb@vger.kernel.org>; Mathias Nyman
>> <mathias.ny...@intel.com>; Dominguez, Jared
>> <jared_doming...@dell.com>
>> Subject: Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)
>>
>> On Fri, Jun 17, 2016 at 12:25 PM, Andy Lutomirski <l...@amacapital.net>
>> wrote:
>> > On Fri, Jun 17, 2016 at 11:46 AM,  <mario_limoncie...@dell.com> wrote:
>> >>> -Original Message-
>> >>> From: Andy Lutomirski [mailto:l...@amacapital.net]
>> >>> Sent: Thursday, June 16, 2016 1:41 PM
>> >>> To: Limonciello, Mario <mario_limoncie...@dell.com>
>> >>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andy
>> Lutomirski
>> >>> <l...@kernel.org>; Mathias Nyman <mathias.ny...@linux.intel.com>;
>> USB
>> >>> list <linux-usb@vger.kernel.org>; Mathias Nyman
>> >>> <mathias.ny...@intel.com>; Dominguez, Jared
>> >>> <jared_doming...@dell.com>
>> >>> Subject: Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350
>> (Skylake)
>> >>>
>> >>> On Sun, Mar 13, 2016 at 7:29 PM, Mario Limonciello
>> >>> <mario_limoncie...@dell.com> wrote:
>> >>> >
>> >>> >
>> >>> > On 03/12/2016 02:33 PM, Andy Lutomirski wrote:
>> >>> >> On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski
>> <l...@amacapital.net>
>> >>> wrote:
>> >>> >> Got it.  I was barking up the wrong tree.
>> >>> >>
>> >>> >> Q: What happens if _Q66 runs concurrently with itself:
>> >>> >>
>> >>> >> A:
>> >>> >>
>> >>> >> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>> >>> >> {
>> >>> >> Acquire (PATM, 0x0064)
>> >>> >> If ((ECRD != One))
>> >>> >> {
>> >>> >> Return (Zero)
>> >>> >> }
>> >>> >>
>> >>> >> NEVT ()
>> >>> >> Release (PATM)
>> >>> >> Return (Zero)
>> >>> >> }
>> >>> >>
>> >>> >> The first one acquires PATM.  The second one fails to acquire PATM
>> due
>> >>> >> to the timeout, does something potentially harmful when it reenters
>> >>> >> NEVT (not sure -- maybe it's fine), then blows up when it tries to
>> >>> >> release PATM, which it doesn't hold.
>> >>> >>
>> >>> >> --Andy
>> >>> > Andy,
>> >>> >
>> >>> > Our team has confirmed this mistake and will issue a fix in a future
>> >>> > BIOS.  For now if you want to build your own DSDT to see if this is
>> >>> > causing your type-C problems the Release(PATM) will be inserted in
>> the
>> >>> > obvious location.
>> >>> >
>> >>> > FWIW this issue will affect many platforms in this generation.  (XPS
>> >>> > 9550, XPS 9350, Precision 5510, and more)
>> >>> >
>> >>>
>> >>> FYI: the H_EC.CHRG issue seems to be fixed in 1.4.3.  The PATM issue
>> >>> is still there by inspection of the code, although I haven't gotten
>> >>> unlucky enough to trigger it yet.
>> >>
>> >> Thanks for the heads up.  I'll poke the team and find out if it was pushed
>> >> out, or it was a different fix than previously planned.  If you trigger 
>> >> it and
>> >> it's still leading to any problems let me know.
>> >>
>> >
>> >
>> > I hit it again.  I haven't seen any observable problem yet, but I also
>> > don't have anything connected to the USB-C port right now.  (I don't
>> > k

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-22 Thread Andy Lutomirski 
On Fri, Jun 17, 2016 at 12:25 PM, Andy Lutomirski <l...@amacapital.net> wrote:
> On Fri, Jun 17, 2016 at 11:46 AM,  <mario_limoncie...@dell.com> wrote:
>>> -Original Message-
>>> From: Andy Lutomirski [mailto:l...@amacapital.net]
>>> Sent: Thursday, June 16, 2016 1:41 PM
>>> To: Limonciello, Mario <mario_limoncie...@dell.com>
>>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andy Lutomirski
>>> <l...@kernel.org>; Mathias Nyman <mathias.ny...@linux.intel.com>; USB
>>> list <linux-usb@vger.kernel.org>; Mathias Nyman
>>> <mathias.ny...@intel.com>; Dominguez, Jared
>>> <jared_doming...@dell.com>
>>> Subject: Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 
>>> (Skylake)
>>>
>>> On Sun, Mar 13, 2016 at 7:29 PM, Mario Limonciello
>>> <mario_limoncie...@dell.com> wrote:
>>> >
>>> >
>>> > On 03/12/2016 02:33 PM, Andy Lutomirski wrote:
>>> >> On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <l...@amacapital.net>
>>> wrote:
>>> >> Got it.  I was barking up the wrong tree.
>>> >>
>>> >> Q: What happens if _Q66 runs concurrently with itself:
>>> >>
>>> >> A:
>>> >>
>>> >> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>>> >> {
>>> >> Acquire (PATM, 0x0064)
>>> >> If ((ECRD != One))
>>> >> {
>>> >> Return (Zero)
>>> >> }
>>> >>
>>> >> NEVT ()
>>> >> Release (PATM)
>>> >> Return (Zero)
>>> >> }
>>> >>
>>> >> The first one acquires PATM.  The second one fails to acquire PATM due
>>> >> to the timeout, does something potentially harmful when it reenters
>>> >> NEVT (not sure -- maybe it's fine), then blows up when it tries to
>>> >> release PATM, which it doesn't hold.
>>> >>
>>> >> --Andy
>>> > Andy,
>>> >
>>> > Our team has confirmed this mistake and will issue a fix in a future
>>> > BIOS.  For now if you want to build your own DSDT to see if this is
>>> > causing your type-C problems the Release(PATM) will be inserted in the
>>> > obvious location.
>>> >
>>> > FWIW this issue will affect many platforms in this generation.  (XPS
>>> > 9550, XPS 9350, Precision 5510, and more)
>>> >
>>>
>>> FYI: the H_EC.CHRG issue seems to be fixed in 1.4.3.  The PATM issue
>>> is still there by inspection of the code, although I haven't gotten
>>> unlucky enough to trigger it yet.
>>
>> Thanks for the heads up.  I'll poke the team and find out if it was pushed
>> out, or it was a different fix than previously planned.  If you trigger it 
>> and
>> it's still leading to any problems let me know.
>>
>
>
> I hit it again.  I haven't seen any observable problem yet, but I also
> don't have anything connected to the USB-C port right now.  (I don't
> know what _Q66 is used for, so maybe it has nothing to do with USB-C.
> But it's certainly not doing whatever it's supposed to do correctly
> given this bug.
>
> [  +0.037295] ACPI Error: Cannot release Mutex [PATM], not acquired
> (20160108/exmutex-393)
>
> [  +0.11] No Local Variables are initialized for method [_Q66]
>
> [  +0.02] No Arguments are initialized for method [_Q66]
>
> [  +0.03] ACPI Error: Method parse/execution failed
> [\_SB.PCI0.LPCB.ECDV._Q66] (Node 8802760ec8c0), AE_AML_MUTEX_NOT_A
>
> --Andy

I got a DA200 adapter.  When I plugged it in, it said:

[106415.096022] ACPI Error: [SPRT] Namespace lookup failure,
AE_ALREADY_EXISTS (20160108/dswload2-330)

[106415.096050] No Local Variables are initialized for method [_E42]

[106415.096057] No Arguments are initialized for method [_E42]

[106415.096063] ACPI Exception: AE_ALREADY_EXISTS, During name
lookup/catalog (20160108/psobject-227)
[106415.096071] ACPI Error: Method parse/execution failed [\_GPE._E42]
(Node 8802760dbe38), AE_ALREADY_EXISTS (20160108/psparse-542)
[106415.096085] ACPI Error: Method parse/execution failed [\_GPE._E42]
(Node 8802760dbe38), AE_ALREADY_EXISTS (20160108/psparse-542)
[106415.096104] ACPI Exception: AE_ALREADY_EXISTS, while evaluating
GPE method [_E42] (20160108/evgpe-592)

Other than that, it seems to work, at least as far as I can tell
without connecting the other end to anything.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-17 Thread Andy Lutomirski 
On Fri, Jun 17, 2016 at 11:46 AM,  <mario_limoncie...@dell.com> wrote:
>> -Original Message-----
>> From: Andy Lutomirski [mailto:l...@amacapital.net]
>> Sent: Thursday, June 16, 2016 1:41 PM
>> To: Limonciello, Mario <mario_limoncie...@dell.com>
>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andy Lutomirski
>> <l...@kernel.org>; Mathias Nyman <mathias.ny...@linux.intel.com>; USB
>> list <linux-usb@vger.kernel.org>; Mathias Nyman
>> <mathias.ny...@intel.com>; Dominguez, Jared
>> <jared_doming...@dell.com>
>> Subject: Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)
>>
>> On Sun, Mar 13, 2016 at 7:29 PM, Mario Limonciello
>> <mario_limoncie...@dell.com> wrote:
>> >
>> >
>> > On 03/12/2016 02:33 PM, Andy Lutomirski wrote:
>> >> On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <l...@amacapital.net>
>> wrote:
>> >> Got it.  I was barking up the wrong tree.
>> >>
>> >> Q: What happens if _Q66 runs concurrently with itself:
>> >>
>> >> A:
>> >>
>> >> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>> >> {
>> >> Acquire (PATM, 0x0064)
>> >> If ((ECRD != One))
>> >> {
>> >> Return (Zero)
>> >> }
>> >>
>> >> NEVT ()
>> >> Release (PATM)
>> >> Return (Zero)
>> >> }
>> >>
>> >> The first one acquires PATM.  The second one fails to acquire PATM due
>> >> to the timeout, does something potentially harmful when it reenters
>> >> NEVT (not sure -- maybe it's fine), then blows up when it tries to
>> >> release PATM, which it doesn't hold.
>> >>
>> >> --Andy
>> > Andy,
>> >
>> > Our team has confirmed this mistake and will issue a fix in a future
>> > BIOS.  For now if you want to build your own DSDT to see if this is
>> > causing your type-C problems the Release(PATM) will be inserted in the
>> > obvious location.
>> >
>> > FWIW this issue will affect many platforms in this generation.  (XPS
>> > 9550, XPS 9350, Precision 5510, and more)
>> >
>>
>> FYI: the H_EC.CHRG issue seems to be fixed in 1.4.3.  The PATM issue
>> is still there by inspection of the code, although I haven't gotten
>> unlucky enough to trigger it yet.
>
> Thanks for the heads up.  I'll poke the team and find out if it was pushed
> out, or it was a different fix than previously planned.  If you trigger it and
> it's still leading to any problems let me know.
>


I hit it again.  I haven't seen any observable problem yet, but I also
don't have anything connected to the USB-C port right now.  (I don't
know what _Q66 is used for, so maybe it has nothing to do with USB-C.
But it's certainly not doing whatever it's supposed to do correctly
given this bug.

[  +0.037295] ACPI Error: Cannot release Mutex [PATM], not acquired
(20160108/exmutex-393)

[  +0.11] No Local Variables are initialized for method [_Q66]

[  +0.02] No Arguments are initialized for method [_Q66]

[  +0.03] ACPI Error: Method parse/execution failed
[\_SB.PCI0.LPCB.ECDV._Q66] (Node 8802760ec8c0), AE_AML_MUTEX_NOT_A

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-06-16 Thread Andy Lutomirski 
On Sun, Mar 13, 2016 at 7:29 PM, Mario Limonciello
<mario_limoncie...@dell.com> wrote:
>
>
> On 03/12/2016 02:33 PM, Andy Lutomirski wrote:
>> On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <l...@amacapital.net> 
>> wrote:
>> Got it.  I was barking up the wrong tree.
>>
>> Q: What happens if _Q66 runs concurrently with itself:
>>
>> A:
>>
>> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>> {
>> Acquire (PATM, 0x0064)
>> If ((ECRD != One))
>> {
>> Return (Zero)
>> }
>>
>> NEVT ()
>> Release (PATM)
>> Return (Zero)
>> }
>>
>> The first one acquires PATM.  The second one fails to acquire PATM due
>> to the timeout, does something potentially harmful when it reenters
>> NEVT (not sure -- maybe it's fine), then blows up when it tries to
>> release PATM, which it doesn't hold.
>>
>> --Andy
> Andy,
>
> Our team has confirmed this mistake and will issue a fix in a future
> BIOS.  For now if you want to build your own DSDT to see if this is
> causing your type-C problems the Release(PATM) will be inserted in the
> obvious location.
>
> FWIW this issue will affect many platforms in this generation.  (XPS
> 9550, XPS 9350, Precision 5510, and more)
>

FYI: the H_EC.CHRG issue seems to be fixed in 1.4.3.  The PATM issue
is still there by inspection of the code, although I haven't gotten
unlucky enough to trigger it yet.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-13 Thread Andy Lutomirski 
On Sun, Mar 13, 2016 at 7:29 PM, Mario Limonciello
<mario_limoncie...@dell.com> wrote:
>
>
> On 03/12/2016 02:33 PM, Andy Lutomirski wrote:
>> On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <l...@amacapital.net> 
>> wrote:
>> Got it.  I was barking up the wrong tree.
>>
>> Q: What happens if _Q66 runs concurrently with itself:
>>
>> A:
>>
>> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>> {
>> Acquire (PATM, 0x0064)
>> If ((ECRD != One))
>> {
>> Return (Zero)
>> }
>>
>> NEVT ()
>> Release (PATM)
>> Return (Zero)
>> }
>>
>> The first one acquires PATM.  The second one fails to acquire PATM due
>> to the timeout, does something potentially harmful when it reenters
>> NEVT (not sure -- maybe it's fine), then blows up when it tries to
>> release PATM, which it doesn't hold.
>>
>> --Andy
> Andy,
>
> Our team has confirmed this mistake and will issue a fix in a future
> BIOS.  For now if you want to build your own DSDT to see if this is
> causing your type-C problems the Release(PATM) will be inserted in the
> obvious location.
>
> FWIW this issue will affect many platforms in this generation.  (XPS
> 9550, XPS 9350, Precision 5510, and more)

I've applied this patch:

DefinitionBlock ("q66patch", "DSDT", 2, "ANDY  ", "LUTO   ", 0x01072009)
{
External (ECRD, IntObj)
External (PATM, UnknownObj)
External (NEVT, MethodObj)

Method (\_SB.PCI0.LPCB._Q66, 0, NotSerialized)  // _Qxx: EC Query
{
if (Acquire (PATM, 0x03e8) != Zero)
{
return (Zero)
}

If ((ECRD != One))
{
Release (PATM)
Return (Zero)
}

NEVT ()
Release (PATM)
Return (Zero)
}
}

IOW, I've increased the timeout, handled the timeout, and added the
missing release.  Still no luck with USB-C.  Maybe I'll try to find a
way to apply that Alpine Ridge firmware update.  But I also could
easily have screwed up the patching process.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-12 Thread Andy Lutomirski 
On Sat, Mar 12, 2016 at 11:35 AM, Andy Lutomirski <l...@amacapital.net> wrote:
> On Wed, Mar 9, 2016 at 5:34 PM, Mario Limonciello
> <mario_limoncie...@dell.com> wrote:
>>
>>
>> On 03/07/2016 02:42 PM, Andy Lutomirski wrote:
>>> On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
>>> <gre...@linuxfoundation.org> wrote:
>>> I'm adding a couple Dell people.
>>>
>>> Hi, Dell people-
>>>
>>> Your latest DSDT has this blatantly buggy method:
>>>
>>> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>>> {
>>> Acquire (PATM, 0x0064)
>>> If ((ECRD != One))
>>> {
>>> Return (Zero)
>>> }
>>>
>>> NEVT ()
>>> Release (PATM)
>>> Return (Zero)
>>> }
>>>
>>> At some point during boot (presumably), this runs with ECRD == 0,
>>> causing PATM to be acquired and never released.  Later on, something
>>> involved in USB-C hotplug (I think -- I can occasionally trigger
>>> errors during hotplug) breaks when it can't acquire PATM.
>>>
>>> Could you ask your BIOS team to please add the obviously missing Release 
>>> (PATM)?
>>>
>>> I don't know if fixing this bug will solve all the USB-C issues, but
>>> it seems unlikely to hurt.  I'm going to try to get it to work with a
>>> custom method, but I may or may not succeed.
>>>
>>> --Andy
>>>
>>>
>> Andy,
>>
>> Sorry I haven't gotten back.  I was hoping to have an answer when I
>> responded but not yet so just wanted to let you know the message wasn't
>> missed.
>> I'm inquiring to my team about this.  I'm unsure the circumstances that
>> the EC isn't ready (race condition?) so it might take some time to get
>> answer.
>
> It looks like ECRD is managed entirely in AML like this:
>
> Method (_REG, 2, NotSerialized)  // _REG: Region Availability
> {
> If (((Arg1 == One) == (Arg0 == 0x03)))
> {
> ECRD = One
> ECIN ()
> }
>
> If (((Arg1 == Zero) && (Arg0 == 0x03)))
> {
> ECRD = Zero
> }
> }
>
> That is also buggy.  (((Arg1 == One) == (Arg0 == 0x03))) should be
> (((Arg1 == One) && (Arg0 == 0x03))).  But that bug goes in the other
> direction, so I don't think it explains how this gets triggered.
>
> I've tried to instrument the code, but I haven't found it yet.

Got it.  I was barking up the wrong tree.

Q: What happens if _Q66 runs concurrently with itself:

A:

Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
{
Acquire (PATM, 0x0064)
If ((ECRD != One))
{
Return (Zero)
}

NEVT ()
Release (PATM)
Return (Zero)
}

The first one acquires PATM.  The second one fails to acquire PATM due
to the timeout, does something potentially harmful when it reenters
NEVT (not sure -- maybe it's fine), then blows up when it tries to
release PATM, which it doesn't hold.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-12 Thread Andy Lutomirski 
On Wed, Mar 9, 2016 at 5:34 PM, Mario Limonciello
<mario_limoncie...@dell.com> wrote:
>
>
> On 03/07/2016 02:42 PM, Andy Lutomirski wrote:
>> On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> I'm adding a couple Dell people.
>>
>> Hi, Dell people-
>>
>> Your latest DSDT has this blatantly buggy method:
>>
>> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
>> {
>> Acquire (PATM, 0x0064)
>> If ((ECRD != One))
>> {
>> Return (Zero)
>> }
>>
>> NEVT ()
>> Release (PATM)
>> Return (Zero)
>> }
>>
>> At some point during boot (presumably), this runs with ECRD == 0,
>> causing PATM to be acquired and never released.  Later on, something
>> involved in USB-C hotplug (I think -- I can occasionally trigger
>> errors during hotplug) breaks when it can't acquire PATM.
>>
>> Could you ask your BIOS team to please add the obviously missing Release 
>> (PATM)?
>>
>> I don't know if fixing this bug will solve all the USB-C issues, but
>> it seems unlikely to hurt.  I'm going to try to get it to work with a
>> custom method, but I may or may not succeed.
>>
>> --Andy
>>
>>
> Andy,
>
> Sorry I haven't gotten back.  I was hoping to have an answer when I
> responded but not yet so just wanted to let you know the message wasn't
> missed.
> I'm inquiring to my team about this.  I'm unsure the circumstances that
> the EC isn't ready (race condition?) so it might take some time to get
> answer.

It looks like ECRD is managed entirely in AML like this:

Method (_REG, 2, NotSerialized)  // _REG: Region Availability
{
If (((Arg1 == One) == (Arg0 == 0x03)))
{
ECRD = One
ECIN ()
}

If (((Arg1 == Zero) && (Arg0 == 0x03)))
{
ECRD = Zero
}
}

That is also buggy.  (((Arg1 == One) == (Arg0 == 0x03))) should be
(((Arg1 == One) && (Arg0 == 0x03))).  But that bug goes in the other
direction, so I don't think it explains how this gets triggered.

I've tried to instrument the code, but I haven't found it yet.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-08 Thread Andy Lutomirski 
On Tue, Mar 8, 2016 at 10:14 AM, Mathias Nyman <mathias.ny...@intel.com> wrote:
> On 07.03.2016 22:13, Greg Kroah-Hartman wrote:
>>
>> On Mon, Mar 07, 2016 at 11:09:36AM -0800, Andy Lutomirski wrote:
>>>
>>> Quick ping here: this is still busted on 4.5-rc6.
>>>
>>> On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski <l...@kernel.org> wrote:
>>>>
>>>> On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
>>>> <mathias.ny...@linux.intel.com> wrote:
>>>>>
>>>>> On 17.02.2016 07:19, Andy Lutomirski wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
>>>>>> <gre...@linuxfoundation.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> I get some warnings at boot on all kernels I've tried.  On 4.5-rc4,
>>>>>>>> I
>>>>>>>> see:
>>>>>>>>
>>>>> ...
>>>>>
>>>>>>>> [1.061036] hub 1-0:1.0: power on to power good time: 20ms
>>>>>>>> [1.061109] hub 1-0:1.0: local power source is good
>>>>>>>> [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
>>>>>>>> according to platform information.
>>>>>>>> [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
>>>>>>>> according to platform information.
>>>>>>>> [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
>>>>>>>> according to platform information.
>>>>>
>>>>>
>>>>> ...
>>>>>>>>
>>>>>>>>
>>>>>>>> [1.085684] usb usb2-port1: peered to usb1-port1
>>>>>>>> [1.086356] usb usb2-port2: peered to usb1-port2
>>>>>>>> [1.087004] usb usb2-port3: peered to usb1-port6
>>>>>>>> [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
>>>>>>>> location (usb2-port4:none) (usb1-port6:usb2-port3)
>>>>>>>> [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
>>>>>>>> [1.087716] usb: port power management may be unreliable
>>>>>>>> [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
>>>>>>>> location (usb2-port5:none) (usb1-port6:usb2-port3)
>>>>>>>> [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
>>>>>>>> [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
>>>>>>>> location (usb2-port6:none) (usb1-port6:usb2-port3)
>>>>>>>> [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Other than your internal hub not liking to be a peer (which seems
>>>>>>> like a
>>>>>>> BIOS/ACPI issue, right Mathias), I don't see an error here.  Are
>>>>>>> things
>>>>>>> working properly?
>>>>
>>>>
>>>> No, actually.
>>>>
>>>> The USB 3.0 ports seem to work fine, although I haven't tried
>>>> suspending with a device inserted.
>>>>
>>>> The USB Type C port is presently screwed up.  I have a Realtek USB C
>>>> to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
>>>> all.  If I suspend while it's plugged in, the screen turns off and the
>>>> system freezes hard without going to sleep.
>>>>
>>>> The Realtek adapter used to work, but that was on an older kernel
>>>> (4.3?) and a different BIOS version.
>>>>
>>>>>
>>>>>
>>>>> Probably.
>>>>> The "by location" message in the log reveals that the peering
>>>>> is based on ACPI DSDT table _PLD entries (Physical Device Location).
>>>>>
>>>>> usb2 ports 3-6 are all peered with usb1 port6.
>>>>> First two ports appear to be sane (usb1_port1 <->usb2_port1)
>>>>>
>>>>> How many physical ports does the DELL XPS have visible and connectable?
>>>>
>>

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-07 Thread Andy Lutomirski 
On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> On Mon, Mar 07, 2016 at 11:09:36AM -0800, Andy Lutomirski wrote:
>> Quick ping here: this is still busted on 4.5-rc6.
>>
>> On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski <l...@kernel.org> wrote:
>> > On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
>> > <mathias.ny...@linux.intel.com> wrote:
>> >> On 17.02.2016 07:19, Andy Lutomirski wrote:
>> >>>
>> >>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
>> >>> <gre...@linuxfoundation.org> wrote:
>> >>>>
>> >>>> On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>> >>>>>
>> >>>>> I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I
>> >>>>> see:
>> >>>>>
>> >> ...
>> >>
>> >>>>> [1.061036] hub 1-0:1.0: power on to power good time: 20ms
>> >>>>> [1.061109] hub 1-0:1.0: local power source is good
>> >>>>> [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
>> >>>>> according to platform information.
>> >>>>> [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
>> >>>>> according to platform information.
>> >>>>> [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
>> >>>>> according to platform information.
>> >>
>> >> ...
>> >>>>>
>> >>>>> [1.085684] usb usb2-port1: peered to usb1-port1
>> >>>>> [1.086356] usb usb2-port2: peered to usb1-port2
>> >>>>> [1.087004] usb usb2-port3: peered to usb1-port6
>> >>>>> [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
>> >>>>> location (usb2-port4:none) (usb1-port6:usb2-port3)
>> >>>>> [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
>> >>>>> [1.087716] usb: port power management may be unreliable
>> >>>>> [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
>> >>>>> location (usb2-port5:none) (usb1-port6:usb2-port3)
>> >>>>> [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
>> >>>>> [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
>> >>>>> location (usb2-port6:none) (usb1-port6:usb2-port3)
>> >>>>> [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
>> >>>>
>> >>>>
>> >>>> Other than your internal hub not liking to be a peer (which seems like a
>> >>>> BIOS/ACPI issue, right Mathias), I don't see an error here.  Are things
>> >>>> working properly?
>> >
>> > No, actually.
>> >
>> > The USB 3.0 ports seem to work fine, although I haven't tried
>> > suspending with a device inserted.
>> >
>> > The USB Type C port is presently screwed up.  I have a Realtek USB C
>> > to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
>> > all.  If I suspend while it's plugged in, the screen turns off and the
>> > system freezes hard without going to sleep.
>> >
>> > The Realtek adapter used to work, but that was on an older kernel
>> > (4.3?) and a different BIOS version.
>> >
>> >>
>> >>
>> >> Probably.
>> >> The "by location" message in the log reveals that the peering
>> >> is based on ACPI DSDT table _PLD entries (Physical Device Location).
>> >>
>> >> usb2 ports 3-6 are all peered with usb1 port6.
>> >> First two ports appear to be sane (usb1_port1 <->usb2_port1)
>> >>
>> >> How many physical ports does the DELL XPS have visible and connectable?
>> >
>> > There are:
>> >
>> >  - Two Super Speed ports.  (If I'm understanding correctly, one is 1-1
>> > and one is 1-2.)
>> >  - One USB Type C port (Alpine Ridge)
>> >  - Two internal M.2 ports
>> >
>> > One of the M.2 ports has an Intel 7265 plugged in, and that's the
>> > Bluetooth device.  The other one has an nvme device plugged in, and I
>> > have no idea whether USB is wired up.
>> >
>> > The Intel 7265 is "removable" in the sense that I personally inserted
>> > it into the p

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-03-07 Thread Andy Lutomirski 
Quick ping here: this is still busted on 4.5-rc6.

On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski <l...@kernel.org> wrote:
> On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
> <mathias.ny...@linux.intel.com> wrote:
>> On 17.02.2016 07:19, Andy Lutomirski wrote:
>>>
>>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
>>> <gre...@linuxfoundation.org> wrote:
>>>>
>>>> On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>>>>>
>>>>> I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I
>>>>> see:
>>>>>
>> ...
>>
>>>>> [1.061036] hub 1-0:1.0: power on to power good time: 20ms
>>>>> [1.061109] hub 1-0:1.0: local power source is good
>>>>> [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
>>>>> according to platform information.
>>>>> [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
>>>>> according to platform information.
>>>>> [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
>>>>> according to platform information.
>>
>> ...
>>>>>
>>>>> [1.085684] usb usb2-port1: peered to usb1-port1
>>>>> [1.086356] usb usb2-port2: peered to usb1-port2
>>>>> [1.087004] usb usb2-port3: peered to usb1-port6
>>>>> [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
>>>>> location (usb2-port4:none) (usb1-port6:usb2-port3)
>>>>> [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
>>>>> [1.087716] usb: port power management may be unreliable
>>>>> [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
>>>>> location (usb2-port5:none) (usb1-port6:usb2-port3)
>>>>> [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
>>>>> [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
>>>>> location (usb2-port6:none) (usb1-port6:usb2-port3)
>>>>> [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
>>>>
>>>>
>>>> Other than your internal hub not liking to be a peer (which seems like a
>>>> BIOS/ACPI issue, right Mathias), I don't see an error here.  Are things
>>>> working properly?
>
> No, actually.
>
> The USB 3.0 ports seem to work fine, although I haven't tried
> suspending with a device inserted.
>
> The USB Type C port is presently screwed up.  I have a Realtek USB C
> to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
> all.  If I suspend while it's plugged in, the screen turns off and the
> system freezes hard without going to sleep.
>
> The Realtek adapter used to work, but that was on an older kernel
> (4.3?) and a different BIOS version.
>
>>
>>
>> Probably.
>> The "by location" message in the log reveals that the peering
>> is based on ACPI DSDT table _PLD entries (Physical Device Location).
>>
>> usb2 ports 3-6 are all peered with usb1 port6.
>> First two ports appear to be sane (usb1_port1 <->usb2_port1)
>>
>> How many physical ports does the DELL XPS have visible and connectable?
>
> There are:
>
>  - Two Super Speed ports.  (If I'm understanding correctly, one is 1-1
> and one is 1-2.)
>  - One USB Type C port (Alpine Ridge)
>  - Two internal M.2 ports
>
> One of the M.2 ports has an Intel 7265 plugged in, and that's the
> Bluetooth device.  The other one has an nvme device plugged in, and I
> have no idea whether USB is wired up.
>
> The Intel 7265 is "removable" in the sense that I personally inserted
> it into the port (with the power off, of course).
>
>>
>> The ACPI DSDt _UPC entry describes if a port is hardwired or hotplug, and
>> the
>> hub descriptor DeviceRemovable field are also involved in all this.
>> The log also shows changing the built in bluetooth and webcam to removable
>> which
>> is a  bit suspicious
>>
>>
>>>
>>>
>>> I think so, but I haven't tested with a USB device plugged in across
>>> suspend/resume.  I do get this on resume, though:
>>>
>>> [  +0.008245] ACPI: Waking up from system sleep state S3
>>> [  +0.137335] xhci_hcd :00:14.0: System wakeup disabled by ACPI
>>> [  +0.000117] PM: noirq resume of devices complete after 12.890 msecs
>>> [  +0.011969] PM: early resume of devices complete after 11.890 msecs
>>> [  +0.000345] usb usb1: root hub lost po

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

2016-03-07 Thread Andy Lutomirski 
On Mon, Mar 7, 2016 at 9:30 AM, Steven Rostedt  wrote:
> On Mon, 7 Mar 2016 18:24:12 +0100 (CET)
> Jiri Kosina  wrote:
>
>> > So, if Clang is producing wrong X86 code here, is it possible to turn
>> > interrupts on/off manually? But, hmm that affects other places as well
>> > in the Linux sources, so.
>>
>> This issue needs to be handled in the compiler.
>>
>
> Exactly. The compiler may get away with this in userspace (maybe), but
> for the kernel, it is definitely a show stopper. Especially if it knows
> that an asm() may be called.

It's broken for user code that fiddles with AC, too.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-02-16 Thread Andy Lutomirski 
On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>> I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I see:
>>
>> [0.229429] usbcore: registered new interface driver usbfs
>> [0.229436] usbcore: registered new interface driver hub
>> [0.229451] usbcore: registered new device driver usb
>> [1.057998] xhci_hcd :00:14.0: xHCI Host Controller
>> [1.058640] xhci_hcd :00:14.0: new USB bus registered, assigned
>> bus number 1
>> [1.059811] xhci_hcd :00:14.0: hcc params 0x200077c1 hci
>> version 0x100 quirks 0x00109810
>> [1.059818] xhci_hcd :00:14.0: cache line size of 64 is not supported
>> [1.059821] xhci_hcd :00:14.0: supports USB remote wakeup
>> [1.060480] usb usb1: default language 0x0409
>> [1.060584] usb usb1: udev 1, busnum 1, minor = 0
>> [1.060587] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
>> [1.060589] usb usb1: New USB device strings: Mfr=3, Product=2,
>> SerialNumber=1
>> [1.060590] usb usb1: Product: xHCI Host Controller
>> [1.060592] usb usb1: Manufacturer: Linux 4.5.0-rc4-acpi+ xhci-hcd
>> [1.060593] usb usb1: SerialNumber: :00:14.0
>> [1.060845] usb usb1: usb_probe_device
>> [1.060848] usb usb1: configuration #1 chosen from 1 choice
>> [1.060914] usb usb1: adding 1-0:1.0 (config #1, interface 0)
>> [1.060937] hub 1-0:1.0: usb_probe_interface
>> [1.060939] hub 1-0:1.0: usb_probe_interface - got id
>> [1.060941] hub 1-0:1.0: USB hub found
>> [1.061028] hub 1-0:1.0: 12 ports detected
>> [1.061030] hub 1-0:1.0: standalone hub
>> [1.061031] hub 1-0:1.0: no power switching (usb 1.0)
>> [1.061032] hub 1-0:1.0: individual port over-current protection
>> [1.061034] hub 1-0:1.0: Single TT
>> [1.061035] hub 1-0:1.0: TT requires at most 8 FS bit times (666 ns)
>> [1.061036] hub 1-0:1.0: power on to power good time: 20ms
>> [1.061109] hub 1-0:1.0: local power source is good
>> [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
>> according to platform information.
>> [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
>> according to platform information.
>> [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
>> according to platform information.
>> [1.084343] hub 1-0:1.0: trying to enable port power on non-switchable hub
>> [1.084438] xhci_hcd :00:14.0: xHCI Host Controller
>> [1.084624] xhci_hcd :00:14.0: new USB bus registered, assigned
>> bus number 2
>> [1.084628] xhci_hcd :00:14.0: supports USB remote wakeup
>> [1.084671] usb usb2: skipped 1 descriptor after endpoint
>> [1.084679] usb usb2: default language 0x0409
>> [1.084691] usb usb2: udev 1, busnum 2, minor = 128
>> [1.084693] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003
>> [1.084694] usb usb2: New USB device strings: Mfr=3, Product=2,
>> SerialNumber=1
>> [1.084695] usb usb2: Product: xHCI Host Controller
>> [1.084697] usb usb2: Manufacturer: Linux 4.5.0-rc4-acpi+ xhci-hcd
>> [1.084698] usb usb2: SerialNumber: :00:14.0
>> [1.084909] usb usb2: usb_probe_device
>> [1.084914] usb usb2: configuration #1 chosen from 1 choice
>> [1.084926] usb usb2: adding 2-0:1.0 (config #1, interface 0)
>> [1.084948] hub 2-0:1.0: usb_probe_interface
>> [1.084949] hub 2-0:1.0: usb_probe_interface - got id
>> [1.084952] hub 2-0:1.0: USB hub found
>> [1.084966] hub 2-0:1.0: 6 ports detected
>> [1.084968] hub 2-0:1.0: standalone hub
>> [1.084969] hub 2-0:1.0: no power switching (usb 1.0)
>> [1.084971] hub 2-0:1.0: individual port over-current protection
>> [1.084972] hub 2-0:1.0: TT requires at most 8 FS bit times (666 ns)
>> [1.084973] hub 2-0:1.0: power on to power good time: 20ms
>> [1.084978] hub 2-0:1.0: local power source is good
>> [1.085684] usb usb2-port1: peered to usb1-port1
>> [1.086356] usb usb2-port2: peered to usb1-port2
>> [1.087004] usb usb2-port3: peered to usb1-port6
>> [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
>> location (usb2-port4:none) (usb1-port6:usb2-port3)
>> [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
>> [1.087716] usb: port power management may be unreliable
>> [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
>> location (usb2-port5:none) (usb1-port6:usb2-port3)
&g

Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

2016-02-16 Thread Andy Lutomirski 
I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I see:

[0.229429] usbcore: registered new interface driver usbfs
[0.229436] usbcore: registered new interface driver hub
[0.229451] usbcore: registered new device driver usb
[1.057998] xhci_hcd :00:14.0: xHCI Host Controller
[1.058640] xhci_hcd :00:14.0: new USB bus registered, assigned
bus number 1
[1.059811] xhci_hcd :00:14.0: hcc params 0x200077c1 hci
version 0x100 quirks 0x00109810
[1.059818] xhci_hcd :00:14.0: cache line size of 64 is not supported
[1.059821] xhci_hcd :00:14.0: supports USB remote wakeup
[1.060480] usb usb1: default language 0x0409
[1.060584] usb usb1: udev 1, busnum 1, minor = 0
[1.060587] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[1.060589] usb usb1: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[1.060590] usb usb1: Product: xHCI Host Controller
[1.060592] usb usb1: Manufacturer: Linux 4.5.0-rc4-acpi+ xhci-hcd
[1.060593] usb usb1: SerialNumber: :00:14.0
[1.060845] usb usb1: usb_probe_device
[1.060848] usb usb1: configuration #1 chosen from 1 choice
[1.060914] usb usb1: adding 1-0:1.0 (config #1, interface 0)
[1.060937] hub 1-0:1.0: usb_probe_interface
[1.060939] hub 1-0:1.0: usb_probe_interface - got id
[1.060941] hub 1-0:1.0: USB hub found
[1.061028] hub 1-0:1.0: 12 ports detected
[1.061030] hub 1-0:1.0: standalone hub
[1.061031] hub 1-0:1.0: no power switching (usb 1.0)
[1.061032] hub 1-0:1.0: individual port over-current protection
[1.061034] hub 1-0:1.0: Single TT
[1.061035] hub 1-0:1.0: TT requires at most 8 FS bit times (666 ns)
[1.061036] hub 1-0:1.0: power on to power good time: 20ms
[1.061109] hub 1-0:1.0: local power source is good
[1.084337] usb usb1-port3: DeviceRemovable is changed to 1
according to platform information.
[1.084339] usb usb1-port4: DeviceRemovable is changed to 1
according to platform information.
[1.084341] usb usb1-port5: DeviceRemovable is changed to 1
according to platform information.
[1.084343] hub 1-0:1.0: trying to enable port power on non-switchable hub
[1.084438] xhci_hcd :00:14.0: xHCI Host Controller
[1.084624] xhci_hcd :00:14.0: new USB bus registered, assigned
bus number 2
[1.084628] xhci_hcd :00:14.0: supports USB remote wakeup
[1.084671] usb usb2: skipped 1 descriptor after endpoint
[1.084679] usb usb2: default language 0x0409
[1.084691] usb usb2: udev 1, busnum 2, minor = 128
[1.084693] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003
[1.084694] usb usb2: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[1.084695] usb usb2: Product: xHCI Host Controller
[1.084697] usb usb2: Manufacturer: Linux 4.5.0-rc4-acpi+ xhci-hcd
[1.084698] usb usb2: SerialNumber: :00:14.0
[1.084909] usb usb2: usb_probe_device
[1.084914] usb usb2: configuration #1 chosen from 1 choice
[1.084926] usb usb2: adding 2-0:1.0 (config #1, interface 0)
[1.084948] hub 2-0:1.0: usb_probe_interface
[1.084949] hub 2-0:1.0: usb_probe_interface - got id
[1.084952] hub 2-0:1.0: USB hub found
[1.084966] hub 2-0:1.0: 6 ports detected
[1.084968] hub 2-0:1.0: standalone hub
[1.084969] hub 2-0:1.0: no power switching (usb 1.0)
[1.084971] hub 2-0:1.0: individual port over-current protection
[1.084972] hub 2-0:1.0: TT requires at most 8 FS bit times (666 ns)
[1.084973] hub 2-0:1.0: power on to power good time: 20ms
[1.084978] hub 2-0:1.0: local power source is good
[1.085684] usb usb2-port1: peered to usb1-port1
[1.086356] usb usb2-port2: peered to usb1-port2
[1.087004] usb usb2-port3: peered to usb1-port6
[1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
location (usb2-port4:none) (usb1-port6:usb2-port3)
[1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
[1.087716] usb: port power management may be unreliable
[1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
location (usb2-port5:none) (usb1-port6:usb2-port3)
[1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
[1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
location (usb2-port6:none) (usb1-port6:usb2-port3)
[1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
[1.089028] hub 2-0:1.0: trying to enable port power on non-switchable hub
[1.089164] usbcore: registered new interface driver usbserial
[1.089170] usbcore: registered new interface driver usbserial_generic
[1.089195] usbserial: USB Serial support registered for generic
[1.097055] usbcore: registered new interface driver usbhid
[1.097056] usbhid: USB HID core driver
[1.184226] usb usb1-port3: status 0101 change 0001
[1.184241] usb usb1-port5: status 0101 change 0001
[1.284270] hub 1-0:1.0: state 7 ports 12 chg 0028 evt 
[1.284302] usb usb1-port3: status

ehci-pci D3cold logspam

2013-09-23 Thread Andy Lutomirski 
I've been getting this on several recent kernel revisions.  Is it
interesting?  If so, I'm happy to help diagnose it.  If not, can the
message be killed or severely ratelimited?  I'm getting so much of
this that it tends to overflow the log ring.

[  287.344991] ehci-pci :00:1d.0: power state changed by ACPI to D0
[  287.445433] ehci-pci :00:1d.0: setting latency timer to 64
[  287.446255] ehci-pci :00:1a.0: power state changed by ACPI to D0
[  287.456094] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
[  287.547205] ehci-pci :00:1a.0: setting latency timer to 64
[  287.557890] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
[  290.126910] ehci-pci :00:1d.0: power state changed by ACPI to D0
[  290.227958] ehci-pci :00:1d.0: setting latency timer to 64
[  290.228416] ehci-pci :00:1a.0: power state changed by ACPI to D0
[  290.238749] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
[  290.328904] ehci-pci :00:1a.0: setting latency timer to 64
[  290.339565] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
[  292.214834] ehci-pci :00:1d.0: power state changed by ACPI to D0
[  292.315458] ehci-pci :00:1d.0: setting latency timer to 64
[  292.315908] ehci-pci :00:1a.0: power state changed by ACPI to D0
[  292.326262] ehci-pci :00:1d.0: power state changed by ACPI to D3cold
[  292.416487] ehci-pci :00:1a.0: setting latency timer to 64
[  292.431075] ehci-pci :00:1a.0: power state changed by ACPI to D3cold
[  295.458048] ehci-pci :00:1d.0: power state changed by ACPI to D0
[  295.558613] ehci-pci :00:1d.0: setting latency timer to 64
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Ejected Nook (usb mass storage) prevents suspend

2013-09-13 Thread Andy Lutomirski 
What's the status of this?  Do I still need to test it?

I won't have access to the Nook that triggered it the first time for a
couple weeks, but I could try to find another device like that.

--Andy

On Fri, Aug 2, 2013 at 7:18 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 2 Aug 2013, Oliver Neukum wrote:

 On Fri, 2013-08-02 at 09:54 -0400, Alan Stern wrote:
  On Fri, 2 Aug 2013, Oliver Neukum wrote:
 
   On Thu, 2013-08-01 at 11:53 -0400, Alan Stern wrote:
On Thu, 1 Aug 2013, Oliver Neukum wrote:
   
 On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote:

  More importantly, if we already know that the medium is not 
  present or
  has been changed since it was last used, then there's no reason to 
  call
  sd_sync_cache() at all.

 Like this?
   
Yes, I like this a lot better, except I would put the test for
!sdkp-media_present in sd_suspend_common() -- no need to print the
  
   But your observation that it makes no sense while no medium is present
   is valid whatever be the reason for wanting to flush.
 
  So do the test in both places.

 Code duplication for what reasons?

 Like I said before, to avoid printing a misleading line in the kernel
 log.

 If you prefer, just get rid of the log message.  Or make it KERN_DEBUG
 instead of KERN_NOTICE, along with the Stopping disk message.  Those
 two things might get pretty annoying when people start using
 block-layer runtime PM.

 Alan Stern




-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ejected Nook (usb mass storage) prevents suspend

2013-07-26 Thread Andy Lutomirski 
This is kernel 3.9.9-302.fc19.x86_64.

I plugged in a BN Nook (a usb mass storage device), used it, and
ejected it.  This makes suspend fail:

[50135.265514] PM: Entering freeze sleep
[50135.265517] Suspending console(s) (use no_console_suspend to debug)
[50135.287724] sd 7:0:0:0: [sdb] Synchronizing SCSI cache
[50135.290413] sd 7:0:0:0: [sdb]
[50135.290415] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[50135.290418] sd 7:0:0:0: [sdb]
[50135.290422] Sense Key : Not Ready [current]
[50135.290424] sd 7:0:0:0: [sdb]
[50135.290429] Add. Sense: Medium not present
[50135.290448] dpm_run_callback(): scsi_bus_suspend+0x0/0x40 returns -5
[50135.290454] PM: Device 7:0:0:0 failed to suspend async: error -5
[50138.486917] PM: Some devices failed to suspend
[50138.525007] PM: resume of devices complete after 38.132 msecs
[50138.525315] pci_pm_runtime_suspend():
hcd_pci_runtime_suspend+0x0/0x50 returns -16
[50138.536357] PM: Finishing wakeup.

Experimenting a bit, here's what happens.

 - Plug in device (so it's working).  Suspend works.
 - eject /dev/sdb.  Suspend fails.
 - Physically unplug the device.  Suspend works again.

This is actually a real issue -- my laptop can continue to power its
usb port while suspended, and the Nook is functional (and charges)
while plugged in but ejected, but I can't suspend my laptop so it can
charge my Nook.

Curiously, this issue seems to be Nook-specific.  I can't reproduce it
with a Corsair Flash Voyager.

--Andy
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Multiple usb_storage problems

2013-03-30 Thread Andy Lutomirski 
I have a Corsair Flash Voyager GT and a Lenovo x220 running Fedora's
3.8.2 kernel.  It doesn't work very well, and the problems vary
depending on whether I'm using usb2 or usb3.

 - On usb3, the device node can't be opened with O_DIRECT (using, for
example, dd oflag=direct).  It returns -EINVAL.  (On usb2, O_DIRECT
works.)
 - Also on usb3, if I dd if=/dev/zero of=/dev/sdb bs=1M, dd finishes
in a second or two and no I/O occurs.  This system only has 2GB of RAM
-- caching shouldn't be that extreme.

On either usb version, if I mount an NTFS partition and write a few GB
of data, I get tons of buffer I/O errors.  The point at which they
happen varies, but this is entirely repeatable.  Oddly, dd
if=/dev/zero of=/dev/sdc oflag=direct bs=1M works fine and
successfully zeroes the whole device.

Other computers (running Windows or OS X) seem fine with this USB
stick.  I don't think the stick is bad.

Any ideas?

--Andy
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple usb_storage problems

2013-03-30 Thread Andy Lutomirski 
On Sat, Mar 30, 2013 at 8:21 PM, Andy Lutomirski l...@amacapital.net wrote:
 I have a Corsair Flash Voyager GT and a Lenovo x220 running Fedora's
 3.8.2 kernel.  It doesn't work very well, and the problems vary
 depending on whether I'm using usb2 or usb3.

  - On usb3, the device node can't be opened with O_DIRECT (using, for
 example, dd oflag=direct).  It returns -EINVAL.  (On usb2, O_DIRECT
 works.)
  - Also on usb3, if I dd if=/dev/zero of=/dev/sdb bs=1M, dd finishes
 in a second or two and no I/O occurs.  This system only has 2GB of RAM
 -- caching shouldn't be that extreme.

*facepalm*.  The issue is that one of the I/O errors caused the sdb
block device to get sufficiently wedged that it still exists in /sys,
but udev removed sdb (and I replaced it with a file).  If I manually
recreate the sdb device node, ejecting it gets an IO error.  (It's not
plugged in any more.)  rmmoding and modprobing usb_storage got rid of
the bogus sdb device.


 On either usb version, if I mount an NTFS partition and write a few GB
 of data, I get tons of buffer I/O errors.  The point at which they
 happen varies, but this is entirely repeatable.  Oddly, dd
 if=/dev/zero of=/dev/sdc oflag=direct bs=1M works fine and
 successfully zeroes the whole device.

 Other computers (running Windows or OS X) seem fine with this USB
 stick.  I don't think the stick is bad.

 Any ideas?

 --Andy
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html