[PATCH 0/6] xhci features and fixes for usb-next

[PATCH 0/6] xhci features and fixes for usb-next

[Mathias Nyman]

Hi Greg

Some xhci features and fixes for usb-next.
Both fixing some possible memory leaks that are too late for usb-linus,
and simple code cleanups for usb-next

Thanks
-Mathias

Jianglei Nie (1):
  usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()

Mario Limonciello (1):
  xhci: Don't show warning for reinit on known broken suspend

Mathias Nyman (3):
  xhci: show fault reason for a failed enable slot command
  xhci: remove unused command member from struct xhci_hcd struct
  xhci: remove unused lpm_failed_dev member from struct xhci_hcd

Rafael Mendonca (1):
  xhci: dbc: Fix memory leak in xhci_alloc_dbc()

 drivers/usb/host/xhci-dbgcap.c | 2 +-
 drivers/usb/host/xhci-mem.c    | 7 ++++++-
 drivers/usb/host/xhci.c        | 6 ++++--
 drivers/usb/host/xhci.h        | 3 ---
 4 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/6] usb: host: xhci: Fix potential memory leak in xhci_alloc_stream_info()

[Mathias Nyman]

From: Jianglei Nie <niejianglei2021@163.com>

xhci_alloc_stream_info() allocates stream context array for stream_info
->stream_ctx_array with xhci_alloc_stream_ctx(). When some error occurs,
stream_info->stream_ctx_array is not released, which will lead to a
memory leak.

We can fix it by releasing the stream_info->stream_ctx_array with
xhci_free_stream_ctx() on the error path to avoid the potential memory
leak.

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8c19e151a945..9e56aa28efcd 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -641,7 +641,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 			num_stream_ctxs, &stream_info->ctx_array_dma,
 			mem_flags);
 	if (!stream_info->stream_ctx_array)
-		goto cleanup_ctx;
+		goto cleanup_ring_array;
 	memset(stream_info->stream_ctx_array, 0,
 			sizeof(struct xhci_stream_ctx)*num_stream_ctxs);
 
@@ -702,6 +702,11 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 	}
 	xhci_free_command(xhci, stream_info->free_streams_command);
 cleanup_ctx:
+	xhci_free_stream_ctx(xhci,
+		stream_info->num_stream_ctxs,
+		stream_info->stream_ctx_array,
+		stream_info->ctx_array_dma);
+cleanup_ring_array:
 	kfree(stream_info->stream_rings);
 cleanup_info:
 	kfree(stream_info);
-- 
2.25.1

[PATCH 2/6] xhci: dbc: Fix memory leak in xhci_alloc_dbc()

[Mathias Nyman]

From: Rafael Mendonca <rafaelmendsr@gmail.com>

If DbC is already in use, then the allocated memory for the xhci_dbc struct
doesn't get freed before returning NULL, which leads to a memleak.

