mboxd: Broadcast the daemon is ready on all transports

The code as it stood only sent the state update at startup on the active
transport, which is somewhat arbitrarily chosen as an implementation
detail of the mbox initialisation function.

If the host firmware is using IPMI, it will not learn of the update
unless it attempts to contact mboxd, which it won't do if it knows the
daemon isn't there, which it may have learned of by receiving a state
update from the daemon's shutdown path. In this circumstance the host
firmware is now stuck.

Relieve the host firmware of this problem by always sending the daemon
state on all supported transports. To avoid some insanity we introduce a
new callback in struct transport_ops that allows use to send the BMC's
entire event state rather than just set or clear updates.

Change-Id: I094ff4089eeebd8be99fbd343b94f7bbef023fb1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

mboxd.c

@@ -51,7 +51,8 @@
 "\t\t\t\t(default: 1MB)\n" \
 "\t-f | --flash\t\tSize of flash in [K|M] bytes\n\n"
 
-static int dbus_init(struct mbox_context *context)
+static int dbus_init(struct mbox_context *context,
+		     const struct transport_ops **ops)
 {
 	int rc;
 
@@ -76,7 +77,7 @@ static int dbus_init(struct mbox_context *context)
 		return rc;
 	}
 
-	rc = transport_dbus_init(context);
+	rc = transport_dbus_init(context, ops);
 	if (rc < 0) {
 		MSG_ERR("Failed to initialise DBus protocol interface: %s\n",
 			strerror(-rc));
@@ -346,6 +347,7 @@ static bool parse_cmdline(int argc, char **argv,
 
 int main(int argc, char **argv)
 {
+	const struct transport_ops *mbox_ops, *dbus_ops;
 	struct mbox_context *context;
 	char *name = argv[0];
 	sigset_t set;
@@ -379,7 +381,7 @@ int main(int argc, char **argv)
 		goto finish;
 	}
 
-	rc = transport_mbox_init(context);
+	rc = transport_mbox_init(context, &mbox_ops);
 	if (rc) {
 		goto finish;
 	}
@@ -400,7 +402,7 @@ int main(int argc, char **argv)
 		goto finish;
 	}
 
-	rc = dbus_init(context);
+	rc = dbus_init(context, &dbus_ops);
 	if (rc) {
 		goto finish;
 	}
@@ -415,7 +417,16 @@ int main(int argc, char **argv)
 		MSG_ERR("LPC configuration failed, RESET required: %d\n", rc);
 	}
 
-	rc = protocol_events_set(context, BMC_EVENT_DAEMON_READY);
+	/* We're ready to go, alert the host */
+	context->bmc_events |= BMC_EVENT_DAEMON_READY;
+
+	/* Alert on all supported transports */
+	rc = protocol_events_put(context, mbox_ops);
+	if (rc) {
+		goto finish;
+	}
+
+	rc = protocol_events_put(context, dbus_ops);
 	if (rc) {
 		goto finish;
 	}
@@ -427,7 +438,11 @@ int main(int argc, char **argv)
 
 finish:
 	MSG_INFO("Daemon Exiting...\n");
-	protocol_events_clear(context, BMC_EVENT_DAEMON_READY);
+	context->bmc_events &= ~BMC_EVENT_DAEMON_READY;
+
+	/* Alert on all supported transports */
+	protocol_events_put(context, mbox_ops);
+	protocol_events_put(context, dbus_ops);
 
 #ifdef VIRTUAL_PNOR_ENABLED
 	destroy_vpnor(context);

protocol.c

