Re: [PATCH v3 1/2] USB: add switch to turn off padding of resume time delays
Hi Todd, [auto build test ERROR on linus/master] [also build test ERROR on next-20161214] [cannot apply to usb/usb-testing balbi-usb/next v4.9] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Todd-Brandt/USB-resume-time-optimization-by-using-spec-minimums/20161214-115712 config: x86_64-randconfig-b0-12141412 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `musb_interrupt': >> (.text+0x1ff088): undefined reference to `usb_timing' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs
On Tue, Dec 13, 2016 at 06:53:03PM -0800, Andy Lutomirski wrote: > On Tue, Dec 13, 2016 at 6:48 PM, Andy Lutomirski 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(&sg_out[1], pad, sizeof pad); > > + sg_set_buf(&sg_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. It's decrypting so I presume it just needs to the extra space for the padding and the result will be thrown away. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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 v3 1/2] USB: add switch to turn off padding of resume time delays
Add a kernel parameter that replaces the USB_RESUME_TIMEOUT
and other hardcoded delay numbers with the USB spec minimums.
The USB subsystem currently uses heavily padded values for TDRSMDN
and TRSTRCY. This patch keeps the current values by default, but if
the kernel is booted with usbcore.timing_minimum=1 they are set to the
spec minimums with no padding. The result is significant performance
improvement in usb device resume.
Use this only if you know your USB hardware and device setup can
handle the spec minimums.
Example analyze_suspend runs are provided here showing the benefits:
https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays
Signed-off-by: Todd Brandt
---
v2 changes:
- moved the core code from hub.c to usb.c
- param name is now usb_timing_minimum
- configured isp1362-hcd and ohci-hub to use the new values
v3 changes:
- changed param to usbcore.timing_minimum
Documentation/admin-guide/kernel-parameters.txt | 7 +++
drivers/usb/core/hub.c | 12 ++--
drivers/usb/core/usb.c | 16
drivers/usb/dwc2/hcd.c | 2 +-
drivers/usb/host/ehci-hcd.c | 4 ++--
drivers/usb/host/ehci-hub.c | 6 +++---
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/isp116x-hcd.c | 2 +-
drivers/usb/host/isp1362-hcd.c | 2 +-
drivers/usb/host/ohci-hub.c | 2 +-
drivers/usb/host/oxu210hp-hcd.c | 4 ++--
drivers/usb/host/r8a66597-hcd.c | 2 +-
drivers/usb/host/sl811-hcd.c| 2 +-
drivers/usb/host/uhci-hub.c | 6 +++---
drivers/usb/host/xhci-hub.c | 6 +++---
drivers/usb/host/xhci-ring.c| 2 +-
drivers/usb/isp1760/isp1760-hcd.c | 2 +-
drivers/usb/musb/musb_core.c| 6 +++---
drivers/usb/musb/musb_virthub.c | 2 +-
include/linux/usb.h | 23 ++-
20 files changed, 77 insertions(+), 33 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index be2d6d0..75f65c6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4024,6 +4024,13 @@
unknown_nmi_panic
[X86] Cause panic on unknown NMI.
+ usbcore.timing_minimum=
+ Set USB timing values to USB 2.0 spec minimums (default
0 = off).
+ This removes any padding added to the values to
accommodate
+ older or troublesome hardware. This will reduce usb
resume
+ time. Use this only if you know your USB hardware and
device
+ setup can handle the spec minimums.
+
usbcore.authorized_default=
[USB] Default USB device authorization:
(default -1 = authorized except for wireless USB,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 143454e..10c0bea 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2841,7 +2841,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
done:
if (status == 0) {
/* TRSTRCY = 10 ms; plus some extra */
- msleep(10 + 40);
+ msleep(usb_timing.trstrcy);
if (udev) {
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
@@ -3433,10 +3433,10 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(&port_dev->dev, "can't resume, status %d\n", status);
} else {
- /* drive resume for USB_RESUME_TIMEOUT msec */
+ /* drive resume for TDRSMDN msec */
dev_dbg(&udev->dev, "usb %sresume\n",
(PMSG_IS_AUTO(msg) ? "auto-" : ""));
- msleep(USB_RESUME_TIMEOUT);
+ msleep(usb_timing.tdrsmdn);
/* Virtual root hubs can trigger on GET_PORT_STATUS to
* stop resume signaling. Then finish the resume
@@ -3445,7 +3445,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
status = hub_port_status(hub, port1, &portstatus, &portchange);
/* TRSMRCY = 10 msec */
- msleep(10);
+ msleep(usb_timing.trsmrcy);
}
SuspendCleared:
@@ -3531,7 +3531,7 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub,
unsigned int port,
if (udev) {
/* TRSMRCY = 10 msec */
- msleep(10);
+ msleep(usb_timing.trsmrcy);
usb_unlock_port(port_dev);
ret = usb_remote_wakeup(udev);
@@ -4108,7 +4108,7 @@ st
[PATCH v3 2/2] USB: usb resume time delay debug
add debugfs support for experimenting with USB timing delay
values on the fly. Values are read/written from debugfs at
/sys/kernel/debug/usb/timing.
Signed-off-by: Todd Brandt
---
drivers/usb/core/usb.c | 55 ++
1 file changed, 55 insertions(+)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2969704..285d879 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -39,6 +39,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -1043,6 +1045,49 @@ struct dentry *usb_debug_root;
EXPORT_SYMBOL_GPL(usb_debug_root);
static struct dentry *usb_debug_devices;
+static struct dentry *usb_debug_timing;
+
+static int usb_timing_show(struct seq_file *s, void *unused)
+{
+ seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn);
+ seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy);
+ seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy);
+ return 0;
+}
+
+static int usb_timing_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, usb_timing_show, inode->i_private);
+}
+
+static ssize_t usb_timing_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ int val;
+ char buf[32];
+
+ if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+ return -EFAULT;
+
+ if (sscanf(buf, "tdrsmdn=%u", &val) == 1)
+ usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN);
+ else if (sscanf(buf, "trsmrcy=%u", &val) == 1)
+ usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN);
+ else if (sscanf(buf, "trstrcy=%u", &val) == 1)
+ usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN);
+ else
+ return -EINVAL;
+
+ return count;
+}
+
+static const struct file_operations usbfs_timing_fops = {
+ .open = usb_timing_open,
+ .write= usb_timing_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
static int usb_debugfs_init(void)
{
@@ -1059,11 +1104,21 @@ static int usb_debugfs_init(void)
return -ENOENT;
}
+ usb_debug_timing = debugfs_create_file("timing", 0644,
+ usb_debug_root, NULL,
+ &usbfs_timing_fops);
+ if (!usb_debug_timing) {
+ debugfs_remove(usb_debug_root);
+ usb_debug_root = NULL;
+ return -ENOENT;
+ }
+
return 0;
}
static void usb_debugfs_cleanup(void)
{
+ debugfs_remove(usb_debug_timing);
debugfs_remove(usb_debug_devices);
debugfs_remove(usb_debug_root);
}
--
2.1.4
--
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 v3 0/2] USB: resume time optimization by using spec minimums
The USB resume code in the kernel currently uses a set of hard coded delay values that are defined in the USB 2.0 spec. Specifically these three have the most effect on resume time: - tdrsmdn: resume signal time (20ms - infinity) usb 2.0 spec 7.1.7.7 - trsmrcy: resume recovery time (10ms) usb 2.0 spec 7.1.7.7 - trstrcy: reset recovery time (0ms - infinity) usb 2.0 spec 7.1.7.5 These values have been padded considerably in order to accomodate non-compliant devices. - tdrsmdn: resume signal time = 40ms - trsmrcy: resume recovery time = 10ms - trstrcy: reset recovery time = 50ms I propose that we provide a kernel parameter that sets the USB timing values to their spec minimums. The following patches do this by creating a global struct which contains these values. By default the values remain as they are now, but if usbcore.timing_minimum=1 is added to the kernel cmd line they're set to their minimums. This struct can be expanded over time to include other hardcoded values that have padding we can remove. I've created a blog entry on 01.org with some analyze_suspend test cases illustrating the benefits: - https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays Todd Brandt (2): USB: add switch to turn off padding of resume time delays USB: usb resume time delay debug Documentation/admin-guide/kernel-parameters.txt | 7 +++ drivers/usb/core/hub.c | 12 ++--- drivers/usb/core/usb.c | 71 + drivers/usb/dwc2/hcd.c | 2 +- drivers/usb/host/ehci-hcd.c | 4 +- drivers/usb/host/ehci-hub.c | 6 +-- drivers/usb/host/fotg210-hcd.c | 2 +- drivers/usb/host/isp116x-hcd.c | 2 +- drivers/usb/host/isp1362-hcd.c | 2 +- drivers/usb/host/ohci-hub.c | 2 +- drivers/usb/host/oxu210hp-hcd.c | 4 +- drivers/usb/host/r8a66597-hcd.c | 2 +- drivers/usb/host/sl811-hcd.c| 2 +- drivers/usb/host/uhci-hub.c | 6 +-- drivers/usb/host/xhci-hub.c | 6 +-- drivers/usb/host/xhci-ring.c| 2 +- drivers/usb/isp1760/isp1760-hcd.c | 2 +- drivers/usb/musb/musb_core.c| 6 +-- drivers/usb/musb/musb_virthub.c | 2 +- include/linux/usb.h | 23 +++- 20 files changed, 132 insertions(+), 33 deletions(-) -- 2.1.4 -- 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 1/2] USB: add switch to turn off padding of resume time delays
Hi Todd,
[auto build test ERROR on linus/master]
[also build test ERROR on next-20161213]
[cannot apply to usb/usb-testing balbi-usb/next v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Todd-Brandt/USB-resume-time-optimization-by-using-spec-minimums/20161214-092721
config: i386-randconfig-s1-201650 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> drivers/usb/core/usb.c:78:52: error: expected ')' before 'bool'
core_param(usb_timing_minimum, usb_timing_minimum, bool, 0644);
^~~~
vim +78 drivers/usb/core/usb.c
72
73 #else
74 #define usb_autosuspend_delay 0
75 #endif
76
77 static bool usb_timing_minimum;
> 78 core_param(usb_timing_minimum, usb_timing_minimum, bool, 0644);
79
80 struct _usb_timing_config usb_timing = {
81 .tdrsmdn = USB_TIMING_TDRSMDN_DEF,
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[PATCH v2 1/1] Increase USB transfer limit
Promote a variable keeping track of USB transfer memory usage to a
wider data type and allow for higher bandwidth transfers from a large
number of USB devices connected to a single host.
Signed-off-by: Mateusz Berezecki
---
drivers/usb/core/devio.c | 48
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 4016dae..99c8e27 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -134,42 +134,37 @@ enum snoop_when {
#define USB_DEVICE_DEV MKDEV(USB_DEVICE_MAJOR, 0)
/* Limit on the total amount of memory we can allocate for transfers */
-static unsigned usbfs_memory_mb = 16;
+static u32 usbfs_memory_mb = 16;
module_param(usbfs_memory_mb, uint, 0644);
MODULE_PARM_DESC(usbfs_memory_mb,
"maximum MB allowed for usbfs buffers (0 = no limit)");
-/* Hard limit, necessary to avoid arithmetic overflow */
-#define USBFS_XFER_MAX (UINT_MAX / 2 - 100)
-
-static atomic_t usbfs_memory_usage;/* Total memory currently allocated */
+static atomic64_t usbfs_memory_usage; /* Total memory currently allocated */
/* Check whether it's okay to allocate more memory for a transfer */
-static int usbfs_increase_memory_usage(unsigned amount)
+static int usbfs_increase_memory_usage(u64 amount)
{
- unsigned lim;
+ u64 lim;
- /*
-* Convert usbfs_memory_mb to bytes, avoiding overflows.
-* 0 means use the hard limit (effectively unlimited).
-*/
lim = ACCESS_ONCE(usbfs_memory_mb);
- if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
- lim = USBFS_XFER_MAX;
- else
- lim <<= 20;
+ lim <<= 20;
- atomic_add(amount, &usbfs_memory_usage);
- if (atomic_read(&usbfs_memory_usage) <= lim)
- return 0;
- atomic_sub(amount, &usbfs_memory_usage);
- return -ENOMEM;
+ atomic64_add(amount, &usbfs_memory_usage);
+
+ if (lim > 0) {
+ if (atomic64_read(&usbfs_memory_usage) <= lim)
+ return 0;
+ atomic64_sub(amount, &usbfs_memory_usage);
+ return -ENOMEM;
+ }
+
+ return 0;
}
/* Memory for a transfer is being deallocated */
-static void usbfs_decrease_memory_usage(unsigned amount)
+static void usbfs_decrease_memory_usage(u64 amount)
{
- atomic_sub(amount, &usbfs_memory_usage);
+ atomic64_sub(amount, &usbfs_memory_usage);
}
static int connected(struct usb_dev_state *ps)
@@ -1191,7 +1186,7 @@ static int proc_bulk(struct usb_dev_state *ps, void
__user *arg)
if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
return -EINVAL;
len1 = bulk.len;
- if (len1 >= USBFS_XFER_MAX)
+ if (len1 >= (INT_MAX - sizeof(struct urb)))
return -EINVAL;
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
if (ret)
@@ -1458,6 +1453,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
struct usbdevfs_urb *uurb
int number_of_packets = 0;
unsigned int stream_id = 0;
void *buf;
+ u32 overhead;
if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
USBDEVFS_URB_SHORT_NOT_OK |
@@ -1584,7 +1580,11 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
struct usbdevfs_urb *uurb
return -EINVAL;
}
- if (uurb->buffer_length >= USBFS_XFER_MAX) {
+ /* check for overflow */
+ overhead = u + sizeof(struct async) + sizeof(struct urb)
+ + num_sgs * sizeof(struct scatterlist);
+
+ if (uurb->buffer_length + overhead < uurb->buffer_length) {
ret = -EINVAL;
goto error;
}
--
2.7.4
--
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
On Tue, Dec 13, 2016 at 6:48 PM, Andy Lutomirski 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(&sg_out[1], pad, sizeof pad); > + sg_set_buf(&sg_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
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
Signed-off-by: Andy Lutomirski
---
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(&sg[1], &scratch->b1, sizeof(scratch->b1));
sg_set_buf(&sg[2], b, blen);
/* 0 if well behaved :) */
- sg_set_buf(&sg[3], bzero, zero_padding);
+ sg_set_page(&sg[3], ZERO_PAGE(0), zero_padding, 0);
sg_init_one(&sg_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
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 Signed-off-by: Andy Lutomirski --- 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(&sg_in[0], epayload->decrypted_data, epayload->decrypted_datalen); - sg_set_buf(&sg_in[1], pad, padlen); + sg_set_page(&sg_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(&sg_out[0], epayload->decrypted_data, epayload->decrypted_datalen); - sg_set_buf(&sg_out[1], pad, sizeof pad); + sg_set_buf(&sg_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 v2 0/2] USB: resume time optimization by using spec minimums
There's an issue with this one so I need to add a fix and resubmit (it fails when USB is configured as a module), so please don't accept this yet. However I would like to get feedback on whether or not this new approach looks ok (it includes fixes based on v1 feedback). Thanks. On Tue, 2016-12-13 at 17:22 -0800, Todd Brandt wrote: > The USB resume code in the kernel currently uses a set of hard coded > delay values that are defined in the USB 2.0 spec. Specifically these > three have the most effect on resume time: > > - tdrsmdn: resume signal time (20ms - infinity) usb 2.0 spec 7.1.7.7 > - trsmrcy: resume recovery time (10ms) usb 2.0 spec 7.1.7.7 > - trstrcy: reset recovery time (0ms - infinity) usb 2.0 spec 7.1.7.5 > > These values have been padded considerably in order to accomodate > non-compliant devices. > > - tdrsmdn: resume signal time = 40ms > - trsmrcy: resume recovery time = 10ms > - trstrcy: reset recovery time = 50ms > > I propose that we provide a kernel parameter that sets the USB timing > values to their spec minimums. The following patches do this by creating > a global struct which contains these values. By default the values remain > as they are now, but if usb_timing_minimum=1 is added to the kernel cmd > line they're set to their minimums. This struct can be expanded over time > to include other hardcoded values that have padding we can remove. > > I've created a blog entry on 01.org with some analyze_suspend test > cases illustrating the benefits: > > - > https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays > > Todd Brandt (2): > USB: add switch to turn off padding of resume time delays > USB: usb resume time delay debug > > Documentation/admin-guide/kernel-parameters.txt | 7 +++ > drivers/usb/core/hub.c | 12 ++--- > drivers/usb/core/usb.c | 70 > + > drivers/usb/dwc2/hcd.c | 2 +- > drivers/usb/host/ehci-hcd.c | 4 +- > drivers/usb/host/ehci-hub.c | 6 +-- > drivers/usb/host/fotg210-hcd.c | 2 +- > drivers/usb/host/isp116x-hcd.c | 2 +- > drivers/usb/host/isp1362-hcd.c | 2 +- > drivers/usb/host/ohci-hub.c | 2 +- > drivers/usb/host/oxu210hp-hcd.c | 4 +- > drivers/usb/host/r8a66597-hcd.c | 2 +- > drivers/usb/host/sl811-hcd.c| 2 +- > drivers/usb/host/uhci-hub.c | 6 +-- > drivers/usb/host/xhci-hub.c | 6 +-- > drivers/usb/host/xhci-ring.c| 2 +- > drivers/usb/isp1760/isp1760-hcd.c | 2 +- > drivers/usb/musb/musb_core.c| 6 +-- > drivers/usb/musb/musb_virthub.c | 2 +- > include/linux/usb.h | 23 +++- > 20 files changed, 131 insertions(+), 33 deletions(-) > -- 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: [REGRESSION] dwc2: 5f54c54b0ba8 Add DDMA chain pointers to dwc2_hsotg_ep structure
On Tue, Dec 13, 2016 at 3:51 PM, John Youn wrote:
> On 12/13/2016 3:23 PM, John Stultz wrote:
>> I've found trying to boot Linus' HEAD today on HiKey, I'm hitting the
>> following WARNINGS/panics, which I bisected down to being introduced
>> with 5f54c54b0ba8 ("Add DDMA chain pointers to dwc2_hsotg_ep
>> structure").
>>
>> It seems the issue is calling dma_alloc_coherent() in
>> dwc2_hsotg_ep_enable() isn't safe, as we may get there from irq
>> context with irqs off.
>>
>> Any thoughts on a fix?
>>
>
> Hi John,
>
> Check this:
> http://marc.info/?l=linux-usb&m=148058361526211&w=2
So digging further here, it seems that patch addresses the allocation
side, but I'm also seeing warnings on the free side, as irqs are
disabled in dwc2_hsotg_vbus_session() before calling
dwc2_hsotg_disconnect().
[ 94.187771] [ cut here ]
[ 94.192454] WARNING: CPU: 0 PID: 3 at
./include/linux/dma-mapping.h:479 dwc2_hsotg_ep_disable+0x180/0x1b0
[ 94.202047]
[ 94.203577] CPU: 0 PID: 3 Comm: kworker/0:0 Tainted: GW
4.9.0-06789-g055090c-dirty #1754
[ 94.212911] Hardware name: HiKey Development Board (DT)
[ 94.218179] Workqueue: events dwc2_hcd_extcon_func
[ 94.223017] task: ffc0387e1900 task.stack: ffc005f44000
[ 94.228972] PC is at dwc2_hsotg_ep_disable+0x180/0x1b0
[ 94.234148] LR is at usb_ep_disable+0x48/0x60
[ 94.238535] pc : [] lr : []
pstate: 81c5
[ 94.245956] sp : ffc005f47bc0
[ 94.249294] x29: ffc005f47bc0 x28: ffc077f2f2a8
[ 94.254669] x27: ffc077f2f078 x26: 7800
[ 94.260045] x25: ff8008013000 x24: ffc074559018
[ 94.265420] x23: ff8008d29dc0 x22:
[ 94.270793] x21: ffc075135c10 x20: 0001
[ 94.276167] x19: ffc074c97918 x18: 0010
[ 94.281540] x17: ce00 x16: 0040
[ 94.286912] x15: ff8088dae367 x14: 0006
[ 94.292284] x13: ff8008dae375 x12: 0007
[ 94.297656] x11: 03a8 x10: 05f5e0ff
[ 94.303028] x9 : 03a9 x8 : ff80083b72f8
[ 94.308400] x7 : ff8008d47190 x6 : 0024
[ 94.313773] x5 : ff8008fa x4 : 0001fff4
[ 94.319144] x3 : 0003 x2 : 0003
[ 94.324516] x1 : ff8008688988 x0 : 01c0
[ 94.329884]
[ 94.331399] ---[ end trace 07f033317005386d ]---
[ 94.336040] Call trace:
[ 94.338519] Exception stack(0xffc005f479f0 to 0xffc005f47b20)
[ 94.344995] 79e0:
ffc074c97918 0080
[ 94.352866] 7a00: ffc005f47bc0 ff8008688b08
03a9 ff80080fec78
[ 94.360733] 7a20: ffc005f47a90 ff80080ff1ac
ff8008db0790 0004
[ 94.368602] 7a40: ff8008dad000 ff8008d35a70
0014
[ 94.376470] 7a60: 0014
0002 01c0
[ 94.384341] 7a80: ff8008db0790 ff8008bc1590
01c0 ff8008688988
[ 94.392210] 7aa0: 0003 0003
0001fff4 ff8008fa
[ 94.400080] 7ac0: 0024 ff8008d47190
ff80083b72f8 03a9
[ 94.407948] 7ae0: 05f5e0ff 03a8
0007 ff8008dae375
[ 94.415816] 7b00: 0006 ff8088dae367
0040 ce00
[ 94.423683] [] dwc2_hsotg_ep_disable+0x180/0x1b0
[ 94.429902] [] usb_ep_disable+0x48/0x60
[ 94.435315] [] ffs_func_eps_disable.isra.8+0x58/0x98
[ 94.441842] [] ffs_func_set_alt+0xb4/0x2a0
[ 94.447501] [] ffs_func_disable+0x14/0x20
[ 94.453075] [] composite_disconnect+0x4c/0xa0
[ 94.458993] [] dwc2_hsotg_disconnect.part.20+0xac/0xe8
[ 94.465693] [] dwc2_hsotg_vbus_session+0xac/0xd0
[ 94.471874] [] usb_gadget_vbus_disconnect+0x1c/0x30
[ 94.478314] [] dwc2_gadget_notify+0x18/0x30
[ 94.484060] [] dwc2_hcd_extcon_func+0x60/0x78
[ 94.489980] [] process_one_work+0x118/0x370
[ 94.495725] [] worker_thread+0x48/0x498
[ 94.501123] [] kthread+0x100/0x130
[ 94.506088] [] ret_from_fork+0x10/0x50
(Note that this is triggered using the extcon patch I have, but the
problematic logic seems to be pre-existing).
thanks
-john
--
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 0/2] USB: resume time optimization by using spec minimums
The USB resume code in the kernel currently uses a set of hard coded delay values that are defined in the USB 2.0 spec. Specifically these three have the most effect on resume time: - tdrsmdn: resume signal time (20ms - infinity) usb 2.0 spec 7.1.7.7 - trsmrcy: resume recovery time (10ms) usb 2.0 spec 7.1.7.7 - trstrcy: reset recovery time (0ms - infinity) usb 2.0 spec 7.1.7.5 These values have been padded considerably in order to accomodate non-compliant devices. - tdrsmdn: resume signal time = 40ms - trsmrcy: resume recovery time = 10ms - trstrcy: reset recovery time = 50ms I propose that we provide a kernel parameter that sets the USB timing values to their spec minimums. The following patches do this by creating a global struct which contains these values. By default the values remain as they are now, but if usb_timing_minimum=1 is added to the kernel cmd line they're set to their minimums. This struct can be expanded over time to include other hardcoded values that have padding we can remove. I've created a blog entry on 01.org with some analyze_suspend test cases illustrating the benefits: - https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays Todd Brandt (2): USB: add switch to turn off padding of resume time delays USB: usb resume time delay debug Documentation/admin-guide/kernel-parameters.txt | 7 +++ drivers/usb/core/hub.c | 12 ++--- drivers/usb/core/usb.c | 70 + drivers/usb/dwc2/hcd.c | 2 +- drivers/usb/host/ehci-hcd.c | 4 +- drivers/usb/host/ehci-hub.c | 6 +-- drivers/usb/host/fotg210-hcd.c | 2 +- drivers/usb/host/isp116x-hcd.c | 2 +- drivers/usb/host/isp1362-hcd.c | 2 +- drivers/usb/host/ohci-hub.c | 2 +- drivers/usb/host/oxu210hp-hcd.c | 4 +- drivers/usb/host/r8a66597-hcd.c | 2 +- drivers/usb/host/sl811-hcd.c| 2 +- drivers/usb/host/uhci-hub.c | 6 +-- drivers/usb/host/xhci-hub.c | 6 +-- drivers/usb/host/xhci-ring.c| 2 +- drivers/usb/isp1760/isp1760-hcd.c | 2 +- drivers/usb/musb/musb_core.c| 6 +-- drivers/usb/musb/musb_virthub.c | 2 +- include/linux/usb.h | 23 +++- 20 files changed, 131 insertions(+), 33 deletions(-) -- 2.1.4 -- 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 2/2] USB: usb resume time delay debug
add debugfs support for experimenting with USB timing delay
values on the fly. Values are read/written from debugfs at
/sys/kernel/debug/usb/timing.
Signed-off-by: Todd Brandt
---
v2 changes:
- moved the debug code from hub.c to usb.c
- use debugfs instead of /sys/kernel/usb
drivers/usb/core/usb.c | 55 ++
1 file changed, 55 insertions(+)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7859e8a..f81f66b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -39,6 +39,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -1042,6 +1044,49 @@ struct dentry *usb_debug_root;
EXPORT_SYMBOL_GPL(usb_debug_root);
static struct dentry *usb_debug_devices;
+static struct dentry *usb_debug_timing;
+
+static int usb_timing_show(struct seq_file *s, void *unused)
+{
+ seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn);
+ seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy);
+ seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy);
+ return 0;
+}
+
+static int usb_timing_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, usb_timing_show, inode->i_private);
+}
+
+static ssize_t usb_timing_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ int val;
+ char buf[32];
+
+ if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+ return -EFAULT;
+
+ if (sscanf(buf, "tdrsmdn=%u", &val) == 1)
+ usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN);
+ else if (sscanf(buf, "trsmrcy=%u", &val) == 1)
+ usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN);
+ else if (sscanf(buf, "trstrcy=%u", &val) == 1)
+ usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN);
+ else
+ return -EINVAL;
+
+ return count;
+}
+
+static const struct file_operations usbfs_timing_fops = {
+ .open = usb_timing_open,
+ .write= usb_timing_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
static int usb_debugfs_init(void)
{
@@ -1058,11 +1103,21 @@ static int usb_debugfs_init(void)
return -ENOENT;
}
+ usb_debug_timing = debugfs_create_file("timing", 0644,
+ usb_debug_root, NULL,
+ &usbfs_timing_fops);
+ if (!usb_debug_timing) {
+ debugfs_remove(usb_debug_root);
+ usb_debug_root = NULL;
+ return -ENOENT;
+ }
+
return 0;
}
static void usb_debugfs_cleanup(void)
{
+ debugfs_remove(usb_debug_timing);
debugfs_remove(usb_debug_devices);
debugfs_remove(usb_debug_root);
}
--
2.1.4
--
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 1/2] USB: add switch to turn off padding of resume time delays
Add a kernel parameter that replaces the USB_RESUME_TIMEOUT
and other hardcoded delay numbers with the USB spec minimums.
The USB subsystem currently uses heavily padded values for TDRSMDN
and TRSTRCY. This patch keeps the current values by default, but if
the kernel is booted with usb_timing_minimum=1 they are set to the
spec minimums with no padding. The result is significant performance
improvement in usb device resume.
Use this only if you know your USB hardware and device setup can
handle the spec minimums.
Example analyze_suspend runs are provided here showing the benefits:
https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays
Signed-off-by: Todd Brandt
---
v2 changes:
- moved the core code from hub.c to usb.c
- changed the module param to a core param
- param name is now usb_timing_minimum
- configured isp1362-hcd and ohci-hub to use the new values
Documentation/admin-guide/kernel-parameters.txt | 7 +++
drivers/usb/core/hub.c | 12 ++--
drivers/usb/core/usb.c | 15 +++
drivers/usb/dwc2/hcd.c | 2 +-
drivers/usb/host/ehci-hcd.c | 4 ++--
drivers/usb/host/ehci-hub.c | 6 +++---
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/isp116x-hcd.c | 2 +-
drivers/usb/host/isp1362-hcd.c | 2 +-
drivers/usb/host/ohci-hub.c | 2 +-
drivers/usb/host/oxu210hp-hcd.c | 4 ++--
drivers/usb/host/r8a66597-hcd.c | 2 +-
drivers/usb/host/sl811-hcd.c| 2 +-
drivers/usb/host/uhci-hub.c | 6 +++---
drivers/usb/host/xhci-hub.c | 6 +++---
drivers/usb/host/xhci-ring.c| 2 +-
drivers/usb/isp1760/isp1760-hcd.c | 2 +-
drivers/usb/musb/musb_core.c| 6 +++---
drivers/usb/musb/musb_virthub.c | 2 +-
include/linux/usb.h | 23 ++-
20 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index be2d6d0..210ab72 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4024,6 +4024,13 @@
unknown_nmi_panic
[X86] Cause panic on unknown NMI.
+ usb_timing_minimum=
+ Set USB timing values to USB 2.0 spec minimums (default
0 = off).
+ This removes any padding added to the values to
accommodate
+ older or troublesome hardware. This will reduce usb
resume
+ time. Use this only if you know your USB hardware and
device
+ setup can handle the spec minimums.
+
usbcore.authorized_default=
[USB] Default USB device authorization:
(default -1 = authorized except for wireless USB,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 143454e..10c0bea 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2841,7 +2841,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
done:
if (status == 0) {
/* TRSTRCY = 10 ms; plus some extra */
- msleep(10 + 40);
+ msleep(usb_timing.trstrcy);
if (udev) {
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
@@ -3433,10 +3433,10 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(&port_dev->dev, "can't resume, status %d\n", status);
} else {
- /* drive resume for USB_RESUME_TIMEOUT msec */
+ /* drive resume for TDRSMDN msec */
dev_dbg(&udev->dev, "usb %sresume\n",
(PMSG_IS_AUTO(msg) ? "auto-" : ""));
- msleep(USB_RESUME_TIMEOUT);
+ msleep(usb_timing.tdrsmdn);
/* Virtual root hubs can trigger on GET_PORT_STATUS to
* stop resume signaling. Then finish the resume
@@ -3445,7 +3445,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
status = hub_port_status(hub, port1, &portstatus, &portchange);
/* TRSMRCY = 10 msec */
- msleep(10);
+ msleep(usb_timing.trsmrcy);
}
SuspendCleared:
@@ -3531,7 +3531,7 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub,
unsigned int port,
if (udev) {
/* TRSMRCY = 10 msec */
- msleep(10);
+ msleep(usb_timing.trsmrcy);
usb_unlock_port(port_dev);
ret = usb_remote_wakeup(udev);
@@ -4108,7 +4108,7 @@ static void hub_usb3_po
Re: [PATCH 1/2] USB: turn off padding of resume time delays
On Mon, 2016-12-05 at 12:06 +0200, Felipe Balbi wrote: > Hi, > > Todd Brandt writes: > > Add a module parameter that replaces the USB_RESUME_TIMEOUT > > and other hardcoded delay numbers with the USB spec minimums. > > By default the patch retains the current values. > > > > The USB subsystem currently uses heavily padded values for TDRSMDN > > and TRSTRCY. This patch keeps the current values by default, but if > > there's a reason for that. We've found many broken devices over the > years and a lot of inconsistencies were also found while with host > controller A or B or C, because they all used different timings. > > Using spec minimum is, actually, a recipe for disaster :-) There's no > way we can guarantee all devices currently in use are respecting that > part of the spec. > > This is a very fragile part of the USB spec which different folks have > different understanding and that's why so many devices are "broken". > > I'd be careful with such a patchset. It's a nice idea, but I'd be > careful :-) > Hi Felipe, I agree that there are a collection of corner cases that will fail if we use the minimums, but do you object to providing the option to use minimums? My focus is on platform developers looking to optimize a specific device build where the range of USB devices is known in advance. Another plus is this patch could be further configured to increase the delay values for buggy devices. I just like the idea of being able to easily change all the timing values. -- 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: [REGRESSION] dwc2: 5f54c54b0ba8 Add DDMA chain pointers to dwc2_hsotg_ep structure
On Tue, Dec 13, 2016 at 3:51 PM, John Youn wrote:
> On 12/13/2016 3:23 PM, John Stultz wrote:
>> I've found trying to boot Linus' HEAD today on HiKey, I'm hitting the
>> following WARNINGS/panics, which I bisected down to being introduced
>> with 5f54c54b0ba8 ("Add DDMA chain pointers to dwc2_hsotg_ep
>> structure").
>>
>> It seems the issue is calling dma_alloc_coherent() in
>> dwc2_hsotg_ep_enable() isn't safe, as we may get there from irq
>> context with irqs off.
>>
>> Any thoughts on a fix?
>>
>
> Hi John,
>
> Check this:
> http://marc.info/?l=linux-usb&m=148058361526211&w=2
Ah! Thanks! That works great!
Sorry for the noise.
-john
--
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: [v5,5/6] usb: dwc3: use bus->sysdev for DMA configuration
Hi!
On 17/11/16 12:43, Sriram Dash wrote:
> From: Arnd Bergmann
>
> The dma ops for dwc3 devices are not set properly. So, use a
> physical device sysdev, which will be inherited from parent,
> to set the hardware / firmware parameters like dma.
>
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Sriram Dash
> Tested-by: Baolin Wang
Successfully tested on arm64/axxia with DWC3 USB host, XHCIs properly inherit
DMA configuration. Therefore:
Tested-by: Alexander Sverdlin
> ---
> Changes in v5:
> - rebase to usb testing/next
>
> Changes in v4:
> - removed the ifdefs for pci
> - made the sysdev as a device property
> - phy create lookup take up the correct device.
>
> Changes in v3:
> - No update
>
> Changes in v2:
> - integrate dwc3 driver changes together
>
> drivers/usb/dwc3/core.c | 27 ++-
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/dwc3-pci.c | 10 ++
> drivers/usb/dwc3/ep0.c | 8
> drivers/usb/dwc3/gadget.c | 33 +
> drivers/usb/dwc3/host.c | 16 ++--
> 6 files changed, 54 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index e951448..e5fbab2 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -202,7 +202,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *dwc)
> static void dwc3_free_one_event_buffer(struct dwc3 *dwc,
> struct dwc3_event_buffer *evt)
> {
> - dma_free_coherent(dwc->dev, evt->length, evt->buf, evt->dma);
> + dma_free_coherent(dwc->sysdev, evt->length, evt->buf, evt->dma);
> }
>
> /**
> @@ -228,7 +228,7 @@ static struct dwc3_event_buffer
> *dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
> if (!evt->cache)
> return ERR_PTR(-ENOMEM);
>
> - evt->buf= dma_alloc_coherent(dwc->dev, length,
> + evt->buf= dma_alloc_coherent(dwc->sysdev, length,
> &evt->dma, GFP_KERNEL);
> if (!evt->buf)
> return ERR_PTR(-ENOMEM);
> @@ -341,11 +341,11 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
> if (!WARN_ON(dwc->scratchbuf))
> return 0;
>
> - scratch_addr = dma_map_single(dwc->dev, dwc->scratchbuf,
> + scratch_addr = dma_map_single(dwc->sysdev, dwc->scratchbuf,
> dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
> DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dwc->dev, scratch_addr)) {
> - dev_err(dwc->dev, "failed to map scratch buffer\n");
> + if (dma_mapping_error(dwc->sysdev, scratch_addr)) {
> + dev_err(dwc->sysdev, "failed to map scratch buffer\n");
> ret = -EFAULT;
> goto err0;
> }
> @@ -369,7 +369,7 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
> return 0;
>
> err1:
> - dma_unmap_single(dwc->dev, dwc->scratch_addr, dwc->nr_scratch *
> + dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
>
> err0:
> @@ -388,7 +388,7 @@ static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
> if (!WARN_ON(dwc->scratchbuf))
> return;
>
> - dma_unmap_single(dwc->dev, dwc->scratch_addr, dwc->nr_scratch *
> + dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> kfree(dwc->scratchbuf);
> }
> @@ -927,6 +927,13 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> dwc->dr_mode = usb_get_dr_mode(dev);
> dwc->hsphy_mode = of_usb_get_phy_mode(dev->of_node);
>
> + dwc->sysdev_is_parent = device_property_read_bool(dev,
> + "linux,sysdev_is_parent");
> + if (dwc->sysdev_is_parent)
> + dwc->sysdev = dwc->dev->parent;
> + else
> + dwc->sysdev = dwc->dev;
> +
> dwc->has_lpm_erratum = device_property_read_bool(dev,
> "snps,has-lpm-erratum");
> device_property_read_u8(dev, "snps,lpm-nyet-threshold",
> @@ -1097,12 +1104,6 @@ static int dwc3_probe(struct platform_device *pdev)
>
> spin_lock_init(&dwc->lock);
>
> - if (!dev->dma_mask) {
> - dev->dma_mask = dev->parent->dma_mask;
> - dev->dma_parms = dev->parent->dma_parms;
> - dma_set_coherent_mask(dev, dev->parent->coherent_dma_mask);
> - }
> -
> pm_runtime_set_active(dev);
> pm_runtime_use_autosuspend(dev);
> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ef81fa5..de5a857 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -819,6 +819,7 @@ struct dwc3_scratchpad_array {
> * @ep0_bounced: true when we used bounce buffer
> * @ep0_expect_in: true when we expect a DATA IN tran
Re: [REGRESSION] dwc2: 5f54c54b0ba8 Add DDMA chain pointers to dwc2_hsotg_ep structure
On 12/13/2016 3:23 PM, John Stultz wrote:
> I've found trying to boot Linus' HEAD today on HiKey, I'm hitting the
> following WARNINGS/panics, which I bisected down to being introduced
> with 5f54c54b0ba8 ("Add DDMA chain pointers to dwc2_hsotg_ep
> structure").
>
> It seems the issue is calling dma_alloc_coherent() in
> dwc2_hsotg_ep_enable() isn't safe, as we may get there from irq
> context with irqs off.
>
> Any thoughts on a fix?
>
Hi John,
Check this:
http://marc.info/?l=linux-usb&m=148058361526211&w=2
Thanks,
John
--
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
[REGRESSION?] 05ee799f2021 dwc2: Move gadget settings into core_params
Hey John,
Wanted to let you know I'm seeing a new WARNING with HiKey as of
05ee799f2021 ("dwc2: Move gadget settings into core_params").
It doesn't seem to kill the system, but seems odd since that patch
seems to be just moving code around and not changing logic.
Any thoughts?
thanks
-john
[8.977899] insufficient fifo memory[8.981366] [
cut here ]
[8.986027] WARNING: CPU: 1 PID: 1 at drivers/usb/dwc2/gadget.c:226
dwc2_hsotg_init_fifo+0x13c/0x188
[8.995176]
[8.996688] CPU: 1 PID: 1 Comm: init Not tainted
4.9.0-06762-g50c812c-dirty #1745
[9.004188] Hardware name: HiKey Development Board (DT)
[9.009432] task: ffc0387e task.stack: ffc0387e8000
[9.015378] PC is at dwc2_hsotg_init_fifo+0x13c/0x188
[9.020450] LR is at dwc2_hsotg_init_fifo+0x13c/0x188
[9.025520] pc : [] lr : []
pstate: 61c5
[9.032935] sp : ffc0387ebbe0
[9.036252] x29: ffc0387ebbe0 x28: ffc0387e8000
[9.041600] x27: ff8008a32000 x26: 0001
[9.046940] x25: ff8008c29af8 x24: ff8008d797bf
[9.052264] x23: 00801000 x22: 1000
[9.057618] x21: ffc074ba50c4 x20: ffc074ba5018
[9.062974] x19: 0104 x18: 0010
[9.068295] x17: x16: ff80081d1098
[9.073614] x15: ff8088d9e367 x14: 0006
[9.078967] x13: ff8008d9e375 x12: 0007
[9.084305] x11: 0357 x10: 05f5e0ff
[9.089652] x9 : 0358 x8 : ff80083b8258
[9.094973] x7 : ff8008d37190 x6 : 0027
[9.100294] x5 : ff8008f8 x4 : 0001fff4
[9.105612] x3 : 0002 x2 : 0002
[9.110948] x1 : 0001 x0 : 0018
[9.116298]
[9.117788] ---[ end trace 727c8ae7c0707f05 ]---
[9.122414] Call trace:
[9.124875] Exception stack(0xffc0387eba10 to 0xffc0387ebb40)
[9.131334] ba00:
0104 0080
[9.139185] ba20: ffc0387ebbe0 ff8008686284
0018
[9.147036] ba40: 01c0
ff8008da0790 ff8008bc1390
[9.154884] ba60: ffc0387ebb00 ff80080ff430
0104 ffc074ba5018
[9.162729] ba80: ffc074ba50c4 1000
00801000 ff8008d797bf
[9.170562] baa0: ff8008c29af8 0001
0018 0001
[9.178396] bac0: 0002 0002
0001fff4 ff8008f8
hikey:/ $ [9.186239] bae0: 0027 ff8008d37190
ff80083b8258 0358
[9.194927] bb00: 05f5e0ff 0357
0007 ff8008d9e375
[9.202760] bb20: 0006 ff8088d9e367
ff80081d1098
[9.210606] [] dwc2_hsotg_init_fifo+0x13c/0x188
[9.216721] [] dwc2_hsotg_core_init_disconnected+0x74/0x2f8
[9.223879] [] dwc2_hsotg_pullup+0x90/0x100
[9.229630] [] usb_udc_connect_control.isra.0+0x78/0x90
[9.236432] [] udc_bind_to_driver+0xc4/0xe8
[9.242182] [] usb_gadget_probe_driver+0xf4/0x148
[9.248448] [] gadget_dev_desc_UDC_store+0xdc/0xf8
[9.254804] [] configfs_write_file+0xc0/0x198
[9.260728] [] __vfs_write+0x1c/0x100
[9.265953] [] vfs_write+0xa0/0x1b8
[9.271003] [] SyS_write+0x44/0xa0
[9.275969] [] el0_svc_naked+0x24/0x28
[9.281308] dwc2 f72c.usb: dwc2_hsotg_enqueue_setup: failed queue (-11)
--
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 3/3] xhci: Remove busy loop from xhci_abort_cmd_ring()
ping about [PATCH 1/3, 2/3, 3/3]?
OGAWA Hirofumi writes:
> Mathias Nyman writes:
>
>>> - Add xhci_handshake_sleep(), and use it.
>>
>> This seems a but overkill, I'd rather don't have xhci_handshake(),
>> xhci_handshake_sleep() and __xhci_handshake() to maintain.
>
> I agree about it. However, on other hand, I thought about the
> possibility/effort to decreasing use-cases of xhci_handshake()
> busyloop. (there are several places to use more than e.g. 500ms
> timeout.) Because long busyloop (especially with interrupt-off) has
> really bad effect to whole system of non-usb.
>
>> As we now can sleep in xhci_abort_cmd_ring() I would rather just first
>> wait for the completion and then maybe handshake check the register.
>> At that point it shouldn't need to busyloop anymore, one read should
>> do it.
>
> I worried about in the case of real internal confusing, and consider
> about chip that doesn't have no stop/abort event.
>
> To handle both case, timeout choice would not be straight-forward.
>
>> But this is all just optimizing an error path, I'm actually fine with
>> taking just patch 1/3 and 2/3 v3, and skipping 3/3. the udelay(1000)
>> to msleep(1) I can add myself
>
> Right. The main point of patchset is 1/3 and 2/3.
>
>>> As related change, current xhci_handshake() is strange behavior,
>>> E.g. xhci_handshake(ptr, mask, done, 1) does
>>>
>>> result = readl(ptr);
>>> /* check result */
>>> udelay(1); <= meaningless delay
>>> return -ETIMEDOUT;
>>
>> Only in the timeout case, so if we after 3 or 5 million register reads
>> + udelays(1) still don't get the value we want then there is an
>> unnecessary udelay(1).
>>
>> Not worth putting much effort on it now.
>
> True. But actual effort for it was very small.
>
> @@ -65,16 +65,18 @@ int xhci_handshake(void __iomem *ptr, u3
> {
> u32 result;
>
> - do {
> + while (1) {
> result = readl(ptr);
> if (result == ~(u32)0) /* card removed */
> return -ENODEV;
> result &= mask;
> if (result == done)
> return 0;
> + if (usec <= 0)
> + break;
> udelay(1);
> usec--;
> - } while (usec > 0);
> + }
> return -ETIMEDOUT;
> }
>
> Thanks.
--
OGAWA Hirofumi
--
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
[REGRESSION] dwc2: 5f54c54b0ba8 Add DDMA chain pointers to dwc2_hsotg_ep structure
I've found trying to boot Linus' HEAD today on HiKey, I'm hitting the
following WARNINGS/panics, which I bisected down to being introduced
with 5f54c54b0ba8 ("Add DDMA chain pointers to dwc2_hsotg_ep
structure").
It seems the issue is calling dma_alloc_coherent() in
dwc2_hsotg_ep_enable() isn't safe, as we may get there from irq
context with irqs off.
Any thoughts on a fix?
thanks
-john
[9.422185] dwc2 f72c.usb: new address 27
[9.445030] configfs-gadget gadget: high-speed config #1: b
[9.450692] [ cut here ]
[9.455343] WARNING: CPU: 0 PID: 0 at kernel/smp.c:405
smp_call_function_many+0xe0/0x2d8
[9.463436]
[9.464930] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
4.9.0-06762-g50c812c-dirty #1739
[9.474076] Hardware name: HiKey Development Board (DT)
[9.479307] task: ff8008d17080 task.stack: ff8008d0
[9.485232] PC is at smp_call_function_many+0xe0/0x2d8
[9.490372] LR is at smp_call_function_many+0x30/0x2d8
[9.495511] pc : [] lr : []
pstate: 61c5
[9.502907] sp : ffc077f29610
[9.506220] x29: ffc077f29610 x28: 0004
[9.511547] x27: ff8008d08000 x26: ff8008d8c880
[9.516868] x25: ff80081659f8 x24: 0001
[9.522191] x23: 0001 x22: ff8008d8c880
[9.527511] x21: ff80081659f8 x20:
[9.532833] x19: ff8008db19c8 x18: 0010
[9.538155] x17: x16:
[9.543474] x15: ff8088d9e367 x14: 0006
[9.548797] x13: ce248c14 x12: 08ca90e4
[9.554118] x11: 0001 x10: ffbf01e43820
[9.559440] x9 : 0800 x8 : ffbf01e01820
[9.564763] x7 : ff8008d8cb58 x6 :
[9.570083] x5 : ff00 x4 : 0040
[9.575406] x3 : 0001 x2 : 00010104
[9.580727] x1 : 0001 x0 : ff8008d2b3e2
[9.586049]
[9.587537] ---[ end trace aa3428adcdcb3db1 ]---
[9.592152] Call trace:
[9.594599] Exception stack(0xffc077f29440 to 0xffc077f29570)
[9.601041] 9440: ff8008db19c8 0080
ffc077f29610 ff80081281d0
[9.608873] 9460:
[9.616705] 9480:
[9.624536] 94a0:
[9.632367] 94c0:
[9.640197] 94e0: ff8008d2b3e2 0001
00010104 0001
[9.648028] 9500: 0040 ff00
ff8008d8cb58
[9.655860] 9520: ffbf01e01820 0800
ffbf01e43820 0001
[9.663692] 9540: 08ca90e4 ce248c14
0006 ff8088d9e367
[9.671523] 9560:
[9.676406] [] smp_call_function_many+0xe0/0x2d8
[9.682589] [] on_each_cpu_mask+0x48/0xd8
[9.688166] [] drain_all_pages+0xf4/0x110
[9.693741] [] start_isolate_page_range+0x7c/0x1f8
[9.700095] [] alloc_contig_range+0xc0/0x340
[9.705927] [] cma_alloc+0xf8/0x1f8
[9.710982] [] dma_alloc_from_contiguous+0x3c/0x48
[9.717340] [] __dma_alloc+0xb4/0x280
[9.722567] [] dwc2_hsotg_ep_enable+0xe0/0x498
[9.728575] [] usb_ep_enable+0x30/0x48
[9.733887] [] ffs_func_set_alt+0x150/0x2a0
[9.739635] [] composite_setup+0xc54/0x17b8
[9.745381] [] dwc2_hsotg_complete_setup+0xbc/0x418
[9.751821] [] usb_gadget_giveback_request+0x10/0x18
[9.758347] [] dwc2_hsotg_complete_request+0x74/0x120
[9.764961] [] dwc2_hsotg_handle_outdone+0xf0/0x1c8
[9.771401] [] dwc2_hsotg_epint+0x51c/0x6b0
[9.777145] [] dwc2_hsotg_irq+0x360/0x740
[9.782718] [] __handle_irq_event_percpu+0x9c/0x128
[9.789157] [] handle_irq_event_percpu+0x1c/0x58
[9.795336] [] handle_irq_event+0x48/0x78
[9.800910] [] handle_fasteoi_irq+0xd4/0x1d8
[9.806744] [] generic_handle_irq+0x28/0x40
[9.812490] [] __handle_domain_irq+0x84/0xf8
[9.818324] [] gic_handle_irq+0x54/0xa8
...
[ 10.057181] BUG: scheduling while atomic: swapper/0/0/0x00010104
[ 10.063213] Preemption disabled at:[ 10.065319] healthd:
wakealarm_init: timerfd_create failed
[ 10.068603] healthd: No battery devices found
[ 10.076392] [] schedule_preempt_disabled+0x1c/0x28
[ 10.082771] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
4.9.0-06762-g50c812c-dirty #1739
[ 10.091903] Hardware name: HiKey Development Board (DT)
[ 10.097124] Call trace:
[ 10.099574] [] dump_backtrace+0x0/0x1a0
[ 10.104970] [] show_stack+0x14/0x20
[ 10.110021] [] dump_stack+0x90/0xb0
[ 10.115071] [] __schedule_bug+0x6c/0xb8
[ 10.120466] [] __schedule+0x4f8/0x5b0
[ 10.125688] [] schedule+0x38/0xa0
[ 10.130563] [] schedule_timeout+0x1b4/0x248
[ 10.136306] [] wait_for_common
Re: [v5,4/6] usb: xhci: use bus->sysdev for DMA configuration
Hi!
On 17/11/16 12:43, Sriram Dash wrote:
> From: Arnd Bergmann
>
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices. So, set
> the dma for xhci from sysdev. sysdev is pointing to device that
> is known to the system firmware or hardware.
>
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Sriram Dash
> Tested-by: Baolin Wang
Successfully tested on arm64/axxia with DWC3 USB host, XHCIs properly inherit
DMA configuration. Therefore:
Tested-by: Alexander Sverdlin
> ---
> Changes in v5:
> - No update
>
> Changes in v4:
> - No update
>
> Changes in v3:
> - No update
>
> Changes in v2:
> - Separate out xhci driver changes apart
>
> drivers/usb/host/xhci-mem.c | 12 ++--
> drivers/usb/host/xhci-plat.c | 33 ++---
> drivers/usb/host/xhci.c | 15 +++
> 3 files changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 48a26d378..eb32de9 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -586,7 +586,7 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
> unsigned int num_stream_ctxs,
> struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
> {
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>
> if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -614,7 +614,7 @@ static struct xhci_stream_ctx
> *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
> unsigned int num_stream_ctxs, dma_addr_t *dma,
> gfp_t mem_flags)
> {
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>
> if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -1644,7 +1644,7 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
> static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
> {
> int i;
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
>
> xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> @@ -1716,7 +1716,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
> {
> int num_sp;
> int i;
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>
> if (!xhci->scratchpad)
> return;
> @@ -1792,7 +1792,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
>
> void xhci_mem_cleanup(struct xhci_hcd *xhci)
> {
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> int size;
> int i, j, num_ports;
>
> @@ -2334,7 +2334,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd
> *xhci, gfp_t flags)
> int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
> {
> dma_addr_t dma;
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> unsigned intval, val2;
> u64 val_64;
> struct xhci_segment *seg;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..beb95c8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -139,6 +140,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
> {
> const struct of_device_id *match;
> const struct hc_driver *driver;
> + struct device *sysdev;
> struct xhci_hcd *xhci;
> struct resource *res;
> struct usb_hcd *hcd;
> @@ -155,22 +157,39 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (irq < 0)
> return -ENODEV;
>
> + /*
> + * sysdev must point to a device that is known to the system firmware
> + * or PCI hardware. We handle these three cases here:
> + * 1. xhci_plat comes from firmware
> + * 2. xhci_plat is child of a device from firmware (dwc3-plat)
> + * 3. xhci_plat is grandchild of a pci device (dwc3-pci)
> + */
> + sysdev = &pdev->dev;
> + if (sysdev->parent && !sysdev->of_node && sysdev->parent->of_node)
> + sysdev = sysdev->parent;
> +#ifdef CONFIG_PCI
> + else if (sysdev->parent && sysdev->parent->parent &&
> + sysdev->parent->parent->bus == &pci_bus_type)
> + sysdev = sysdev->parent->parent;
> +#endif
> +
> /* Try to set 64-bit DMA first */
> - if (WARN_ON(!pdev->dev.dma_mask))
> + if (WARN
Re: [RFC][PATCH 2/5] usb: dwc2: Workaround case where GOTGCTL state is wrong
On Tue, Dec 13, 2016 at 4:28 AM, Vardan Mikayelyan
wrote:
> On 12/13/2016 11:12 AM, John Stultz wrote:
>> When removing a USB-A to USB-otg adapter cable, we get a change
>> status irq, and then in dwc2_conn_id_status_change, we
>> erroniously see the GOTGCTL_CONID_B flag set. This causes us to
>> get stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
>> spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
>> until it fails out many seconds later.
>>
>> This patch works around the issue by re-reading the GOTGCTL
>> state to check if the GOTGCTL_CONID_B is still set and if not
>> restarting the change status logic.
>>
>> I suspect this isn't the best solution, but it seems to work
>> well for me.
>>
>> Feedback would be greatly appreciated!
>>
>> Cc: Wei Xu
>> Cc: Guodong Xu
>> Cc: Amit Pundir
>> Cc: Rob Herring
>> Cc: John Youn
>> Cc: Douglas Anderson
>> Cc: Chen Yu
>> Cc: Kishon Vijay Abraham I
>> Cc: Felipe Balbi
>> Cc: Greg Kroah-Hartman
>> Cc: linux-usb@vger.kernel.org
>> Acked-by: John Youn
>> Signed-off-by: John Stultz
>> ---
>> drivers/usb/dwc2/hcd.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index df5a065..a3f34dd 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -3199,7 +3199,7 @@ static void dwc2_conn_id_status_change(struct
>> work_struct *work)
>> dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl);
>> dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n",
>> !!(gotgctl & GOTGCTL_CONID_B));
>> -
>> +again:
>> /* B-Device connector (Device Mode) */
>> if (gotgctl & GOTGCTL_CONID_B) {
>> /* Wait for switch to device mode */
>> @@ -3210,6 +3210,9 @@ static void dwc2_conn_id_status_change(struct
>> work_struct *work)
>>dwc2_is_host_mode(hsotg) ? "Host" :
>>"Peripheral");
>> usleep_range(2, 4);
>> + gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
>> + if (!(gotgctl & GOTGCTL_CONID_B))
>> + goto again;
>> if (++count > 250)
>> break;
>> }
>>
> Hi John Stultz,
>
> When it goes to ":again", it will go to else anyways. I'll suggest
> alternative way to do this. Please see below.
Sounds good. I've made the change.
> Also you can add "Reviewed-by: Vardan Mikayelyan "
Thanks so much for your review!
-john
--
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: Fwd: usb cellphone storage device disconnection causes my usb mouse to be disconnected
Besiyata Dishmaya, Iv'e downloaded the specified kernel version, and I confirm that the bug was fixed... and I managed to complete the file transfer... thank you Alan and thank you Greg On Tue, Dec 13, 2016 at 9:55 PM, Alan Stern wrote: > On Tue, 13 Dec 2016, or dv wrote: > >> kernel version: >> Linux version 4.8.0-30-generic (buildd@lcy01-08) (gcc version 6.2.0 >> 20161005 (Ubuntu 6.2.0-5ubuntu12) ) #32-Ubuntu SMP Fri Dec 2 03:43:27 >> UTC 2016 >> >> kernel log shows: > >> Dec 13 21:31:27 merits kernel: [ 5835.972008] ohci-pci :00:04.0: >> HcDoneHead not written back; disabled >> Dec 13 21:31:27 merits kernel: [ 5835.972014] ohci-pci :00:04.0: >> HC died; cleaning up >> Dec 13 21:31:27 merits kernel: [ 5835.972335] usb 2-2: USB disconnect, >> device number 2 >> Dec 13 21:31:27 merits kernel: [ 5835.973969] usb 2-4: USB disconnect, >> device number 3 >> Dec 13 21:31:27 merits kernel: [ 5836.012277] usb 2-9: USB disconnect, >> device number 5 > > This is a known bug in the 4.8 kernel. You should upgrade to the > latest 4.8-stable version (currently 4.8.14); it contains a fix for > this bug. > > Alan Stern > -- 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: [v5,6/6] usb: dwc3: Do not set dma coherent mask
Hi! On 17/11/16 12:43, Sriram Dash wrote: > From: Arnd Bergmann > > The dma mask is correctly set up by the DT probe function, no > need to override it any more. > > Signed-off-by: Arnd Bergmann > Signed-off-by: Sriram Dash Successfully tested on arm64/axxia with DWC3 USB host, XHCIs properly inherit DMA configuration. Therefore: Tested-by: Alexander Sverdlin > --- > Changes in v5: > - No update > > Changes in v4: > - No update > > Changes in v3: > - No update > > Changes in v2: > - club the cleanup for dma coherent mask for device > > drivers/usb/dwc3/dwc3-exynos.c | 10 -- > drivers/usb/dwc3/dwc3-st.c | 1 - > 2 files changed, 11 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index 2f1fb7e..e27899b 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -20,7 +20,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -117,15 +116,6 @@ static int dwc3_exynos_probe(struct platform_device > *pdev) > if (!exynos) > return -ENOMEM; > > - /* > - * Right now device-tree probed devices don't get dma_mask set. > - * Since shared usb code relies on it, set it here for now. > - * Once we move to full device tree support this will vanish off. > - */ > - ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); > - if (ret) > - return ret; > - > platform_set_drvdata(pdev, exynos); > > exynos->dev = dev; > diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c > index 89a2f71..4d7439c 100644 > --- a/drivers/usb/dwc3/dwc3-st.c > +++ b/drivers/usb/dwc3/dwc3-st.c > @@ -218,7 +218,6 @@ static int st_dwc3_probe(struct platform_device *pdev) > if (IS_ERR(regmap)) > return PTR_ERR(regmap); > > - dma_set_coherent_mask(dev, dev->coherent_dma_mask); > dwc3_data->dev = dev; > dwc3_data->regmap = regmap; > > -- Best regards, Alexander Sverdlin. -- 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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
"On Tue, Dec 13, 2016 at 9:09 PM, Alan Stern wrote: > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > >> On Tue, Dec 13, 2016 at 7:38 PM, Alan Stern >> wrote: >> > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: >> > >> >> On Tue, Dec 13, 2016 at 4:52 PM, Alan Stern >> >> wrote: >> >> > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: >> >> > >> >> >> >> > If it is >> >> >> >> > not a bug in kernel source code, then it must not produce a >> >> >> >> > WARNING. >> >> >> > >> >> >> > What about a memory allocation failure? The memory management part >> >> >> > of >> >> >> > the kernel produces a WARNING message if an allocation fails and the >> >> >> > caller did not specify __GFP_NOWARN. >> >> >> > >> >> >> > There is no way for a driver to guarantee that a memory allocation >> >> >> > request will succeed -- failure is always an option. But obviously >> >> >> > memory allocation failures are not bugs in the kernel. >> >> >> > >> >> >> > Are you saying that mm/page_alloc.c:warn_alloc() should produce >> >> >> > something other than a WARNING? >> >> >> >> >> >> >> >> >> The main thing I am saying is that we absolutely need a way for a >> >> >> human or a computer program to be able to determine if there is >> >> >> anything wrong with kernel or not. >> >> > Doesn't it also produce a WARNING under other circumstances? >> >> >> >> No. >> >> >> >> OOM is not a WARNING and is easily distinguishable from BUG/WARNING. >> > >> >> Memory allocator does not print WARNINGs on allocation failures. >> > >> > Do you count dev_warn the same as WARN or WARN_ON? What about dev_WARN >> > or pr_warn() or printk(KERN_WARNING...)? Maybe we're not talking about >> > the same messages. >> > >> > The USB subsystem has got tons of dev_warn() and dev_err() calls. >> > Relatively few (if any) of them are for kernel bugs. >> >> >> I grep for "WARNING:". It is not possible to understand what function >> printed messages on console. >> Here are my current regexps: >> https://github.com/google/syzkaller/blob/master/report/report.go#L29 > > Ah, okay. > > So the take-home message is that we should use WARN* or dev_WARN or > related functions only when reporting an actual kernel bug, whereas in > other circumstances we should avoid mentioning "WARNING" or "BUG" in > log output. In addition, memory allocations where the size is given by > the user (and not limited) should always use the __GFP_NOWARN flag. Yes, such simple convention that allows to "grep" for bugs would be awesome! > I can audit the parts of the USB stack that I'm familiar with for these > things. Anything else? What about "ERROR"? Your regexps don't appear > to search for that. Well, I don't know. Are there messages that are prefixed with ERROR and indicate kernel bugs? I can't possibly manually inspect all generated kernel output. I add errors on case-by-case basis. BUG: and WARNING: where obvious starting points. Then I discovered "INFO:", "Unable to handle kernel paging request", "general protection fault:", "Kernel panic", "kernel BUG", "Kernel BUG", "divide error:", "invalid opcode:", "unreferenced object" and "UBSAN:". There are chances that there are some bugs with ERROR: that I am just missing. For example my dmesg is full of: [2536674.524238] vmwrite error: reg 2800 value (err -255) [2536674.524256] Call Trace: [2536674.524260] [] dump_stack+0x64/0x82 [2536674.524267] [] vmwrite_error+0x2c/0x2e [kvm_intel] [2536674.524272] [] vmcs_writel+0x1f/0x30 [kvm_intel] [2536674.524278] [] free_nested.part.64+0x70/0x170 [kvm_intel] [2536674.524283] [] vmx_free_vcpu+0x33/0x70 [kvm_intel] [2536674.524295] [] kvm_arch_vcpu_free+0x44/0x50 [kvm] [2536674.524308] [] kvm_arch_destroy_vm+0xf2/0x1f0 [kvm] [2536674.524311] [] ? synchronize_srcu+0x1d/0x20 [2536674.524323] [] kvm_put_kvm+0x106/0x1c0 [kvm] [2536674.524334] [] kvm_vm_release+0x21/0x30 [kvm] [2536674.524337] [] __fput+0xe4/0x260 [2536674.524340] [] fput+0xe/0x10 [2536674.524343] [] task_work_run+0xac/0xd0 [2536674.524346] [] do_exit+0x2b0/0xa60 [2536674.524349] [] do_group_exit+0x3f/0xa0 [2536674.524352] [] get_signal_to_deliver+0x1d0/0x6f0 [2536674.524356] [] do_signal+0x48/0xa10 [2536674.524359] [] ? SYSC_sendto+0x17e/0x1c0 [2536674.524362] [] ? do_vfs_ioctl+0x2e0/0x4c0 [2536674.524365] [] do_notify_resume+0x69/0xb0 [2536674.524368] [] int_signal+0x12/0x17 But I have no idea if it is a bug or not. It's kind of printed with pr_err. But on the other hand it is not printed WARN or BUG. Who knows?.. -- 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: Fwd: usb cellphone storage device disconnection causes my usb mouse to be disconnected
On Tue, 13 Dec 2016, or dv wrote: > kernel version: > Linux version 4.8.0-30-generic (buildd@lcy01-08) (gcc version 6.2.0 > 20161005 (Ubuntu 6.2.0-5ubuntu12) ) #32-Ubuntu SMP Fri Dec 2 03:43:27 > UTC 2016 > > kernel log shows: > Dec 13 21:31:27 merits kernel: [ 5835.972008] ohci-pci :00:04.0: > HcDoneHead not written back; disabled > Dec 13 21:31:27 merits kernel: [ 5835.972014] ohci-pci :00:04.0: > HC died; cleaning up > Dec 13 21:31:27 merits kernel: [ 5835.972335] usb 2-2: USB disconnect, > device number 2 > Dec 13 21:31:27 merits kernel: [ 5835.973969] usb 2-4: USB disconnect, > device number 3 > Dec 13 21:31:27 merits kernel: [ 5836.012277] usb 2-9: USB disconnect, > device number 5 This is a known bug in the 4.8 kernel. You should upgrade to the latest 4.8-stable version (currently 4.8.14); it contains a fix for this bug. Alan Stern -- 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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > On Tue, Dec 13, 2016 at 7:38 PM, Alan Stern wrote: > > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > > > >> On Tue, Dec 13, 2016 at 4:52 PM, Alan Stern > >> wrote: > >> > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > >> > > >> >> >> > If it is > >> >> >> > not a bug in kernel source code, then it must not produce a > >> >> >> > WARNING. > >> >> > > >> >> > What about a memory allocation failure? The memory management part of > >> >> > the kernel produces a WARNING message if an allocation fails and the > >> >> > caller did not specify __GFP_NOWARN. > >> >> > > >> >> > There is no way for a driver to guarantee that a memory allocation > >> >> > request will succeed -- failure is always an option. But obviously > >> >> > memory allocation failures are not bugs in the kernel. > >> >> > > >> >> > Are you saying that mm/page_alloc.c:warn_alloc() should produce > >> >> > something other than a WARNING? > >> >> > >> >> > >> >> The main thing I am saying is that we absolutely need a way for a > >> >> human or a computer program to be able to determine if there is > >> >> anything wrong with kernel or not. > >> > Doesn't it also produce a WARNING under other circumstances? > >> > >> No. > >> > >> OOM is not a WARNING and is easily distinguishable from BUG/WARNING. > > > >> Memory allocator does not print WARNINGs on allocation failures. > > > > Do you count dev_warn the same as WARN or WARN_ON? What about dev_WARN > > or pr_warn() or printk(KERN_WARNING...)? Maybe we're not talking about > > the same messages. > > > > The USB subsystem has got tons of dev_warn() and dev_err() calls. > > Relatively few (if any) of them are for kernel bugs. > > > I grep for "WARNING:". It is not possible to understand what function > printed messages on console. > Here are my current regexps: > https://github.com/google/syzkaller/blob/master/report/report.go#L29 Ah, okay. So the take-home message is that we should use WARN* or dev_WARN or related functions only when reporting an actual kernel bug, whereas in other circumstances we should avoid mentioning "WARNING" or "BUG" in log output. In addition, memory allocations where the size is given by the user (and not limited) should always use the __GFP_NOWARN flag. I can audit the parts of the USB stack that I'm familiar with for these things. Anything else? What about "ERROR"? Your regexps don't appear to search for that. Alan Stern -- 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
Andy Lutomirski wrote: > I don't know whether you're right, but that sounds a bit silly to me. > This is a *tiny* amount of memory. Assuming a 1MiB kernel image in 4K pages, that gets you back a couple of pages I think - useful if you've only got a few MiB of RAM. David -- 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] tools: usb: usbip: Update README
Update README file: - remove outdated parts - clarify terminology and general structure - add some description of vUDC Signed-off-by: Krzysztof Opasiak --- tools/usb/usbip/README | 56 +- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tools/usb/usbip/README b/tools/usb/usbip/README index 831f49fea3ce..148d13814306 100644 --- a/tools/usb/usbip/README +++ b/tools/usb/usbip/README @@ -4,10 +4,32 @@ # Copyright (C) 2011 matt mooney # 2005-2008 Takahiro Hirofuchi +[Overview] +USB/IP protocol allows to pass USB device from server to client over the +network. The USB device may be either physical device connected to a server or +software entity created on a server using USB gadget subsystem. +Whole project consists of four parts: + +- usbip-vhci +Kernel module which provides a virtual USB Host Controller and allows +to import a USB device from a remote machine. Used on a client side. + +- usbip-host (stub driver) +Kernel module which provides a USB device driver which can be bound to +a physical USB device to make it exportable. Used on a server side. + +- usbip-vudc +Kernel module which provides a virtual USB Device Controller and allows +to export a USB device created using USB Gadget Subsystem. Used on +a server side. + +- usbip-utils +A set of userspace tools used to handle connection and management. +Used on both sides. [Requirements] - USB/IP device drivers - Found in the staging directory of the Linux kernel. +Found in the drivers/usb/usbip/ directory of the Linux kernel tree. - libudev >= 2.0 libudev library @@ -36,6 +58,10 @@ [Usage] +On a server side there are two entities which can be shared. +First of them is physical usb device connected to the machine. +To make it available below steps should be executed: + server:# (Physically attach your USB device.) server:# insmod usbip-core.ko @@ -52,6 +78,30 @@ - The USB device 1-2 is now exportable to other hosts! - Use `usbip unbind --busid 1-2' to stop exporting the device. +Second of shareable entities is USB Gadget created using USB Gadget Subsystem +on a server machine. To make it available below steps should be executed: + +server:# (Create your USB gadget) +- Currently the most preferable way of creating a new USB gadget + is ConfigFS Composite Gadget. Please refer to its documentation + for details. +- See vudc_server_example.sh for a short example of USB gadget creation + +server:# insmod usbip-core.ko +server:# insmod usbip-vudc.ko +- To create more than one instance of vudc use num module param + +server:# (Bind gadget to one of available vudc) +- Assign your new gadget to USB/IP UDC +- Using ConfigFS interface you may do this simply by: +server:# cd /sys/kernel/config/usb_gadget/ +server:# echo "usbip-vudc.0" > UDC + +server:# usbipd -D --device +- Start usbip daemon. + +To attach new device to client machine below commands should be used: + client:# insmod usbip-core.ko client:# insmod vhci-hcd.ko @@ -60,6 +110,8 @@ client:# usbip attach --remote --busid 1-2 - Connect the remote USB device. + - When using vudc on a server side busid is really vudc instance name. + For example: usbip-vudc.0 client:# usbip port - Show virtual port status. @@ -192,6 +244,8 @@ Detach the imported device: - http://usbip.wiki.sourceforge.net/how-to-debug-usbip - usbip-host.ko must be bound to the target device. - See /proc/bus/usb/devices and find "Driver=..." lines of the device. +- Target USB gadget must be bound to vudc + (using USB gadget susbsys, not usbip bind command) - Shutdown firewall. - usbip now uses TCP port 3240. - Disable SELinux. -- 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: Fwd: usb cellphone storage device disconnection causes my usb mouse to be disconnected
kernel version: Linux version 4.8.0-30-generic (buildd@lcy01-08) (gcc version 6.2.0 20161005 (Ubuntu 6.2.0-5ubuntu12) ) #32-Ubuntu SMP Fri Dec 2 03:43:27 UTC 2016 kernel log shows: Dec 13 21:31:07 merits kernel: [ 5815.576016] usb 2-9: new full-speed USB device number 4 using ohci-pci Dec 13 21:31:07 merits kernel: [ 5815.812966] usb 2-9: New USB device found, idVendor=0421, idProduct=0550 Dec 13 21:31:07 merits kernel: [ 5815.812969] usb 2-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Dec 13 21:31:07 merits kernel: [ 5815.812971] usb 2-9: Product: Nokia C2-01 Dec 13 21:31:07 merits kernel: [ 5815.812973] usb 2-9: Manufacturer: Nokia Dec 13 21:31:07 merits kernel: [ 5815.812974] usb 2-9: SerialNumber: 359309040628341 Dec 13 21:31:13 merits gvfs-gphoto2-vo[1438]: device (null) has no BUSNUM property, ignoring Dec 13 21:31:16 merits gnome-shell[1383]: clutter_layout_manager_get_child_meta: assertion 'CLUTTER_IS_LAYOUT_MANAGER (manager)' failed Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:16 merits gnome-shell[1383]: clutter_layout_manager_get_child_meta: assertion 'CLUTTER_IS_LAYOUT_MANAGER (manager)' failed Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:16 merits gnome-shell[1383]: clutter_layout_manager_get_child_meta: assertion 'CLUTTER_IS_LAYOUT_MANAGER (manager)' failed Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:25 merits kernel: [ 5833.374996] usb 2-9: USB disconnect, device number 4 Dec 13 21:31:25 merits kernel: [ 5834.128021] usb 2-9: new full-speed USB device number 5 using ohci-pci Dec 13 21:31:26 merits kernel: [ 5834.376908] usb 2-9: New USB device found, idVendor=0421, idProduct=054f Dec 13 21:31:26 merits kernel: [ 5834.376911] usb 2-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Dec 13 21:31:26 merits kernel: [ 5834.376913] usb 2-9: Product: Nokia C2-01 Dec 13 21:31:26 merits kernel: [ 5834.376915] usb 2-9: Manufacturer: Nokia Dec 13 21:31:26 merits kernel: [ 5834.376916] usb 2-9: SerialNumber: 359309040628341 Dec 13 21:31:16 merits gnome-shell[1383]: g_object_set: assertion 'G_IS_OBJECT (object)' failed Dec 13 21:31:26 merits kernel: [ 5834.519454] usb-storage 2-9:1.0: USB Mass Storage device detected Dec 13 21:31:26 merits kernel: [ 5834.519760] scsi host6: usb-storage 2-9:1.0 Dec 13 21:31:26 merits kernel: [ 5834.519991] usbcore: registered new interface driver usb-storage Dec 13 21:31:26 merits kernel: [ 5834.560598] usbcore: registered new interface driver uas Dec 13 21:31:27 merits kernel: [ 5835.525874] scsi 6:0:0:0: Direct-Access NokiaNokia C2-01 PQ: 0 ANSI: 4 Dec 13 21:31:27 merits kernel: [ 5835.530263] sd 6:0:0:0: Attached scsi generic sg2 type 0 Dec 13 21:31:27 merits kernel: [ 5835.536860] sd 6:0:0:0: [sdb] Adjusting the sector count from its reported value: 7744513 Dec 13 21:31:27 merits kernel: [ 5835.536865] sd 6:0:0:0: [sdb] 7744512 512-byte logical blocks: (3.97 GB/3.69 GiB) Dec 13 21:31:27 merits kernel: [ 5835.540848] sd 6:0:0:0: [sdb] Write Protect is off Dec 13 21:31:27 merits kernel: [ 5835.540851] sd 6:0:0:0: [sdb] Mode Sense: 04 00 00 00 Dec 13 21:31:27 merits kernel: [ 5835.544858] sd 6:0:0:0: [sdb] No Caching mode page found Dec 13 21:31:27 merits kernel: [ 5835.544863] sd 6:0:0:0: [sdb] Assuming drive cache: write through Dec 13 21:31:27 merits kernel: [ 5835.558854] sd 6:0:0:0: [sdb] Adjusting the sector count from its reported value: 7744513 Dec 13 21:31:27 merits kernel: [ 5835.574872] sdb: sdb1 Dec 13 21:31:27 merits kernel: [ 5835.582847] sd 6:0:0:0: [sdb] Adjusting the sector count from its reported value: 7744513 Dec 13 21:31:27 merits kernel: [ 5835.593015] sd 6:0:0:0: [sdb] Attached SCSI removable disk Dec 13 21:31:27 merits kernel: [ 5835.972008] ohci-pci :00:04.0: HcDoneHead not written back; disabled Dec 13 21:31:27 merits kernel: [ 5835.972014] ohci-pci :00:04.0: HC died; cleaning up Dec 13 21:31:27 merits kernel: [ 5835.972335] usb 2-2: USB disconnect, device number 2 Dec 13 21:31:27 merits kernel: [ 5835.973969] usb 2-4: USB disconnect, device number 3 Dec 13 21:31:27 merits kernel: [ 5836.012277] usb 2-9: USB disconnect, device number 5 Dec 13 21:31:27 merits kernel: [ 5836.024023] scsi 6:0:0:0: rejecting I/O to offline device Dec 13 21:31:27 merits kernel: [ 5836.024029] scsi 6:0:0:0: [sdb] killing request Dec 13 21:31:27 merits kernel: [ 5836.024042] scsi 6:0:0:0: [sdb] FAILED Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK Dec 13 21:31:27 merits kernel: [ 5836.024046] scsi 6:0:0:0: [sdb] CDB: Read(10) 28 00 00 00 00 80 00 00 90 00 Dec 13 21:31:27 merits kernel: [ 5836.024048] blk_update_request: I/O error, de
Re: [RFC][PATCH 5/5] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220
On Tue, Dec 13, 2016 at 5:24 AM, Alan Stern wrote: > On Mon, 12 Dec 2016, John Stultz wrote: > >> From: Chen Yu >> >> The Hi6220's usb controller is limited in that it does not >> support "Split Transactions", so it does not support communicating >> with low-speed and full-speed devices behind a high-speed hub. >> >> Thus it requires a quirk so that we can manually drop the usb >> speed when low/full-speed are attached, and bump back to high >> speed when they are removed. > > Just out of curiosity (I know nothing about this hardware), what If your interested in details, page 12 of the pdf here has some details: https://github.com/96boards/documentation/blob/master/ConsumerEdition/HiKey/AdditionalDocs/HiKey_Hardware_User_Manual_Rev0.2.pdf There's also schematics for the board available (if you are interested in that sort of stuff). You can find the USB bits on Page 4 here: https://github.com/96boards/documentation/blob/master/ConsumerEdition/HiKey/HardwareDocs/HiKey_schematics_LeMaker_version_Rev_A1.pdf > happens if there is a high-speed hub plugged into the host controller > and both a high-speed and a full-speed device plugged into the hub? > > Do you end up forcing the high-speed device to run at full speed? Yes. It drops back to full-speed. thanks -john -- 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 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller
On Fri, Dec 09, 2016 at 05:06:39PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB device controller.
>
> Signed-off-by: Sergio Prado
> ---
> .../devicetree/bindings/usb/s3c2410-usb.txt| 28
> ++
> 1 file changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> index e45b38ce2986..4d3f9894c2d4 100644
> --- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> @@ -20,3 +20,31 @@ usb0: ohci@4900 {
> clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
> clock-names = "usb-bus-host", "usb-host";
> };
> +
> +Samsung S3C2410 and compatible USB device controller
> +
> +Required properties:
> + - compatible: Should be one of the following
> +"samsung,s3c2410-udc"
> +"samsung,s3c2440-udc"
> + - reg: address and length of the controller memory mapped region
> + - interrupts: interrupt number for the USB device controller
> + - clocks: Should reference the bus and host clocks
> + - clock-names: Should contain two strings
> + "usb-bus-gadget" for the USB bus clock
Pretty sure the h/w clock name in the datasheet does not use the Linux
term gadget.
> + "usb-device" for the USB device clock
> +
> +Optional properties:
> + - samsung,vbus-gpio: If present, specifies a gpio that needs to be
> + activated for the bus to be powered.
Isn't it the host side that controls Vbus?
> + - samsung,pullup-gpio: If present, specifies a gpio to control the
Both GPIOs need to specify the active state.
> + USB D+ pullup.
> +
> +usb1: udc@5200 {
> + compatible = "samsung,s3c2440-udc";
> + reg = <0x5200 0x10>;
> + interrupts = <0 0 25 3>;
> + clocks = <&clocks UCLK>, <&clocks HCLK_USBD>;
> + clock-names = "usb-bus-gadget", "usb-device";
> + samsung,pullup-gpio = <&gpc 5 GPIO_ACTIVE_HIGH>;
> +};
> --
> 1.9.1
>
--
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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Tue, Dec 13, 2016 at 7:38 PM, Alan Stern wrote: > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > >> On Tue, Dec 13, 2016 at 4:52 PM, Alan Stern >> wrote: >> > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: >> > >> >> >> > If it is >> >> >> > not a bug in kernel source code, then it must not produce a WARNING. >> >> > >> >> > What about a memory allocation failure? The memory management part of >> >> > the kernel produces a WARNING message if an allocation fails and the >> >> > caller did not specify __GFP_NOWARN. >> >> > >> >> > There is no way for a driver to guarantee that a memory allocation >> >> > request will succeed -- failure is always an option. But obviously >> >> > memory allocation failures are not bugs in the kernel. >> >> > >> >> > Are you saying that mm/page_alloc.c:warn_alloc() should produce >> >> > something other than a WARNING? >> >> >> >> >> >> The main thing I am saying is that we absolutely need a way for a >> >> human or a computer program to be able to determine if there is >> >> anything wrong with kernel or not. >> > Doesn't it also produce a WARNING under other circumstances? >> >> No. >> >> OOM is not a WARNING and is easily distinguishable from BUG/WARNING. > >> Memory allocator does not print WARNINGs on allocation failures. > > Do you count dev_warn the same as WARN or WARN_ON? What about dev_WARN > or pr_warn() or printk(KERN_WARNING...)? Maybe we're not talking about > the same messages. > > The USB subsystem has got tons of dev_warn() and dev_err() calls. > Relatively few (if any) of them are for kernel bugs. I grep for "WARNING:". It is not possible to understand what function printed messages on console. Here are my current regexps: https://github.com/google/syzkaller/blob/master/report/report.go#L29 -- 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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > On Tue, Dec 13, 2016 at 4:52 PM, Alan Stern wrote: > > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > > > >> >> > If it is > >> >> > not a bug in kernel source code, then it must not produce a WARNING. > >> > > >> > What about a memory allocation failure? The memory management part of > >> > the kernel produces a WARNING message if an allocation fails and the > >> > caller did not specify __GFP_NOWARN. > >> > > >> > There is no way for a driver to guarantee that a memory allocation > >> > request will succeed -- failure is always an option. But obviously > >> > memory allocation failures are not bugs in the kernel. > >> > > >> > Are you saying that mm/page_alloc.c:warn_alloc() should produce > >> > something other than a WARNING? > >> > >> > >> The main thing I am saying is that we absolutely need a way for a > >> human or a computer program to be able to determine if there is > >> anything wrong with kernel or not. > > Doesn't it also produce a WARNING under other circumstances? > > No. > > OOM is not a WARNING and is easily distinguishable from BUG/WARNING. > Memory allocator does not print WARNINGs on allocation failures. Do you count dev_warn the same as WARN or WARN_ON? What about dev_WARN or pr_warn() or printk(KERN_WARNING...)? Maybe we're not talking about the same messages. The USB subsystem has got tons of dev_warn() and dev_err() calls. Relatively few (if any) of them are for kernel bugs. Alan Stern -- 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: Fwd: usb cellphone storage device disconnection causes my usb mouse to be disconnected
On Tue, Dec 13, 2016 at 08:20:56PM +0200, or dv wrote: > I have a nokia C2 cellphone. when I connect it with a micro usb cable, > and then try to move a file to the memory card that resides in my cellphone, > in the middle of the file transfer the device disconnects, > and then I can't move my mouse, > and the device can't be mounted anymore until reboot... > and so do the mouse can't be detected anymore until reboot > > the bug is reproducible... What kernel version is this? Also, is there any error messages in the kernel log when this happens? thanks, greg k-h -- 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
Fwd: usb cellphone storage device disconnection causes my usb mouse to be disconnected
I have a nokia C2 cellphone. when I connect it with a micro usb cable, and then try to move a file to the memory card that resides in my cellphone, in the middle of the file transfer the device disconnects, and then I can't move my mouse, and the device can't be mounted anymore until reboot... and so do the mouse can't be detected anymore until reboot the bug is reproducible... -- 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
[GIT PULL] USB/PHY driver changes patches for 4.10-rc1
The following changes since commit e5517c2a5a49ed5e99047008629f1cd60246ea0e: Linux 4.9-rc7 (2016-11-27 13:08:04 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.10-rc1 for you to fetch changes up to d8a12b7117b42fd708f1e908498350232bdbd5ff: USB: serial: option: add dlink dwm-158 (2016-12-09 11:44:11 +0100) USB/PHY patches for 4.10-rc1 Here's the big set of USB/PHY patches for 4.10-rc1. A number of new drivers are here in this set of changes. We have a new USB controller type "mtu3", a new usb-serial driver, and the usual churn in the gadget subsystem and the xhci host controller driver, along with a few other new small drivers added. And lots of little other changes all over the USB and PHY driver tree. Full details are in the shortlog All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Aidan Thornton (4): USB: serial: ch341: add register and USB request definitions USB: serial: ch341: reinitialize chip on reconfiguration USB: serial: ch341: add support for parity, frame length, stop bits USB: serial: ch341: add debug output for chip version Alexandre Bailon (3): phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround usb: musb: da8xx: Call earlier clk_prepare_enable() usb: musb: da8xx: Set phy in OTG mode by default Alexey Khoroshilov (2): usb: gadget: mv_u3d: add check for dma mapping error usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring Amitesh Singh (1): usb: fix a typo in usb_class_driver documentation Anson Jacob (1): usb: gadget: Fix checkpatch error for braces Arnd Bergmann (5): usb: dwc3: ep0: avoid empty-body warning usb: dwc3: use bus->sysdev for DMA configuration usb: dwc3: Do not set dma coherent mask phy: rockchip-inno-usb2: fix uninitialized tmout variable phy: rockchip-inno-usb2: select USB_COMMON Axel Haslam (7): usb: ohci-da8xx: rename driver to ohci-da8xx USB: ohci: da8xx: use ohci priv data instead of globals USB: ohci: da8xx: Add wrappers for platform callbacks USB: ohci: da8xx: Allow a regulator to handle VBUS USB: ohci: da8xx: Add devicetree bindings USB: ohci: da8xx: Allow probing from DT USB: ohci: da8xx: Resume the entire host controller Baolin Wang (2): usb: dwc3: gadget: don't clear RUN/STOP when it's invalid to do so usb: dwc3: gadget: wait for End Transfer to complete Changming Huang (1): fsl/usb: Workarourd for USB erratum-A005697 Christian Lamparter (2): usb: dwc2: fixes host_dma logic usb: dwc2: add amcc,dwc-otg support Chunfeng Yun (10): dt-bindings: mt8173-xhci: support host side of dual-role mode dt-bindings: mt8173-mtu3: add devicetree bindings usb: xhci-mtk: make IPPC register optional usb: Add MediaTek USB3 DRD driver usb: mtu3: Super-Speed Peripheral mode support usb: mtu3: host only mode support usb: mtu3: dual-role mode support arm64: dts: mediatek: add USB3 DRD driver usb: mtu3: enable auto switch from U3 to U2 usb: mtu3: fix U3 port link issue Dan Carpenter (1): usb: xhci-mem: use passed in GFP flags instead of GFP_KERNEL Daniel Wagner (1): usb: gadget: f_fs: use complete() instead complete_all() Daniele Palmas (1): USB: serial: option: add support for Telit LE922A PIDs 0x1040, 0x1041 Dave Penkler (1): usbtmc: Add, clarify and fix comments David Lechner (1): usb: ohci-da8xx: Remove code that references mach Fabian Frederick (1): usb: isp1301-omap: don't opencode IS_REACHABLE() Fabien Lahoudere (2): usb: chipidea: imx: configure imx for ULPI phy usb: chipidea: imx: Disable internal 60Mhz clock with ULPI PHY Felipe Balbi (74): usb: add helper to extract bits 12:11 of wMaxPacketSize usb: gadget: composite: correctly initialize ep->maxpacket usb: gadget: composite: always set ep->mult to a sensible value usb: dwc3: gadget: set PCM1 field of isochronous-first TRBs usb: dwc3: gadget: properly check ep cmd usb: dwc3: gadget: make use of No Response Update Transfer usb: dwc3: gadget: giveback request if we can't kick it usb: dwc3: gadget: conditionally disable Link State change events usb: dwc3: debug: decode control endpoint phase too usb: dwc3: gadget: remove redundant trace prints usb: dwc3: debug: move dwc3_ep0_state_string() to debug.h usb: dwc3: trace: print out ep0state also from XferComplete media: usbtv: core: make use of new usb_endpoint_maxp_mult() media: usb: uvc: make use of new usb_endpoint_maxp_mult() usb: chipidea: udc: make use of new usb_endpoint_maxp_mult() usb: core: devices: make use
Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs
On Tue, Dec 13, 2016 at 8:45 AM, David Howells wrote: > Andy Lutomirski 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
Andy Lutomirski writes: > On Tue, Dec 13, 2016 at 3:35 AM, Kalle Valo wrote: >> Andy Lutomirski 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 >>> Signed-off-by: Andy Lutomirski >> >> "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. Makes sense now, thanks. I'll add that to the commit log and queue this to 4.10. -- Kalle Valo -- 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
Andy Lutomirski 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. David -- 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
On Tue, Dec 13, 2016 at 3:35 AM, Kalle Valo wrote: > Andy Lutomirski 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 >> Signed-off-by: Andy Lutomirski > > "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
[add some people who might know] On Tue, Dec 13, 2016 at 4:20 AM, David Laight 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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Tue, Dec 13, 2016 at 4:52 PM, Alan Stern wrote: > On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > >> >> > If it is >> >> > not a bug in kernel source code, then it must not produce a WARNING. >> > >> > What about a memory allocation failure? The memory management part of >> > the kernel produces a WARNING message if an allocation fails and the >> > caller did not specify __GFP_NOWARN. >> > >> > There is no way for a driver to guarantee that a memory allocation >> > request will succeed -- failure is always an option. But obviously >> > memory allocation failures are not bugs in the kernel. >> > >> > Are you saying that mm/page_alloc.c:warn_alloc() should produce >> > something other than a WARNING? >> >> >> The main thing I am saying is that we absolutely need a way for a >> human or a computer program to be able to determine if there is >> anything wrong with kernel or not. > > Okay, I agree with that. I'm not at all sure that this decision should > be based on whether a message is a WARNING. > > In my experience, there is no general consensus on what conditions > should qualify as a WARNING. There aren't any hard-and-fast rules, so > people are forced to rely on their own judgment. The general > interpretation seems to be that WARNING is appropriate for notifying > the user whenever something important has gone unexpectedly wrong. > That covers a lot of ground, including things (like hardware failures) > that are not kernel bugs. see below >> Page_alloc produces a WARNING iff you ask for an amount of memory that >> can't possibly be allocated under any circumstances (order >= >> MAX_ORDER). > > Doesn't it also produce a WARNING under other circumstances? No. OOM is not a WARNING and is easily distinguishable from BUG/WARNING. >> That's not just an allocation failure. Kernel itself >> should not generally ask for such large amounts of memory. If the >> allocation size is dictated by user, then kernel code should either >> use __GFP_NOWARN, or impose own stricter limit dictated by context >> (e.g. if it's a text command of known format, then limit can be as >> small as say 128 bytes). > > All right, yes, it makes sense to use __GFP_NOWARN when the allocation > size is dictated by the user. > > But still, even when a stricter limit has been imposed, a memory > allocation request may fail. In fact, as far as I know, the kernel has > _no_ guarantee at all that any memory allocation request will succeed! > The best it offers is that sufficiently small requests may wait > indefinitely for memory to become available, which isn't much better > than failing. Memory allocator does not print WARNINGs on allocation failures. >> Re fake hardware. panic_on_warn will definitely cause problems. I >> don't know if it used in any paranoid production systems or not, >> though. But more generally, I don't see how it is different from >> incorrect syscall arguments or nonsensical network packets received >> from free internet. In ideal productions environments none of these >> incorrect inputs to kernel should happen. I don't see any single >> reason to not protect kernel from incorrect input in this case as >> well, as we do in all other cases. > > I _do_ see a reason. Real computers don't live in ideal production > environments. Why go to extra trouble to add protection for some > particular incorrect input when the kernel is _already_ capable of > handling this input in a valid manner (even though this may include > logging a WARNING message)? One reason is to be able to distinguish kernel bugs from incorrect inputs to kernel. also see below >> In particular, it would resolve a >> very real and practical issue for us -- fuzzer will not reboot machine >> every minute, and we will not spend time looking at these WARNINGs, >> and we will not spend your time by reporting these WARNINGs. > > Maybe you should decide that ERROR messages indicate a kernel bug, > rather than WARNING messages. Even that is questionable, but you will > get far fewer false positives. > > Even better, you should publicize this decision (in the kernel > documentation, on various mailing lists, on LWN, and so forth), and > enforce it by reducing existing ERROR severity levels to WARNINGs in > places where they do not indicate a kernel bug. OK, I have some numbers on my hands. To date we've run about 1e12 random programs covering 1000+ syscalls and have found 300+ bugs. 50+ of these bugs are found on WARNINGs (search by "warning" here https://github.com/google/syzkaller/wiki/Found-Bugs). Some of the bugs found on WARNINGs are as severe as VMM exploitations. With that coverage the _only_ case I know of now where a WARNING does not mean bug in kernel source code is basically the one we are arguing about now. Provided that total number of WARN/WARN_ON in kernel code is ~1. There is currently a very-very-very-very-very clear WARNING usage practice that suggests that WARNING is-a bug in kernel code. If we stop treating WARNINGs as bu
Re: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Tue, 13 Dec 2016, Dmitry Vyukov wrote: > >> > If it is > >> > not a bug in kernel source code, then it must not produce a WARNING. > > > > What about a memory allocation failure? The memory management part of > > the kernel produces a WARNING message if an allocation fails and the > > caller did not specify __GFP_NOWARN. > > > > There is no way for a driver to guarantee that a memory allocation > > request will succeed -- failure is always an option. But obviously > > memory allocation failures are not bugs in the kernel. > > > > Are you saying that mm/page_alloc.c:warn_alloc() should produce > > something other than a WARNING? > > > The main thing I am saying is that we absolutely need a way for a > human or a computer program to be able to determine if there is > anything wrong with kernel or not. Okay, I agree with that. I'm not at all sure that this decision should be based on whether a message is a WARNING. In my experience, there is no general consensus on what conditions should qualify as a WARNING. There aren't any hard-and-fast rules, so people are forced to rely on their own judgment. The general interpretation seems to be that WARNING is appropriate for notifying the user whenever something important has gone unexpectedly wrong. That covers a lot of ground, including things (like hardware failures) that are not kernel bugs. > Page_alloc produces a WARNING iff you ask for an amount of memory that > can't possibly be allocated under any circumstances (order >= > MAX_ORDER). Doesn't it also produce a WARNING under other circumstances? > That's not just an allocation failure. Kernel itself > should not generally ask for such large amounts of memory. If the > allocation size is dictated by user, then kernel code should either > use __GFP_NOWARN, or impose own stricter limit dictated by context > (e.g. if it's a text command of known format, then limit can be as > small as say 128 bytes). All right, yes, it makes sense to use __GFP_NOWARN when the allocation size is dictated by the user. But still, even when a stricter limit has been imposed, a memory allocation request may fail. In fact, as far as I know, the kernel has _no_ guarantee at all that any memory allocation request will succeed! The best it offers is that sufficiently small requests may wait indefinitely for memory to become available, which isn't much better than failing. > Re fake hardware. panic_on_warn will definitely cause problems. I > don't know if it used in any paranoid production systems or not, > though. But more generally, I don't see how it is different from > incorrect syscall arguments or nonsensical network packets received > from free internet. In ideal productions environments none of these > incorrect inputs to kernel should happen. I don't see any single > reason to not protect kernel from incorrect input in this case as > well, as we do in all other cases. I _do_ see a reason. Real computers don't live in ideal production environments. Why go to extra trouble to add protection for some particular incorrect input when the kernel is _already_ capable of handling this input in a valid manner (even though this may include logging a WARNING message)? > In particular, it would resolve a > very real and practical issue for us -- fuzzer will not reboot machine > every minute, and we will not spend time looking at these WARNINGs, > and we will not spend your time by reporting these WARNINGs. Maybe you should decide that ERROR messages indicate a kernel bug, rather than WARNING messages. Even that is questionable, but you will get far fewer false positives. Even better, you should publicize this decision (in the kernel documentation, on various mailing lists, on LWN, and so forth), and enforce it by reducing existing ERROR severity levels to WARNINGs in places where they do not indicate a kernel bug. Alan Stern -- 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: add URB_ZERO_PACKET in transfer_flags for usb-skeleton.c
On Tue, Dec 13, 2016 at 02:03:08PM +0900, Kim Jaejoong wrote: > Hi Oliver and Greg > > 2016-12-12 19:45 GMT+09:00 Oliver Neukum : > > On Thu, 2016-12-08 at 16:16 +0100, Greg KH wrote: > >> On Fri, Dec 09, 2016 at 12:09:46AM +0900, Jae Joong Kim wrote: > >> > > Do you have a driver that was based on this skeleton that needed this > >> > > flag to be added to work properly? Has it been updated in the kernel > >> > > source tree properly for it? > >> > > > >> > > >> > Yes, Based on the skeleton driver, I made a bulk driver with > >> > URB_ZERO_PACKET and > >> > it works fine both my notebook(ubuntu 15.04/kernel 3.19) and my target > >> > board(Embedded Linux/kernel 2.6) > >> > With the bulk driver, I am developing USB logging daemon. > >> > >> That's great. But, the biggest problem with the skeleton driver is > >> that it is not really needed anymore. Very rarely do we need to ever > >> write a new USB driver these days. Things like a "USB logging daemon" > >> using bulk endpoints can be easier to write in userspace using libusb, > >> and have it work on all operating systems. Kernel USB drivers are a > >> rare thing for brand new types of devices anymore, which is nice. > > My target board(kernel 2.6) does not support libusb because there is > no USB_DEVICEFS config (long long ago kernel USB config). > That's why I made a bulk driver based on skeleton driver for USB logging > daemon. usbfs has been around since the 2.4 kernel days, you have support for it (might be called usbdevfs or something else like that, I can't remember decades old code, sorry...) So you still can do this just fine for your 2.6 kernel from userspace, using libusb. I recommend it, as then it will "just work" when you move to a kernel that is actually supported and maintained and has been released in the past decade :) good luck! greg k-h -- 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 1/1] Increase USB transfer limit
> Il giorno 13 dic 2016, alle ore 05:22, Alan Stern
> ha scritto:
>
>> On Mon, 12 Dec 2016, Mateusz Berezecki wrote:
>>
>> Promote a variable keeping track of USB transfer memory usage to a
>> wider data type and allow for higher bandwidth transfers from a large
>> number of USB devices connected to a single host.
>> ---
>> drivers/usb/core/devio.c | 34 --
>> 1 file changed, 12 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
>> index 4016dae..bead975 100644
>> --- a/drivers/usb/core/devio.c
>> +++ b/drivers/usb/core/devio.c
>> @@ -134,42 +134,32 @@ enum snoop_when {
>> #define USB_DEVICE_DEVMKDEV(USB_DEVICE_MAJOR, 0)
>>
>> /* Limit on the total amount of memory we can allocate for transfers */
>> -static unsigned usbfs_memory_mb = 16;
>> +static u32 usbfs_memory_mb = 16;
>> module_param(usbfs_memory_mb, uint, 0644);
>> MODULE_PARM_DESC(usbfs_memory_mb,
>>"maximum MB allowed for usbfs buffers (0 = no limit)");
>>
>> -/* Hard limit, necessary to avoid arithmetic overflow */
>> -#define USBFS_XFER_MAX(UINT_MAX / 2 - 100)
>> -
>> -static atomic_t usbfs_memory_usage;/* Total memory currently allocated
>> */
>> +static atomic64_t usbfs_memory_usage;/* Total memory currently
>> allocated */
>>
>> /* Check whether it's okay to allocate more memory for a transfer */
>> -static int usbfs_increase_memory_usage(unsigned amount)
>> +static int usbfs_increase_memory_usage(u64 amount)
>> {
>> -unsigned lim;
>> +u64 lim;
>>
>> -/*
>> - * Convert usbfs_memory_mb to bytes, avoiding overflows.
>> - * 0 means use the hard limit (effectively unlimited).
>> - */
>>lim = ACCESS_ONCE(usbfs_memory_mb);
>> -if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
>> -lim = USBFS_XFER_MAX;
>> -else
>> -lim <<= 20;
>> +lim <<= 20;
>
> You need to handle the case where lim == 0 (see the MODULE_PARM_DESC
> text).
Good catch.
>
>>
>> -atomic_add(amount, &usbfs_memory_usage);
>> -if (atomic_read(&usbfs_memory_usage) <= lim)
>> +atomic64_add(amount, &usbfs_memory_usage);
>> +if (atomic64_read(&usbfs_memory_usage) <= lim)
>>return 0;
>> -atomic_sub(amount, &usbfs_memory_usage);
>> +atomic64_sub(amount, &usbfs_memory_usage);
>>return -ENOMEM;
>> }
>>
>> /* Memory for a transfer is being deallocated */
>> -static void usbfs_decrease_memory_usage(unsigned amount)
>> +static void usbfs_decrease_memory_usage(u64 amount)
>> {
>> -atomic_sub(amount, &usbfs_memory_usage);
>> +atomic64_sub(amount, &usbfs_memory_usage);
>> }
>>
>> static int connected(struct usb_dev_state *ps)
>> @@ -1191,7 +1181,7 @@ static int proc_bulk(struct usb_dev_state *ps, void
>> __user *arg)
>>if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
>>return -EINVAL;
>>len1 = bulk.len;
>> -if (len1 >= USBFS_XFER_MAX)
>> +if (len1 >= (INT_MAX - sizeof(struct urb)))
>>return -EINVAL;
>>ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
>>if (ret)
>> @@ -1584,7 +1574,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
>> struct usbdevfs_urb *uurb
>>return -EINVAL;
>>}
>>
>> -if (uurb->buffer_length >= USBFS_XFER_MAX) {
>> +if (uurb->buffer_length >= (INT_MAX - sizeof(struct urb))) {
>>ret = -EINVAL;
>>goto error;
>>}
>
> This isn't right. See how the u variable gets used in this routine.
> You have to guarantee that the computation of u won't overflow.
Ok. Let me revise, test and resubmit in a few hours.
Thanks for the feedback and reviewing!
Mateusz
--
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: usb/core: warning in usb_create_ep_devs/sysfs_create_dir_ns
On Mon, Dec 12, 2016 at 11:04 PM, Alan Stern wrote: > On Mon, 12 Dec 2016, Alan Stern wrote: > >> On Mon, 12 Dec 2016, Dmitry Vyukov wrote: >> >> > On Mon, Dec 12, 2016 at 10:05 PM, Alan Stern >> > wrote: >> > > On Mon, 12 Dec 2016, Andrey Konovalov wrote: >> > > >> > >> Hi! >> > >> >> > >> While running the syzkaller fuzzer I've got the following error report. >> > >> >> > >> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dev 2). >> > >> >> > >> WARNING: CPU: 2 PID: 865 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x8a/0xa0 >> > >> gadgetfs: disconnected >> > >> sysfs: cannot create duplicate filename >> > >> '/devices/platform/dummy_hcd.0/usb2/2-1/2-1:64.0/ep_05' >> > >> Kernel panic - not syncing: panic_on_warn set ... >> > > >> > > I suppose we could check for USB devices that claim to have two >> > > endpoints with the same address. But is it really worthwhile? A >> > > kernel warning isn't so bad when you're dealing with buggy device >> > > firmware. >> > >> > We need a clear distinction between what is a bug in kernel source >> > code and what is incorrect user-space code. Otherwise no automated >> > testing is possible. WARNING means bug in kernel source code. >> >> I don't necessarily agree with that. Is it documented anywhere? >> >> > If it is >> > not a bug in kernel source code, then it must not produce a WARNING. > > What about a memory allocation failure? The memory management part of > the kernel produces a WARNING message if an allocation fails and the > caller did not specify __GFP_NOWARN. > > There is no way for a driver to guarantee that a memory allocation > request will succeed -- failure is always an option. But obviously > memory allocation failures are not bugs in the kernel. > > Are you saying that mm/page_alloc.c:warn_alloc() should produce > something other than a WARNING? The main thing I am saying is that we absolutely need a way for a human or a computer program to be able to determine if there is anything wrong with kernel or not. Page_alloc produces a WARNING iff you ask for an amount of memory that can't possibly be allocated under any circumstances (order >= MAX_ORDER). That's not just an allocation failure. Kernel itself should not generally ask for such large amounts of memory. If the allocation size is dictated by user, then kernel code should either use __GFP_NOWARN, or impose own stricter limit dictated by context (e.g. if it's a text command of known format, then limit can be as small as say 128 bytes). Re fake hardware. panic_on_warn will definitely cause problems. I don't know if it used in any paranoid production systems or not, though. But more generally, I don't see how it is different from incorrect syscall arguments or nonsensical network packets received from free internet. In ideal productions environments none of these incorrect inputs to kernel should happen. I don't see any single reason to not protect kernel from incorrect input in this case as well, as we do in all other cases. In particular, it would resolve a very real and practical issue for us -- fuzzer will not reboot machine every minute, and we will not spend time looking at these WARNINGs, and we will not spend your time by reporting these WARNINGs. -- 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: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
On Tue, Dec 13, 2016 at 2:34 PM, Hans Verkuil wrote: > Try again, this time with Krzysztof's new email address... > > > On 13/12/16 13:20, Hans Verkuil wrote: >> >> Hi Krzysztof, >> >> This still seems to be broken with the latest 4.9 kernel, right? >> >> Has there been any progress on this? Do you have an updated patch series >> for me to use? Hi, I think it is not fixed. Still. I left this work to others. AFAIK, Peter Chen is working on a new generic approach: https://lwn.net/Articles/703556/ On top of his patchset, Odroid would need some DTS code as well (and maybe something in usb3503). However I do not plan to work on this anymore as I do not have Odroid U3 anymore. Marek and Bartlomiej from Samsung Poland are in CC-list so maybe they would like to continue the work? Best regards, Krzysztof -- 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: usb/gadget: warning in ep_write_iter/__alloc_pages_nodemask
On Tue, 13 Dec 2016, Michal Hocko wrote: > > > That being said, what ep_write_iter does sounds quite stupit. It just > > > allocates a large continuous buffer which seems to be under user > > > control... Aka no good! It should do that per pages or something like > > > that. Something worth fixing > > > > It's not important enough to make the driver do all this work. If > > users want to send large amounts of data, they can send it a page at a > > time (or something like that). > > Is it really necessary to allocate the full iov_iter_count? Why cannot > we process the from buffer one page at a time? We could (although one page is really too small -- USB 3.1 can transfer 800 KB per ms so we ought to handle at least 128 KB at a time). But turn the argument around: If the user wants to transfer that much data, why can't he _submit_ it one page at a time? > > If you really want to prevent the driver from attempting to allocate a > > large buffer, all that's needed is an upper limit on the total size. > > For example, 64 KB. > > Well, my point was that it is not really hard to imagine to deplete > larger contiguous memory blocks (say PAGE_ALLOC_COSTLY_ORDER). Those are > still causing the OOM killer and chances are that a controlled flood of > these requests could completely DoS the system. Putting a limit on the total size of a single transfer would prevent this. Alan Stern -- 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: FTD driverI issue?
Please do not drop the linux-usb list from CC. Resending my reply to the list and dropping that (private) sourceforge list you added from CC. On Tue, Dec 13, 2016 at 01:47:45PM +0100, Sylvain LG wrote: > Hello Johan, > > I am using the 4.9 kernel provided by Robert C Nelson, from > Beagleboard > > https://github.com/beagleboard/linux/tree/4.9 Please verify that you have the same issue when using a mainline 4.9-kernel, so that this is not just a problem with that tree. > I'm using FTDI products from StarTech.com: > > ICUSB2321F > https://www.startech.com/Cards-Adapters/Serial-Cards-Adapters/1-Port-USB-Serial-RS232-Adapter-Cable~ICUSB2321F That should be an FT232RL. > ICUSB2322F > https://www.startech.com/Cards-Adapters/Serial-Cards-Adapters/2-Port-USB-RS232-Serial-Cable~ICUSB2322F And an FT2232D. > and also an USB Cape which provides 2 USB ports and 2 FTDI serial ports: > > USB-2COM-BB > http://www.titan.tw/product/USB_2COM_BB.html I assume that cape is not supported by a mainline kernel, so please use the on-board BBB host port. > Currently, I could perform tests with StarTech 2 ports ICUSB2322F and > BBB cape USB-2COM-BB. > (But results are the same with all the other stuff). What happens when you use the on-board BBB host port instead of the cape? > lsusb -v returns: > > Bus 001 Device 001: ID 1d6b:0002 > Bus 001 Device 002: ID 05e3:0610 > Bus 001 Device 005: ID 0403:6010 > Bus 001 Device 004: ID 0403:6010 That's not the verbose output (-v) of the (libusb) lsusb. But never mind for now, this sounds like a BBB issue. > After applying your diagnostics patch to ftdio_sio.c, kernel messages are: > > [ 421.937699] ftdi_sio ttyUSB3: ftdi_set_termios FAILED to set > databits/stopbits/parity: -110 So that's an -ETIMEDOUT when no response to this request is received within 1000 ms when the port is opened. Do you see any other error messages when connecting the device(s)? There are a couple of control requests that are sent as part of probe, which could also be failing. > [ 422.961642] ftdi_sio ttyUSB3: ftdi_set_termios urb failed to set baudrate > [ 428.081820] ftdi_sio ttyUSB3: urb failed to clear flow control > > > Thank you for your help, You're welcome. Johan -- 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: [RFC][PATCH 5/5] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220
On Mon, 12 Dec 2016, John Stultz wrote: > From: Chen Yu > > The Hi6220's usb controller is limited in that it does not > support "Split Transactions", so it does not support communicating > with low-speed and full-speed devices behind a high-speed hub. > > Thus it requires a quirk so that we can manually drop the usb > speed when low/full-speed are attached, and bump back to high > speed when they are removed. Just out of curiosity (I know nothing about this hardware), what happens if there is a high-speed hub plugged into the host controller and both a high-speed and a full-speed device plugged into the hub? Do you end up forcing the high-speed device to run at full speed? Alan Stern -- 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 1/1] Increase USB transfer limit
On Mon, 12 Dec 2016, Mateusz Berezecki wrote:
> Promote a variable keeping track of USB transfer memory usage to a
> wider data type and allow for higher bandwidth transfers from a large
> number of USB devices connected to a single host.
> ---
> drivers/usb/core/devio.c | 34 --
> 1 file changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 4016dae..bead975 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -134,42 +134,32 @@ enum snoop_when {
> #define USB_DEVICE_DEV MKDEV(USB_DEVICE_MAJOR, 0)
>
> /* Limit on the total amount of memory we can allocate for transfers */
> -static unsigned usbfs_memory_mb = 16;
> +static u32 usbfs_memory_mb = 16;
> module_param(usbfs_memory_mb, uint, 0644);
> MODULE_PARM_DESC(usbfs_memory_mb,
> "maximum MB allowed for usbfs buffers (0 = no limit)");
>
> -/* Hard limit, necessary to avoid arithmetic overflow */
> -#define USBFS_XFER_MAX (UINT_MAX / 2 - 100)
> -
> -static atomic_t usbfs_memory_usage; /* Total memory currently allocated */
> +static atomic64_t usbfs_memory_usage;/* Total memory currently
> allocated */
>
> /* Check whether it's okay to allocate more memory for a transfer */
> -static int usbfs_increase_memory_usage(unsigned amount)
> +static int usbfs_increase_memory_usage(u64 amount)
> {
> - unsigned lim;
> + u64 lim;
>
> - /*
> - * Convert usbfs_memory_mb to bytes, avoiding overflows.
> - * 0 means use the hard limit (effectively unlimited).
> - */
> lim = ACCESS_ONCE(usbfs_memory_mb);
> - if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
> - lim = USBFS_XFER_MAX;
> - else
> - lim <<= 20;
> + lim <<= 20;
You need to handle the case where lim == 0 (see the MODULE_PARM_DESC
text).
>
> - atomic_add(amount, &usbfs_memory_usage);
> - if (atomic_read(&usbfs_memory_usage) <= lim)
> + atomic64_add(amount, &usbfs_memory_usage);
> + if (atomic64_read(&usbfs_memory_usage) <= lim)
> return 0;
> - atomic_sub(amount, &usbfs_memory_usage);
> + atomic64_sub(amount, &usbfs_memory_usage);
> return -ENOMEM;
> }
>
> /* Memory for a transfer is being deallocated */
> -static void usbfs_decrease_memory_usage(unsigned amount)
> +static void usbfs_decrease_memory_usage(u64 amount)
> {
> - atomic_sub(amount, &usbfs_memory_usage);
> + atomic64_sub(amount, &usbfs_memory_usage);
> }
>
> static int connected(struct usb_dev_state *ps)
> @@ -1191,7 +1181,7 @@ static int proc_bulk(struct usb_dev_state *ps, void
> __user *arg)
> if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
> return -EINVAL;
> len1 = bulk.len;
> - if (len1 >= USBFS_XFER_MAX)
> + if (len1 >= (INT_MAX - sizeof(struct urb)))
> return -EINVAL;
> ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
> if (ret)
> @@ -1584,7 +1574,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
> struct usbdevfs_urb *uurb
> return -EINVAL;
> }
>
> - if (uurb->buffer_length >= USBFS_XFER_MAX) {
> + if (uurb->buffer_length >= (INT_MAX - sizeof(struct urb))) {
> ret = -EINVAL;
> goto error;
> }
This isn't right. See how the u variable gets used in this routine.
You have to guarantee that the computation of u won't overflow.
Alan Stern
--
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] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack
On Mon, 2016-12-12 at 12:54 -0800, Andy Lutomirski wrote:
> 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
> Signed-off-by: Andy Lutomirski
> ---
>
> 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(&sgin, in, 8);
> - sg_init_one(&sgout, 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, &sgin, &sgout, 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
Acked-by: Jeff Layton
--
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] usb: host: xhci: refactor xhci_hub_control()
Hi,
kbuild test robot writes:
> Hi Felipe,
>
> [auto build test ERROR on usb/usb-testing]
> [also build test ERROR on v4.9 next-20161213]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Felipe-Balbi/usb-host-xhci-refactor-xhci_hub_control/20161213-192036
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> usb-testing
> config: blackfin-allmodconfig (attached as .config)
> compiler: bfin-uclinux-gcc (GCC) 6.2.0
> reproduce:
> wget
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=blackfin
>
> All errors (new ones prefixed by >>):
>
>drivers/usb/host/xhci-hub.c: In function
> 'xhci_ctrl_set_port_feature.isra.5':
>>> drivers/usb/host/xhci-hub.c:1159:1: error: unable to find a register to
>>> spill in class 'CCREGS'
> }
> ^
>>> drivers/usb/host/xhci-hub.c:1159:1: error: this is the insn:
>(insn 977 434 439 44 (set (reg:SI 0 R0 [433])
>(zero_extend:SI (reg:BI 16 I0 [599])))
> drivers/usb/host/xhci-hub.c:363 30 {zero_extendbisi2}
> (nil))
>drivers/usb/host/xhci-hub.c:1159: confused by earlier errors, bailing out
>
> vim +/CCREGS +1159 drivers/usb/host/xhci-hub.c
>
> 1153return -EPIPE;
> 1154}
> 1155/* unblock any posted writes */
> 1156temp = readl(port_array[wIndex]);
> 1157
> 1158return 0;
>> 1159 }
no idea what this means :-) It compiled just fine for x86 here. Any ideas?
--
balbi
signature.asc
Description: PGP signature
Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
Try again, this time with Krzysztof's new email address... On 13/12/16 13:20, Hans Verkuil wrote: Hi Krzysztof, This still seems to be broken with the latest 4.9 kernel, right? Has there been any progress on this? Do you have an updated patch series for me to use? Regards, Hans On 05/05/16 14:34, Krzysztof Kozlowski wrote: Hi, This is a different, second try to fix usb3503+lan on Odroid U3 board if it was initialized by bootloader (e.g. for TFTP boot). First version: http://www.spinics.net/lists/linux-usb/msg140042.html Problem === When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP), the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset is required, e.g. by suspend to RAM. The actual TFTP boot does not have to happen. Just "usb start" from U-Boot is sufficient. -- 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: [RFC][PATCH 2/5] usb: dwc2: Workaround case where GOTGCTL state is wrong
On 12/13/2016 11:12 AM, John Stultz wrote:
> When removing a USB-A to USB-otg adapter cable, we get a change
> status irq, and then in dwc2_conn_id_status_change, we
> erroniously see the GOTGCTL_CONID_B flag set. This causes us to
> get stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
> spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
> until it fails out many seconds later.
>
> This patch works around the issue by re-reading the GOTGCTL
> state to check if the GOTGCTL_CONID_B is still set and if not
> restarting the change status logic.
>
> I suspect this isn't the best solution, but it seems to work
> well for me.
>
> Feedback would be greatly appreciated!
>
> Cc: Wei Xu
> Cc: Guodong Xu
> Cc: Amit Pundir
> Cc: Rob Herring
> Cc: John Youn
> Cc: Douglas Anderson
> Cc: Chen Yu
> Cc: Kishon Vijay Abraham I
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Acked-by: John Youn
> Signed-off-by: John Stultz
> ---
> drivers/usb/dwc2/hcd.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index df5a065..a3f34dd 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3199,7 +3199,7 @@ static void dwc2_conn_id_status_change(struct
> work_struct *work)
> dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl);
> dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n",
> !!(gotgctl & GOTGCTL_CONID_B));
> -
> +again:
> /* B-Device connector (Device Mode) */
> if (gotgctl & GOTGCTL_CONID_B) {
> /* Wait for switch to device mode */
> @@ -3210,6 +3210,9 @@ static void dwc2_conn_id_status_change(struct
> work_struct *work)
>dwc2_is_host_mode(hsotg) ? "Host" :
>"Peripheral");
> usleep_range(2, 4);
> + gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
> + if (!(gotgctl & GOTGCTL_CONID_B))
> + goto again;
> if (++count > 250)
> break;
> }
>
Hi John Stultz,
When it goes to ":again", it will go to else anyways. I'll suggest
alternative way to do this. Please see below.
Also you can add "Reviewed-by: Vardan Mikayelyan "
Thanks,
Vardan.
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b7311a6..128311b 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3242,6 +3242,9 @@ static void dwc2_conn_id_status_change(struct
work_struct *work)
dwc2_is_host_mode(hsotg) ? "Host" :
"Peripheral");
usleep_range(2, 4);
+ gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+ if (!(gotgctl & GOTGCTL_CONID_B))
+ goto host;
if (++count > 250)
break;
}
@@ -3256,6 +3259,7 @@ static void dwc2_conn_id_status_change(struct
work_struct *work)
spin_unlock_irqrestore(&hsotg->lock, flags);
dwc2_hsotg_core_connect(hsotg);
} else {
+host:
/* A-Device connector (Host Mode) */
dev_dbg(hsotg->dev, "connId A\n");
while (!dwc2_is_host_mode(hsotg)) {
--
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: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
Hi Krzysztof, This still seems to be broken with the latest 4.9 kernel, right? Has there been any progress on this? Do you have an updated patch series for me to use? Regards, Hans On 05/05/16 14:34, Krzysztof Kozlowski wrote: Hi, This is a different, second try to fix usb3503+lan on Odroid U3 board if it was initialized by bootloader (e.g. for TFTP boot). First version: http://www.spinics.net/lists/linux-usb/msg140042.html Problem === When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP), the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset is required, e.g. by suspend to RAM. The actual TFTP boot does not have to happen. Just "usb start" from U-Boot is sufficient. -- 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
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. David -- 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] usb: host: xhci: refactor xhci_hub_control()
Hi Felipe,
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.9 next-20161213]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Felipe-Balbi/usb-host-xhci-refactor-xhci_hub_control/20161213-192036
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All errors (new ones prefixed by >>):
drivers/usb/host/xhci-hub.c: In function 'xhci_ctrl_set_port_feature.isra.5':
>> drivers/usb/host/xhci-hub.c:1159:1: error: unable to find a register to
>> spill in class 'CCREGS'
}
^
>> drivers/usb/host/xhci-hub.c:1159:1: error: this is the insn:
(insn 977 434 439 44 (set (reg:SI 0 R0 [433])
(zero_extend:SI (reg:BI 16 I0 [599])))
drivers/usb/host/xhci-hub.c:363 30 {zero_extendbisi2}
(nil))
drivers/usb/host/xhci-hub.c:1159: confused by earlier errors, bailing out
vim +/CCREGS +1159 drivers/usb/host/xhci-hub.c
1153 return -EPIPE;
1154 }
1155 /* unblock any posted writes */
1156 temp = readl(port_array[wIndex]);
1157
1158 return 0;
> 1159 }
1160
1161 static int xhci_ctrl_clear_port_feature(struct usb_hcd *hcd, u16 wValue,
1162 u16 wIndex, char *buf, u16 wLength, unsigned long flags)
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack
Hello! On 12/12/2016 11:54 PM, Andy Lutomirski wrote: smbencrypt() points a scatterlist to the stack, which is breaks if s/is//. 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 Signed-off-by: Andy Lutomirski [...] MBR, Sergei -- 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
Andy Lutomirski 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 > Signed-off-by: Andy Lutomirski "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? I'm just wondering should I push this to 4.10 or -next. This is a driver for ancient hardware so I'm starting to lean for -next. -- Kalle Valo -- 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] usb: host: xhci: refactor xhci_hub_control()
In order to make xhci_hub_control() easier to read,
let's break it into smaller functions. This will aid
maintainability and readability of the code. There
are no functional changes here, just shuffling code
around.
Signed-off-by: Felipe Balbi
---
drivers/usb/host/xhci-hub.c | 652
1 file changed, 363 insertions(+), 289 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef16900efed..96f83389f78e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -872,17 +872,97 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
return status;
}
-int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
+static int xhci_ctrl_get_hub_status(struct usb_hcd *hcd, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
+ /* No power source, over-current reported per port */
+ memset(buf, 0, 4);
+ return 0;
+}
+
+static int xhci_ctrl_get_hub_descriptor(struct usb_hcd *hcd, u16 wValue,
+ u16 wIndex, char *buf, u16 wLength)
+{
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+ /* Check to make sure userspace is asking for the USB 3.0 hub
+* descriptor for the USB 3.0 roothub. If not, we stall the
+* endpoint, like external hubs do.
+*/
+ if (hcd->speed >= HCD_USB3 &&
+ (wLength < USB_DT_SS_HUB_SIZE ||
+ wValue != (USB_DT_SS_HUB << 8))) {
+ xhci_dbg(xhci, "Wrong hub descriptor type for "
+ "USB 3.0 roothub.\n");
+ return -EPIPE;
+ }
+ xhci_hub_descriptor(hcd, xhci,
+ (struct usb_hub_descriptor *) buf);
+
+ return 0;
+}
+
+static int xhci_ctrl_get_port_status(struct usb_hcd *hcd, u16 wValue,
+ u16 wIndex, char *buf, u16 wLength, unsigned long flags)
+{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct xhci_bus_state *bus_state;
+
+ u32 status;
+ u32 temp;
+
int max_ports;
- unsigned long flags;
- u32 temp, status;
- int retval = 0;
+
__le32 __iomem **port_array;
- int slot_id;
+
+ max_ports = xhci_get_ports(hcd, &port_array);
+ bus_state = &xhci->bus_state[hcd_index(hcd)];
+
+ if (!wIndex || wIndex > max_ports)
+ return -EINVAL;
+
+ wIndex--;
+ temp = readl(port_array[wIndex]);
+ if (temp == 0x)
+ return -ENODEV;
+ status = xhci_get_port_status(hcd, bus_state, port_array,
+ wIndex, temp, flags);
+ if (status == 0x)
+ return -ENODEV;
+
+ xhci_dbg(xhci, "get port status, actual port %d status = 0x%x\n",
+ wIndex, temp);
+ xhci_dbg(xhci, "Get port status returned 0x%x\n", status);
+
+ put_unaligned(cpu_to_le32(status), (__le32 *) buf);
+ /* if USB 3.1 extended port status return additional 4 bytes */
+ if (wValue == 0x02) {
+ u32 port_li;
+
+ if (hcd->speed < HCD_USB31 || wLength != 8) {
+ xhci_err(xhci, "get ext port status invalid
parameter\n");
+ return -EINVAL;
+ }
+ port_li = readl(port_array[wIndex] + PORTLI);
+ status = xhci_get_ext_port_status(temp, port_li);
+ put_unaligned_le32(cpu_to_le32(status), &buf[4]);
+ }
+
+ return 0;
+}
+
+static int xhci_ctrl_set_port_feature(struct usb_hcd *hcd, u16 wValue,
+ u16 wIndex, char *buf, u16 wLength, unsigned long flags)
+{
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_bus_state *bus_state;
+
+ int max_ports;
+ int slot_id;
+ u32 temp;
+
+ __le32 __iomem **port_array;
+
u16 link_state = 0;
u16 wake_mask = 0;
u16 timeout = 0;
@@ -890,328 +970,322 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
max_ports = xhci_get_ports(hcd, &port_array);
bus_state = &xhci->bus_state[hcd_index(hcd)];
- spin_lock_irqsave(&xhci->lock, flags);
- switch (typeReq) {
- case GetHubStatus:
- /* No power source, over-current reported per port */
- memset(buf, 0, 4);
- break;
- case GetHubDescriptor:
- /* Check to make sure userspace is asking for the USB 3.0 hub
-* descriptor for the USB 3.0 roothub. If not, we stall the
-* endpoint, like external hubs do.
+ if (wValue == USB_PORT_FEAT_LINK_STATE)
+ link_state = (wIndex & 0xff00) >> 3;
+ if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK)
+ wake_mask = wIndex & 0xff00;
+ /* The MSB of wIndex is the U1/U2 timeout */
+ timeout = (wIndex & 0xff00) >> 8;
+ wIndex &= 0xff;
+ if (!wIndex || wIndex > max_port
Re: [PATCH 2/2] usb: xhci: mem: convert to a switch statement
Hi, Felipe Balbi writes: > when getting endpoint type a switch statement looks > better than a series of if () branches. There are no > functional changes with this patch, cleanup only. > > Signed-off-by: Felipe Balbi ping here too. Any comments? -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: xhci: switch to running avg trb length
Hi, Felipe Balbi writes: > It's unlikely that we will ever know the avg so > instead of assuming it'll be something really large, > we will calculate the avg as we go as mentioned in > XHCI specification section 4.14.1.1. > > Signed-off-by: Felipe Balbi ping?!? This has been here for quite some time. -- balbi signature.asc Description: PGP signature
Re: Fwd: UAS erorrs
On Mon, 2016-12-12 at 10:55 -0800, Alex Ruiz wrote: > I noticed it as being an issue when trying to run "rsync" local to > local. But it effected everything including long times to mount, > partitioning, ddrescue, and etc. > > > This is the hardware: http://www.iguanamicro.com/mousb30sahad.html > > > The fix: > > > [root@big-kahuna ~]# cat /etc/modprobe.d/blacklist_uas_152d.conf > ## view https://bbs.archlinux.org/viewtopic.php?pid=1428782#p1428782 > ## view https://ubuntuforums.org/archive/index.php/t-2307662.html > options usb-storage quirks=0x152d:0x3562:u That is not really a fix. Performance will suffer a lot. Have you tried any other flags? Would you test a patch if I sent it to you? Regards Oliver -- 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] USB3/DWC3: Enable undefined length INCR burst type
While enabling undefined length INCR burst type and INCR16 burst type, get better write performance on NXP Layerscape platform: around 3% improvement (from 364MB/s to 375MB/s). Signed-off-by: Changming Huang Signed-off-by: Rajesh Bhagat --- drivers/usb/dwc3/core.c |6 ++ drivers/usb/dwc3/core.h | 13 + 2 files changed, 19 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index fea4469..0e11891 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -621,6 +621,12 @@ static int dwc3_core_init(struct dwc3 *dwc) goto err0; } + /* Enable Undefined Length INCR Burst Type and Enable INCR16 Burst */ + reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0); + reg &= ~DWC3_GSBUSCFG0_INCRBRSTMASK; + reg |= DWC3_GSBUSCFG0_INCR16BRSTENA | DWC3_GSBUSCFG0_INCRBRSTENA; + dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg); + /* * Write Linux Version Code to our GUID register so it's easy to figure * out which kernel version a bug was found. diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 6b60e42..8bfdb77 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -156,6 +156,19 @@ /* Bit fields */ +/* Global SoC Bus Configuration Register 0 */ +#define DWC3_GSBUSCFG0_DATABIGEND (1 << 11) +#define DWC3_GSBUSCFG0_DESCBIGEND (1 << 10) +#define DWC3_GSBUSCFG0_INCR256BRSTENA (1 << 7) +#define DWC3_GSBUSCFG0_INCR128BRSTENA (1 << 6) +#define DWC3_GSBUSCFG0_INCR64BRSTENA (1 << 5) +#define DWC3_GSBUSCFG0_INCR32BRSTENA (1 << 4) +#define DWC3_GSBUSCFG0_INCR16BRSTENA (1 << 3) +#define DWC3_GSBUSCFG0_INCR8BRSTENA(1 << 2) +#define DWC3_GSBUSCFG0_INCR4BRSTENA(1 << 1) +#define DWC3_GSBUSCFG0_INCRBRSTENA (1 << 0) +#define DWC3_GSBUSCFG0_INCRBRSTMASK0xff + /* Global Debug Queue/FIFO Space Available Register */ #define DWC3_GDBGFIFOSPACE_NUM(n) ((n) & 0x1f) #define DWC3_GDBGFIFOSPACE_TYPE(n) (((n) << 5) & 0x1e0) -- 1.7.9.5 -- 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] usb: host: xhci: Clean up commands when stop endpoint command is timeout
Since the stop endpoint command is timeout, we will halt the xHCI controller and issuing the usb_hc_died() to report abnormal shutdown of a host controller, but before that, we should clean up the command queue to free commands memory and complete the pending commands. Signed-off-by: Baolin Wang --- drivers/usb/host/xhci-ring.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index edc9ac2..92b48fe 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -904,6 +904,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) xhci_kill_endpoint_urbs(xhci, i, j); } spin_unlock_irqrestore(&xhci->lock, flags); + xhci_cleanup_command_queue(xhci); xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Calling usb_hc_died()"); usb_hc_died(xhci_to_hcd(xhci)); -- 1.7.9.5 -- 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: usb/gadget: warning in ep_write_iter/__alloc_pages_nodemask
On Mon 12-12-16 16:12:16, Alan Stern wrote: > On Mon, 12 Dec 2016, Michal Hocko wrote: > > > On Mon 12-12-16 21:32:35, Andrey Konovalov wrote: > > > On Mon, Dec 12, 2016 at 9:31 PM, Andrey Konovalov > > > wrote: > > > > Hi! > > > > > > > > While running the syzkaller fuzzer I've got the following error report. > > > > > > > > The issue is that the len argument is not checked for being too big. > > > > Well, the value is checked in kmalloc_slab. There is a discrepancy > > though. While the page allocator enforces order < MAX_ORDER, slab > > allocators enforce size <= KMALLOC_MAX_SIZE and > > > > KMALLOC_MAX_SIZE is 1UL << (MAX_ORDER + PAGE_SHIFT) for SLUB which is > > what I expect you have or 1UL << (MAX_ORDER + PAGE_SHIFT - 1) for SLAB > > on most archs. This means that KMALLOC_MAX_SIZE might be MAX_ORDER for > > SLUB which would explain your warning. > > > > Let's CC Christoph, this is nothing really new, but I suspect that SLUB > > should change the maximum size to something like SLAB does. > > > > That being said, what ep_write_iter does sounds quite stupit. It just > > allocates a large continuous buffer which seems to be under user > > control... Aka no good! It should do that per pages or something like > > that. Something worth fixing > > It's not important enough to make the driver do all this work. If > users want to send large amounts of data, they can send it a page at a > time (or something like that). Is it really necessary to allocate the full iov_iter_count? Why cannot we process the from buffer one page at a time? > If you really want to prevent the driver from attempting to allocate a > large buffer, all that's needed is an upper limit on the total size. > For example, 64 KB. Well, my point was that it is not really hard to imagine to deplete larger contiguous memory blocks (say PAGE_ALLOC_COSTLY_ORDER). Those are still causing the OOM killer and chances are that a controlled flood of these requests could completely DoS the system. -- Michal Hocko SUSE Labs -- 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
