From owner-svn-src-stable-8@FreeBSD.ORG Fri Mar 13 20:10:11 2015 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 840EBF37; Fri, 13 Mar 2015 20:10:11 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6D668EBA; Fri, 13 Mar 2015 20:10:11 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t2DKAB94080093; Fri, 13 Mar 2015 20:10:11 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t2DKAA1U080084; Fri, 13 Mar 2015 20:10:10 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201503132010.t2DKAA1U080084@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Fri, 13 Mar 2015 20:10:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r279964 - in stable: 8/sys/dev/ipmi 9/sys/dev/ipmi X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Mar 2015 20:10:11 -0000 Author: jhb Date: Fri Mar 13 20:10:09 2015 New Revision: 279964 URL: https://svnweb.freebsd.org/changeset/base/279964 Log: MFC 278321: Use direct hardware access for internal requests for KCS and SMIC. In particular, updates to the watchdog should no longer sleep. - Add a new IPMI_IO_LOCK for low-level I/O access. Use this for kcs_polled_request() and smic_polled_request(). - Add a new backend callback "ipmi_driver_request" to handle a driver request. The new callback performs the request sychronously for KCS and SMIC. SSIF still defers the work to the worker thread since the worker thread sleeps during request processing anyway. - Allocate driver requests on the stack rather than using malloc(). Modified: stable/8/sys/dev/ipmi/ipmi.c stable/8/sys/dev/ipmi/ipmi_kcs.c stable/8/sys/dev/ipmi/ipmi_smic.c stable/8/sys/dev/ipmi/ipmi_ssif.c stable/8/sys/dev/ipmi/ipmivars.h Directory Properties: stable/8/sys/ (props changed) stable/8/sys/dev/ (props changed) stable/8/sys/dev/ipmi/ (props changed) Changes in other areas also in this revision: Modified: stable/9/sys/dev/ipmi/ipmi.c stable/9/sys/dev/ipmi/ipmi_kcs.c stable/9/sys/dev/ipmi/ipmi_smic.c stable/9/sys/dev/ipmi/ipmi_ssif.c stable/9/sys/dev/ipmi/ipmivars.h Directory Properties: stable/9/sys/ (props changed) stable/9/sys/dev/ (props changed) Modified: stable/8/sys/dev/ipmi/ipmi.c ============================================================================== --- stable/8/sys/dev/ipmi/ipmi.c Fri Mar 13 20:08:35 2015 (r279963) +++ stable/8/sys/dev/ipmi/ipmi.c Fri Mar 13 20:10:09 2015 (r279964) @@ -49,6 +49,23 @@ __FBSDID("$FreeBSD$"); #include #endif +/* + * Driver request structures are allocated on the stack via alloca() to + * avoid calling malloc(), especially for the watchdog handler. + * To avoid too much stack growth, a previously allocated structure can + * be reused via IPMI_INIT_DRIVER_REQUEST(), but the caller should ensure + * that there is adequate reply/request space in the original allocation. + */ +#define IPMI_INIT_DRIVER_REQUEST(req, addr, cmd, reqlen, replylen) \ + bzero((req), sizeof(struct ipmi_request)); \ + ipmi_init_request((req), NULL, 0, (addr), (cmd), (reqlen), (replylen)) + +#define IPMI_ALLOC_DRIVER_REQUEST(req, addr, cmd, reqlen, replylen) \ + (req) = __builtin_alloca(sizeof(struct ipmi_request) + \ + (reqlen) + (replylen)); \ + IPMI_INIT_DRIVER_REQUEST((req), (addr), (cmd), (reqlen), \ + (replylen)) + #ifdef IPMB static int ipmi_ipmb_checksum(u_char, int); static int ipmi_ipmb_send_message(device_t, u_char, u_char, u_char, @@ -180,8 +197,8 @@ ipmi_dtor(void *arg) */ dev->ipmi_closing = 1; while (dev->ipmi_requests > 0) { - msleep(&dev->ipmi_requests, &sc->ipmi_lock, PWAIT, - "ipmidrain", 0); + msleep(&dev->ipmi_requests, &sc->ipmi_requests_lock, + PWAIT, "ipmidrain", 0); ipmi_purge_completed_requests(dev); } } @@ -214,7 +231,7 @@ ipmi_ipmb_send_message(device_t dev, u_c u_char slave_addr = 0x52; int error; - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_SEND_MSG, data_len + 8, 0); req->ir_request[0] = channel; req->ir_request[1] = slave_addr; @@ -230,7 +247,6 @@ ipmi_ipmb_send_message(device_t dev, u_c ipmi_submit_driver_request(sc, req); error = req->ir_error; - ipmi_free_request(req); return (error); } @@ -242,7 +258,7 @@ ipmi_handle_attn(struct ipmi_softc *sc) int error; device_printf(sc->ipmi_dev, "BMC has a message\n"); - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_GET_MSG_FLAGS, 0, 1); ipmi_submit_driver_request(sc, req); @@ -256,9 +272,7 @@ ipmi_handle_attn(struct ipmi_softc *sc) "watchdog about to go off"); } if (req->ir_reply[0] & IPMI_MSG_AVAILABLE) { - ipmi_free_request(req); - - req = ipmi_alloc_driver_request( + IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_GET_MSG, 0, 16); @@ -267,7 +281,6 @@ ipmi_handle_attn(struct ipmi_softc *sc) } } error = req->ir_error; - ipmi_free_request(req); return (error); } @@ -477,15 +490,11 @@ ipmi_ioctl(struct cdev *cdev, u_long cmd * Request management. */ -/* Allocate a new request with request and reply buffers. */ -struct ipmi_request * -ipmi_alloc_request(struct ipmi_device *dev, long msgid, uint8_t addr, - uint8_t command, size_t requestlen, size_t replylen) +static __inline void +ipmi_init_request(struct ipmi_request *req, struct ipmi_device *dev, long msgid, + uint8_t addr, uint8_t command, size_t requestlen, size_t replylen) { - struct ipmi_request *req; - req = malloc(sizeof(struct ipmi_request) + requestlen + replylen, - M_IPMI, M_WAITOK | M_ZERO); req->ir_owner = dev; req->ir_msgid = msgid; req->ir_addr = addr; @@ -498,6 +507,18 @@ ipmi_alloc_request(struct ipmi_device *d req->ir_reply = (char *)&req[1] + requestlen; req->ir_replybuflen = replylen; } +} + +/* Allocate a new request with request and reply buffers. */ +struct ipmi_request * +ipmi_alloc_request(struct ipmi_device *dev, long msgid, uint8_t addr, + uint8_t command, size_t requestlen, size_t replylen) +{ + struct ipmi_request *req; + + req = malloc(sizeof(struct ipmi_request) + requestlen + replylen, + M_IPMI, M_WAITOK | M_ZERO); + ipmi_init_request(req, dev, msgid, addr, command, requestlen, replylen); return (req); } @@ -532,21 +553,13 @@ ipmi_complete_request(struct ipmi_softc } } -/* Enqueue an internal driver request and wait until it is completed. */ +/* Perform an internal driver request. */ int ipmi_submit_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo) { - int error; - IPMI_LOCK(sc); - error = sc->ipmi_enqueue_request(sc, req); - if (error == 0) - error = msleep(req, &sc->ipmi_lock, 0, "ipmireq", timo); - if (error == 0) - error = req->ir_error; - IPMI_UNLOCK(sc); - return (error); + return (sc->ipmi_driver_request(sc, req, timo)); } /* @@ -563,7 +576,7 @@ ipmi_dequeue_request(struct ipmi_softc * IPMI_LOCK_ASSERT(sc); while (!sc->ipmi_detaching && TAILQ_EMPTY(&sc->ipmi_pending_requests)) - cv_wait(&sc->ipmi_request_added, &sc->ipmi_lock); + cv_wait(&sc->ipmi_request_added, &sc->ipmi_requests_lock); if (sc->ipmi_detaching) return (NULL); @@ -597,7 +610,7 @@ ipmi_set_watchdog(struct ipmi_softc *sc, if (sec > 0xffff / 10) return (EINVAL); - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_SET_WDOG, 6, 0); if (sec) { @@ -621,9 +634,7 @@ ipmi_set_watchdog(struct ipmi_softc *sc, if (error) device_printf(sc->ipmi_dev, "Failed to set watchdog\n"); else if (sec) { - ipmi_free_request(req); - - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_RESET_WDOG, 0, 0); error = ipmi_submit_driver_request(sc, req, 0); @@ -632,7 +643,6 @@ ipmi_set_watchdog(struct ipmi_softc *sc, "Failed to reset watchdog\n"); } - ipmi_free_request(req); return (error); /* dump_watchdog(sc); @@ -676,7 +686,8 @@ ipmi_startup(void *arg) dev = sc->ipmi_dev; /* Initialize interface-independent state. */ - mtx_init(&sc->ipmi_lock, device_get_nameunit(dev), "ipmi", MTX_DEF); + mtx_init(&sc->ipmi_requests_lock, "ipmi requests", NULL, MTX_DEF); + mtx_init(&sc->ipmi_io_lock, "ipmi io", NULL, MTX_DEF); cv_init(&sc->ipmi_request_added, "ipmireq"); TAILQ_INIT(&sc->ipmi_pending_requests); @@ -689,28 +700,24 @@ ipmi_startup(void *arg) } /* Send a GET_DEVICE_ID request. */ - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_GET_DEVICE_ID, 0, 15); error = ipmi_submit_driver_request(sc, req, MAX_TIMEOUT); if (error == EWOULDBLOCK) { device_printf(dev, "Timed out waiting for GET_DEVICE_ID\n"); - ipmi_free_request(req); return; } else if (error) { device_printf(dev, "Failed GET_DEVICE_ID: %d\n", error); - ipmi_free_request(req); return; } else if (req->ir_compcode != 0) { device_printf(dev, "Bad completion code for GET_DEVICE_ID: %d\n", req->ir_compcode); - ipmi_free_request(req); return; } else if (req->ir_replylen < 5) { device_printf(dev, "Short reply for GET_DEVICE_ID: %d\n", req->ir_replylen); - ipmi_free_request(req); return; } @@ -720,9 +727,7 @@ ipmi_startup(void *arg) req->ir_reply[2] & 0x7f, req->ir_reply[3] >> 4, req->ir_reply[3] & 0x0f, req->ir_reply[4] & 0x0f, req->ir_reply[4] >> 4); - ipmi_free_request(req); - - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_CLEAR_FLAGS, 1, 0); ipmi_submit_driver_request(sc, req, 0); @@ -734,25 +739,21 @@ ipmi_startup(void *arg) if (req->ir_compcode == 0xc1) { device_printf(dev, "Clear flags illegal\n"); } - ipmi_free_request(req); for (i = 0; i < 8; i++) { - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_GET_CHANNEL_INFO, 1, 0); req->ir_request[0] = i; ipmi_submit_driver_request(sc, req, 0); - if (req->ir_compcode != 0) { - ipmi_free_request(req); + if (req->ir_compcode != 0) break; - } - ipmi_free_request(req); } device_printf(dev, "Number of channels %d\n", i); /* probe for watchdog */ - req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0), + IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0), IPMI_GET_WDOG, 0, 0); ipmi_submit_driver_request(sc, req, 0); @@ -763,7 +764,6 @@ ipmi_startup(void *arg) sc->ipmi_watchdog_tag = EVENTHANDLER_REGISTER(watchdog_list, ipmi_wd_event, sc, 0); } - ipmi_free_request(req); sc->ipmi_cdev = make_dev(&ipmi_cdevsw, device_get_unit(dev), UID_ROOT, GID_OPERATOR, 0660, "ipmi%d", device_get_unit(dev)); @@ -830,14 +830,16 @@ ipmi_detach(device_t dev) sc->ipmi_detaching = 1; if (sc->ipmi_kthread) { cv_broadcast(&sc->ipmi_request_added); - msleep(sc->ipmi_kthread, &sc->ipmi_lock, 0, "ipmi_wait", 0); + msleep(sc->ipmi_kthread, &sc->ipmi_requests_lock, 0, + "ipmi_wait", 0); } IPMI_UNLOCK(sc); if (sc->ipmi_irq) bus_teardown_intr(dev, sc->ipmi_irq_res, sc->ipmi_irq); ipmi_release_resources(dev); - mtx_destroy(&sc->ipmi_lock); + mtx_destroy(&sc->ipmi_io_lock); + mtx_destroy(&sc->ipmi_requests_lock); return (0); } Modified: stable/8/sys/dev/ipmi/ipmi_kcs.c ============================================================================== --- stable/8/sys/dev/ipmi/ipmi_kcs.c Fri Mar 13 20:08:35 2015 (r279963) +++ stable/8/sys/dev/ipmi/ipmi_kcs.c Fri Mar 13 20:10:09 2015 (r279964) @@ -321,6 +321,8 @@ kcs_polled_request(struct ipmi_softc *sc u_char *cp, data; int i, state; + IPMI_IO_LOCK(sc); + /* Send the request. */ if (!kcs_start_write(sc)) { device_printf(sc->ipmi_dev, "KCS: Failed to start write\n"); @@ -444,6 +446,7 @@ kcs_polled_request(struct ipmi_softc *sc } i++; } + IPMI_IO_UNLOCK(sc); req->ir_replylen = i; #ifdef KCS_DEBUG device_printf(sc->ipmi_dev, "KCS: READ finished (%d bytes)\n", i); @@ -457,6 +460,7 @@ kcs_polled_request(struct ipmi_softc *sc return (1); fail: kcs_error(sc); + IPMI_IO_UNLOCK(sc); return (0); } @@ -490,6 +494,21 @@ kcs_startup(struct ipmi_softc *sc) device_get_nameunit(sc->ipmi_dev))); } +static int +kcs_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo) +{ + int i, ok; + + ok = 0; + for (i = 0; i < 3 && !ok; i++) + ok = kcs_polled_request(sc, req); + if (ok) + req->ir_error = 0; + else + req->ir_error = EIO; + return (req->ir_error); +} + int ipmi_kcs_attach(struct ipmi_softc *sc) { @@ -498,6 +517,7 @@ ipmi_kcs_attach(struct ipmi_softc *sc) /* Setup function pointers. */ sc->ipmi_startup = kcs_startup; sc->ipmi_enqueue_request = ipmi_polled_enqueue_request; + sc->ipmi_driver_request = kcs_driver_request; /* See if we can talk to the controller. */ status = INB(sc, KCS_CTL_STS); Modified: stable/8/sys/dev/ipmi/ipmi_smic.c ============================================================================== --- stable/8/sys/dev/ipmi/ipmi_smic.c Fri Mar 13 20:08:35 2015 (r279963) +++ stable/8/sys/dev/ipmi/ipmi_smic.c Fri Mar 13 20:10:09 2015 (r279964) @@ -363,8 +363,11 @@ smic_loop(void *arg) IPMI_LOCK(sc); while ((req = ipmi_dequeue_request(sc)) != NULL) { ok = 0; - for (i = 0; i < 3 && !ok; i++) + for (i = 0; i < 3 && !ok; i++) { + IPMI_IO_LOCK(sc); ok = smic_polled_request(sc, req); + IPMI_IO_UNLOCK(sc); + } if (ok) req->ir_error = 0; else @@ -383,6 +386,24 @@ smic_startup(struct ipmi_softc *sc) "%s: smic", device_get_nameunit(sc->ipmi_dev))); } +static int +smic_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo) +{ + int i, ok; + + ok = 0; + for (i = 0; i < 3 && !ok; i++) { + IPMI_IO_LOCK(sc); + ok = smic_polled_request(sc, req); + IPMI_IO_UNLOCK(sc); + } + if (ok) + req->ir_error = 0; + else + req->ir_error = EIO; + return (req->ir_error); +} + int ipmi_smic_attach(struct ipmi_softc *sc) { @@ -391,6 +412,7 @@ ipmi_smic_attach(struct ipmi_softc *sc) /* Setup function pointers. */ sc->ipmi_startup = smic_startup; sc->ipmi_enqueue_request = ipmi_polled_enqueue_request; + sc->ipmi_driver_request = smic_driver_request; /* See if we can talk to the controller. */ flags = INB(sc, SMIC_FLAGS); Modified: stable/8/sys/dev/ipmi/ipmi_ssif.c ============================================================================== --- stable/8/sys/dev/ipmi/ipmi_ssif.c Fri Mar 13 20:08:35 2015 (r279963) +++ stable/8/sys/dev/ipmi/ipmi_ssif.c Fri Mar 13 20:10:09 2015 (r279964) @@ -359,6 +359,22 @@ ssif_startup(struct ipmi_softc *sc) "%s: ssif", device_get_nameunit(sc->ipmi_dev))); } +static int +ssif_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo) +{ + int error; + + IPMI_LOCK(sc); + error = ipmi_polled_enqueue_request(sc, req); + if (error == 0) + error = msleep(req, &sc->ipmi_requests_lock, 0, "ipmireq", + timo); + if (error == 0) + error = req->ir_error; + IPMI_UNLOCK(sc); + return (error); +} + int ipmi_ssif_attach(struct ipmi_softc *sc, device_t smbus, int smbus_address) { @@ -370,6 +386,7 @@ ipmi_ssif_attach(struct ipmi_softc *sc, /* Setup function pointers. */ sc->ipmi_startup = ssif_startup; sc->ipmi_enqueue_request = ipmi_polled_enqueue_request; + sc->ipmi_driver_request = ssif_driver_request; return (0); } Modified: stable/8/sys/dev/ipmi/ipmivars.h ============================================================================== --- stable/8/sys/dev/ipmi/ipmivars.h Fri Mar 13 20:08:35 2015 (r279963) +++ stable/8/sys/dev/ipmi/ipmivars.h Fri Mar 13 20:10:09 2015 (r279964) @@ -95,6 +95,7 @@ struct ipmi_softc { } _iface; int ipmi_io_rid; int ipmi_io_type; + struct mtx ipmi_io_lock; struct resource *ipmi_io_res[MAX_RES]; int ipmi_io_spacing; int ipmi_irq_rid; @@ -107,12 +108,13 @@ struct ipmi_softc { eventhandler_tag ipmi_watchdog_tag; int ipmi_watchdog_active; struct intr_config_hook ipmi_ich; - struct mtx ipmi_lock; + struct mtx ipmi_requests_lock; struct cv ipmi_request_added; struct proc *ipmi_kthread; driver_intr_t *ipmi_intr; int (*ipmi_startup)(struct ipmi_softc *); int (*ipmi_enqueue_request)(struct ipmi_softc *, struct ipmi_request *); + int (*ipmi_driver_request)(struct ipmi_softc *, struct ipmi_request *, int); }; #define ipmi_ssif_smbus_address _iface.ssif.smbus_address @@ -183,12 +185,13 @@ struct ipmi_ipmb { #define IPMI_ADDR(netfn, lun) ((netfn) << 2 | (lun)) #define IPMI_REPLY_ADDR(addr) ((addr) + 0x4) -#define IPMI_LOCK(sc) mtx_lock(&(sc)->ipmi_lock) -#define IPMI_UNLOCK(sc) mtx_unlock(&(sc)->ipmi_lock) -#define IPMI_LOCK_ASSERT(sc) mtx_assert(&(sc)->ipmi_lock, MA_OWNED) - -#define ipmi_alloc_driver_request(addr, cmd, reqlen, replylen) \ - ipmi_alloc_request(NULL, 0, (addr), (cmd), (reqlen), (replylen)) +#define IPMI_LOCK(sc) mtx_lock(&(sc)->ipmi_requests_lock) +#define IPMI_UNLOCK(sc) mtx_unlock(&(sc)->ipmi_requests_lock) +#define IPMI_LOCK_ASSERT(sc) mtx_assert(&(sc)->ipmi_requests_lock, MA_OWNED) + +#define IPMI_IO_LOCK(sc) mtx_lock(&(sc)->ipmi_io_lock) +#define IPMI_IO_UNLOCK(sc) mtx_unlock(&(sc)->ipmi_io_lock) +#define IPMI_IO_LOCK_ASSERT(sc) mtx_assert(&(sc)->ipmi_io_lock, MA_OWNED) #if __FreeBSD_version < 601105 #define bus_read_1(r, o) \