@@ -23,7 +23,23 @@ static inline uint8_t protocol_get_bmc_event_mask(struct mbox_context *context)
 }
 
 /*
- * protocol_events_set() - Set BMC events
+ * protocol_events_put() - Push the full set/cleared state of BMC events on the
+ * 			   provided transport
+ * @context:    The mbox context pointer
+ * @ops:	The operations struct for the transport of interest
+ *
+ * Return:      0 on success otherwise negative error code
+ */
+int protocol_events_put(struct mbox_context *context,
+			const struct transport_ops *ops)
+{
+	const uint8_t mask = protocol_get_bmc_event_mask(context);
+
+	return ops->put_events(context, mask);
+}
+
+/*
+ * protocol_events_set() - Update the set BMC events on the active transport
  * @context:	The mbox context pointer
  * @bmc_event:	The bits to set
  *
@@ -44,7 +60,8 @@ int protocol_events_set(struct mbox_context *context, uint8_t bmc_event)
 }
 
 /*
- * protocol_events_clear() - Clear BMC events
+ * protocol_events_clear() - Update the cleared BMC events on the active
+ *                           transport
  * @context:	The mbox context pointer
  * @bmc_event:	The bits to clear
  *

protocol.h

@@ -5,6 +5,7 @@
 #define PROTOCOL_H
 
 struct mbox_context;
+struct transport_ops;
 
 /*
  * The GET_MBOX_INFO command is special as it can change the interface based on
@@ -120,6 +121,8 @@ void protocol_free(struct mbox_context *context);
 
 int protocol_negotiate_version(struct mbox_context *context, uint8_t requested);
 
+int protocol_events_put(struct mbox_context *context,
+			const struct transport_ops *ops);
 int protocol_events_set(struct mbox_context *context, uint8_t bmc_event);
 int protocol_events_clear(struct mbox_context *context, uint8_t bmc_event);
 

transport.h

@@ -7,6 +7,7 @@
 struct mbox_context;
 
 struct transport_ops {
+	int (*put_events)(struct mbox_context *context, uint8_t mask);
 	int (*set_events)(struct mbox_context *context, uint8_t events,
 			  uint8_t mask);
 	int (*clear_events)(struct mbox_context *context, uint8_t events,

transport_dbus.c

@@ -38,16 +38,11 @@ static int transport_dbus_property_update(struct mbox_context *context,
 	return (rc < 0) ? rc : 0;
 }
 
-static int transport_dbus_set_events(struct mbox_context *context,
-				     uint8_t events, uint8_t mask)
+static int transport_dbus_signal_update(struct mbox_context *context,
+					uint8_t events)
 {
 	int rc;
 
-	rc = transport_dbus_property_update(context, events & mask);
-	if (rc < 0) {
-		return rc;
-	}
-
 	/*
 	 * Handle signals - edge triggered, only necessary when they're
 	 * asserted
@@ -91,6 +86,38 @@ static int transport_dbus_set_events(struct mbox_context *context,
 	return 0;
 }
 
+static int transport_dbus_put_events(struct mbox_context *context, uint8_t mask)
+{
+	int rc;
+
+	/* Always update all properties */
+	rc = transport_dbus_property_update(context, mask);
+	if (rc < 0) {
+		return rc;
+	}
+
+	/*
+	 * Still test signals against the values set as sending them indicates
+	 * the event has been asserted, so we must not send them if the bits
+	 * are not set.
+	 */
+	return transport_dbus_signal_update(context,
+					    context->bmc_events & mask);
+}
+
+static int transport_dbus_set_events(struct mbox_context *context,
+				     uint8_t events, uint8_t mask)
+{
+	int rc;
+
+	rc = transport_dbus_property_update(context, events & mask);
+	if (rc < 0) {
+		return rc;
+	}
+
+	return transport_dbus_signal_update(context, events & mask);
+}
+
 static int transport_dbus_clear_events(struct mbox_context *context,
 				       uint8_t events, uint8_t mask)
 {
@@ -99,6 +126,7 @@ static int transport_dbus_clear_events(struct mbox_context *context,
 }
 
 static const struct transport_ops transport_dbus_ops = {
+	.put_events = transport_dbus_put_events,
 	.set_events = transport_dbus_set_events,
 	.clear_events = transport_dbus_clear_events,
 };
@@ -509,7 +537,8 @@ static const sd_bus_vtable protocol_v2_vtable[] = {
 	SD_BUS_VTABLE_END
 };
 
-int transport_dbus_init(struct mbox_context *context)
+int transport_dbus_init(struct mbox_context *context,
+			const struct transport_ops **ops)
 {
 	int rc;
 
@@ -526,8 +555,15 @@ int transport_dbus_init(struct mbox_context *context)
 					MBOX_DBUS_OBJECT,
 					MBOX_DBUS_PROTOCOL_IFACE_V2,
 					protocol_v2_vtable, context);
+	if (rc < 0) {
+		return rc;
+	}
+
+	if (ops) {
+		*ops = &transport_dbus_ops;
+	}
 
-	return rc;
+	return 0;
 }
 
 #define __unused __attribute__((unused))

transport_dbus.h

@@ -5,8 +5,10 @@
 #define TRANSPORT_DBUS_H
 
 #include "dbus.h"
+#include "transport.h"
 
-int transport_dbus_init(struct mbox_context *context);
+int transport_dbus_init(struct mbox_context *context,
+			const struct transport_ops **ops);
 void transport_dbus_free(struct mbox_context *context);
 
 #endif /* TRANSPORT_DBUS_H */

transport_mbox.c

@@ -121,21 +121,22 @@ static int transport_mbox_flush_events(struct mbox_context *context, uint8_t eve
 	return 0;
 }
 
-static int transport_mbox_set_events(struct mbox_context *context,
-				     uint8_t events, uint8_t mask)
+static int transport_mbox_put_events(struct mbox_context *context,
+					uint8_t mask)
 {
 	return transport_mbox_flush_events(context, context->bmc_events & mask);
 }
 
-static int transport_mbox_clear_events(struct mbox_context *context,
-				       uint8_t events, uint8_t mask)
+static int transport_mbox_update_events(struct mbox_context *context,
+					uint8_t events, uint8_t mask)
 {
 	return transport_mbox_flush_events(context, context->bmc_events & mask);
 }
 
 static const struct transport_ops transport_mbox_ops = {
-	.set_events = transport_mbox_set_events,
-	.clear_events = transport_mbox_clear_events,
+	.put_events = transport_mbox_put_events,
+	.set_events = transport_mbox_update_events,
+	.clear_events = transport_mbox_update_events,
 };
 
 /* Command Handlers */
@@ -665,9 +666,20 @@ int __transport_mbox_init(struct mbox_context *context, const char *path)
 	return 0;
 }
 
-int transport_mbox_init(struct mbox_context *context)
+int transport_mbox_init(struct mbox_context *context,
+			const struct transport_ops **ops)
 {
-	return __transport_mbox_init(context, MBOX_HOST_PATH);
+	int rc;
+
+	rc = __transport_mbox_init(context, MBOX_HOST_PATH);
+	if (rc)
+		return rc;
+
+	if (ops) {
+		*ops = &transport_mbox_ops;
+	}
+
+	return 0;
 }
 
 void transport_mbox_free(struct mbox_context *context)

transport_mbox.h

@@ -7,6 +7,7 @@
 #include <stdint.h>
 
 struct mbox_context;
+struct transport_ops;
 
 /* Command Values */
 #define MBOX_C_RESET_STATE		0x01
@@ -52,7 +53,8 @@ union mbox_regs {
 };
 
 int transport_mbox_dispatch(struct mbox_context *context);
-int transport_mbox_init(struct mbox_context *context);
+int transport_mbox_init(struct mbox_context *context,
+			const struct transport_ops **ops);
 void transport_mbox_free(struct mbox_context *context);
 
 #endif /* MBOXD_MSG_H */