| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGCq0LYh13qaPpuM0jPyu2LV+EXqJrfoKvD-TOuhkZYScFnTNg@mail.gmail.com>
Date: Fri, 28 Mar 2025 15:42:00 +0800
From: Puma Hsu <pumahsu@...gle.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>
Cc: srinivas.kandagatla@...aro.org, mathias.nyman@...el.com, perex@...ex.cz,
conor+dt@...nel.org, dmitry.torokhov@...il.com, corbet@....net,
broonie@...nel.org, lgirdwood@...il.com, krzk+dt@...nel.org,
pierre-louis.bossart@...ux.intel.com, Thinh.Nguyen@...opsys.com,
tiwai@...e.com, robh@...nel.org, gregkh@...uxfoundation.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-sound@...r.kernel.org, linux-input@...r.kernel.org,
linux-usb@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v36 02/31] usb: host: xhci-mem: Cleanup pending secondary
event ring events
On Wed, Mar 19, 2025 at 8:53 AM Wesley Cheng <quic_wcheng@...cinc.com> wrote:
>
> As part of xHCI bus suspend, the xHCI is halted. However, if there are
> pending events in the secondary event ring, it is observed that the xHCI
> controller stops responding to further commands upon host or device
> initiated bus resume. Iterate through all pending events and update the
> dequeue pointer to the beginning of the event ring.
>
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
Tested-by: Puma Hsu <pumahsu@...gle.com>
> ---
> drivers/usb/host/xhci-mem.c | 7 +++++-
> drivers/usb/host/xhci-ring.c | 47 ++++++++++++++++++++++++++++++------
> drivers/usb/host/xhci.c | 2 +-
> drivers/usb/host/xhci.h | 7 ++++++
> 4 files changed, 54 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index d698095fc88d..daea0f76e844 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1805,7 +1805,7 @@ xhci_remove_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
> tmp &= ERST_SIZE_MASK;
> writel(tmp, &ir->ir_set->erst_size);
>
> - xhci_write_64(xhci, ERST_EHB, &ir->ir_set->erst_dequeue);
> + xhci_update_erst_dequeue(xhci, ir, true);
> }
> }
>
> @@ -1848,6 +1848,11 @@ void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrup
> return;
> }
>
> + /*
> + * Cleanup secondary interrupter to ensure there are no pending events.
> + * This also updates event ring dequeue pointer back to the start.
> + */
> + xhci_skip_sec_intr_events(xhci, ir->event_ring, ir);
> intr_num = ir->intr_num;
>
> xhci_remove_interrupter(xhci, ir);
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5d64c297721c..bfef765dd78c 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3054,9 +3054,9 @@ static int xhci_handle_event_trb(struct xhci_hcd *xhci, struct xhci_interrupter
> * - When all events have finished
> * - To avoid "Event Ring Full Error" condition
> */
> -static void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
> - struct xhci_interrupter *ir,
> - bool clear_ehb)
> +void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
> + struct xhci_interrupter *ir,
> + bool clear_ehb)
> {
> u64 temp_64;
> dma_addr_t deq;
> @@ -3099,10 +3099,11 @@ static void xhci_clear_interrupt_pending(struct xhci_interrupter *ir)
> * Handle all OS-owned events on an interrupter event ring. It may drop
> * and reaquire xhci->lock between event processing.
> */
> -static int xhci_handle_events(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
> +static int xhci_handle_events(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
> + bool skip_events)
> {
> int event_loop = 0;
> - int err;
> + int err = 0;
> u64 temp;
>
> xhci_clear_interrupt_pending(ir);
> @@ -3125,7 +3126,8 @@ static int xhci_handle_events(struct xhci_hcd *xhci, struct xhci_interrupter *ir
>
> /* Process all OS owned event TRBs on this event ring */
> while (unhandled_event_trb(ir->event_ring)) {
> - err = xhci_handle_event_trb(xhci, ir, ir->event_ring->dequeue);
> + if (!skip_events)
> + err = xhci_handle_event_trb(xhci, ir, ir->event_ring->dequeue);
>
> /*
> * If half a segment of events have been handled in one go then
> @@ -3152,6 +3154,37 @@ static int xhci_handle_events(struct xhci_hcd *xhci, struct xhci_interrupter *ir
> return 0;
> }
>
> +/*
> + * Move the event ring dequeue pointer to skip events kept in the secondary
> + * event ring. This is used to ensure that pending events in the ring are
> + * acknowledged, so the xHCI HCD can properly enter suspend/resume. The
> + * secondary ring is typically maintained by an external component.
> + */
> +void xhci_skip_sec_intr_events(struct xhci_hcd *xhci,
> + struct xhci_ring *ring, struct xhci_interrupter *ir)
> +{
> + union xhci_trb *current_trb;
> + u64 erdp_reg;
> + dma_addr_t deq;
> +
> + /* disable irq, ack pending interrupt and ack all pending events */
> + xhci_disable_interrupter(ir);
> +
> + /* last acked event trb is in erdp reg */
> + erdp_reg = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
> + deq = (dma_addr_t)(erdp_reg & ERST_PTR_MASK);
> + if (!deq) {
> + xhci_err(xhci, "event ring handling not required\n");
> + return;
> + }
> +
> + current_trb = ir->event_ring->dequeue;
> + /* read cycle state of the last acked trb to find out CCS */
> + ring->cycle_state = le32_to_cpu(current_trb->event_cmd.flags) & TRB_CYCLE;
> +
> + xhci_handle_events(xhci, ir, true);
> +}
> +
> /*
> * xHCI spec says we can get an interrupt, and if the HC has an error condition,
> * we might get bad data out of the event ring. Section 4.10.2.7 has a list of
> @@ -3196,7 +3229,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
> writel(status, &xhci->op_regs->status);
>
> /* This is the handler of the primary interrupter */
> - xhci_handle_events(xhci, xhci->interrupters[0]);
> + xhci_handle_events(xhci, xhci->interrupters[0], false);
> out:
> spin_unlock(&xhci->lock);
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 83a4adf57bae..61950a350432 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -335,7 +335,7 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir)
> return 0;
> }
>
> -static int xhci_disable_interrupter(struct xhci_interrupter *ir)
> +int xhci_disable_interrupter(struct xhci_interrupter *ir)
> {
> u32 iman;
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 39db228f0b84..3fa8669e3b2d 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1856,6 +1856,9 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
> u32 imod_interval);
> void xhci_remove_secondary_interrupter(struct usb_hcd
> *hcd, struct xhci_interrupter *ir);
> +void xhci_skip_sec_intr_events(struct xhci_hcd *xhci,
> + struct xhci_ring *ring,
> + struct xhci_interrupter *ir);
>
> /* xHCI host controller glue */
> typedef void (*xhci_get_quirks_t)(struct device *, struct xhci_hcd *);
> @@ -1895,6 +1898,7 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci,
> struct usb_tt *tt, gfp_t mem_flags);
> int xhci_set_interrupter_moderation(struct xhci_interrupter *ir,
> u32 imod_interval);
> +int xhci_disable_interrupter(struct xhci_interrupter *ir);
>
> /* xHCI ring, segment, TRB, and TD functions */
> dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb);
> @@ -1939,6 +1943,9 @@ unsigned int count_trbs(u64 addr, u64 len);
> int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
> int suspend, gfp_t gfp_flags);
> void xhci_process_cancelled_tds(struct xhci_virt_ep *ep);
> +void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
> + struct xhci_interrupter *ir,
> + bool clear_ehb);
>
> /* xHCI roothub code */
> void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
>
Powered by blists - more mailing lists