[Pdbg] [PATCH 26/29] libpdbg/htm: Add POWER8 HTM (both Core and Nest)

[Alistair Popple]

> +static int do_chtm_reset(struct htm *htm, uint64_t *r_base, uint64_t *r_size)
> +{
> +	struct htm_status status;
> +
> +	if (HTM_ERR(get_status(htm, &status)))
> +		return -1;
> +
> +	if (!is_resetable(&status) || !is_configured(htm)) {
> +		if (HTM_ERR(configure_chtm(htm))) {
> +			PR_ERROR("Couldn't setup Core HTM during reset\n");
> +			return -1;
> +		}
> +	}
> +
> +	if (HTM_ERR(configure_memory(htm, r_base, r_size)))
> +		return -1;
> +
> +	return 1;
> +}
> +
> +static int do_nhtm_reset(struct htm *htm, uint64_t *r_base, uint64_t *r_size)
> +{
> +	struct htm_status status;
> +
> +	if (HTM_ERR(get_status(htm, &status)))
> +		return -1;
> +
> +	if (!is_resetable(&status) || !is_configured(htm)) {
> +		if (HTM_ERR(configure_nhtm(htm))) {
> +			PR_ERROR("Couldn't setup Nest HTM during reset\n");
> +			return -1;
> +		}

The majority of this code is the same as do_chtm_reset() above. Personally I
think it might be nicer to have `if (pdbg_target_class_name(target) == "nhtm")
...` rather than duplicate the code, but I'll admit this is mostly a personal
preference.

> +	}
> +
> +	if (HTM_ERR(configure_memory(htm, r_base, r_size)))
> +		return -1;
> +
>  	return 1;
>  }
>  
> @@ -834,28 +915,59 @@ static int do_htm_dump(struct htm *htm, uint64_t size, const char *basename)
>  	return 1;
>  }
>  
> -static int nhtm_probe(struct pdbg_target *target)
> +static bool is_debugfs_memtrace_ok(void)
>  {
> -	uint64_t val;
> -
>  	if (access(DEBUGFS_MEMTRACE, F_OK) != 0) {
>  		PR_DEBUG("Couldn't open debugfs\n");

Previously I think PR_DEBUG was defined in the HTM code, is that still the case?

Printing messages in the probe functions is problematic as they are called on
all platforms and are expected to fail on at least some. Although printing at
debug level is ok, we just need to make sure it's not enabled by default. And I
will admit our error logging could use some improvement :-)

> +struct htm p9nhtm = {
> +	.target = {
> +		.name =	"POWER9 Nest HTM",
> +		.compatible = "ibm,power9-nhtm",
> +		.class = "nhtm",
> +		.probe = nhtm_probe,
> +	},
> +	.start = do_htm_start,
> +	.stop = do_htm_stop,
> +	.reset = do_nhtm_reset,
> +	.pause = do_htm_pause,
> +	.status = do_htm_status,
> +	.dump = do_htm_dump,
> +};
> +DECLARE_HW_UNIT(p9nhtm);
> +
> +struct htm p8nhtm = {
>  	.target = {
> -		.name =	"Nest HTM",
> -		.compatible = "ibm,nhtm",
> +		.name = "POWER8 Nest HTM",
> +		.compatible = "ibm,power8-nhtm",
>  		.class = "nhtm",
>  		.probe = nhtm_probe,
>  	},
> @@ -866,4 +978,20 @@ struct htm nhtm = {
>  	.status = do_htm_status,
>  	.dump = do_htm_dump,
>  };
> -DECLARE_HW_UNIT(nhtm);
> +DECLARE_HW_UNIT(p8nhtm);

Is there any reason at present to have a POWER8 vs. POWER9 version of these
classes? I would be quite happy to have a single nhtm class with `compatible =
"ibm,nhtm", "ibm,power?-nhtm"` to differentiate between the two flavours of
almost identical hardware interfaces.

> +
> +struct htm chtm = {
> +	.target = {
> +		.name = "POWER8 Core HTM",
> +		.compatible = "ibm,power8-chtm",
> +		.class = "chtm",
> +		.probe = chtm_probe,
> +	},
> +	.start = do_htm_start,
> +	.stop = do_htm_stop,
> +	.reset = do_chtm_reset,
> +	.pause = do_htm_pause,
> +	.status = do_htm_status,
> +	.dump = do_htm_dump,
> +};
> +DECLARE_HW_UNIT(chtm);

In fact if you merge the do_?htm_reset() functions this could also be merged as
another flavour of the hardware with `compatible = "ibm,htm",
"ibm,power8-html"`. I guess this could make the command parsing harder though,
so I'm not going to get too hung up on this if you don't think it makes sense.

- Alistair

> diff --git a/p8-pib.dts.m4 b/p8-pib.dts.m4
> index de1d3bf..bdc02c5 100644
> --- a/p8-pib.dts.m4
> +++ b/p8-pib.dts.m4
> @@ -7,6 +7,11 @@ define(`CORE', `core at CORE_BASE($1) {
>  	compatible = "ibm,power8-core";
>  	reg = <0x0 HEX(CORE_BASE($1)) 0xfffff>;
>  	index = <0x$2>;
> +	chtm at 11000 {
> +		compatible = "ibm,power8-chtm";
> +		reg = <0x0 0x11000 0xB>;
> +		index = <0x0>;
> +	};
>  
>  	THREAD(0);
>  	THREAD(1);
> @@ -42,4 +47,10 @@ adu at 2020000 {
>  	reg = <0x0 0x2020000 0x4>;
>  };
>  
> +nhtm at 2010880 {
> +	compatible = "ibm,power8-nhtm";
> +	reg = <0x0 0x2010880 0x8>;
> +	index = <0x0>;
> +};
> +
>  PROC_CORES;
>