Re: [PATCH 3/7] usb: xhci: improve Stream Context register debugging

[Michal Pecio <michal.pecio@gmail.com>]

On Wed,  3 Sep 2025 19:01:23 +0200, Niklas Neronin wrote:
> Improve the debugging output for Stream Context registers in the xHCI
> driver. The Stream Context registers consist of the following fields:
>  bit 0 - Dequeue Cycle State.
>  bits 3:1 - Stream Context Type.
>  bits 63:4 - TR Dequeue Pointer, is 16-byte aligned.
> 
> Instead of printing the entire 64-bit register as a single block, each
> field is now printed separately. This approach enhances the readability.
> 
> Remove xhci_dbg() in xhci_alloc_stream_info(). A detailed trace message is
> printed after xhci_update_stream_mapping() call.
> 
> xHCI specification, section 6.2.4.1.
> 
> Why not use 'dma_addr_t' for the address?
> The 'dma_addr_t' type can vary between 32 and 64 bits depending on the
> system architecture or xHCI driver flags, whereas the 64-bit address field
> size remains constant. Since hardware cannot be fully trusted, it's better
> to print the entire 64-bit address to detect any non-zero values in the
> upper 32 bits. This approach ensures that potential issues are easily
> detected.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-debugfs.c | 16 ++++++++++------
>  drivers/usb/host/xhci-mem.c     |  1 -
>  drivers/usb/host/xhci-trace.h   |  5 +++--
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
> index c6d44977193f..35398b95c5a2 100644
> --- a/drivers/usb/host/xhci-debugfs.c
> +++ b/drivers/usb/host/xhci-debugfs.c
> @@ -521,6 +521,7 @@ static int xhci_stream_context_array_show(struct seq_file *s, void *unused)
>  	struct xhci_ep_priv	*epriv = s->private;
>  	struct xhci_stream_ctx	*stream_ctx;
>  	dma_addr_t		dma;
> +	u64			ctx;
>  	int			id;
>  
>  	if (!epriv->stream_info)
> @@ -533,12 +534,15 @@ static int xhci_stream_context_array_show(struct seq_file *s, void *unused)
>  	for (id = 0; id < epriv->stream_info->num_stream_ctxs; id++) {
>  		stream_ctx = epriv->stream_info->stream_ctx_array + id;
>  		dma = epriv->stream_info->ctx_array_dma + id * 16;
> -		if (id < epriv->stream_info->num_streams)
> -			seq_printf(s, "%pad stream id %d deq %016llx\n", &dma,
> -				   id, le64_to_cpu(stream_ctx->stream_ring));
> -		else
> -			seq_printf(s, "%pad stream context entry not used deq %016llx\n",
> -				   &dma, le64_to_cpu(stream_ctx->stream_ring));
> +
> +		if (id < epriv->stream_info->num_streams) {
> +			ctx = le64_to_cpu(stream_ctx->stream_ring);
> +			seq_printf(s, "%pad stream %d: deq %016llx SCT %llu cycle %llu\n",
> +				   &dma, id, ctx & TR_DEQ_PTR_MASK, CTX_TO_SCT(ctx),
> +				   ctx & EP_CTX_CYCLE_MASK);

That SCT could benefit from decoding to human readable form,
but AFAIK the driver currently isn't using it anyway, so it
doesn't matter very much.

> +		} else {
> +			seq_printf(s, "%pad stream %d: entry not used\n", &dma, id);
> +		}
>  	}
>  
>  	return 0;
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 2a414dee7233..9520e7c6e774 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -676,7 +676,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
>  			cur_ring->cycle_state;
>  		stream_info->stream_ctx_array[cur_stream].stream_ring =
>  			cpu_to_le64(addr);
> -		xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n", cur_stream, addr);
>  
>  		ret = xhci_update_stream_mapping(cur_ring, mem_flags);
>  
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 8451e9386aa9..f6a2b4cedb8d 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -329,9 +329,10 @@ DECLARE_EVENT_CLASS(xhci_log_stream_ctx,
>  		__entry->ctx_array_dma = info->ctx_array_dma + stream_id * 16;
>  
>  	),
> -	TP_printk("stream %u ctx @%pad: SCT %llu deq %llx", __entry->stream_id,
> +	TP_printk("stream %u ctx @%pad: SCT %llu deq %llx cycle %llu", __entry->stream_id,
>  		&__entry->ctx_array_dma, CTX_TO_SCT(__entry->stream_ring),
> -		__entry->stream_ring
> +		__entry->stream_ring & TR_DEQ_PTR_MASK,
> +		__entry->stream_ring & EP_CTX_CYCLE_MASK
>  	)
>  );
>  
> -- 
> 2.50.1
>