From patchwork Tue Sep 3 07:59:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alistair Popple X-Patchwork-Id: 1156861 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46Mzrv1QSzz9s7T for ; Tue, 3 Sep 2019 18:00:15 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=popple.id.au Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 46Mzrr4G7fzDqhH for ; Tue, 3 Sep 2019 18:00:12 +1000 (AEST) X-Original-To: pdbg@lists.ozlabs.org Delivered-To: pdbg@lists.ozlabs.org Received: from ozlabs.org (bilbo.ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 46MzrZ31NVzDqYM for ; Tue, 3 Sep 2019 17:59:58 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=popple.id.au Received: from authenticated.ozlabs.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.ozlabs.org (Postfix) with ESMTPSA id 46MzrY11tYz9s7T; Tue, 3 Sep 2019 17:59:57 +1000 (AEST) From: Alistair Popple To: pdbg@lists.ozlabs.org Date: Tue, 3 Sep 2019 17:59:33 +1000 Message-Id: <20190903075934.3525-1-alistair@popple.id.au> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [Pdbg] [PATCH 1/2] libpdbg: Create struct pdbg_target_priv X-BeenThere: pdbg@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "mailing list for https://github.com/open-power/pdbg development" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: pdbg-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Pdbg" In order to allow external programs to register their own hardware units it is required to expose the definition of struct pdbg_target. However this means the struct definition now becomes part of the library ABI/API, making it difficult to change fields without breaking applications. It also means we would have to install the ccan list headers, which themselves depend on other ccan modules. Instead create a seperate struct definition (struct pdbg_target_priv) which encapsulates the libpdbg specific fields which should not be accessed directly by applications and switch the library to use those. This should allow a stable definition of struct pdbg_target to be exported to applications without preventing changes to the underlying library fields. Signed-off-by: Alistair Popple --- libpdbg/device.c | 95 +++++++++++++++++++++++++++-------------------- libpdbg/hwunit.c | 3 ++ libpdbg/libpdbg.c | 30 +++++++++------ libpdbg/target.c | 12 +++--- libpdbg/target.h | 8 +++- 5 files changed, 89 insertions(+), 59 deletions(-) diff --git a/libpdbg/device.c b/libpdbg/device.c index efa9ce4..e009a1a 100644 --- a/libpdbg/device.c +++ b/libpdbg/device.c @@ -67,6 +67,7 @@ static const char *take_name(const char *name) /* Adds information representing an actual target */ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) { + struct pdbg_target_priv *target_priv; struct pdbg_target *target; struct pdbg_target_class *target_class; const struct hw_unit_info *hw_info = NULL; @@ -98,19 +99,21 @@ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) /* Couldn't find anything implementing this target */ return NULL; - target = calloc(1, size); - if (!target) { + target_priv = calloc(1, sizeof(struct pdbg_target_priv) - sizeof(struct pdbg_target) + size); + if (!target_priv) { prerror("Failed to allocate node\n"); abort(); } + target = &target_priv->target; + /* hw_info->hw_unit points to a per-target struct type. This * works because the first member in the per-target struct is * guaranteed to be the struct pdbg_target (see the comment * above DECLARE_HW_UNIT). */ memcpy(target, hw_info->hw_unit, size); target_class = get_target_class(target); - list_add_tail(&target_class->targets, &target->class_link); + list_add_tail(&target_class->targets, &target_priv->class_link); return target; } @@ -118,31 +121,36 @@ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) static struct pdbg_target *dt_new_node(const char *name, const void *fdt, int node_offset) { struct pdbg_target *node = NULL; - size_t size = sizeof(*node); + struct pdbg_target_priv *target_priv; - if (fdt) + if (fdt) { node = dt_pdbg_target_new(fdt, node_offset); - - if (!node) - node = calloc(1, size); + target_priv = pdbg_target_to_priv(node); + } if (!node) { + target_priv = calloc(1, sizeof(*target_priv)); + node = &target_priv->target; + } + + if (!node && !target_priv) { prerror("Failed to allocate node\n"); abort(); } - node->dn_name = take_name(name); + target_priv->dn_name = take_name(name); node->parent = NULL; - list_head_init(&node->properties); - list_head_init(&node->children); - node->phandle = ++last_phandle; + list_head_init(&target_priv->properties); + list_head_init(&target_priv->children); + target_priv->phandle = ++last_phandle; return node; } static const char *get_unitname(const struct pdbg_target *node) { - const char *c = strchr(node->dn_name, '@'); + struct pdbg_target_priv *target_priv = pdbg_target_to_priv(node); + const char *c = strchr(target_priv->dn_name, '@'); if (!c) return NULL; @@ -152,13 +160,14 @@ static const char *get_unitname(const struct pdbg_target *node) static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target *b) { + struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a), *priv_b = pdbg_target_to_priv(b); const char *a_unit = get_unitname(a); const char *b_unit = get_unitname(b); - ptrdiff_t basenamelen = a_unit - a->dn_name; + ptrdiff_t basenamelen = a_unit - priv_a->dn_name; /* sort hex unit addresses by number */ - if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name, basenamelen)) { + if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b->dn_name, basenamelen)) { unsigned long long a_num, b_num; char *a_end, *b_end; @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target return (a_num > b_num) - (a_num < b_num); } - return strcmp(a->dn_name, b->dn_name); + return strcmp(priv_a->dn_name, priv_b->dn_name); } static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root) { - struct pdbg_target *node = NULL; + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); + struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent); + struct pdbg_target_priv *node = NULL; assert(!root->parent); - if (list_empty(&parent->children)) { - list_add(&parent->children, &root->list); + if (list_empty(&parent_priv->children)) { + list_add(&parent_priv->children, &root_priv->list); root->parent = parent; return true; } - dt_for_each_child(parent, node) { - int cmp = dt_cmp_subnodes(node, root); + dt_for_each_child(parent_priv, node) { + int cmp = dt_cmp_subnodes(&node->target, root); /* Look for duplicates */ if (cmp == 0) { prerror("DT: %s failed, duplicate %s\n", - __func__, root->dn_name); + __func__, root_priv->dn_name); return false; } @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root) break; } - list_add_before(&parent->children, &root->list, &node->list); + list_add_before(&parent_priv->children, &root_priv->list, &node->list); root->parent = parent; return true; @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target *node) return strdup(""); for (n = node; n; n = n->parent) { - len += strlen(n->dn_name); + len += strlen(pdbg_target_to_priv(n)->dn_name); if (n->parent || n == node) len++; } @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target *node) assert(path); p = path + len; for (n = node; n; n = n->parent) { - len = strlen(n->dn_name); + len = strlen(pdbg_target_to_priv(n)->dn_name); p -= len; - memcpy(p, n->dn_name, len); + memcpy(p, pdbg_target_to_priv(n)->dn_name, len); if (n->parent || n == node) *(--p) = '/'; } @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p, static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char *path) { - struct pdbg_target *n; + struct pdbg_target_priv *n; + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL; unsigned int pnl, pal, nnl, nal; bool match; @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char /* Compare with each child node */ match = false; - list_for_each(&root->children, n, list) { + list_for_each(&root_priv->children, n, list) { match = true; __dt_path_split(n->dn_name, &nn, &nnl, &na, &nal); if (pnl && (pnl != nnl || strncmp(pn, nn, pnl))) @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char if (pal && (pal != nal || strncmp(pa, na, pal))) match = false; if (match) { - root = n; + root = &n->target; break; } } @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const struct pdbg_target *node, { struct dt_property *i = NULL; - list_for_each(&node->properties, i, list) + list_for_each(&pdbg_target_to_priv(node)->properties, i, list) if (strcmp(i->name, name) == 0) return i; return NULL; @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct pdbg_target *node, p->name = take_name(name); p->len = size; - list_add_tail(&node->properties, &p->list); + list_add_tail(&pdbg_target_to_priv(node)->properties, &p->list); return p; } @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct pdbg_target *node, const char *name, const void *val, size_t size) { + struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node); struct dt_property *p; /* @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct pdbg_target *node, if (strcmp(name, "linux,phandle") == 0 || strcmp(name, "phandle") == 0) { assert(size == 4); - node->phandle = *(const u32 *)val; - if (node->phandle >= last_phandle) - last_phandle = node->phandle; + node_priv->phandle = *(const u32 *)val; + if (node_priv->phandle >= last_phandle) + last_phandle = node_priv->phandle; return NULL; } @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct dt_property *prop, u32 index) /* First child of this node. */ static struct pdbg_target *dt_first(const struct pdbg_target *root) { - return list_top(&root->children, struct pdbg_target, list); + return &list_top(&pdbg_target_to_priv(root)->children, struct pdbg_target_priv, list)->target; } /* Return next node, or NULL. */ static struct pdbg_target *dt_next(const struct pdbg_target *root, const struct pdbg_target *prev) { + struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev); + /* Children? */ - if (!list_empty(&prev->children)) + if (!list_empty(&prev_priv->children)) return dt_first(prev); do { /* More siblings? */ - if (prev->list.next != &prev->parent->children.n) - return list_entry(prev->list.next, struct pdbg_target,list); + if (prev_priv->list.next != &pdbg_target_to_priv(prev->parent)->children.n) + return &list_entry(prev_priv->list.next, struct pdbg_target_priv, list)->target; /* No more siblings, move up to parent. */ prev = prev->parent; @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target *node, const void *fdt, int fdt_nod name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); if (strcmp("index", name) == 0) { memcpy(&data, prop->data, sizeof(data)); - node->index = fdt32_to_cpu(data); + pdbg_target_to_priv(node)->index = fdt32_to_cpu(data); } if (strcmp("status", name) == 0) - node->status = str_to_status(prop->data); + pdbg_target_to_priv(node)->status = str_to_status(prop->data); dt_add_property(node, name, prop->data, fdt32_to_cpu(prop->len)); diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c index c7ec63d..5c40d23 100644 --- a/libpdbg/hwunit.c +++ b/libpdbg/hwunit.c @@ -21,6 +21,9 @@ #define MAX_HW_UNITS 1024 +struct pdbg_target pdbg_target_size; +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)]; + static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS]; static int g_hw_unit_count; diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c index 0c5a451..bec2488 100644 --- a/libpdbg/libpdbg.c +++ b/libpdbg/libpdbg.c @@ -9,6 +9,7 @@ static pdbg_progress_tick_t progress_tick; struct pdbg_target *__pdbg_next_target(const char *class, struct pdbg_target *parent, struct pdbg_target *last) { struct pdbg_target *next, *tmp; + struct pdbg_target_priv *last_priv; struct pdbg_target_class *target_class; if (class && !find_target_class(class)) @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char *class, struct pdbg_target *pa target_class = require_target_class(class); retry: + last_priv = pdbg_target_to_priv(last); + /* No more targets left to check in this class */ - if ((last && last->class_link.next == &target_class->targets.n) || + if ((last && last_priv->class_link.next == &target_class->targets.n) || list_empty(&target_class->targets)) return NULL; if (last) - next = list_entry(last->class_link.next, struct pdbg_target, class_link); + next = &list_entry(last_priv->class_link.next, struct pdbg_target_priv, class_link)->target; else - if (!(next = list_top(&target_class->targets, struct pdbg_target, class_link))) + if (!(next = &list_top(&target_class->targets, struct pdbg_target_priv, class_link)->target)) return NULL; if (!parent) @@ -45,21 +48,24 @@ retry: struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last) { - if (!parent || list_empty(&parent->children)) + struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent); + struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last); + + if (!parent || list_empty(&parent_priv->children)) return NULL; if (!last) - return list_top(&parent->children, struct pdbg_target, list); + return &list_top(&parent_priv->children, struct pdbg_target_priv, list)->target; - if (last->list.next == &parent->children.n) + if (last_priv->list.next == &parent_priv->children.n) return NULL; - return list_entry(last->list.next, struct pdbg_target, list); + return &list_entry(last_priv->list.next, struct pdbg_target_priv, list)->target; } enum pdbg_target_status pdbg_target_status(struct pdbg_target *target) { - return target->status; + return pdbg_target_to_priv(target)->status; } void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status status) @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status * blow up obviously if this happens */ assert(status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_MUSTEXIST); - target->status = status; + pdbg_target_to_priv(target)->status = status; } /* Searches up the tree and returns the first valid index found */ @@ -76,12 +82,12 @@ uint32_t pdbg_target_index(struct pdbg_target *target) { struct pdbg_target *dn; - for (dn = target; dn && dn->index == -1; dn = dn->parent); + for (dn = target; dn && pdbg_target_to_priv(dn)->index == -1; dn = dn->parent); if (!dn) return -1; else - return dn->index; + return pdbg_target_to_priv(dn)->index; } /* Find a target parent from the given class */ @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target *target) const char *pdbg_target_dn_name(struct pdbg_target *target) { - return target->dn_name; + return pdbg_target_to_priv(target)->dn_name; } int pdbg_target_u32_property(struct pdbg_target *target, const char *name, uint32_t *val) diff --git a/libpdbg/target.c b/libpdbg/target.c index 5808d02..ce2bb2d 100644 --- a/libpdbg/target.c +++ b/libpdbg/target.c @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) case PDBG_TARGET_NONEXISTENT: /* The parent doesn't exist neither does it's * children */ - target->status = PDBG_TARGET_NONEXISTENT; + pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT; return PDBG_TARGET_NONEXISTENT; case PDBG_TARGET_DISABLED: @@ -427,11 +427,11 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) if (target->probe && target->probe(target)) { /* Could not find the target */ assert(pdbg_target_status(target) != PDBG_TARGET_MUSTEXIST); - target->status = PDBG_TARGET_NONEXISTENT; + pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT; return PDBG_TARGET_NONEXISTENT; } - target->status = PDBG_TARGET_ENABLED; + pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED; return PDBG_TARGET_ENABLED; } @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target *target) /* Release the target */ if (target->release) target->release(target); - target->status = PDBG_TARGET_RELEASED; + pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED; } /* @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target *target, const char *class) void *pdbg_target_priv(struct pdbg_target *target) { - return target->priv; + return pdbg_target_to_priv(target)->priv; } void pdbg_target_priv_set(struct pdbg_target *target, void *priv) { - target->priv = priv; + pdbg_target_to_priv(target)->priv = priv; } diff --git a/libpdbg/target.h b/libpdbg/target.h index 2ecbf98..ba2b20d 100644 --- a/libpdbg/target.h +++ b/libpdbg/target.h @@ -39,19 +39,25 @@ struct pdbg_target { int (*probe)(struct pdbg_target *target); void (*release)(struct pdbg_target *target); uint64_t (*translate)(struct pdbg_target *target, uint64_t addr); + struct pdbg_target *parent; +}; + +struct pdbg_target_priv { int index; enum pdbg_target_status status; const char *dn_name; struct list_node list; struct list_head properties; struct list_head children; - struct pdbg_target *parent; u32 phandle; bool probed; struct list_node class_link; + struct pdbg_target target; void *priv; }; +#define pdbg_target_to_priv(x) (x ? container_of(x, struct pdbg_target_priv, target) : NULL) + struct pdbg_target *require_target_parent(struct pdbg_target *target); struct pdbg_target_class *find_target_class(const char *name); struct pdbg_target_class *require_target_class(const char *name);