[PATCH V2 4/4] Display VERSION partition info on BMC machines

Sam Mendoza-Jonas sam at mendozajonas.com
Mon Jan 18 09:47:09 AEDT 2016


On Mon, Jan 18, 2016 at 09:16:28AM +1100, Cyril Bur wrote:
> On Fri, 15 Jan 2016 15:14:57 +1100
> Sam Mendoza-Jonas <sam at mendozajonas.com> wrote:
> 
> > From: Samuel Mendoza-Jonas <sam.mj at au1.ibm.com>
> > 
> > On supported platforms read the VERSION partition on startup and display
> > the available versions strings in the System Information screen.
> > 
> > This adds a skeleton hostboot.c to support possible additional BMC
> > platform support.
> > 
> > Signed-off-by: Sam Mendoza-Jonas <sam at mendozajonas.com>
> > ---
> > V2 Changes: Update the format of platform versions in nc-sysinfo
> > 
> >  discover/Makefile.am          | 14 ++++++++++++++
> >  discover/hostboot.c           | 26 ++++++++++++++++++++++++++
> >  discover/hostboot.h           | 15 +++++++++++++++
> >  discover/platform-powerpc.c   | 10 ++++++++++
> >  lib/pb-protocol/pb-protocol.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  lib/types/types.h             |  4 ++++
> >  ui/ncurses/nc-sysinfo.c       | 16 ++++++++++++++++
> >  7 files changed, 124 insertions(+)
> >  create mode 100644 discover/hostboot.c
> >  create mode 100644 discover/hostboot.h
> > 
> > diff --git a/discover/Makefile.am b/discover/Makefile.am
> > index 1672035..f55f1cd 100644
> > --- a/discover/Makefile.am
> > +++ b/discover/Makefile.am
> > @@ -79,7 +79,21 @@ discover_platform_ro_SOURCES = \
> >  	discover/ipmi.h \
> >  	discover/dt.c \
> >  	discover/dt.h \
> > +	discover/hostboot.h \
> >  	discover/platform-powerpc.c
> >  
> > +discover_platform_ro_CPPFLAGS = \
> > +	$(AM_CPPFLAGS)
> > +
> > +if ENABLE_MTD
> > +discover_platform_ro_SOURCES += \
> > +	discover/hostboot.c
> > +
> > +discover_platform_ro_LDFLAGS = \
> > +	$(core_lib) \
> > +	$(UDEV_LIBS)
> > +
> > +endif
> > +
> >  discover_platform_ro_LINK = \
> >  	$(LD) -r -o $@
> > diff --git a/discover/hostboot.c b/discover/hostboot.c
> > new file mode 100644
> > index 0000000..9a27b0f
> > --- /dev/null
> > +++ b/discover/hostboot.c
> > @@ -0,0 +1,26 @@
> > +#include <string.h>
> > +#include <stdlib.h>
> > +#include <errno.h>
> > +
> > +#include <log/log.h>
> > +#include <talloc/talloc.h>
> > +#include <flash/flash.h>
> > +
> > +#include "hostboot.h"
> > +
> > +void hostboot_load_versions(struct system_info *info)
> > +{
> > +	int n = 0;
> > +
> > +	n = flash_parse_version(info, &info->platform_current, true);
> > +	if (n < 0)
> > +		pb_log("Failed to read platform versions for current side\n");
> > +	else
> > +		info->n_current = n;
> > +
> > +	n = flash_parse_version(info, &info->platform_other, false);
> > +	if (n < 0)
> > +		pb_log("Failed to read platform versions for other side\n");
> > +	else
> > +		info->n_other = n;
> > +}
> 
> On a platform with only one side is "Failed to read platform versions for other
> side\n" always going to appear in the logs? Is having an error seeming message
> wise when it won't be an error at all?
> 

Good point - I double checked how the error propagates up, and in
flash_parse_version() we have this snippet:

	info = flash_setup_buffer(ctx, "VERSION");
	if (!info)
		return 0;

	if (current && !info->other_side)
		return 0;

