Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1697668 > unrolled thread

[PATCH v2 0/5] usb: xhci: Handle USB transaction error on address command

Started byLu Baolu <baolu.lu@linux.intel.com>
First post2017-07-27 04:30 +0200
Last post2017-07-27 04:30 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/5] usb: xhci: Handle USB transaction error on address command Lu Baolu <baolu.lu@linux.intel.com> - 2017-07-27 04:30 +0200
    [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command Lu Baolu <baolu.lu@linux.intel.com> - 2017-07-27 04:30 +0200
      Re: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command Felipe Balbi <felipe.balbi@linux.intel.com> - 2017-07-27 10:00 +0200
        Re: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address  command Lu Baolu <baolu.lu@linux.intel.com> - 2017-07-27 11:00 +0200
    [PATCH v2 3/5] usb: xhci: Fix memory leak when xhci_disable_slot() returns error Lu Baolu <baolu.lu@linux.intel.com> - 2017-07-27 04:30 +0200

#1697668 — [PATCH v2 0/5] usb: xhci: Handle USB transaction error on address command

FromLu Baolu <baolu.lu@linux.intel.com>
Date2017-07-27 04:30 +0200
Subject[PATCH v2 0/5] usb: xhci: Handle USB transaction error on address command
Message-ID<u7GIN-em-3@gated-at.bofh.it>
Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.

The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.

The related discussion threads can be found through below links.

http://marc.info/?l=linux-usb&m=149362010728921&w=2
http://marc.info/?l=linux-usb&m=149252752825755&w=2

This patch set includes some fixes in xhci_disable_slot() as well
which will be used to handle USB transaction error on address
command.

---
Change log:

v1->v2:
 - include 4 fixes in xhci_disable_slot which will be used
   to handle USB transaction error on address command.

Lu Baolu (5):
  usb: xhci: Disable slot even virt-dev is null
  usb: xhci: Fix potential memory leak in xhci_disable_slot()
  usb: xhci: Fix memory leak when xhci_disable_slot() returns error
  usb: xhci: Return error when host is dead in xhci_disable_slot()
  usb: xhci: Handle USB transaction error on address command

 drivers/usb/host/xhci-hub.c |  2 +-
 drivers/usb/host/xhci.c     | 52 ++++++++++++++++++---------------------------
 drivers/usb/host/xhci.h     |  3 +--
 3 files changed, 23 insertions(+), 34 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1697669 — [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command

FromLu Baolu <baolu.lu@linux.intel.com>
Date2017-07-27 04:30 +0200
Subject[PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command
Message-ID<u7GIN-em-17@gated-at.bofh.it>
In reply to#1697668
Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.

The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.

This patch handles USB transaction errors on address command
completion events. The related discussion threads can be found
through below links.

http://marc.info/?l=linux-usb&m=149362010728921&w=2
http://marc.info/?l=linux-usb&m=149252752825755&w=2

Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/usb/host/xhci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d6b728d..95780f8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 		break;
 	case COMP_USB_TRANSACTION_ERROR:
 		dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
+
+		ret = xhci_disable_slot(xhci, udev->slot_id);
+		if (!ret)
+			xhci_alloc_dev(hcd, udev);
+
 		ret = -EPROTO;
 		break;
 	case COMP_INCOMPATIBLE_DEVICE_ERROR:
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1697783 — Re: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command

FromFelipe Balbi <felipe.balbi@linux.intel.com>
Date2017-07-27 10:00 +0200
SubjectRe: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command
Message-ID<u7LS9-3l2-1@gated-at.bofh.it>
In reply to#1697669

[Multipart message — attachments visible in raw view] — view raw

Hi,

Lu Baolu <baolu.lu@linux.intel.com> writes:
> Xhci driver handles USB transaction errors on transfer events,
> but transaction errors are possible on address device command
> completion events as well.
>
> The xHCI specification (section 4.6.5) says: A USB Transaction
> Error Completion Code for an Address Device Command may be due
> to a Stall response from a device. Software should issue a Disable
> Slot Command for the Device Slot then an Enable Slot Command to
> recover from this error.
>
> This patch handles USB transaction errors on address command
> completion events. The related discussion threads can be found
> through below links.
>
> http://marc.info/?l=linux-usb&m=149362010728921&w=2
> http://marc.info/?l=linux-usb&m=149252752825755&w=2
>
> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index d6b728d..95780f8 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
>  		break;
>  	case COMP_USB_TRANSACTION_ERROR:
>  		dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
> +
> +		ret = xhci_disable_slot(xhci, udev->slot_id);
> +		if (!ret)
> +			xhci_alloc_dev(hcd, udev);

aren't you leaking previously allocated virt_dev ?

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1697818 — Re: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command

FromLu Baolu <baolu.lu@linux.intel.com>
Date2017-07-27 11:00 +0200
SubjectRe: [PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command
Message-ID<u7MOe-3UB-23@gated-at.bofh.it>
In reply to#1697783
Hi,

On 07/27/2017 03:55 PM, Felipe Balbi wrote:
> Hi,
>
> Lu Baolu <baolu.lu@linux.intel.com> writes:
>> Xhci driver handles USB transaction errors on transfer events,
>> but transaction errors are possible on address device command
>> completion events as well.
>>
>> The xHCI specification (section 4.6.5) says: A USB Transaction
>> Error Completion Code for an Address Device Command may be due
>> to a Stall response from a device. Software should issue a Disable
>> Slot Command for the Device Slot then an Enable Slot Command to
>> recover from this error.
>>
>> This patch handles USB transaction errors on address command
>> completion events. The related discussion threads can be found
>> through below links.
>>
>> http://marc.info/?l=linux-usb&m=149362010728921&w=2
>> http://marc.info/?l=linux-usb&m=149252752825755&w=2
>>
>> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>  drivers/usb/host/xhci.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index d6b728d..95780f8 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
>>  		break;
>>  	case COMP_USB_TRANSACTION_ERROR:
>>  		dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
>> +
>> +		ret = xhci_disable_slot(xhci, udev->slot_id);
>> +		if (!ret)
>> +			xhci_alloc_dev(hcd, udev);
> aren't you leaking previously allocated virt_dev ?
>

When disable slot command completes, the command completion handler
will release the previous allocated virt_dev.

Best regards,
Lu Baolu

[toc] | [prev] | [next] | [standalone]

#1697670 — [PATCH v2 3/5] usb: xhci: Fix memory leak when xhci_disable_slot() returns error

FromLu Baolu <baolu.lu@linux.intel.com>
Date2017-07-27 04:30 +0200
Subject[PATCH v2 3/5] usb: xhci: Fix memory leak when xhci_disable_slot() returns error
Message-ID<u7GIO-em-23@gated-at.bofh.it>
In reply to#1697668
If xhci_disable_slot() returns success, a disable slot command
trb was queued in the command ring. The command completion
handler will free the virtual device data structure associated
with the slot. On the other hand, when xhci_disable_slot()
returns error, the invokers should take the responsibilities to
free the slot related data structure. Otherwise, memory leakage
happens.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/usb/host/xhci.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index cb2461a..2df601e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3547,11 +3547,9 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
 		del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
 	}
 
-	xhci_disable_slot(xhci, udev->slot_id);
-	/*
-	 * Event command completion handler will free any data structures
-	 * associated with the slot.  XXX Can free sleep?
-	 */
+	ret = xhci_disable_slot(xhci, udev->slot_id);
+	if (ret)
+		xhci_free_virt_device(xhci, udev->slot_id);
 }
 
 int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
@@ -3697,7 +3695,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	return 1;
 
 disable_slot:
-	return xhci_disable_slot(xhci, udev->slot_id);
+	ret = xhci_disable_slot(xhci, udev->slot_id);
+	if (ret)
+		xhci_free_virt_device(xhci, udev->slot_id);
+
+	return 0;
 }
 
 /*
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web