From 2eeba925149a3e56d2ffd169d29357f34bf4816a Mon Sep 17 00:00:00 2001 From: patrick Date: Wed, 25 Jul 2018 20:47:45 +0000 Subject: [PATCH] Implement a MSGBUF control packet mechanism based on the command request ids. So far we were only able to have one command in flight at a time and race conditions could easily lead to unexpected behaviour. With this rework we send and enqueue a control packet command and wait for replies to happen. Thus we can have multiple control packets in flight and a reply with the correct id will wake us up. --- sys/dev/pci/if_bwfm_pci.c | 164 ++++++++++++++++++++++++-------------- 1 file changed, 102 insertions(+), 62 deletions(-) diff --git a/sys/dev/pci/if_bwfm_pci.c b/sys/dev/pci/if_bwfm_pci.c index 704d9ffacc4..afeabceefe6 100644 --- a/sys/dev/pci/if_bwfm_pci.c +++ b/sys/dev/pci/if_bwfm_pci.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_bwfm_pci.c,v 1.25 2018/07/06 12:36:53 patrick Exp $ */ +/* $OpenBSD: if_bwfm_pci.c,v 1.26 2018/07/25 20:47:45 patrick Exp $ */ /* * Copyright (c) 2010-2016 Broadcom Corporation * Copyright (c) 2017 Patrick Wildt @@ -56,9 +56,11 @@ #define BWFM_NUM_TX_MSGRINGS 2 #define BWFM_NUM_RX_MSGRINGS 3 +#define BWFM_NUM_IOCTL_PKTIDS 8 #define BWFM_NUM_TX_PKTIDS 2048 #define BWFM_NUM_RX_PKTIDS 1024 +#define BWFM_NUM_IOCTL_DESCS 1 #define BWFM_NUM_TX_DESCS 1 #define BWFM_NUM_RX_DESCS 1 @@ -95,6 +97,14 @@ struct bwfm_pci_msgring { uint8_t mac[ETHER_ADDR_LEN]; }; +struct bwfm_pci_ioctl { + uint16_t transid; + uint16_t retlen; + int16_t status; + struct mbuf *m; + TAILQ_ENTRY(bwfm_pci_ioctl) next; +}; + struct bwfm_pci_buf { bus_dmamap_t bb_map; struct mbuf *bb_m; @@ -158,17 +168,14 @@ struct bwfm_pci_softc { struct bwfm_pci_dmamem *sc_scratch_buf; struct bwfm_pci_dmamem *sc_ringupd_buf; - struct bwfm_pci_dmamem *sc_ioctl_buf; - int sc_ioctl_reqid; - uint32_t sc_ioctl_resp_pktid; - uint32_t sc_ioctl_resp_ret_len; - uint32_t sc_ioctl_resp_status; - int sc_ioctl_poll; + TAILQ_HEAD(, bwfm_pci_ioctl) sc_ioctlq; + uint16_t sc_ioctl_transid; struct if_rxring sc_ioctl_ring; struct if_rxring sc_event_ring; struct if_rxring sc_rxbuf_ring; + struct bwfm_pci_pkts sc_ioctl_pkts; struct bwfm_pci_pkts sc_rx_pkts; struct bwfm_pci_pkts sc_tx_pkts; int sc_tx_pkts_full; @@ -271,6 +278,8 @@ int bwfm_pci_msgbuf_query_dcmd(struct bwfm_softc *, int, int, char *, size_t *); int bwfm_pci_msgbuf_set_dcmd(struct bwfm_softc *, int, int, char *, size_t); +void bwfm_pci_msgbuf_rxioctl(struct bwfm_pci_softc *, + struct msgbuf_ioctl_resp_hdr *); struct bwfm_buscore_ops bwfm_pci_buscore_ops = { .bc_read = bwfm_pci_buscore_read, @@ -632,10 +641,6 @@ bwfm_pci_preinit(struct bwfm_softc *bwfm) sc->sc_shared_address + BWFM_SHARED_DMA_RINGUPD_LEN, BWFM_DMA_D2H_RINGUPD_BUF_LEN); - if ((sc->sc_ioctl_buf = bwfm_pci_dmamem_alloc(sc, - BWFM_DMA_H2D_IOCTL_BUF_LEN, 8)) == NULL) - goto cleanup; - bwfm_pci_select_core(sc, BWFM_AGENT_CORE_PCIE2); bwfm_pci_intr_enable(sc); @@ -657,6 +662,15 @@ bwfm_pci_preinit(struct bwfm_softc *bwfm) BWFM_NUM_TX_DESCS, MSGBUF_MAX_PKT_SIZE, 0, BUS_DMA_WAITOK, &sc->sc_tx_pkts.pkts[i].bb_map); + /* Maps IOCTL mbufs to a packet id and back. */ + sc->sc_ioctl_pkts.npkt = BWFM_NUM_IOCTL_PKTIDS; + sc->sc_ioctl_pkts.pkts = malloc(BWFM_NUM_IOCTL_PKTIDS + * sizeof(struct bwfm_pci_buf), M_DEVBUF, M_WAITOK | M_ZERO); + for (i = 0; i < BWFM_NUM_IOCTL_PKTIDS; i++) + bus_dmamap_create(sc->sc_dmat, MSGBUF_MAX_PKT_SIZE, + BWFM_NUM_IOCTL_DESCS, MSGBUF_MAX_PKT_SIZE, 0, BUS_DMA_WAITOK, + &sc->sc_ioctl_pkts.pkts[i].bb_map); + /* * For whatever reason, could also be a bug somewhere in this * driver, the firmware needs a bunch of RX buffers otherwise @@ -668,6 +682,8 @@ bwfm_pci_preinit(struct bwfm_softc *bwfm) if_rxr_init(&sc->sc_event_ring, 8, 8); bwfm_pci_fill_rx_rings(sc); + TAILQ_INIT(&sc->sc_ioctlq); + #ifdef BWFM_DEBUG sc->sc_console_readidx = 0; bwfm_pci_debug_console(sc); @@ -677,8 +693,6 @@ bwfm_pci_preinit(struct bwfm_softc *bwfm) return 0; cleanup: - if (sc->sc_ioctl_buf) - bwfm_pci_dmamem_free(sc, sc->sc_ioctl_buf); if (sc->sc_ringupd_buf) bwfm_pci_dmamem_free(sc, sc->sc_ringupd_buf); if (sc->sc_scratch_buf) @@ -771,7 +785,6 @@ bwfm_pci_detach(struct device *self, int flags) /* FIXME: free TX buffers */ /* FIXME: free more memory */ - bwfm_pci_dmamem_free(sc, sc->sc_ioctl_buf); bwfm_pci_dmamem_free(sc, sc->sc_ringupd_buf); bwfm_pci_dmamem_free(sc, sc->sc_scratch_buf); bwfm_pci_dmamem_free(sc, sc->sc_rx_complete.ring); @@ -1335,15 +1348,17 @@ bwfm_pci_msg_rx(struct bwfm_pci_softc *sc, void *buf) ring->status = RING_CLOSED; break; case MSGBUF_TYPE_IOCTLPTR_REQ_ACK: + m = bwfm_pci_pktid_free(sc, &sc->sc_ioctl_pkts, + letoh32(msg->request_id)); + if (m == NULL) + break; + m_freem(m); break; case MSGBUF_TYPE_IOCTL_CMPLT: resp = (struct msgbuf_ioctl_resp_hdr *)buf; - sc->sc_ioctl_resp_pktid = letoh32(resp->msg.request_id); - sc->sc_ioctl_resp_ret_len = letoh16(resp->resp_len); - sc->sc_ioctl_resp_status = letoh16(resp->compl_hdr.status); + bwfm_pci_msgbuf_rxioctl(sc, resp); if_rxr_put(&sc->sc_ioctl_ring, 1); bwfm_pci_fill_rx_rings(sc); - wakeup(&sc->sc_ioctl_buf); break; case MSGBUF_TYPE_WL_EVENT: event = (struct msgbuf_rx_event *)buf; @@ -1898,71 +1913,73 @@ bwfm_pci_msgbuf_query_dcmd(struct bwfm_softc *bwfm, int ifidx, { struct bwfm_pci_softc *sc = (void *)bwfm; struct msgbuf_ioctl_req_hdr *req; + struct bwfm_pci_ioctl *ctl; struct mbuf *m; + uint32_t pktid; + paddr_t paddr; size_t buflen; - int s; - s = splnet(); - sc->sc_ioctl_resp_pktid = -1; + buflen = min(*len, BWFM_DMA_H2D_IOCTL_BUF_LEN); + m = MCLGETI(NULL, M_DONTWAIT, NULL, buflen); + if (m == NULL) + return 1; + m->m_len = m->m_pkthdr.len = buflen; + + if (buf) + memcpy(mtod(m, char *), buf, buflen); + else + memset(mtod(m, char *), 0, buflen); + req = bwfm_pci_ring_write_reserve(sc, &sc->sc_ctrl_submit); if (req == NULL) { - printf("%s: cannot reserve for write\n", DEVNAME(sc)); - splx(s); + m_freem(m); return 1; } + + if (bwfm_pci_pktid_new(sc, &sc->sc_ioctl_pkts, m, &pktid, &paddr)) { + bwfm_pci_ring_write_cancel(sc, &sc->sc_ctrl_submit, 1); + m_freem(m); + return 1; + } + + ctl = malloc(sizeof(*ctl), M_TEMP, M_WAITOK|M_ZERO); + ctl->transid = sc->sc_ioctl_transid++; + TAILQ_INSERT_TAIL(&sc->sc_ioctlq, ctl, next); + req->msg.msgtype = MSGBUF_TYPE_IOCTLPTR_REQ; req->msg.ifidx = 0; req->msg.flags = 0; - req->msg.request_id = htole32(MSGBUF_IOCTL_REQ_PKTID); + req->msg.request_id = htole32(pktid); req->cmd = htole32(cmd); req->output_buf_len = htole16(*len); - req->trans_id = htole16(sc->sc_ioctl_reqid++); + req->trans_id = htole16(ctl->transid); - buflen = min(*len, BWFM_DMA_H2D_IOCTL_BUF_LEN); - req->input_buf_len = htole16(buflen); - req->req_buf_addr.high_addr = - htole32((uint64_t)BWFM_PCI_DMA_DVA(sc->sc_ioctl_buf) >> 32); - req->req_buf_addr.low_addr = - htole32((uint64_t)BWFM_PCI_DMA_DVA(sc->sc_ioctl_buf) & 0xffffffff); - if (buf) - memcpy(BWFM_PCI_DMA_KVA(sc->sc_ioctl_buf), buf, buflen); - else - memset(BWFM_PCI_DMA_KVA(sc->sc_ioctl_buf), 0, buflen); - - bus_dmamap_sync(sc->sc_dmat, BWFM_PCI_DMA_MAP(sc->sc_ioctl_buf), - 0, buflen, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); + req->input_buf_len = htole16(m->m_len); + req->req_buf_addr.high_addr = htole32((uint64_t)paddr >> 32); + req->req_buf_addr.low_addr = htole32(paddr & 0xffffffff); bwfm_pci_ring_write_commit(sc, &sc->sc_ctrl_submit); - splx(s); - if (sc->sc_ioctl_poll) { - int i; - for (i = 0; i < 100; i++) { - if (sc->sc_ioctl_resp_pktid != -1) - break; - delay(10 * 1000); - } - if (i == 100) { - printf("%s: timeout waiting for ioctl response\n", - DEVNAME(sc)); - return 1; - } - } else if (tsleep(&sc->sc_ioctl_buf, PCATCH, "bwfm", hz)) { - printf("%s: timeout waiting for ioctl response\n", - DEVNAME(sc)); - return 1; - } + tsleep(ctl, PWAIT, "bwfm", hz); + TAILQ_REMOVE(&sc->sc_ioctlq, ctl, next); - m = bwfm_pci_pktid_free(sc, &sc->sc_rx_pkts, sc->sc_ioctl_resp_pktid); - if (m == NULL) + if (ctl->m == NULL) { + free(ctl, M_TEMP, sizeof(*ctl)); return 1; + } - *len = min(buflen, sc->sc_ioctl_resp_ret_len); + *len = min(ctl->retlen, m->m_len); + *len = min(*len, buflen); if (buf) - memcpy(buf, mtod(m, char *), *len); - m_freem(m); - splx(s); + m_copydata(ctl->m, 0, *len, (caddr_t)buf); + m_freem(ctl->m); + if (ctl->status < 0) { + free(ctl, M_TEMP, sizeof(*ctl)); + return 1; + } + + free(ctl, M_TEMP, sizeof(*ctl)); return 0; } @@ -1972,3 +1989,26 @@ bwfm_pci_msgbuf_set_dcmd(struct bwfm_softc *bwfm, int ifidx, { return bwfm_pci_msgbuf_query_dcmd(bwfm, ifidx, cmd, buf, &len); } + +void +bwfm_pci_msgbuf_rxioctl(struct bwfm_pci_softc *sc, + struct msgbuf_ioctl_resp_hdr *resp) +{ + struct bwfm_pci_ioctl *ctl, *tmp; + struct mbuf *m; + + m = bwfm_pci_pktid_free(sc, &sc->sc_rx_pkts, + letoh32(resp->msg.request_id)); + + TAILQ_FOREACH_SAFE(ctl, &sc->sc_ioctlq, next, tmp) { + if (ctl->transid != letoh16(resp->trans_id)) + continue; + ctl->m = m; + ctl->retlen = letoh16(resp->resp_len); + ctl->status = letoh16(resp->compl_hdr.status); + wakeup(ctl); + return; + } + + m_free(m); +} -- 2.20.1