Which means well but is wrong :) It *should* be !current. When fixed
though, if we fail to find a second side to flash, we'll just return 0
(0 strings found).
So thanks for getting me to look at that again!

Cheers,
Sam

> > diff --git a/discover/hostboot.h b/discover/hostboot.h
> > new file mode 100644
> > index 0000000..cb1e497
> > --- /dev/null
> > +++ b/discover/hostboot.h
> > @@ -0,0 +1,15 @@
> > +#ifndef HOSTBOOT_H
> > +#define HOSTBOOT_H
> > +
> > +#include "config.h"
> > +#include <types/types.h>
> > +
> > +#ifdef MTD_SUPPORT
> > +void hostboot_load_versions(struct system_info *info);
> > +#else
> > +static inline void hostboot_load_versions(struct system_info *info)
> > +{
> > +	(void)info;
> > +}
> > +#endif
> > +#endif /* HOSTBOOT_H */
> > diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
> > index 5abc4d7..8434d6f 100644
> > --- a/discover/platform-powerpc.c
> > +++ b/discover/platform-powerpc.c
> > @@ -14,7 +14,9 @@
> >  #include <list/list.h>
> >  #include <log/log.h>
> >  #include <process/process.h>
> > +#include <types/types.h>
> >  
> > +#include "hostboot.h"
> >  #include "platform.h"
> >  #include "ipmi.h"
> >  #include "dt.h"
> > @@ -43,6 +45,7 @@ struct platform_powerpc {
> >  				bool persistent);
> >  	int 		(*set_os_boot_sensor)(
> >  				struct platform_powerpc *platform);
> > +	void		(*get_platform_versions)(struct system_info *info);
> >  };
> >  
> >  static const char *known_params[] = {
> > @@ -1089,6 +1092,9 @@ static int get_sysinfo(struct platform *p, struct system_info *sysinfo)
> >  	if (platform->ipmi)
> >  		get_ipmi_bmc_mac(p, sysinfo->bmc_mac);
> >  
> > +	if (platform->get_platform_versions)
> > +		platform->get_platform_versions(sysinfo);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1126,6 +1132,10 @@ static bool probe(struct platform *p, void *ctx)
> >  		pb_log("platform: no IPMI parameter support\n");
> >  	}
> >  
> > +	rc = stat("/proc/device-tree/bmc", &statbuf);
> > +	if (!rc)
> > +		platform->get_platform_versions = hostboot_load_versions;
> > +
> >  	return true;
> >  }
> >  
> > diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
> > index 42506e2..e99ce86 100644
> > --- a/lib/pb-protocol/pb-protocol.c
> > +++ b/lib/pb-protocol/pb-protocol.c
> > @@ -225,6 +225,13 @@ int pb_protocol_system_info_len(const struct system_info *sysinfo)
> >  		4 + optional_strlen(sysinfo->identifier) +
> >  		4 + 4;
> >  
> > +	len +=	4;
> > +	for (i = 0; i < sysinfo->n_current; i++)
> > +		len += 4 + optional_strlen(sysinfo->platform_current[i]);
> > +	len +=	4;
> > +	for (i = 0; i < sysinfo->n_other; i++)
> > +		len += 4 + optional_strlen(sysinfo->platform_other[i]);
> > +
> >  	for (i = 0; i < sysinfo->n_interfaces; i++) {
> >  		struct interface_info *if_info = sysinfo->interfaces[i];
> >  		len +=	4 + if_info->hwaddr_size +
> > @@ -395,6 +402,16 @@ int pb_protocol_serialise_system_info(const struct system_info *sysinfo,
> >  	pos += pb_protocol_serialise_string(pos, sysinfo->type);
> >  	pos += pb_protocol_serialise_string(pos, sysinfo->identifier);
> >  
> > +	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_current);
> > +	pos += sizeof(uint32_t);
> > +	for (i = 0; i < sysinfo->n_current; i++)
> > +		pos += pb_protocol_serialise_string(pos, sysinfo->platform_current[i]);
> > +
> > +	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_other);
> > +	pos += sizeof(uint32_t);
> > +	for (i = 0; i < sysinfo->n_other; i++)
> > +		pos += pb_protocol_serialise_string(pos, sysinfo->platform_other[i]);
> > +
> >  	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_interfaces);
> >  	pos += sizeof(uint32_t);
> >  
> > @@ -798,6 +815,7 @@ int pb_protocol_deserialise_system_info(struct system_info *sysinfo,
> >  	unsigned int len, i;
> >  	const char *pos;
> >  	int rc = -1;
> > +	char *tmp;
> >  
> >  	len = message->payload_len;
> >  	pos = message->payload;
> > @@ -809,6 +827,27 @@ int pb_protocol_deserialise_system_info(struct system_info *sysinfo,
> >  	if (read_string(sysinfo, &pos, &len, &sysinfo->identifier))
> >  		goto out;
> >  
> > +	/* versions strings for openpower platforms */
> > +	if (read_u32(&pos, &len, &sysinfo->n_current))
> > +		goto out;
> > +	sysinfo->platform_current = talloc_array(sysinfo, char *,
> > +						sysinfo->n_current);
> > +	for (i = 0; i < sysinfo->n_current; i++) {
> > +		if (read_string(sysinfo, &pos, &len, &tmp))
> > +			goto out;
> > +		sysinfo->platform_current[i] = talloc_strdup(sysinfo, tmp);
> > +	}
> > +
> > +	if (read_u32(&pos, &len, &sysinfo->n_other))
> > +		goto out;
> > +	sysinfo->platform_other = talloc_array(sysinfo, char *,
> > +						sysinfo->n_other);
> > +	for (i = 0; i < sysinfo->n_other; i++) {
> > +		if (read_string(sysinfo, &pos, &len, &tmp))
> > +			goto out;
> > +		sysinfo->platform_other[i] = talloc_strdup(sysinfo, tmp);
> > +	}
> > +
> >  	/* number of interfaces */
> >  	if (read_u32(&pos, &len, &sysinfo->n_interfaces))
> >  		goto out;
> > diff --git a/lib/types/types.h b/lib/types/types.h
> > index c2de8a5..db4d892 100644
> > --- a/lib/types/types.h
> > +++ b/lib/types/types.h
> > @@ -93,6 +93,10 @@ struct blockdev_info {
> >  struct system_info {
> >  	char			*type;
> >  	char			*identifier;
> > +	char			**platform_current;
> > +	char			**platform_other;
> > +	unsigned int		n_current;
> > +	unsigned int		n_other;
> >  	uint8_t			*bmc_mac;
> >  	struct interface_info	**interfaces;
> >  	unsigned int		n_interfaces;
> > diff --git a/ui/ncurses/nc-sysinfo.c b/ui/ncurses/nc-sysinfo.c
> > index 5ced871..6947f83 100644
> > --- a/ui/ncurses/nc-sysinfo.c
> > +++ b/ui/ncurses/nc-sysinfo.c
> > @@ -65,6 +65,22 @@ static void sysinfo_screen_populate(struct sysinfo_screen *screen,
> >  	line("%-12s %s", _("System type:"), sysinfo->type ?: "");
> >  	line("%-12s %s", _("System id:"),   sysinfo->identifier ?: "");
> >  
> > +	if (sysinfo->n_current) {
> > +		line(NULL);
> > +		line("%s", _("Platform versions - current:"));
> > +		for (i = 0; i < sysinfo->n_current; i++) {
> > +			line("%s", sysinfo->platform_current[i] ?: "");
> > +		}
> > +	}
> > +
> > +	if (sysinfo->n_other) {
> > +		line(NULL);
> > +		line("%s", _("Platform versions - alternate:"));
> > +		for (i = 0; i < sysinfo->n_other; i++) {
> > +			line("%s", sysinfo->platform_other[i] ?: "");
> > +		}
> > +	}
> > +
> >  	if (sysinfo->n_blockdevs) {
> >  		line(NULL);
> >  		line(_("Storage devices"));
> 



More information about the Petitboot mailing list