Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Oct 2023 20:36:55 GMT
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 73765a5fa7d0 - stable/12 - Advertise the MPI Message Version that's contained in the IOCFacts message in the sysctl block for the driver. mpsutil/mprutil needs this so it can know how big of a buffer to allocate when requesting the IOCFacts from the controller. This eliminates the kernel console messages about wrong allocation sizes.
Message-ID:  <202310052036.395KatmI013852@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=73765a5fa7d0daa72387d41661c36922f7860bcb

commit 73765a5fa7d0daa72387d41661c36922f7860bcb
Author:     Scott Long <scottl@FreeBSD.org>
AuthorDate: 2020-02-07 12:15:39 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-05 20:36:00 +0000

    Advertise the MPI Message Version that's contained in the IOCFacts message
    in the sysctl block for the driver.  mpsutil/mprutil needs this so it can
    know how big of a buffer to allocate when requesting the IOCFacts from the
    controller.  This eliminates the kernel console messages about wrong
    allocation sizes.
    
    Reported by:    imp
    
    (cherry picked from commit 69e85eb8ae4919e0806bc2957cbc4a33f9138b54)
    
    mprutil: "fix user reply buffer (64)..." warnings
    
    Depending on the card's firmware version, it may return different length
    responses for MPI2_FUNCTION_IOC_FACTS.  But the first part of the
    response contains the length of the rest, so query it first to get the
    length and then use that to size the buffer for the full response.
    
    Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST.  It only worked
    by luck before.
    
    PR:             264848
    Reported by:    Julien Cigar <julien@perdition.city>
    Sponsored by:   Axcient
    Reviewed by:    scottl, imp
    Differential Revision: https://reviews.freebsd.org/D38739
    
    (cherry picked from commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83)
---
 sys/dev/mpr/mpr.c          | 14 ++++++++++++--
 sys/dev/mpr/mpr_user.c     |  6 +++---
 sys/dev/mpr/mprvar.h       |  1 +
 sys/dev/mps/mps.c          | 14 ++++++++++++--
 sys/dev/mps/mps_user.c     |  6 +++---
 sys/dev/mps/mpsvar.h       |  1 +
 usr.sbin/mpsutil/mps_cmd.c | 35 +++++++++++++++++++++++++++++++----
 7 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/sys/dev/mpr/mpr.c b/sys/dev/mpr/mpr.c
index 562d629c7afa..b36e10d103d3 100644
--- a/sys/dev/mpr/mpr.c
+++ b/sys/dev/mpr/mpr.c
@@ -521,6 +521,12 @@ mpr_iocfacts_allocate(struct mpr_softc *sc, uint8_t attaching)
 	    sc->facts->FWVersion.Struct.Unit,
 	    sc->facts->FWVersion.Struct.Dev);
 
+	snprintf(sc->msg_version, sizeof(sc->msg_version), "%d.%d",
+	    (sc->facts->MsgVersion & MPI2_IOCFACTS_MSGVERSION_MAJOR_MASK) >>
+	    MPI2_IOCFACTS_MSGVERSION_MAJOR_SHIFT,
+	    (sc->facts->MsgVersion & MPI2_IOCFACTS_MSGVERSION_MINOR_MASK) >>
+	    MPI2_IOCFACTS_MSGVERSION_MINOR_SHIFT);
+
 	mpr_dprint(sc, MPR_INFO, "Firmware: %s, Driver: %s\n", sc->fw_version,
 	    MPR_DRIVER_VERSION);
 	mpr_dprint(sc, MPR_INFO,
@@ -1904,13 +1910,17 @@ mpr_setup_sysctl(struct mpr_softc *sc)
 	    "Total number of event frames allocated");
 
 	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
-	    OID_AUTO, "firmware_version", CTLFLAG_RW, sc->fw_version,
+	    OID_AUTO, "firmware_version", CTLFLAG_RD, sc->fw_version,
 	    strlen(sc->fw_version), "firmware version");
 
 	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
-	    OID_AUTO, "driver_version", CTLFLAG_RW, MPR_DRIVER_VERSION,
+	    OID_AUTO, "driver_version", CTLFLAG_RD, MPR_DRIVER_VERSION,
 	    strlen(MPR_DRIVER_VERSION), "driver version");
 
+	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
+	    OID_AUTO, "msg_version", CTLFLAG_RD, sc->msg_version,
+	    strlen(sc->msg_version), "message interface version (deprecated)");
+
 	SYSCTL_ADD_INT(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
 	    OID_AUTO, "io_cmds_active", CTLFLAG_RD,
 	    &sc->io_cmds_active, 0, "number of currently active commands");
