MFC 304786,304788

304786
    hyperv/ic: Cleanup heartbeat channel callback.

    Sponsored by:   Microsoft
    Differential Revision:  https://reviews.freebsd.org/D7626

304788
    hyperv/ic: Update total message size if negotiate message size grows.

    Sponsored by:   Microsoft
    Differential Revision:  https://reviews.freebsd.org/D7627

6 files changed, 86 insertions, 60 deletions

diff --git a/sys/dev/hyperv/utilities/hv_heartbeat.c b/sys/dev/hyperv/utilities/hv_heartbeat.c
index d9017ca..5e91cd1 100644
--- a/sys/dev/hyperv/utilities/hv_heartbeat.c
+++ b/sys/dev/hyperv/utilities/hv_heartbeat.c
@@ -22,23 +22,22 @@
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * $FreeBSD$
  */
 
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
 #include <sys/param.h>
-#include <sys/kernel.h>
 #include <sys/bus.h>
-#include <sys/malloc.h>
+#include <sys/kernel.h>
 #include <sys/module.h>
-#include <sys/timetc.h>
-#include <sys/syscallsubr.h>
 #include <sys/systm.h>
 
 #include <dev/hyperv/include/hyperv.h>
 #include <dev/hyperv/include/vmbus.h>
-#include <dev/hyperv/utilities/hv_utilreg.h>
-#include "hv_util.h"
+#include <dev/hyperv/utilities/hv_util.h>
+#include <dev/hyperv/utilities/vmbus_icreg.h>
+
 #include "vmbus_if.h"
 
 static const struct vmbus_ic_desc vmbus_heartbeat_descs[] = {
@@ -51,55 +50,64 @@ static const struct vmbus_ic_desc vmbus_heartbeat_descs[] = {
 	VMBUS_IC_DESC_END
 };
 
-/**
- * Process heartbeat message
- */
 static void
-hv_heartbeat_cb(struct vmbus_channel *channel, void *context)
+vmbus_heartbeat_cb(struct vmbus_channel *chan, void *xsc)
 {
-	uint8_t*		buf;
-	int			recvlen;
-	uint64_t		requestid;
-	int			ret;
-
-	struct hv_vmbus_heartbeat_msg_data*	heartbeat_msg;
-	struct hv_vmbus_icmsg_hdr*		icmsghdrp;
-	hv_util_sc			*softc;
-
-	softc = (hv_util_sc*)context;
-	buf = softc->receive_buffer;;
-
-	recvlen = softc->ic_buflen;
-	ret = vmbus_chan_recv(channel, buf, &recvlen, &requestid);
-	KASSERT(ret != ENOBUFS, ("hvheartbeat recvbuf is not large enough"));
-	/* XXX check recvlen to make sure that it contains enough data */
-
-	if ((ret == 0) && recvlen > 0) {
-
-	    icmsghdrp = (struct hv_vmbus_icmsg_hdr *)
-		&buf[sizeof(struct hv_vmbus_pipe_hdr)];
-
-	    if (icmsghdrp->icmsgtype == HV_ICMSGTYPE_NEGOTIATE) {
-	    	int error;
-
-		error = vmbus_ic_negomsg(softc, buf, recvlen);
+	struct hv_util_sc *sc = xsc;
+	struct vmbus_icmsg_hdr *hdr;
+	int dlen, error;
+	uint64_t xactid;
+	void *data;
+
+	/*
+	 * Receive request.
+	 */
+	data = sc->receive_buffer;
+	dlen = sc->ic_buflen;
+	error = vmbus_chan_recv(chan, data, &dlen, &xactid);
+	KASSERT(error != ENOBUFS, ("icbuf is not large enough"));
+	if (error)
+		return;
+
+	if (dlen < sizeof(struct vmbus_icmsg_hdr)) {
+		device_printf(sc->ic_dev, "invalid data len %d\n", dlen);
+		return;
+	}
+	hdr = data;
+
+	/*
+	 * Update request, which will be echoed back as response.
+	 */
+	switch (hdr->ic_type) {
+	case VMBUS_ICMSG_TYPE_NEGOTIATE:
+		error = vmbus_ic_negomsg(sc, data, &dlen);
 		if (error)
 			return;
-	    } else {
-		heartbeat_msg =
-		    (struct hv_vmbus_heartbeat_msg_data *)
-			&buf[sizeof(struct hv_vmbus_pipe_hdr) +
-			     sizeof(struct hv_vmbus_icmsg_hdr)];
+		break;
 
-		heartbeat_msg->seq_num += 1;
-	    }
-
-	    icmsghdrp->icflags = HV_ICMSGHDRFLAG_TRANSACTION |
-				 HV_ICMSGHDRFLAG_RESPONSE;
+	case VMBUS_ICMSG_TYPE_HEARTBEAT:
+		/* Only ic_seq is a must */
+		if (dlen < VMBUS_ICMSG_HEARTBEAT_SIZE_MIN) {
+			device_printf(sc->ic_dev, "invalid heartbeat len %d\n",
+			    dlen);
+			return;
+		}
+		((struct vmbus_icmsg_heartbeat *)data)->ic_seq++;
+		break;
 
-	    vmbus_chan_send(channel, VMBUS_CHANPKT_TYPE_INBAND, 0,
-	        buf, recvlen, requestid);
+	default:
+		device_printf(sc->ic_dev, "got 0x%08x icmsg\n", hdr->ic_type);
+		break;
 	}
+
+	/*
+	 * Send response by echoing the updated request back.
+	 */
+	hdr->ic_flags = VMBUS_ICMSG_FLAG_XACT | VMBUS_ICMSG_FLAG_RESP;
+	error = vmbus_chan_send(chan, VMBUS_CHANPKT_TYPE_INBAND, 0,
+	    data, dlen, xactid);
+	if (error)
+		device_printf(sc->ic_dev, "resp send failed: %d\n", error);
 }
 
 static int
@@ -112,7 +120,8 @@ hv_heartbeat_probe(device_t dev)
 static int
 hv_heartbeat_attach(device_t dev)
 {
-	return hv_util_attach(dev, hv_heartbeat_cb);
+
+	return (hv_util_attach(dev, vmbus_heartbeat_cb));
 }
 
 static device_method_t heartbeat_methods[] = {
diff --git a/sys/dev/hyperv/utilities/hv_shutdown.c b/sys/dev/hyperv/utilities/hv_shutdown.c
index 8fda4db..f29e32c 100644
--- a/sys/dev/hyperv/utilities/hv_shutdown.c
+++ b/sys/dev/hyperv/utilities/hv_shutdown.c
@@ -87,7 +87,7 @@ hv_shutdown_cb(struct vmbus_channel *channel, void *context)
 	    if (icmsghdrp->icmsgtype == HV_ICMSGTYPE_NEGOTIATE) {
 		int error;
 
-		error = vmbus_ic_negomsg(softc, buf, recv_len);
+		error = vmbus_ic_negomsg(softc, buf, &recv_len);
 		if (error)
 			return;
 	    } else {
diff --git a/sys/dev/hyperv/utilities/hv_timesync.c b/sys/dev/hyperv/utilities/hv_timesync.c
index a71eeb4..632a45e 100644
--- a/sys/dev/hyperv/utilities/hv_timesync.c
+++ b/sys/dev/hyperv/utilities/hv_timesync.c
@@ -162,7 +162,7 @@ hv_timesync_cb(struct vmbus_channel *channel, void *context)
 	    if (icmsghdrp->icmsgtype == HV_ICMSGTYPE_NEGOTIATE) {
 	    	int error;
 
-		error = vmbus_ic_negomsg(&softc->util_sc, time_buf, recvlen);
+		error = vmbus_ic_negomsg(&softc->util_sc, time_buf, &recvlen);
 		if (error)
 			return;
 	    } else {
diff --git a/sys/dev/hyperv/utilities/hv_util.c b/sys/dev/hyperv/utilities/hv_util.c
index bb5b110..3fc16c9 100644
--- a/sys/dev/hyperv/utilities/hv_util.c
+++ b/sys/dev/hyperv/utilities/hv_util.c
@@ -48,13 +48,16 @@
 
 #define VMBUS_IC_BRSIZE		(4 * PAGE_SIZE)
 
-CTASSERT(sizeof(struct vmbus_icmsg_negotiate) < VMBUS_IC_BRSIZE);
+#define VMBUS_IC_VERCNT		2
+#define VMBUS_IC_NEGOSZ		\
+	__offsetof(struct vmbus_icmsg_negotiate, ic_ver[VMBUS_IC_VERCNT])
+CTASSERT(VMBUS_IC_NEGOSZ < VMBUS_IC_BRSIZE);
 
 int
-vmbus_ic_negomsg(struct hv_util_sc *sc, void *data, int dlen)
+vmbus_ic_negomsg(struct hv_util_sc *sc, void *data, int *dlen0)
 {
 	struct vmbus_icmsg_negotiate *nego;
-	int cnt, major;
+	int cnt, major, dlen = *dlen0;
 
 	/*
 	 * Preliminary message size verification
@@ -87,9 +90,13 @@ vmbus_ic_negomsg(struct hv_util_sc *sc, void *data, int dlen)
 	nego->ic_msgver_cnt = 1;
 	nego->ic_ver[1] = VMBUS_IC_VERSION(major, 0);
 
-	/* Data contains two versions */
-	nego->ic_hdr.ic_dsize = __offsetof(struct vmbus_icmsg_negotiate,
-	    ic_ver[2]) - sizeof(struct vmbus_icmsg_hdr);
+	/* Update data size */
+	nego->ic_hdr.ic_dsize = VMBUS_IC_NEGOSZ -
+	    sizeof(struct vmbus_icmsg_hdr);
+
+	/* Update total size, if necessary */
+	if (dlen < VMBUS_IC_NEGOSZ)
+		*dlen0 = VMBUS_IC_NEGOSZ;
 
 	return 0;
 }
diff --git a/sys/dev/hyperv/utilities/hv_util.h b/sys/dev/hyperv/utilities/hv_util.h
index 130e4b7..012cdee 100644
--- a/sys/dev/hyperv/utilities/hv_util.h
+++ b/sys/dev/hyperv/utilities/hv_util.h
@@ -54,6 +54,6 @@ struct vmbus_ic_desc {
 int		hv_util_attach(device_t dev, vmbus_chan_callback_t cb);
 int		hv_util_detach(device_t dev);
 int		vmbus_ic_probe(device_t dev, const struct vmbus_ic_desc descs[]);
-int		vmbus_ic_negomsg(struct hv_util_sc *, void *data, int dlen);
+int		vmbus_ic_negomsg(struct hv_util_sc *, void *data, int *dlen);
 
 #endif
diff --git a/sys/dev/hyperv/utilities/vmbus_icreg.h b/sys/dev/hyperv/utilities/vmbus_icreg.h
index 2cc263a..38643e3 100644
--- a/sys/dev/hyperv/utilities/vmbus_icreg.h
+++ b/sys/dev/hyperv/utilities/vmbus_icreg.h
@@ -81,4 +81,14 @@ struct vmbus_icmsg_negotiate {
 	uint32_t		ic_ver[];
 } __packed;
 
+/* VMBUS_ICMSG_TYPE_HEARTBEAT */
+struct vmbus_icmsg_heartbeat {
+	struct vmbus_icmsg_hdr	ic_hdr;
+	uint64_t		ic_seq;
+	uint32_t		ic_rsvd[8];
+} __packed;
+
+#define VMBUS_ICMSG_HEARTBEAT_SIZE_MIN	\
+	__offsetof(struct vmbus_icmsg_heartbeat, ic_rsvd[0])
+
 #endif	/* !_VMBUS_ICREG_H_ */