[Skiboot] [PATCH 1/4] core: Add debugfs framework

[Frederic Barrat]

On 05/11/2020 17:15, Cédric Le Goater wrote:
> This is a very simplistic framework adding a read/write interface to
> opal drivers. Drivers should define a set of 'opal_debug' handlers,
> each with custom read/write operations. The device tree is then
> populated with nodes such as :
> 
> 	debug at 0 {
> 		ibm,chip-id = <0x00>;
> 		label = "xive-ivt";
> 		compatible = "ibm,opal-debug";
> 		reg = <0x00>;
> 		phandle = <0x805e>;
> 	};
> 
> which can be freely extended by the driver if needed.
> 
> The Linux driver can choose to expose the contents of each 'opal_debug'
> handler through debugfs using the OPAL_DEBUG_READ call. It is relatively
> easy to implement a simple command interface with the OPAL_DEBUG_WRITE
> call, to set some values or reset some counters.
> 
> Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
>   include/opal-api.h   |  4 +-
>   include/opal-debug.h | 27 ++++++++++++
>   core/opal-debug.c    | 97 ++++++++++++++++++++++++++++++++++++++++++++
>   core/Makefile.inc    |  1 +
>   4 files changed, 128 insertions(+), 1 deletion(-)
>   create mode 100644 include/opal-debug.h
>   create mode 100644 core/opal-debug.c
> 
> diff --git a/include/opal-api.h b/include/opal-api.h
> index e90cab1e9f65..03c414946651 100644
> --- a/include/opal-api.h
> +++ b/include/opal-api.h
> @@ -227,7 +227,9 @@
>   #define OPAL_SECVAR_ENQUEUE_UPDATE		178
>   #define OPAL_PHB_SET_OPTION			179
>   #define OPAL_PHB_GET_OPTION			180
> -#define OPAL_LAST				180
> +#define OPAL_DEBUG_READ				181
> +#define OPAL_DEBUG_WRITE			182
> +#define OPAL_LAST				182
>   
>   #define QUIESCE_HOLD			1 /* Spin all calls at entry */
>   #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
> diff --git a/include/opal-debug.h b/include/opal-debug.h
> new file mode 100644
> index 000000000000..fc8e3980f8a6
> --- /dev/null
> +++ b/include/opal-debug.h
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2020 IBM Corp. */
> +
> +#ifndef __OPAL_DEBUG_H
> +#define __OPAL_DEBUG_H
> +
> +struct opal_debug;
> +
> +struct opal_debug_ops {
> +	const char *compat;
> +	int  (*read)(struct opal_debug *d, void *buf, uint64_t size);
> +	int  (*write)(struct opal_debug *d, void *buf, uint64_t size);
> +};
> +
> +struct opal_debug {
> +	struct list_node link;
> +	uint64_t id;
> +	struct dt_node *node;
> +	const char *name;
> +	void *private;
> +	const struct opal_debug_ops *ops;
> +};
> +
> +struct opal_debug *opal_debug_create(const char *name, struct dt_node *node,
> +			     void* private, const struct opal_debug_ops *ops);
> +
> +#endif
> diff --git a/core/opal-debug.c b/core/opal-debug.c
> new file mode 100644
> index 000000000000..a9ed2552e908
> --- /dev/null
> +++ b/core/opal-debug.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2020 IBM Corp. */
> +
> +#define pr_fmt(fmt) "DEBUG: " fmt
> +
> +#include <cpu.h>
> +#include <opal.h>
> +#include <opal-debug.h>
> +
> +static LIST_HEAD(opal_debug_handlers);
> +static uint64_t opal_debug_index;
> +
> +/* This would need some locking */
> +struct opal_debug *opal_debug_create(const char *name, struct dt_node *node,
> +				     void* private, const struct opal_debug_ops *ops)
> +{
> +	const char *compat = ops->compat ? ops->compat : "ibm,opal-debug";
> +	struct opal_debug *d;
> +
> +	d = zalloc(sizeof(*d));
> +	if (!d) {
> +		prlog(PR_ERR, "Failed to allocate debug handler!\n");
> +		return NULL;
> +	}
> +
> +	d->id = opal_debug_index;
> +	d->name = name;


Duplicating the name would be safer and avoid making assumptions on the 
caller.

  Fred


> +	d->private = private;
> +	d->ops = ops;
> +
> +	d->node = dt_new_addr(node, "debug", opal_debug_index);
> +	dt_add_property_cells(d->node, "reg", opal_debug_index);
> +	dt_add_property_string(d->node, "compatible", compat);
> +	dt_add_property_string(d->node, "label", d->name);
> +
> +	list_add_tail(&opal_debug_handlers, &d->link);
> +
> +	opal_debug_index++;
> +
> +	return d;
> +}
> +
> +static struct opal_debug *opal_debug_find(uint64_t id)
> +{
> +	struct opal_debug *d;
> +
> +	list_for_each(&opal_debug_handlers, d, link) {
> +		if (d->id == id)
> +			return d;
> +	}
> +	return NULL;
> +}
> +static int64_t opal_debug_read(uint64_t id, uint64_t buf, uint64_t size)
> +{
> +	struct opal_debug *d;
> +
> +	if (id >= opal_debug_index)
> +		return OPAL_PARAMETER;
> +
> +	if (!opal_addr_valid((void *)buf) || !size)
> +		return OPAL_PARAMETER;
> +
> +	d = opal_debug_find(id);
> +	if (!d) {
> +		prlog(PR_ERR, "No debug handler %lld!\n", id);
> +		return OPAL_INTERNAL_ERROR;
> +	}
> +
> +	if (!d->ops->read)
> +		return OPAL_UNSUPPORTED;
> +
> +	return d->ops->read(d, (void *)buf, size);
> +}
> +opal_call(OPAL_DEBUG_READ, opal_debug_read, 3);
> +
> +static int64_t opal_debug_write(uint64_t id, uint64_t buf, uint64_t size)
> +{
> +	struct opal_debug *d;
> +
> +	if (id >= opal_debug_index)
> +		return OPAL_PARAMETER;
> +
> +	if (!opal_addr_valid((void *)buf) || !size)
> +		return OPAL_PARAMETER;
> +
> +	d = opal_debug_find(id);
> +	if (!d) {
> +		prlog(PR_ERR, "No debug handler %lld!\n", id);
> +		return OPAL_INTERNAL_ERROR;
> +	}
> +
> +	if (!d->ops->write)
> +		return OPAL_UNSUPPORTED;
> +
> +	return d->ops->write(d, (void *)buf, size);
> +}
> +opal_call(OPAL_DEBUG_WRITE, opal_debug_write, 3);
> diff --git a/core/Makefile.inc b/core/Makefile.inc
> index 829800e5b27f..ed7003ed7a7a 100644
> --- a/core/Makefile.inc
> +++ b/core/Makefile.inc
> @@ -13,6 +13,7 @@ CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
>   CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o powercap.o psr.o
>   CORE_OBJS += pci-dt-slot.o direct-controls.o cpufeatures.o
>   CORE_OBJS += flash-firmware-versions.o opal-dump.o
> +CORE_OBJS += opal-debug.o
>   
>   ifeq ($(SKIBOOT_GCOV),1)
>   CORE_OBJS += gcov-profiling.o
>