diff --git a/sys/dev/mpr/mpr_user.c b/sys/dev/mpr/mpr_user.c
index 53b3f94ea152..62111b91af0b 100644
--- a/sys/dev/mpr/mpr_user.c
+++ b/sys/dev/mpr/mpr_user.c
@@ -855,8 +855,8 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
 		if ((cm != NULL) && (cm->cm_reply != NULL)) {
 			rpl = (MPI2_DEFAULT_REPLY *)cm->cm_reply;
 			sz = rpl->MsgLength * 4;
-	
-			if (sz > data->ReplySize) {
+
+			if (bootverbose && sz > data->ReplySize) {
 				mpr_printf(sc, "%s: user reply buffer (%d) "
 				    "smaller than returned buffer (%d)\n",
 				    __func__, data->ReplySize, sz);
@@ -1081,7 +1081,7 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
 		rpl = (MPI2_DEFAULT_REPLY *)cm->cm_reply;
 		sz = rpl->MsgLength * 4;
 
-		if (sz > data->ReplySize) {
+		if (bootverbose && sz > data->ReplySize) {
 			mpr_printf(sc, "%s: user reply buffer (%d) smaller "
 			    "than returned buffer (%d)\n", __func__,
 			    data->ReplySize, sz);
diff --git a/sys/dev/mpr/mprvar.h b/sys/dev/mpr/mprvar.h
index 8eb9379d8cff..bda6e9c7da17 100644
--- a/sys/dev/mpr/mprvar.h
+++ b/sys/dev/mpr/mprvar.h
@@ -369,6 +369,7 @@ struct mpr_softc {
 	struct sysctl_ctx_list		sysctl_ctx;
 	struct sysctl_oid		*sysctl_tree;
 	char                            fw_version[16];
+	char				msg_version[8];
 	struct mpr_command		*commands;
 	struct mpr_chain		*chains;
 	struct mpr_prp_page		*prps;
diff --git a/sys/dev/mps/mps.c b/sys/dev/mps/mps.c
index 1fb7cf09eae8..7bb7d97cb538 100644
--- a/sys/dev/mps/mps.c
+++ b/sys/dev/mps/mps.c
@@ -498,6 +498,12 @@ mps_iocfacts_allocate(struct mps_softc *sc, uint8_t attaching)
 	    sc->facts->FWVersion.Struct.Unit,
 	    sc->facts->FWVersion.Struct.Dev);
 
+	snprintf(sc->msg_version, sizeof(sc->msg_version), "%d.%d",
+	    (sc->facts->MsgVersion & MPI2_IOCFACTS_MSGVERSION_MAJOR_MASK) >>
+	    MPI2_IOCFACTS_MSGVERSION_MAJOR_SHIFT, 
+	    (sc->facts->MsgVersion & MPI2_IOCFACTS_MSGVERSION_MINOR_MASK) >>
+	    MPI2_IOCFACTS_MSGVERSION_MINOR_SHIFT);
+
 	mps_dprint(sc, MPS_INFO, "Firmware: %s, Driver: %s\n", sc->fw_version,
 	    MPS_DRIVER_VERSION);
 	mps_dprint(sc, MPS_INFO, "IOCCapabilities: %b\n",
@@ -1738,13 +1744,17 @@ mps_setup_sysctl(struct mps_softc *sc)
 	    "Total number of event frames allocated");
 
 	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
-	    OID_AUTO, "firmware_version", CTLFLAG_RW, sc->fw_version,
+	    OID_AUTO, "firmware_version", CTLFLAG_RD, sc->fw_version,
 	    strlen(sc->fw_version), "firmware version");
 
 	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
-	    OID_AUTO, "driver_version", CTLFLAG_RW, MPS_DRIVER_VERSION,
+	    OID_AUTO, "driver_version", CTLFLAG_RD, MPS_DRIVER_VERSION,
 	    strlen(MPS_DRIVER_VERSION), "driver version");
 
+	SYSCTL_ADD_STRING(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
+	    OID_AUTO, "msg_version", CTLFLAG_RD, sc->msg_version,
+	    strlen(sc->msg_version), "message interface version (deprecated)");
+
 	SYSCTL_ADD_INT(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree),
 	    OID_AUTO, "io_cmds_active", CTLFLAG_RD,
 	    &sc->io_cmds_active, 0, "number of currently active commands");
diff --git a/sys/dev/mps/mps_user.c b/sys/dev/mps/mps_user.c
index e7e376288961..a065112bf138 100644
--- a/sys/dev/mps/mps_user.c
+++ b/sys/dev/mps/mps_user.c
@@ -866,8 +866,8 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
 		if ((cm != NULL) && (cm->cm_reply != NULL)) {
 			rpl = (MPI2_DEFAULT_REPLY *)cm->cm_reply;
 			sz = rpl->MsgLength * 4;
-	
-			if (sz > data->ReplySize) {
+
+			if (bootverbose && sz > data->ReplySize) {
 				mps_printf(sc, "%s: user reply buffer (%d) "
 				    "smaller than returned buffer (%d)\n",
 				    __func__, data->ReplySize, sz);
@@ -1021,7 +1021,7 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
 		rpl = (MPI2_DEFAULT_REPLY *)cm->cm_reply;
 		sz = rpl->MsgLength * 4;
 
-		if (sz > data->ReplySize) {
+		if (bootverbose && sz > data->ReplySize) {
 			mps_printf(sc, "%s: user reply buffer (%d) smaller "
 			    "than returned buffer (%d)\n", __func__,
 			    data->ReplySize, sz);
diff --git a/sys/dev/mps/mpsvar.h b/sys/dev/mps/mpsvar.h
index bb0087bf0346..2326084fee3d 100644
--- a/sys/dev/mps/mpsvar.h
+++ b/sys/dev/mps/mpsvar.h
@@ -327,6 +327,7 @@ struct mps_softc {
 	struct sysctl_ctx_list		sysctl_ctx;
 	struct sysctl_oid		*sysctl_tree;
 	char                            fw_version[16];
+	char				msg_version[8];
 	struct mps_command		*commands;
 	struct mps_chain		*chains;
 	struct callout			periodic;
diff --git a/usr.sbin/mpsutil/mps_cmd.c b/usr.sbin/mpsutil/mps_cmd.c
index 51a0e77822d9..8d7432651495 100644
--- a/usr.sbin/mpsutil/mps_cmd.c
+++ b/usr.sbin/mpsutil/mps_cmd.c
@@ -727,28 +727,55 @@ mps_pass_command(int fd, void *req, uint32_t req_len, void *reply,
 	return (0);
 }
 
+/* Return the length in bytes of the device's MPI2_IOC_FACTS reply */
+static size_t
+mps_get_ioc_factslen(int fd)
+{
+	MPI2_IOC_FACTS_REQUEST req;
+	const size_t factslen = 4;
+	char factsbuf[4] = {0};
+	MPI2_IOC_FACTS_REPLY *facts = (MPI2_IOC_FACTS_REPLY*)factsbuf;
+	int error;
+
+	bzero(&req, sizeof(req));
+	req.Function = MPI2_FUNCTION_IOC_FACTS;
+	error = mps_pass_command(fd, &req, sizeof(MPI2_IOC_FACTS_REQUEST),
+	    factsbuf, factslen, NULL, 0, NULL, 0, 10);
+
+	if (error)
+		return (0);
+
+	/* The card's response is measured in dwords */
+	return (facts->MsgLength * 4);
+}
+
 MPI2_IOC_FACTS_REPLY *
 mps_get_iocfacts(int fd)
 {
 	MPI2_IOC_FACTS_REPLY *facts;
 	MPI2_IOC_FACTS_REQUEST req;
+	size_t factslen;
 	int error;
 
-	facts = malloc(sizeof(MPI2_IOC_FACTS_REPLY));
+	factslen = mps_get_ioc_factslen(fd);
+	if (factslen == 0)
+		return (NULL);
+
+	facts = malloc(factslen);
 	if (facts == NULL) {
 		errno = ENOMEM;
 		return (NULL);
 	}
 
-	bzero(&req, sizeof(MPI2_IOC_FACTS_REQUEST));
+	bzero(&req, sizeof(req));
 	req.Function = MPI2_FUNCTION_IOC_FACTS;
 
 #if 1
 	error = mps_pass_command(fd, &req, sizeof(MPI2_IOC_FACTS_REQUEST),
-	    facts, sizeof(MPI2_IOC_FACTS_REPLY), NULL, 0, NULL, 0, 10);
+	    facts, factslen, NULL, 0, NULL, 0, 10);
 #else
 	error = mps_user_command(fd, &req, sizeof(MPI2_IOC_FACTS_REQUEST),
-	    facts, sizeof(MPI2_IOC_FACTS_REPLY), NULL, 0, 0);
+	    facts, factslen, NULL, 0, 0);
 #endif
 	if (error) {
 		free(facts);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310052036.395KatmI013852>