[Pdbg] [PATCH] libpdbg: Add support for getring on POWER9
Balbir Singh
bsingharora at gmail.com
Tue May 1 14:30:09 AEST 2018
On Mon, 30 Apr 2018 17:28:13 +1000
Alistair Popple <alistair at popple.id.au> wrote:
> Add basic support for a getring operation on POWER9.
>
Honestly a very hard patch to review -- see below
> Signed-off-by: Alistair Popple <alistair at popple.id.au>
> ---
> Makefile.am | 2 +-
> libpdbg/chip.c | 12 +++++++
> libpdbg/libpdbg.h | 2 ++
> libpdbg/p9chip.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> libpdbg/target.c | 22 +++++++++++++
> libpdbg/target.h | 2 ++
> src/main.c | 14 ++++++++
> src/ring.c | 81 +++++++++++++++++++++++++++++++++++++++++++++
> src/ring.h | 17 ++++++++++
> 9 files changed, 246 insertions(+), 5 deletions(-)
> create mode 100644 src/ring.c
> create mode 100644 src/ring.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 099a035..c6668d8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -12,7 +12,7 @@ AM_CFLAGS = -I$(top_srcdir)/ccan/array_size -Wall -Werror -O2
>
> pdbg_SOURCES = \
> src/main.c src/cfam.c src/scom.c src/reg.c src/mem.c src/thread.c \
> - src/htm.c src/options_ at ARCH@.c
> + src/ring.c src/htm.c src/options_ at ARCH@.c
>
> pdbg_LDADD = fake.dtb.o p8-fsi.dtb.o p8-i2c.dtb.o p9w-fsi.dtb.o p8-host.dtb.o \
> p9z-fsi.dtb.o p9r-fsi.dtb.o p9-kernel.dtb.o libpdbg.la libfdt.la \
> diff --git a/libpdbg/chip.c b/libpdbg/chip.c
> index 06ea4e9..79bc87d 100644
> --- a/libpdbg/chip.c
> +++ b/libpdbg/chip.c
> @@ -272,3 +272,15 @@ int ram_getmem(struct pdbg_target *thread, uint64_t addr, uint64_t *value)
> *value = results[3];
> return 0;
> }
> +
> +/*
> + * Read the given ring from the given chiplet. Result must be large enough to hold ring_len bits.
> + */
> +int getring(struct pdbg_target *chiplet_target, uint64_t ring_addr, uint64_t ring_len, uint32_t result[])
> +{
> + struct chiplet *chiplet;
> +
> + assert(!strcmp(chiplet_target->class, "chiplet"));
> + chiplet = target_to_chiplet(chiplet_target);
> + return chiplet->getring(chiplet, ring_addr, ring_len, result);
> +}
> diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> index 15537b8..f843816 100644
> --- a/libpdbg/libpdbg.h
> +++ b/libpdbg/libpdbg.h
> @@ -78,6 +78,7 @@ int fsi_write(struct pdbg_target *target, uint32_t addr, uint32_t val);
>
> int pib_read(struct pdbg_target *target, uint64_t addr, uint64_t *val);
> int pib_write(struct pdbg_target *target, uint64_t addr, uint64_t val);
> +int pib_wait(struct pdbg_target *pib_dt, uint64_t addr, uint64_t mask, uint64_t data);
>
> int ram_putmsr(struct pdbg_target *target, uint64_t val);
> int ram_putnia(struct pdbg_target *target, uint64_t val);
> @@ -92,6 +93,7 @@ int ram_step_thread(struct pdbg_target *target, int steps);
> int ram_stop_thread(struct pdbg_target *target);
> int ram_sreset_thread(struct pdbg_target *target);
> uint64_t thread_status(struct pdbg_target *target);
> +int getring(struct pdbg_target *chiplet_target, uint64_t ring_addr, uint64_t ring_len, uint32_t result[]);
>
> #define THREAD_STATUS_DISABLED PPC_BIT(0)
> #define THREAD_STATUS_ACTIVE PPC_BIT(63)
> diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
> index 41f694e..380cb59 100644
> --- a/libpdbg/p9chip.c
> +++ b/libpdbg/p9chip.c
> @@ -35,11 +35,34 @@
> #define P9_SPR_MODE 0x10a84
> #define P9_SCR0_REG 0x10a86
>
> +#define CHIPLET_CTRL0_WOR 0x10
> +#define CHIPLET_CTRL0_CLEAR 0x20
> +#define CHIPLET_CTRL0_CTRL_CC_ABIST_MUXSEL_DC PPC_BIT(0)
> +#define CHIPLET_CTRL0_TC_UNIT_SYNCCLK_MUXSEL_DC PPC_BIT(1)
> +#define CHIPLET_CTRL0_CTRL_CC_FLUSHMODE_INH_DC PPC_BIT(2)
> +
> +#define CHIPLET_CTRL1_WOR 0x11
> +#define CHIPLET_CTRL1_TC_VITL_REGION_FENCE PPC_BIT(3)
> +
> +#define CHIPLET_STAT0 0x100
> +#define CHIPLET_STAT0_CC_CTRL_OPCG_DONE_DC PPC_BIT(8)
> +
> +#define CHIPLET_SCAN_REGION_TYPE 0x30005
> +#define CHIPLET_CLK_REGION 0x30006
> +#define CHIPLET_CLK_REGION_CLOCK_CMD PPC_BITMASK(0, 1)
> +#define CHIPLET_CLK_REGION_CLOCK_CMD_STOP 0x2
> +#define CHIPLET_CLK_REGION_SLAVE_MODE PPC_BIT(2)
> +#define CHIPLET_CLK_REGION_MASTER_MODE PPC_BIT(3)
> +#define CHIPLET_CLK_REGION_REGIONS PPC_BITMASK(4, 14)
> +#define CHIPLET_CLK_REGION_SEL_THOLD PPC_BITMASK(48, 50)
> +
Some comments on these definitions would be nice
> /* PCB Slave Registers */
> -#define NET_CTRL0 0xf0040
> -#define NET_CTRL0_CHIPLET_ENABLE PPC_BIT(0)
> -#define PPM_GPMMR 0xf0100
> -#define PPM_SPWKUP_OTR 0xf010a
> +#define NET_CTRL0 0xf0040
> +#define NET_CTRL0_CHIPLET_ENABLE PPC_BIT(0)
> +#define NET_CTRL0_FENCE_EN PPC_BIT(18)
> +#define NET_CTRL0_WOR 0xf0042
> +#define PPM_GPMMR 0xf0100
> +#define PPM_SPWKUP_OTR 0xf010a
> #define SPECIAL_WKUP_DONE PPC_BIT(1)
>
> #define RAS_STATUS_TIMEOUT 100
> @@ -215,6 +238,73 @@ struct thread p9_thread = {
> };
> DECLARE_HW_UNIT(p9_thread);
>
> +#define HEADER_CHECK_DATA ((uint64_t) 0xc0ffee03 << 32)
> +
> +static int p9_chiplet_getring(struct chiplet *chiplet, uint64_t ring_addr, int64_t ring_len, uint32_t result[])
> +{
> + uint64_t scan_type_addr;
> + uint64_t scan_data_addr;
> + uint64_t scan_header_addr;
> + uint64_t scan_type_data;
> + uint64_t set_pulse = 1;
> + uint64_t bits = 32;
> + uint64_t data;
> +
> + /* We skip the first word in the results so we can write it later as it
> + * should contain the header read out at the end */
> + int i = 0;
> +
> + scan_type_addr = (ring_addr & 0x7fff0000) | 0x7;
> + scan_data_addr = (scan_type_addr & 0xffff0000) | 0x8000;
> + scan_header_addr = scan_data_addr & 0xffffe000;
> +
> + scan_type_data = (ring_addr & 0xfff0) << 13;
> + scan_type_data |= 0x800 >> (ring_addr & 0xf);
> + scan_type_data <<= 32;
> +
What's this block doing?
> + pib_write(&chiplet->target, scan_type_addr, scan_type_data);
> + pib_write(&chiplet->target, scan_header_addr, HEADER_CHECK_DATA);
> +
> + /* The final 32 bit read is the header which we do at the end */
> + ring_len -= 32;
> + i = 1;
> +
> + while (ring_len > 0) {
> + ring_len -= bits;
> + if (set_pulse) {
> + scan_data_addr |= 0x4000;
> + set_pulse = 0;
> + } else
> + scan_data_addr &= ~0x4000ULL;
> +
> + scan_data_addr &= ~0xffull;
> + scan_data_addr |= bits;
> + pib_read(&chiplet->target, scan_data_addr, &data);
> +
> + /* Discard lower 32 bits */
> + /* TODO: We always read 64-bits from the ring on P9 so we could
> + * optimise here by reading 64-bits at a time, but I'm not
> + * confident I've figured that out and 32-bits is what Hostboot
> + * does and seems to work. */
> + data >>= 32;
> +
> + /* Left-align data */
> + data <<= 32 - bits;
> + result[i++] = data;
> + if (ring_len > 0 && (ring_len < bits))
> + bits = ring_len;
> + }
> +
> + pib_read(&chiplet->target, scan_header_addr | 0x20, &data);
> + data &= 0xffffffff00000000;
> + result[0] = data >> 32;
> + if (data != HEADER_CHECK_DATA)
> + printf("WARNING: Header check failed. Make sure you specified the right ring length!\n"
> + "Ring data is probably corrupt now.\n");
> +
> + return 0;
> +}
> +
> static int p9_core_probe(struct pdbg_target *target)
> {
> int i = 0;
> @@ -271,5 +361,6 @@ struct chiplet p9_chiplet = {
> .class = "chiplet",
> .probe = p9_chiplet_probe,
> },
> + .getring = p9_chiplet_getring,
> };
> DECLARE_HW_UNIT(p9_chiplet);
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index 7e42b58..704d7d5 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -139,6 +139,28 @@ int pib_write(struct pdbg_target *pib_dt, uint64_t addr, uint64_t data)
> return rc;
> }
>
> +/* Wait for a SCOM register addr to match value & mask == data */
> +int pib_wait(struct pdbg_target *pib_dt, uint64_t addr, uint64_t mask, uint64_t data)
> +{
> + struct pib *pib;
> + uint64_t tmp;
> + int rc;
> +
> + pib_dt = get_class_target_addr(pib_dt, "pib", &addr);
> + pib = target_to_pib(pib_dt);
> +
> + do {
> + if (addr & PPC_BIT(0))
> + rc = pib_indirect_read(pib, addr, &tmp);
> + else
> + rc = pib->read(pib, addr, &tmp);
> + if (rc)
> + return rc;
> + } while ((tmp & mask) != data);
> +
> + return 0;
> +}
> +
> int opb_read(struct pdbg_target *opb_dt, uint32_t addr, uint32_t *data)
> {
> struct opb *opb;
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index dc91d68..eba26cb 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -157,5 +157,7 @@ struct thread {
> /* Place holder for chiplets which we just want translation for */
> struct chiplet {
> struct pdbg_target target;
> + int (*getring)(struct chiplet *, uint64_t, int64_t, uint32_t[]);
> };
> +#define target_to_chiplet(x) container_of(x, struct chiplet, target)
> #endif
> diff --git a/src/main.c b/src/main.c
> index c91066f..025ad56 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -37,6 +37,7 @@
> #include "cfam.h"
> #include "scom.h"
> #include "reg.h"
> +#include "ring.h"
> #include "mem.h"
> #include "thread.h"
> #include "htm.h"
> @@ -84,6 +85,7 @@ static struct {
> { "putspr", "<spr> <value>", "Write Special Purpose Register (SPR)", &handle_spr },
> { "getmsr", "", "Get Machine State Register (MSR)", &handle_msr },
> { "putmsr", "<value>", "Write Machine State Register (MSR)", &handle_msr },
> + { "getring", "<addr> <len>", "Read a ring. Length must be correct", &handle_getring },
How? Where are they documented? What is addr? How can length be correct?
> { "start", "", "Start thread", &thread_start },
> { "step", "<count>", "Set a thread <count> instructions", &thread_step },
> { "stop", "", "Stop thread", &thread_stop },
> @@ -423,6 +425,18 @@ static int target_selection(void)
> } else
> target_unselect(chip);
> }
> +
> + /* This is kinda broken as we're overloading what '-c'
> + * means - it's now up to each command to select targets
> + * based on core/chiplet. We really need a better
> + * solution to target selection. */
> + pdbg_for_each_target("chiplet", pib, chip) {
> + int chip_index = pdbg_target_index(chip);
> + if (chipsel[proc_index][chip_index]) {
> + target_select(chip);
> + } else
> + target_unselect(chip);
> + }
> } else
> target_unselect(pib);
> }
> diff --git a/src/ring.c b/src/ring.c
> new file mode 100644
> index 0000000..b0c9376
> --- /dev/null
> +++ b/src/ring.c
> @@ -0,0 +1,81 @@
> +/* Copyright 2018 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 <errno.h>
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <target.h>
> +#include <operations.h>
> +
> +#include "main.h"
> +
> +static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *len)
> +{
> + uint32_t *result;
> + int i, words;
> + int ring_len = *len;
> +
> + words = (ring_len + 32 - 1)/32;
> +
> + result = calloc(words, sizeof(*result));
> + assert(result);
> +
> + getring(target, *addr, ring_len, result);
> +
> + for (i = 0; i < ring_len/32; i++)
> + printf("%08" PRIx32, result[i]);
> +
> + ring_len -= i*32;
> +
> + /* Print out remaining bits */
> + for (i = 0; i < (ring_len + 4 - 1)/4; i++)
> + printf("%01" PRIx32, (result[words - 1] >> (28 - i*4)) & 0xf);
> +
> + printf("\n");
> +
> + return 1;
> +}
> +
> +int handle_getring(int optind, int argc, char *argv[])
> +{
> + uint64_t ring_addr, ring_len;
> + char *endptr;
> +
> + if (optind + 2 >= argc) {
> + printf("%s: command '%s' requires two arguments (address and length)\n",
> + argv[0], argv[optind]);
> + return -1;
> + }
> +
> + errno = 0;
> + ring_addr = strtoull(argv[optind + 1], &endptr, 0);
> + if (errno || *endptr != '\0') {
> + printf("%s: command '%s' couldn't parse ring address '%s'\n",
> + argv[0], argv[optind], argv[optind + 1]);
> + return -1;
> + }
> +
> + ring_len = strtoull(argv[optind + 2], &endptr, 0);
> + if (errno || *endptr != '\0') {
> + printf("%s: command '%s' couldn't parse ring length '%s'\n",
> + argv[0], argv[optind], argv[optind + 2]);
> + return -1;
> + }
> +
> + return for_each_target("chiplet", pdbg_getring, &ring_addr, &ring_len);
> +}
> diff --git a/src/ring.h b/src/ring.h
> new file mode 100644
> index 0000000..a72c875
> --- /dev/null
> +++ b/src/ring.h
> @@ -0,0 +1,17 @@
> +/* Copyright 2018 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.
> + */
> +
> +int handle_getring(int optind, int argc, char *argv[]);
Balbir
More information about the Pdbg
mailing list