| Message ID | 20240229141438.619372-3-mathias.nyman@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 06790c19086fe8f54afcd49184916132c7a8da4e |
| Headers | show |
| Series | xhci features for usb-next | expand |
Hi Mathias, On Thu, Feb 29, 2024, Mathias Nyman wrote: > From: Niklas Neronin <niklas.neronin@intel.com> > > Variables real & fake port do not convey their purpose, thus they are > replaced with a pointer to the root hub port 'struct xhci_port *rhub_port'. > 'rhub_port' contains real & fake ports in zero-based format, which happens > to be more widely used inside the xHCI driver: > - 'real_port' is ('rhub_port->hw_portnum' + 1) > - 'fake_port' is ('rhub_port->hcd_portnum' + 1) > > One reason for real port being one-based, is to signal other functions in > case struct 'xhci_virt_device' initialization failed, in this case the > value will remain 0. This is no longer needed, instead we check whether > or not 'rhub_port' is 'NULL'. > > Signed-off-by: Niklas Neronin <niklas.neronin@intel.com> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> > --- > drivers/usb/host/xhci-hub.c | 4 +++- > drivers/usb/host/xhci-mem.c | 35 ++++++++++++++------------------- > drivers/usb/host/xhci-mtk-sch.c | 14 +++++-------- > drivers/usb/host/xhci-trace.h | 12 +++++------ > drivers/usb/host/xhci.c | 12 +++++------ > drivers/usb/host/xhci.h | 3 +-- > 6 files changed, 35 insertions(+), 45 deletions(-) > > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c > index 0980ade2a234..37128a777a58 100644 We're getting a NULL pointer dereference bug for this patch. To reproduce this, just unload and reload the xhci driver while a device is connected. It may take a few times to hit the issue. [ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c [ 1515.737629] #PF: supervisor read access in kernel mode [ 1515.737631] #PF: error_code(0x0000) - not-present page [ 1515.737633] PGD 0 P4D 0 [ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI [ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8 [ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019 [ 1515.737643] Workqueue: usb_hub_wq hub_event [ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] [ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0 [ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086 [ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4 [ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8 [ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010 [ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001 [ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000 [ 1515.737734] FS: 0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000 [ 1515.737736] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0 [ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1515.737743] Call Trace: [ 1515.737746] <TASK> [ 1515.737748] ? __die_body+0x1a/0x5c [ 1515.737751] ? page_fault_oops+0x2ea/0x380 [ 1515.737806] ? sched_clock+0x10/0x23 [ 1515.737808] ? trace_clock_local+0x10/0x23 [ 1515.737812] ? exc_page_fault+0xfe/0x11e [ 1515.737815] ? asm_exc_page_fault+0x26/0x30 [ 1515.737818] ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] [ 1515.737832] ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd] [ 1515.737844] xhci_free_virt_device+0x76/0x1d8 [xhci_hcd] [ 1515.737856] xhci_free_dev+0x111/0x12f [xhci_hcd] [ 1515.737867] hub_event+0xca6/0x137e [ 1515.737870] ? __schedule+0x656/0x69f [ 1515.737873] process_scheduled_works+0x1a4/0x2e3 [ 1515.737876] worker_thread+0x191/0x1e9 [ 1515.737878] ? __pfx_worker_thread+0x10/0x10 [ 1515.737881] kthread+0xe6/0xf1 [ 1515.737883] ? __pfx_kthread+0x10/0x10 [ 1515.737885] ret_from_fork+0x20/0x37 [ 1515.737887] ? __pfx_kthread+0x10/0x10 [ 1515.737889] ret_from_fork_asm+0x1a/0x30 [ 1515.737892] </TASK> To capture the tracepoints and avoid the NULL pointer, I made this change: diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 1740000d54c2..b4b20e3f7539 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __field(void *, vdev) __field(unsigned long long, out_ctx) __field(unsigned long long, in_ctx) - __field(int, hcd_portnum) - __field(int, hw_portnum) + __field(struct xhci_port *, rhub_port) __field(u16, current_mel) ), @@ -181,13 +180,14 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __entry->vdev = vdev; __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; - __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum; - __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum; + __entry->rhub_port = (struct xhci_port *) vdev->rhub_port; __entry->current_mel = (u16) vdev->current_mel; ), TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d", __entry->vdev, __entry->in_ctx, __entry->out_ctx, - __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel + __entry->rhub_port ? __entry->rhub_port->hcd_portnum : -1, + __entry->rhub_port ? __entry->rhub_port->hw_portnum : -1, + __entry->current_mel ) ); You should see this line where the NULL deref happened in the attached log: kworker/0:1-8 [000] d..1. 250.442611: xhci_free_virt_device: vdev 00000000a84fbabe ctx 159dd6000 | 1a7d60000 hcd_portnum -1 hw_portnum -1 current_mel 0 Please review and help fix it. Thanks, Thinh
Hi Thinh On 2.4.2024 3.50, Thinh Nguyen wrote: > Hi Mathias, > > We're getting a NULL pointer dereference bug for this patch. To > reproduce this, just unload and reload the xhci driver while a device is > connected. It may take a few times to hit the issue. > > [ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c > [ 1515.737629] #PF: supervisor read access in kernel mode > [ 1515.737631] #PF: error_code(0x0000) - not-present page > [ 1515.737633] PGD 0 P4D 0 > [ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI > [ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8 > [ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019 > [ 1515.737643] Workqueue: usb_hub_wq hub_event > [ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] > [ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0 > [ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086 > [ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4 > [ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8 > [ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010 > [ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001 > [ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000 > [ 1515.737734] FS: 0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000 > [ 1515.737736] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0 > [ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 1515.737743] Call Trace: > [ 1515.737746] <TASK> > [ 1515.737748] ? __die_body+0x1a/0x5c > [ 1515.737751] ? page_fault_oops+0x2ea/0x380 > [ 1515.737806] ? sched_clock+0x10/0x23 > [ 1515.737808] ? trace_clock_local+0x10/0x23 > [ 1515.737812] ? exc_page_fault+0xfe/0x11e > [ 1515.737815] ? asm_exc_page_fault+0x26/0x30 > [ 1515.737818] ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] > [ 1515.737832] ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd] > [ 1515.737844] xhci_free_virt_device+0x76/0x1d8 [xhci_hcd] > [ 1515.737856] xhci_free_dev+0x111/0x12f [xhci_hcd] > [ 1515.737867] hub_event+0xca6/0x137e Thanks for the report, and for debugging this. I was able to reproduce it myself using your steps. This triggers if xhci tracing is enabled and usb device is freed before it's addressed as vdev->rhub_port is only set during addressing. > > > To capture the tracepoints and avoid the NULL pointer, I made this > change: > I think we could skip printing the hcd_portnum and hw_portnum during xhci_free_virt_dev() tracing. I haven't really found them useful. It would make sense to print the slot_id instead. how about this: diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 1740000d54c2..5762564b9d73 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __field(void *, vdev) __field(unsigned long long, out_ctx) __field(unsigned long long, in_ctx) - __field(int, hcd_portnum) - __field(int, hw_portnum) + __field(int, slot_id) __field(u16, current_mel) ), @@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __entry->vdev = vdev; __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; - __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum; - __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum; + __entry->slot_id = (int) vdev->slot_id; __entry->current_mel = (u16) vdev->current_mel; ), - TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d", - __entry->vdev, __entry->in_ctx, __entry->out_ctx, - __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel + TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d", + __entry->vdev, __entry->slot_id, __entry->in_ctx, + __entry->out_ctx, __entry->current_mel ) ); It works for me Thanks Mathias
On Tue, Apr 02, 2024, Mathias Nyman wrote: > Hi Thinh > > On 2.4.2024 3.50, Thinh Nguyen wrote: > > Hi Mathias, > > > > We're getting a NULL pointer dereference bug for this patch. To > > reproduce this, just unload and reload the xhci driver while a device is > > connected. It may take a few times to hit the issue. > > > > [ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c > > [ 1515.737629] #PF: supervisor read access in kernel mode > > [ 1515.737631] #PF: error_code(0x0000) - not-present page > > [ 1515.737633] PGD 0 P4D 0 > > [ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI > > [ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8 > > [ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019 > > [ 1515.737643] Workqueue: usb_hub_wq hub_event > > [ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] > > [ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0 > > [ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086 > > [ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4 > > [ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8 > > [ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010 > > [ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001 > > [ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000 > > [ 1515.737734] FS: 0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000 > > [ 1515.737736] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > [ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0 > > [ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > [ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > [ 1515.737743] Call Trace: > > [ 1515.737746] <TASK> > > [ 1515.737748] ? __die_body+0x1a/0x5c > > [ 1515.737751] ? page_fault_oops+0x2ea/0x380 > > [ 1515.737806] ? sched_clock+0x10/0x23 > > [ 1515.737808] ? trace_clock_local+0x10/0x23 > > [ 1515.737812] ? exc_page_fault+0xfe/0x11e > > [ 1515.737815] ? asm_exc_page_fault+0x26/0x30 > > [ 1515.737818] ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd] > > [ 1515.737832] ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd] > > [ 1515.737844] xhci_free_virt_device+0x76/0x1d8 [xhci_hcd] > > [ 1515.737856] xhci_free_dev+0x111/0x12f [xhci_hcd] > > [ 1515.737867] hub_event+0xca6/0x137e > > Thanks for the report, and for debugging this. > I was able to reproduce it myself using your steps. > > This triggers if xhci tracing is enabled and usb device is freed before it's addressed > as vdev->rhub_port is only set during addressing. > > > > > > > To capture the tracepoints and avoid the NULL pointer, I made this > > change: > > > > I think we could skip printing the hcd_portnum and hw_portnum during > xhci_free_virt_dev() tracing. I haven't really found them useful. > > It would make sense to print the slot_id instead. > > how about this: > > diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h > index 1740000d54c2..5762564b9d73 100644 > --- a/drivers/usb/host/xhci-trace.h > +++ b/drivers/usb/host/xhci-trace.h > @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, > __field(void *, vdev) > __field(unsigned long long, out_ctx) > __field(unsigned long long, in_ctx) > - __field(int, hcd_portnum) > - __field(int, hw_portnum) > + __field(int, slot_id) > __field(u16, current_mel) > ), > @@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, > __entry->vdev = vdev; > __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; > __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; > - __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum; > - __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum; > + __entry->slot_id = (int) vdev->slot_id; > __entry->current_mel = (u16) vdev->current_mel; > ), > - TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d", > - __entry->vdev, __entry->in_ctx, __entry->out_ctx, > - __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel > + TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d", > + __entry->vdev, __entry->slot_id, __entry->in_ctx, > + __entry->out_ctx, __entry->current_mel > ) > ); > > It works for me > That looks good to me. Can you submit the change? On an unrelated note, often we have to debug xHCI driver on a system with multiple xHCI controllers. I'm not sure if there's a good way to filter the xHCI tracepoints to a specific controller? I needed to print the devname for each tracepoint just to get around this, which doesn't seem like a great solution. Any idea? Thanks, Thinh
On 3.4.2024 2.13, Thinh Nguyen wrote: > On Tue, Apr 02, 2024, Mathias Nyman wrote: >> Hi Thinh >> >> On 2.4.2024 3.50, Thinh Nguyen wrote: >>> Hi Mathias, >>> >>> We're getting a NULL pointer dereference bug for this patch. To >>> reproduce this, just unload and reload the xhci driver while a device is >>> connected. It may take a few times to hit the issue. >>> >> how about this: >> >> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h >> index 1740000d54c2..5762564b9d73 100644 >> --- a/drivers/usb/host/xhci-trace.h >> +++ b/drivers/usb/host/xhci-trace.h >> @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, >> __field(void *, vdev) >> __field(unsigned long long, out_ctx) >> __field(unsigned long long, in_ctx) >> - __field(int, hcd_portnum) >> - __field(int, hw_portnum) >> + __field(int, slot_id) >> __field(u16, current_mel) >> ), >> @@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, >> __entry->vdev = vdev; >> __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; >> __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; >> - __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum; >> - __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum; >> + __entry->slot_id = (int) vdev->slot_id; >> __entry->current_mel = (u16) vdev->current_mel; >> ), >> - TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d", >> - __entry->vdev, __entry->in_ctx, __entry->out_ctx, >> - __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel >> + TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d", >> + __entry->vdev, __entry->slot_id, __entry->in_ctx, >> + __entry->out_ctx, __entry->current_mel >> ) >> ); > > That looks good to me. Can you submit the change? yes, I'll submit the change > > On an unrelated note, often we have to debug xHCI driver on a system > with multiple xHCI controllers. I'm not sure if there's a good way to > filter the xHCI tracepoints to a specific controller? I needed to print > the devname for each tracepoint just to get around this, which doesn't > seem like a great solution. Any idea? I'm facing similar debugging issues, I'll look into it. Thanks Mathias
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0980ade2a234..37128a777a58 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -464,13 +464,15 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, int i; enum usb_device_speed speed; + /* 'hcd_portnum' is zero-based, thus convert one-based 'port' to zero-based */ + port -= 1; slot_id = 0; for (i = 0; i < MAX_HC_SLOTS; i++) { if (!xhci->devs[i] || !xhci->devs[i]->udev) continue; speed = xhci->devs[i]->udev->speed; if (((speed >= USB_SPEED_SUPER) == (hcd->speed >= HCD_USB3)) - && xhci->devs[i]->fake_port == port) { + && xhci->devs[i]->rhub_port->hcd_portnum == port) { slot_id = i; break; } diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 92160b96d4c0..9fa68fa17ac7 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -789,15 +789,14 @@ static void xhci_free_tt_info(struct xhci_hcd *xhci, bool slot_found = false; /* If the device never made it past the Set Address stage, - * it may not have the real_port set correctly. + * it may not have the root hub port pointer set correctly. */ - if (virt_dev->real_port == 0 || - virt_dev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) { - xhci_dbg(xhci, "Bad real port.\n"); + if (!virt_dev->rhub_port) { + xhci_dbg(xhci, "Bad rhub port.\n"); return; } - tt_list_head = &(xhci->rh_bw[virt_dev->real_port - 1].tts); + tt_list_head = &(xhci->rh_bw[virt_dev->rhub_port->hw_portnum].tts); list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { /* Multi-TT hubs will have more than one entry */ if (tt_info->slot_id == slot_id) { @@ -834,7 +833,7 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci, goto free_tts; INIT_LIST_HEAD(&tt_info->tt_list); list_add(&tt_info->tt_list, - &xhci->rh_bw[virt_dev->real_port - 1].tts); + &xhci->rh_bw[virt_dev->rhub_port->hw_portnum].tts); tt_info->slot_id = virt_dev->udev->slot_id; if (tt->multi) tt_info->ttport = i+1; @@ -929,13 +928,12 @@ static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_i if (!vdev) return; - if (vdev->real_port == 0 || - vdev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) { - xhci_dbg(xhci, "Bad vdev->real_port.\n"); + if (!vdev->rhub_port) { + xhci_dbg(xhci, "Bad rhub port.\n"); goto out; } - tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts); + tt_list_head = &(xhci->rh_bw[vdev->rhub_port->hw_portnum].tts); list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { /* is this a hub device that added a tt_info to the tts list */ if (tt_info->slot_id == slot_id) { @@ -1082,7 +1080,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud struct xhci_virt_device *dev; struct xhci_ep_ctx *ep0_ctx; struct xhci_slot_ctx *slot_ctx; - struct xhci_port *rhub_port; u32 max_packets; dev = xhci->devs[udev->slot_id]; @@ -1124,14 +1121,12 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud return -EINVAL; } /* Find the root hub port this device is under */ - rhub_port = xhci_find_rhub_port(xhci, udev); - if (!rhub_port) + dev->rhub_port = xhci_find_rhub_port(xhci, udev); + if (!dev->rhub_port) return -EINVAL; - dev->real_port = rhub_port->hw_portnum + 1; - dev->fake_port = rhub_port->hcd_portnum + 1; - slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->real_port)); - xhci_dbg(xhci, "Set root hub portnum to %d\n", dev->real_port); - xhci_dbg(xhci, "Set fake root hub portnum to %d\n", dev->fake_port); + slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->rhub_port->hw_portnum + 1)); + xhci_dbg(xhci, "Slot ID %d: HW portnum %d, hcd portnum %d\n", + udev->slot_id, dev->rhub_port->hw_portnum, dev->rhub_port->hcd_portnum); /* Find the right bandwidth table that this device will be a part of. * If this is a full speed device attached directly to a root port (or a @@ -1140,12 +1135,12 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud * will never be created for the HS root hub. */ if (!udev->tt || !udev->tt->hub->parent) { - dev->bw_table = &xhci->rh_bw[dev->real_port - 1].bw_table; + dev->bw_table = &xhci->rh_bw[dev->rhub_port->hw_portnum].bw_table; } else { struct xhci_root_port_bw_info *rh_bw; struct xhci_tt_bw_info *tt_bw; - rh_bw = &xhci->rh_bw[dev->real_port - 1]; + rh_bw = &xhci->rh_bw[dev->rhub_port->hw_portnum]; /* Find the right TT. */ list_for_each_entry(tt_bw, &rh_bw->tts, tt_list) { if (tt_bw->slot_id != udev->tt->hub->slot_id) diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c index 61f3f8bbdcea..27eb384a3963 100644 --- a/drivers/usb/host/xhci-mtk-sch.c +++ b/drivers/usb/host/xhci-mtk-sch.c @@ -122,10 +122,6 @@ static u32 get_bw_boundary(enum usb_device_speed speed) * each HS root port is treated as a single bandwidth domain, * but each SS root port is treated as two bandwidth domains, one for IN eps, * one for OUT eps. -* @real_port value is defined as follow according to xHCI spec: -* 1 for SSport0, ..., N+1 for SSportN, N+2 for HSport0, N+3 for HSport1, etc -* so the bandwidth domain array is organized as follow for simplification: -* SSport0-OUT, SSport0-IN, ..., SSportX-OUT, SSportX-IN, HSport0, ..., HSportY */ static struct mu3h_sch_bw_info * get_bw_info(struct xhci_hcd_mtk *mtk, struct usb_device *udev, @@ -136,19 +132,19 @@ get_bw_info(struct xhci_hcd_mtk *mtk, struct usb_device *udev, int bw_index; virt_dev = xhci->devs[udev->slot_id]; - if (!virt_dev->real_port) { - WARN_ONCE(1, "%s invalid real_port\n", dev_name(&udev->dev)); + if (!virt_dev->rhub_port) { + WARN_ONCE(1, "%s invalid rhub port\n", dev_name(&udev->dev)); return NULL; } if (udev->speed >= USB_SPEED_SUPER) { if (usb_endpoint_dir_out(&ep->desc)) - bw_index = (virt_dev->real_port - 1) * 2; + bw_index = (virt_dev->rhub_port->hw_portnum) * 2; else - bw_index = (virt_dev->real_port - 1) * 2 + 1; + bw_index = (virt_dev->rhub_port->hw_portnum) * 2 + 1; } else { /* add one more for each SS port */ - bw_index = virt_dev->real_port + xhci->usb3_rhub.num_ports - 1; + bw_index = virt_dev->rhub_port->hw_portnum + xhci->usb3_rhub.num_ports; } return &mtk->sch_array[bw_index]; diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index ac47b1c0544a..1740000d54c2 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -172,8 +172,8 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __field(void *, vdev) __field(unsigned long long, out_ctx) __field(unsigned long long, in_ctx) - __field(u8, fake_port) - __field(u8, real_port) + __field(int, hcd_portnum) + __field(int, hw_portnum) __field(u16, current_mel) ), @@ -181,13 +181,13 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev, __entry->vdev = vdev; __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; - __entry->fake_port = (u8) vdev->fake_port; - __entry->real_port = (u8) vdev->real_port; + __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum; + __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum; __entry->current_mel = (u16) vdev->current_mel; ), - TP_printk("vdev %p ctx %llx | %llx fake_port %d real_port %d current_mel %d", + TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d", __entry->vdev, __entry->in_ctx, __entry->out_ctx, - __entry->fake_port, __entry->real_port, __entry->current_mel + __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel ) ); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index b405b8236134..c50d5881e214 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2273,7 +2273,7 @@ static int xhci_check_tt_bw_table(struct xhci_hcd *xhci, struct xhci_tt_bw_info *tt_info; /* Find the bandwidth table for the root port this TT is attached to. */ - bw_table = &xhci->rh_bw[virt_dev->real_port - 1].bw_table; + bw_table = &xhci->rh_bw[virt_dev->rhub_port->hw_portnum].bw_table; tt_info = virt_dev->tt_info; /* If this TT already had active endpoints, the bandwidth for this TT * has already been added. Removing all periodic endpoints (and thus @@ -2391,7 +2391,7 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci, if (virt_dev->tt_info) { xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "Recalculating BW for rootport %u", - virt_dev->real_port); + virt_dev->rhub_port->hw_portnum + 1); if (xhci_check_tt_bw_table(xhci, virt_dev, old_active_eps)) { xhci_warn(xhci, "Not enough bandwidth on HS bus for " "newly activated TT.\n"); @@ -2404,7 +2404,7 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci, } else { xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "Recalculating BW for rootport %u", - virt_dev->real_port); + virt_dev->rhub_port->hw_portnum + 1); } /* Add in how much bandwidth will be used for interval zero, or the @@ -2501,14 +2501,12 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci, bw_used += overhead + packet_size; if (!virt_dev->tt_info && virt_dev->udev->speed == USB_SPEED_HIGH) { - unsigned int port_index = virt_dev->real_port - 1; - /* OK, we're manipulating a HS device attached to a * root port bandwidth domain. Include the number of active TTs * in the bandwidth used. */ bw_used += TT_HS_OVERHEAD * - xhci->rh_bw[port_index].num_active_tts; + xhci->rh_bw[virt_dev->rhub_port->hw_portnum].num_active_tts; } xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, @@ -2695,7 +2693,7 @@ void xhci_update_tt_active_eps(struct xhci_hcd *xhci, if (!virt_dev->tt_info) return; - rh_bw_info = &xhci->rh_bw[virt_dev->real_port - 1]; + rh_bw_info = &xhci->rh_bw[virt_dev->rhub_port->hw_portnum]; if (old_active_eps == 0 && virt_dev->tt_info->active_eps != 0) { rh_bw_info->num_active_tts += 1; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 6e09b9130fae..634ab517a776 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -739,8 +739,7 @@ struct xhci_virt_device { /* Used for addressing devices and configuration changes */ struct xhci_container_ctx *in_ctx; struct xhci_virt_ep eps[EP_CTX_PER_DEV]; - u8 fake_port; - u8 real_port; + struct xhci_port *rhub_port; struct xhci_interval_bw_table *bw_table; struct xhci_tt_bw_info *tt_info; /*