The branch main has been updated by seuros: URL: https://cgit.FreeBSD.org/src/commit/?id=a14a81016b14b54a9cd660bb76ef505c6f70dd73
commit a14a81016b14b54a9cd660bb76ef505c6f70dd73 Author: Abdelkader Boudih <[email protected]> AuthorDate: 2026-09-08 00:19:36 +0000 Commit: Abdelkader Boudih <[email protected]> CommitDate: 2026-09-08 00:19:36 +0000 bce_vhci: fix duplicate endpoint queue registration, ratelimit failure logging bce_vhci_endpoint_create() was registering both the IN and OUT submission queues on every call, regardless of which direction the endpoint actually needed. The per-stage firmware registration failures weren't rate-limited, so a stuck registration will spam the console. Tested on: MacBookPro15,2 Macmini8,1 MacBookPro15,3 MacBookPro16,2 Reviewed by: adrian Differential Revision: https://reviews.freebsd.org/D58831 --- sys/dev/apple_bce/apple_bce_vhci.c | 220 ++++++++++++++++++++++--------------- sys/dev/apple_bce/apple_bce_vhci.h | 2 + 2 files changed, 135 insertions(+), 87 deletions(-) diff --git a/sys/dev/apple_bce/apple_bce_vhci.c b/sys/dev/apple_bce/apple_bce_vhci.c index e8bef8566cd6..e91ba8b37dc8 100644 --- a/sys/dev/apple_bce/apple_bce_vhci.c +++ b/sys/dev/apple_bce/apple_bce_vhci.c @@ -31,6 +31,7 @@ #include <sys/sema.h> #include <sys/taskqueue.h> +#include <sys/time.h> #include <sys/endian.h> #include <machine/bus.h> #include <machine/atomic.h> @@ -195,12 +196,7 @@ static int bce_vhci_cmd_execute(struct bce_vhci_softc *vhci, struct bce_vhci_message *req, struct bce_vhci_message *reply, int timeout_ticks); -/* - * Convert USB endpoint address to tq[] index. - * ep0 (0x00) maps to index 0. For other endpoints, IN and OUT get - * separate slots: OUT 0x01 -> 1, IN 0x81 -> 2, OUT 0x02 -> 3, etc. - * Maximum index is 30 (ep 0x8F), fits in BCE_VHCI_MAX_ENDPOINTS (32). - */ +/* Convert USB endpoint address to tq[] index -- EP0 shared, others per-direction. */ static inline uint8_t bce_vhci_ep_index(uint8_t ep_addr) { @@ -1185,12 +1181,12 @@ bce_vhci_create_task(void *arg, int pending __unused) usbd_transfer_done(xfer, USB_ERR_STALLED); USB_BUS_UNLOCK(&vhci->sc_bus); - device_printf(vhci->sc_dev, - "create_task: ep create " - "failed: dev=%d ep=0x%02x " - "err=%d\n", - dev->fw_dev_id, ep_addr, - ep_err); + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "endpoint create failed: " + "dev=%d ep=0x%02x err=%d\n", + dev->fw_dev_id, ep_addr, ep_err); continue; } @@ -1980,12 +1976,20 @@ bce_vhci_endpoint_create(struct bce_vhci_softc *vhci, char name[0x20]; uint32_t status; int error, cq_qid, out_qid, in_qid, i; - uint8_t ep_idx; + uint8_t ep_idx, ep_num; + int want_out, want_in; ep_idx = bce_vhci_ep_index(ep_addr); if (ep_idx >= BCE_VHCI_MAX_ENDPOINTS) return (EINVAL); + /* EP0 needs both SQs; other endpoints only need their own direction's. */ + ep_num = ep_addr & 0x0F; + want_out = (ep_num == 0) || ((ep_addr & 0x80) == 0); + want_in = (ep_num == 0) || ((ep_addr & 0x80) != 0); + out_qid = -1; + in_qid = -1; + tq = &dev->tq[ep_idx]; if (tq->active) return (EEXIST); @@ -2069,6 +2073,12 @@ bce_vhci_endpoint_create(struct bce_vhci_softc *vhci, cfg.vector_or_cq = 4; status = bce_cmd_register_queue(sc->sc_cmd_cmdq, sc, &cfg, NULL, 0); if (status != 0) { + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "failed to register CQ dev=%d ep=0x%02x " + "qid=%d: %u\n", + dev->fw_dev_id, ep_addr, cq_qid, status); error = EIO; goto fail_cq; } @@ -2088,74 +2098,92 @@ bce_vhci_endpoint_create(struct bce_vhci_softc *vhci, if (inserted == 0) { sc->sc_queues[cq_qid] = NULL; mtx_unlock(&sc->sc_queues_lock); - device_printf(vhci->sc_dev, - "CQ list full, cannot add endpoint CQ\n"); + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "CQ list full, cannot add endpoint " + "CQ\n"); error = ENOSPC; goto fail_cq_reg; } } mtx_unlock(&sc->sc_queues_lock); - /* Allocate OUT SQ (host -> device) */ - out_qid = bce_vhci_alloc_qid(vhci); - if (out_qid < 0) { - error = ENOSPC; - goto fail_cq_reg; - } - tq->sq_out = bce_alloc_sq(sc, out_qid, - sizeof(struct bce_qe_submission), BCE_VHCI_TQ_EL, - bce_vhci_tq_completion, tq); - if (tq->sq_out == NULL) { - error = ENOMEM; - goto fail_sq_out_alloc; - } + /* Allocate OUT SQ (host -> device), only if this direction needs it */ + if (want_out) { + out_qid = bce_vhci_alloc_qid(vhci); + if (out_qid < 0) { + error = ENOSPC; + goto fail_cq_reg; + } + tq->sq_out = bce_alloc_sq(sc, out_qid, + sizeof(struct bce_qe_submission), BCE_VHCI_TQ_EL, + bce_vhci_tq_completion, tq); + if (tq->sq_out == NULL) { + error = ENOMEM; + goto fail_sq_out_alloc; + } - snprintf(name, sizeof(name), "VHC1-%d-%02x", - dev->fw_dev_id, ep_addr & 0x0F); - bce_get_sq_memcfg(tq->sq_out, tq->cq, &cfg); - status = bce_cmd_register_queue(sc->sc_cmd_cmdq, sc, &cfg, name, 1); - if (status != 0) { - device_printf(vhci->sc_dev, - "failed to register OUT SQ '%s': %u\n", name, status); - error = EIO; - goto fail_sq_out; + snprintf(name, sizeof(name), "VHC1-%d-%02x", + dev->fw_dev_id, ep_addr & 0x0F); + bce_get_sq_memcfg(tq->sq_out, tq->cq, &cfg); + status = bce_cmd_register_queue(sc->sc_cmd_cmdq, sc, &cfg, + name, 1); + if (status != 0) { + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "failed to register OUT SQ '%s' " + "qid=%d cq_qid=%d: %u\n", name, + out_qid, cq_qid, status); + error = EIO; + goto fail_sq_out; + } + + mtx_lock(&sc->sc_queues_lock); + sc->sc_queues[out_qid] = tq->sq_out; + sc->sc_int_sq_list[out_qid] = tq->sq_out; + mtx_unlock(&sc->sc_queues_lock); } - mtx_lock(&sc->sc_queues_lock); - sc->sc_queues[out_qid] = tq->sq_out; - sc->sc_int_sq_list[out_qid] = tq->sq_out; - mtx_unlock(&sc->sc_queues_lock); + /* Allocate IN SQ (device -> host), only if this direction needs it */ + if (want_in) { + in_qid = bce_vhci_alloc_qid(vhci); + if (in_qid < 0) { + error = ENOSPC; + goto fail_sq_out_reg; + } + tq->sq_in = bce_alloc_sq(sc, in_qid, + sizeof(struct bce_qe_submission), BCE_VHCI_TQ_EL, + bce_vhci_tq_completion, tq); + if (tq->sq_in == NULL) { + error = ENOMEM; + goto fail_sq_in_alloc; + } - /* Allocate IN SQ (device -> host) */ - in_qid = bce_vhci_alloc_qid(vhci); - if (in_qid < 0) { - error = ENOSPC; - goto fail_sq_out_reg; - } - tq->sq_in = bce_alloc_sq(sc, in_qid, - sizeof(struct bce_qe_submission), BCE_VHCI_TQ_EL, - bce_vhci_tq_completion, tq); - if (tq->sq_in == NULL) { - error = ENOMEM; - goto fail_sq_in_alloc; - } + snprintf(name, sizeof(name), "VHC1-%d-%02x", + dev->fw_dev_id, ep_addr | 0x80); + bce_get_sq_memcfg(tq->sq_in, tq->cq, &cfg); + status = bce_cmd_register_queue(sc->sc_cmd_cmdq, sc, &cfg, + name, 0); + if (status != 0) { + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "failed to register IN SQ '%s' " + "qid=%d cq_qid=%d out_qid=%d: %u\n", + name, in_qid, cq_qid, out_qid, + status); + error = EIO; + goto fail_sq_in; + } - snprintf(name, sizeof(name), "VHC1-%d-%02x", - dev->fw_dev_id, ep_addr | 0x80); - bce_get_sq_memcfg(tq->sq_in, tq->cq, &cfg); - status = bce_cmd_register_queue(sc->sc_cmd_cmdq, sc, &cfg, name, 0); - if (status != 0) { - device_printf(vhci->sc_dev, - "failed to register IN SQ '%s': %u\n", name, status); - error = EIO; - goto fail_sq_in; + mtx_lock(&sc->sc_queues_lock); + sc->sc_queues[in_qid] = tq->sq_in; + sc->sc_int_sq_list[in_qid] = tq->sq_in; + mtx_unlock(&sc->sc_queues_lock); } - mtx_lock(&sc->sc_queues_lock); - sc->sc_queues[in_qid] = tq->sq_in; - sc->sc_int_sq_list[in_qid] = tq->sq_in; - mtx_unlock(&sc->sc_queues_lock); - /* Tell firmware to create the endpoint */ memset(&cmd, 0, sizeof(cmd)); cmd.cmd = BCE_VHCI_CMD_ENDPOINT_CREATE; @@ -2183,9 +2211,12 @@ bce_vhci_endpoint_create(struct bce_vhci_softc *vhci, error = bce_vhci_cmd_execute(vhci, &cmd, &reply, BCE_VHCI_CMD_TIMEOUT_SHORT); if (error != 0) { - device_printf(vhci->sc_dev, - "ENDPOINT_CREATE(dev=%d, ep=0x%02x) failed: %d\n", - dev->fw_dev_id, ep_addr, error); + if (ppsratecheck(&tq->create_fail_lastprint, + &tq->create_fail_pps, 1)) + device_printf(vhci->sc_dev, + "ENDPOINT_CREATE(dev=%d, ep=0x%02x) " + "failed: %d\n", + dev->fw_dev_id, ep_addr, error); goto fail_sq_in_reg; } @@ -2199,27 +2230,37 @@ bce_vhci_endpoint_create(struct bce_vhci_softc *vhci, return (0); fail_sq_in_reg: - bce_cmd_unregister_queue(sc->sc_cmd_cmdq, sc, in_qid); - mtx_lock(&sc->sc_queues_lock); - sc->sc_queues[in_qid] = NULL; - sc->sc_int_sq_list[in_qid] = NULL; - mtx_unlock(&sc->sc_queues_lock); + if (want_in) { + bce_cmd_unregister_queue(sc->sc_cmd_cmdq, sc, in_qid); + mtx_lock(&sc->sc_queues_lock); + sc->sc_queues[in_qid] = NULL; + sc->sc_int_sq_list[in_qid] = NULL; + mtx_unlock(&sc->sc_queues_lock); + } fail_sq_in: - bce_free_sq(sc, tq->sq_in); - tq->sq_in = NULL; + if (want_in) { + bce_free_sq(sc, tq->sq_in); + tq->sq_in = NULL; + } fail_sq_in_alloc: - bce_vhci_free_qid(vhci, in_qid); + if (in_qid >= 0) + bce_vhci_free_qid(vhci, in_qid); fail_sq_out_reg: - bce_cmd_unregister_queue(sc->sc_cmd_cmdq, sc, out_qid); - mtx_lock(&sc->sc_queues_lock); - sc->sc_queues[out_qid] = NULL; - sc->sc_int_sq_list[out_qid] = NULL; - mtx_unlock(&sc->sc_queues_lock); + if (want_out) { + bce_cmd_unregister_queue(sc->sc_cmd_cmdq, sc, out_qid); + mtx_lock(&sc->sc_queues_lock); + sc->sc_queues[out_qid] = NULL; + sc->sc_int_sq_list[out_qid] = NULL; + mtx_unlock(&sc->sc_queues_lock); + } fail_sq_out: - bce_free_sq(sc, tq->sq_out); - tq->sq_out = NULL; + if (want_out) { + bce_free_sq(sc, tq->sq_out); + tq->sq_out = NULL; + } fail_sq_out_alloc: - bce_vhci_free_qid(vhci, out_qid); + if (out_qid >= 0) + bce_vhci_free_qid(vhci, out_qid); fail_cq_reg: bce_cmd_unregister_queue(sc->sc_cmd_cmdq, sc, cq_qid); mtx_lock(&sc->sc_queues_lock); @@ -2436,6 +2477,11 @@ bce_vhci_device_create(struct bce_vhci_softc *vhci, uint8_t port) "device created: port=%d fw_dev_id=%d\n", port, fw_dev_id); dev = &vhci->sc_devs[fw_dev_id]; + if (dev->allocated != 0) + device_printf(vhci->sc_dev, + "device created: fw_dev_id=%d reused while still " + "marked allocated (old port=%d, new port=%d)\n", + fw_dev_id, dev->port, port); memset(dev, 0, sizeof(*dev)); dev->allocated = 1; dev->fw_dev_id = fw_dev_id; diff --git a/sys/dev/apple_bce/apple_bce_vhci.h b/sys/dev/apple_bce/apple_bce_vhci.h index c85dd326b277..71cba4a5fd32 100644 --- a/sys/dev/apple_bce/apple_bce_vhci.h +++ b/sys/dev/apple_bce/apple_bce_vhci.h @@ -207,6 +207,8 @@ struct bce_vhci_transfer_queue { int create_pending; /* Deferred ep create */ struct usb_endpoint_descriptor *create_edesc; struct usb_xfer *create_xfer; /* Deferred xfer */ + struct timeval create_fail_lastprint; + int create_fail_pps; /* DMA buffer for data transfer */ bus_dma_tag_t dma_tag;
