[Pdbg] [PATCH v3 1/2] libpdbg: Add default device-tree selection to libpdbg

Amitay Isaacs amitay at ozlabs.org
Wed Jul 3 14:06:59 AEST 2019


Apart from one cosmetic change, it looks good.

Replace AM_V_CPPAS with GEN_V to indicate generated file.

Reviewed-by: Amitay Isaacs <amitay at ozlabs.org>

On Wed, 2019-07-03 at 13:20 +1000, Alistair Popple wrote:
> For portability reasons libpdbg intentionally did not deal with the
> selection and retrieval of the device tree for a given system. This
> means that by design each application was responsible for selecting
> and loading the correct device tree for the system it was on.
> 
> This is convenient for embedded systems as it removes the dependency
> on some libc functions such as open. However there are currently no
> embedded users of libpdbg and forcing end user applications to select
> the correct device tree is likely to be error prone and inconsistent.
> 
> Instead this patch adds the ability for applications to call
> pdbg_targets_init(NULL) and have libpdbg attempt to detect and load
> the correct device tree for the current system. This doesn't change
> any existing behaviour as applications may still pass an explicit
> device-tree to pdbg_targets_init() which will always be used.
> 
> An environment variable is provided so users may override the
> auto-detected device-tree when the application does not provide one.
> 
> This patch will bloat the overall size of the library as it will now
> include all the device-trees. It also likely makes things harder for
> embedded users. However if either of these is an issue we should be
> able to easily add a build time option to exclude this and/or only
> build in specific device-trees.
> 
> Signed-off-by: Alistair Popple <alistair at popple.id.au>
> ---
>  .gitignore        |   1 +
>  Makefile.am       |  19 +++-
>  configure.ac      |   1 +
>  libpdbg/device.c  |  10 ++
>  libpdbg/dtb.c     | 270
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libpdbg/i2c.c     |   6 +-
>  libpdbg/libpdbg.h |   5 +
>  libpdbg/target.h  |   2 +
>  8 files changed, 309 insertions(+), 5 deletions(-)
>  create mode 100644 libpdbg/dtb.c
> 
> diff --git a/.gitignore b/.gitignore
> index 79f1ef9..13f21ff 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -35,6 +35,7 @@ m4
>  *.dtsi
>  *.dts
>  *.dt.h
> +*.dtb.S
>  optcmd_test
>  *.trs
>  *.log
> diff --git a/Makefile.am b/Makefile.am
> index 22faf6e..b317b96 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -55,20 +55,23 @@ EXTRA_DIST = \
>  	tests/test_driver.sh \
>  	$(PDBG_TESTS)
>  
> -if TARGET_ARM
>  DT_ARM = p8-fsi.dts p8-i2c.dts p8-kernel.dts \
>  	 p9w-fsi.dts p9r-fsi.dts p9z-fsi.dts p9-kernel.dts
> +
> +if TARGET_ARM
>  ARCH_FLAGS="-DTARGET_ARM=1"
>  endif
>  
> -if TARGET_PPC
>  DT_PPC = p8-host.dts p9-host.dts
> +
> +if TARGET_PPC
>  ARCH_FLAGS="-DTARGET_PPC=1"
>  endif
>  
>  DT = fake.dts $(DT_ARM) $(DT_PPC)
>  
> -DT_objects = $(DT:.dts=.dtb.o)
> +DT_objects = $(DT:.dts=.dtb.lo)
> +DT_sources = $(DT:.dts=.dtb.S)
>  DT_headers = $(DT:.dts=.dt.h)
>  
>  optcmd_test_SOURCES = src/optcmd.c src/parsers.c
> src/tests/optcmd_test.c
> @@ -127,6 +130,7 @@ pdbg_CFLAGS += -DDISABLE_GDBSERVER
>  endif
>  
>  src/main.c: $(DT_headers)
> +libpdbg/dtb.c: $(DT_headers)
>  
>  src/pdbg-gdb_parser.$(OBJEXT): CFLAGS+=-Wno-unused-const-variable
>  
> @@ -136,6 +140,7 @@ pdbg_LDADD = $(DT_objects) libpdbg.la libccan.a \
>  lib_LTLIBRARIES = libpdbg.la
>  
>  libpdbg_la_SOURCES = \
> +	$(DT_sources) \
>  	libpdbg/adu.c \
>  	libpdbg/backend.h \
>  	libpdbg/bitutils.h \
> @@ -147,6 +152,7 @@ libpdbg_la_SOURCES = \
>  	libpdbg/debug.c \
>  	libpdbg/debug.h \
>  	libpdbg/device.c \
> +	libpdbg/dtb.c \
>  	libpdbg/fake.c \
>  	libpdbg/host.c \
>  	libpdbg/htm.c \
> @@ -247,10 +253,15 @@ p9z-fsi.dts: p9z-fsi.dts.m4 p9-fsi.dtsi
>  %.dt.h: %.dts
>  	$(GEN_V)$(srcdir)/generate_dt_header.sh $< > $@
>  
> +%.dtb.S: %.dtb
> +	$(AM_V_CPPAS)$(CPP) $(srcdir)/template.S \
> +	-DSYMBOL_PREFIX=$(shell basename $< .S | tr '.-' '_')_o
> -DFILENAME=\"$<\" -o \
> +	$@
> +
>  %.dtb.o: %.dtb
>  	$(AM_V_CC)$(CC) -c $(srcdir)/template.S -DSYMBOL_PREFIX=$(shell
> basename $@ | tr '.-' '_') -DFILENAME=\"$<\" -o $@
>  
>  %.c: %.rl
>  	$(RAGEL_V)$(RAGEL) -o $@ $<
>  
> -MOSTLYCLEANFILES = *.dtb *.dts *.dt.h p9-fsi.dtsi src/gdb_parser.c
> +MOSTLYCLEANFILES = *.dtb.S *.dtb *.dts *.dt.h p9-fsi.dtsi
> src/gdb_parser.c
> diff --git a/configure.ac b/configure.ac
> index c62ec05..c41e13a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3,6 +3,7 @@ AM_INIT_AUTOMAKE([subdir-objects])
>  AM_SILENT_RULES([yes])
>  
>  AC_PROG_CC
> +AM_PROG_AS
>  AC_PROG_LIBTOOL
>  
>  AC_PATH_PROG([M4], [m4])
> diff --git a/libpdbg/device.c b/libpdbg/device.c
> index b7fd49f..ba9e05e 100644
> --- a/libpdbg/device.c
> +++ b/libpdbg/device.c
> @@ -638,7 +638,17 @@ uint64_t pdbg_target_address(struct pdbg_target
> *target, uint64_t *out_size)
>  
>  void pdbg_targets_init(void *fdt)
>  {
> +	/* Root node needs to be valid when this function returns */
>  	pdbg_dt_root = dt_new_node("", NULL, 0);
> +
> +	if (!fdt)
> +		fdt = pdbg_default_dtb();
> +
> +	if (!fdt) {
> +		pdbg_log(PDBG_ERROR, "Could not find a system device
> tree\n");
> +		return;
> +	}
> +
>  	dt_expand(fdt);
>  }
>  
> diff --git a/libpdbg/dtb.c b/libpdbg/dtb.c
> new file mode 100644
> index 0000000..76c6cf9
> --- /dev/null
> +++ b/libpdbg/dtb.c
> @@ -0,0 +1,270 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * 	http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions
> and
> + * limitations under the License.
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +#include <assert.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +
> +#include "libpdbg.h"
> +
> +#include "fake.dt.h"
> +
> +#include "p8-i2c.dt.h"
> +#include "p8-fsi.dt.h"
> +#include "p8-kernel.dt.h"
> +#include "p9w-fsi.dt.h"
> +#include "p9r-fsi.dt.h"
> +#include "p9z-fsi.dt.h"
> +#include "p9-kernel.dt.h"
> +#include "p8-host.dt.h"
> +#include "p9-host.dt.h"
> +
> +#define AMI_BMC "/proc/ractrends/Helper/FwInfo"
> +#define OPENFSI_BMC "/sys/bus/platform/devices/gpio-fsi/fsi0/"
> +#define FSI_CFAM_ID "/sys/devices/platform/gpio-fsi/fsi0/slave at 00:00
> /cfam_id"
> +#define XSCOM_BASE_PATH "/sys/kernel/debug/powerpc/scom"
> +
> +#define CHIP_ID_P8  0xea
> +#define CHIP_ID_P8P 0xd3
> +#define CHIP_ID_P9  0xd1
> +
> +static enum pdbg_backend pdbg_backend = PDBG_DEFAULT_BACKEND;
> +static const char *pdbg_backend_option;
> +
> +/* Determines the most appropriate backend for the host system we
> are
> + * running on. */
> +static enum pdbg_backend default_backend(void)
> +{
> +	int rc;
> +
> +	rc = access(XSCOM_BASE_PATH, F_OK);
> +	if (rc == 0) /* PowerPC Host System */
> +		return PDBG_BACKEND_HOST;
> +
> +	rc = access(AMI_BMC, F_OK);
> +	if (rc == 0) /* AMI BMC */
> +		return PDBG_BACKEND_I2C;
> +
> +	rc = access(OPENFSI_BMC, F_OK);
> +	if (rc == 0) /* Kernel interface. OpenBMC */
> +		return PDBG_BACKEND_KERNEL;
> +
> +	return PDBG_BACKEND_FAKE;
> +}
> +
> +/* Try and determine what system type we are on by reading
> + * /proc/cpuinfo */
> +static void *ppc_target(void)
> +{
> +	const char *pos = NULL;
> +	char line[256];
> +	FILE *cpuinfo;
> +
> +	if (!strcmp(pdbg_backend_option, "p8"))
> +		return &_binary_p8_host_dtb_o_start;
> +	else if (!strcmp(pdbg_backend_option, "p9"))
> +		return &_binary_p9_host_dtb_o_start;
> +
> +	cpuinfo = fopen("/proc/cpuinfo", "r");
> +	if (!cpuinfo)
> +		return NULL;
> +
> +	while ((pos = fgets(line, sizeof(line), cpuinfo)))
> +		if (strncmp(line, "cpu", 3) == 0)
> +			break;
> +	fclose(cpuinfo);
> +
> +	if (!pos) {
> +		/* Got to EOF without a break */
> +		pdbg_log(PDBG_ERROR, "Unable to parse
> /proc/cpuinfo\n");
> +		return NULL;
> +	}
> +
> +	pos = strchr(line, ':');
> +	if (!pos || (*(pos + 1) == '\0')) {
> +		pdbg_log(PDBG_ERROR, "Unable to parse
> /proc/cpuinfo\n");
> +		return NULL;
> +	}
> +
> +	pos += 2;
> +
> +	if (strncmp(pos, "POWER8", 6) == 0) {
> +		pdbg_log(PDBG_INFO, "Found a POWER8 PPC host
> system\n");
> +		return &_binary_p8_host_dtb_o_start;
> +	}
> +
> +	if (strncmp(pos, "POWER9", 6) == 0) {
> +		pdbg_log(PDBG_INFO, "Found a POWER9 PPC host
> system\n");
> +		return &_binary_p9_host_dtb_o_start;
> +	}
> +
> +	pdbg_log(PDBG_ERROR, "Unsupported CPU type '%s'\n", pos);
> +	return NULL;
> +}
> +
> +static void *bmc_target(void)
> +{
> +	FILE *cfam_id_file;
> +	uint32_t cfam_id = 0;
> +	uint32_t chip_id = 0;
> +	int rc;
> +
> +	if (!pdbg_backend_option) {
> +		/* Try and determine the correct device type */
> +		cfam_id_file = fopen(FSI_CFAM_ID, "r");
> +		if (!cfam_id_file) {
> +			pdbg_log(PDBG_ERROR, "Unabled to open CFAM ID
> file\n");
> +			return NULL;
> +		}
> +
> +		rc = fscanf(cfam_id_file, "0x%" PRIx32, &cfam_id);
> +		if (rc != 1) {
> +			pdbg_log(PDBG_ERROR, "Unable to read CFAM ID:
> %s", strerror(errno));
> +		}
> +		fclose(cfam_id_file);
> +		chip_id = (cfam_id >> 4) & 0xff;
> +	} else {
> +		if (!strcmp(pdbg_backend_option, "p9"))
> +			chip_id = CHIP_ID_P9;
> +		else if (!strcmp(pdbg_backend_option, "p8"))
> +			chip_id = CHIP_ID_P8;
> +		else
> +			pdbg_log(PDBG_WARNING, "Invalid OpenBMC system
> type '%s' specified\n",
> +				 pdbg_backend_option);
> +	}
> +
> +	switch(chip_id) {
> +	case CHIP_ID_P9:
> +		pdbg_log(PDBG_INFO, "Found a POWER9 OpenBMC based
> system\n");
> +		return &_binary_p9_kernel_dtb_o_start;
> +		break;
> +
> +	case CHIP_ID_P8:
> +	case CHIP_ID_P8P:
> +		pdbg_log(PDBG_INFO, "Found a POWER8/8+ OpenBMC based
> system\n");
> +		return &_binary_p8_kernel_dtb_o_start;
> +		break;
> +
> +	default:
> +		pdbg_log(PDBG_ERROR, "Unrecognised Chip ID register
> 0x%08" PRIx32 "\n", chip_id);
> +		return NULL;
> +	}
> +}
> +
> +/* Opens a dtb at the given path */
> +static void *mmap_dtb(char *file)
> +{
> +	int fd;
> +	void *dtb;
> +	struct stat statbuf;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		pdbg_log(PDBG_ERROR, "Unable to open dtb file '%s'\n",
> file);
> +		return NULL;
> +	}
> +
> +	if (fstat(fd, &statbuf) == -1) {
> +		pdbg_log(PDBG_ERROR, "Failed to stat file '%s'\n",
> file);
> +		goto fail;
> +	}
> +
> +	dtb = mmap(NULL, statbuf.st_size, PROT_READ, MAP_PRIVATE, fd,
> 0);
> +	if (dtb == MAP_FAILED) {
> +		pdbg_log(PDBG_ERROR, "Failed top mmap file '%s'\n",
> file);
> +		goto fail;
> +	}
> +
> +	return dtb;
> +
> +fail:
> +	close(fd);
> +	return NULL;
> +}
> +
> +int pdbg_set_backend(enum pdbg_backend backend, const char
> *backend_option)
> +{
> +	pdbg_backend = backend;
> +	pdbg_backend_option = backend_option;
> +
> +	return 0;
> +}
> +
> +const char *pdbg_get_backend_option(void)
> +{
> +	return pdbg_backend_option;
> +}
> +
> +/* Determines what platform we are running on and returns a pointer
> to
> + * the fdt that is most likely to work on the system. */
> +void *pdbg_default_dtb(void)
> +{
> +	char *dtb = getenv("PDBG_DTB");
> +
> +	if (dtb)
> +		return mmap_dtb(dtb);
> +
> +	if (!pdbg_backend)
> +		pdbg_backend = default_backend();
> +
> +	switch(pdbg_backend) {
> +	case PDBG_BACKEND_HOST:
> +		return ppc_target();
> +		break;
> +
> +	case PDBG_BACKEND_I2C:
> +		/* I2C is only supported on POWER8 */
> +		pdbg_log(PDBG_INFO, "Found a POWER8 AMI BMC based
> system\n");
> +		return &_binary_p8_i2c_dtb_o_start;
> +		break;
> +
> +	case PDBG_BACKEND_KERNEL:
> +		return bmc_target();
> +		break;
> +
> +	case PDBG_BACKEND_FSI:
> +		if (!strcmp(pdbg_backend_option, "p8"))
> +			return &_binary_p8_fsi_dtb_o_start;
> +		else if (!strcmp(pdbg_backend_option, "p9w"))
> +			return &_binary_p9w_fsi_dtb_o_start;
> +		else if (!strcmp(pdbg_backend_option, "p9r"))
> +			return &_binary_p9r_fsi_dtb_o_start;
> +		else if (!strcmp(pdbg_backend_option, "p9z"))
> +			return &_binary_p9z_fsi_dtb_o_start;
> +		else {
> +			pdbg_log(PDBG_WARNING, "Invalid device type
> specified, using a fake one\n");
> +			return &_binary_fake_dtb_o_start;
> +		}
> +
> +		break;
> +
> +	default:
> +		pdbg_log(PDBG_WARNING, "Unable to determine a valid
> default backend, using a fake one for testing purposes\n");
> +		/* Fall through */
> +
> +	case PDBG_BACKEND_FAKE:
> +		return &_binary_fake_dtb_o_start;
> +		break;
> +	}
> +}
> diff --git a/libpdbg/i2c.c b/libpdbg/i2c.c
> index 227c8a0..1a5d089 100644
> --- a/libpdbg/i2c.c
> +++ b/libpdbg/i2c.c
> @@ -131,7 +131,11 @@ int i2c_target_probe(struct pdbg_target *target)
>  	const char *bus;
>  	int addr;
>  
> -	bus = pdbg_target_property(&pib->target, "bus", NULL);
> +	bus = pdbg_get_backend_option();
> +
> +	if (!bus)
> +		bus = pdbg_target_property(&pib->target, "bus", NULL);
> +
>  	if (!bus)
>  		bus = "/dev/i2c4";
>  
> diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> index 14a41ab..ae763e5 100644
> --- a/libpdbg/libpdbg.h
> +++ b/libpdbg/libpdbg.h
> @@ -52,6 +52,9 @@ enum pdbg_target_status {PDBG_TARGET_UNKNOWN = 0,
> PDBG_TARGET_ENABLED,
>  			 PDBG_TARGET_DISABLED, PDBG_TARGET_MUSTEXIST,
>  			 PDBG_TARGET_NONEXISTENT,
> PDBG_TARGET_RELEASED};
>  
> +enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0, PDBG_BACKEND_FSI,
> PDBG_BACKEND_I2C,
> +		    PDBG_BACKEND_KERNEL, PDBG_BACKEND_FAKE,
> PDBG_BACKEND_HOST };
> +
>  #define pdbg_for_each_compatible(parent, target, compat)		
> \
>          for (target =
> NULL;                                             \
>               (target = __pdbg_next_compatible_node(parent, target,
> compat)) != NULL;)
> @@ -103,6 +106,8 @@ enum pdbg_target_status pdbg_target_probe(struct
> pdbg_target *target);
>  void pdbg_target_release(struct pdbg_target *target);
>  enum pdbg_target_status pdbg_target_status(struct pdbg_target
> *target);
>  void pdbg_target_status_set(struct pdbg_target *target, enum
> pdbg_target_status status);
> +int pdbg_set_backend(enum pdbg_backend backend, const char
> *backend_option);
> +void *pdbg_default_dtb(void);
>  uint32_t pdbg_target_index(struct pdbg_target *target);
>  char *pdbg_target_path(const struct pdbg_target *target);
>  struct pdbg_target *pdbg_target_from_path(struct pdbg_target
> *target, const char *path);
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 233f085..193c0e0 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -61,4 +61,6 @@ bool pdbg_target_is_class(struct pdbg_target
> *target, const char *class);
>  extern struct list_head empty_list;
>  extern struct list_head target_classes;
>  
> +const char *pdbg_get_backend_option(void);
> +
>  #endif
> -- 
> 2.11.0
> 

Amitay.
-- 

Wealth is what you are, not what you have.



More information about the Pdbg mailing list