Fixes: 534675942e90 ("xhci: dbc: refactor xhci_dbc_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-dbgcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index e61155fa6379..f1367b53b260 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -988,7 +988,7 @@ xhci_alloc_dbc(struct device *dev, void __iomem *base, const struct dbc_driver *
 	dbc->driver = driver;
 
 	if (readl(&dbc->regs->control) & DBC_CTRL_DBC_ENABLE)
-		return NULL;
+		goto err;
 
 	INIT_DELAYED_WORK(&dbc->event_work, xhci_dbc_handle_events);
 	spin_lock_init(&dbc->lock);
-- 
2.25.1

[PATCH 3/6] xhci: Don't show warning for reinit on known broken suspend

[Mathias Nyman]

From: Mario Limonciello <mario.limonciello@amd.com>

commit 8b328f8002bc ("xhci: re-initialize the HC during resume if HCE was
set") introduced a new warning message when the host controller error
was set and re-initializing.

This is expected behavior on some designs which already set
`xhci->broken_suspend` so the new warning is alarming to some users.

Modify the code to only show the warning if this was a surprising behavior
to the XHCI driver.

Fixes: 8b328f8002bc ("xhci: re-initialize the HC during resume if HCE was set")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216470
Reported-by: Artem S. Tashkinov <aros@gmx.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 38649284ff88..a7ef675f00fd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1183,7 +1183,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 	/* re-initialize the HC on Restore Error, or Host Controller Error */
 	if (temp & (STS_SRE | STS_HCE)) {
 		reinit_xhc = true;
-		xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp);
+		if (!xhci->broken_suspend)
+			xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp);
 	}
 
 	if (reinit_xhc) {
-- 
2.25.1

[PATCH 4/6] xhci: show fault reason for a failed enable slot command

[Mathias Nyman]

Show the completion code of a unsuccessful "enable slot" command.
Add it in a human readable form to the existing error message.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a7ef675f00fd..17f5dda913be 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4096,7 +4096,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	slot_id = command->slot_id;
 
 	if (!slot_id || command->status != COMP_SUCCESS) {
-		xhci_err(xhci, "Error while assigning device slot ID\n");
+		xhci_err(xhci, "Error while assigning device slot ID: %s\n",
+			 xhci_trb_comp_code_string(command->status));
 		xhci_err(xhci, "Max number of devices this xHCI host supports is %u.\n",
 				HCS_MAX_SLOTS(
 					readl(&xhci->cap_regs->hcs_params1)));
-- 
2.25.1

[PATCH 5/6] xhci: remove unused command member from struct xhci_hcd struct

[Mathias Nyman]

The u32 command was added to struct xhci_hcd over 10 years ago in
commit 9777e3ce907d ("USB: xHCI: bus power management implementation")

It wasn't even used back then, so remove it.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7caa0db5e826..fa352fb24867 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1827,7 +1827,6 @@ struct xhci_hcd {
 	/* Host controller watchdog timer structures */
 	unsigned int		xhc_state;
 	unsigned long		run_graceperiod;
-	u32			command;
 	struct s3_save		s3;
 /* Host controller is dying - not responding to commands. "I'm not dead yet!"
  *
-- 
2.25.1

[PATCH 6/6] xhci: remove unused lpm_failed_dev member from struct xhci_hcd

[Mathias Nyman]

xhci used to test if link power management (LPM) capable USB2 devices
really could enter and exit L1 state link state.
Failed devices were added to a lpm_failed_dev list.

This feature was removed 9 years ago in
commit de68bab4fa96 ("usb: Don't enable USB 2.0 Link PM by default.")
but lpm_failed_dev member was still left.

Remove it now.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index fa352fb24867..807fc4e47959 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1807,8 +1807,6 @@ struct xhci_hcd {
 	struct xhci_erst	erst;
 	/* Scratchpad */
 	struct xhci_scratchpad  *scratchpad;
-	/* Store LPM test failed devices' information */
-	struct list_head	lpm_failed_devs;
 
 	/* slot enabling and address device helpers */
 	/* these are not thread safe so use mutex */
-- 
2.25.1

Re: [PATCH 0/6] xhci features and fixes for usb-next

[Greg KH]

On Wed, Sep 21, 2022 at 03:34:44PM +0300, Mathias Nyman wrote:
> Hi Greg
> 
> Some xhci features and fixes for usb-next.
> Both fixing some possible memory leaks that are too late for usb-linus,
> and simple code cleanups for usb-next

Did you miss this one:
	https://lore.kernel.org/r/20220915011134.58400-1-liulongfang@huawei.com

or was there some issue with it?

thanks,

greg k-h

Re: [PATCH 0/6] xhci features and fixes for usb-next

[Mathias Nyman]

On 22.9.2022 15.08, Greg KH wrote:
> On Wed, Sep 21, 2022 at 03:34:44PM +0300, Mathias Nyman wrote:
>> Hi Greg
>>
>> Some xhci features and fixes for usb-next.
>> Both fixing some possible memory leaks that are too late for usb-linus,
>> and simple code cleanups for usb-next
> 
> Did you miss this one:
> 	https://lore.kernel.org/r/20220915011134.58400-1-liulongfang@huawei.com
> 
> or was there some issue with it?

Got distracted while looking at it.

Can be added but probably not that useful.

In a host controller error (HCE) case all host activity stops, so it would
make more sense to check HCE bit in timeout cases than in interrupt handler.

-Mathias

Re: [PATCH 0/6] xhci features and fixes for usb-next

[Greg KH]

On Thu, Sep 22, 2022 at 03:43:04PM +0300, Mathias Nyman wrote:
> On 22.9.2022 15.08, Greg KH wrote:
> > On Wed, Sep 21, 2022 at 03:34:44PM +0300, Mathias Nyman wrote:
> > > Hi Greg
> > > 
> > > Some xhci features and fixes for usb-next.
> > > Both fixing some possible memory leaks that are too late for usb-linus,
> > > and simple code cleanups for usb-next
> > 
> > Did you miss this one:
> > 	https://lore.kernel.org/r/20220915011134.58400-1-liulongfang@huawei.com
> > 
> > or was there some issue with it?
> 
> Got distracted while looking at it.
> 
> Can be added but probably not that useful.
> 
> In a host controller error (HCE) case all host activity stops, so it would
> make more sense to check HCE bit in timeout cases than in interrupt handler.

Ok, no worries, I'll drop it from my review queue, but you should
respond to the patch submission with this information.

thanks,

greg k-h