Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore phandles from binary representations #151

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion dtc.c
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,15 @@ int main(int argc, char *argv[])
if (auto_label_aliases)
generate_label_tree(dti, "aliases", false);

if (generate_symbols)
if (generate_symbols) {
generate_label_tree(dti, "__symbols__", true);
generate_labels_from_tree(dti, "__symbols__");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd not immediately sure if it make sense to only do this when generate_symbols is set.

}

if (generate_fixups) {
generate_fixups_tree(dti, "__fixups__");
generate_local_fixups_tree(dti, "__local_fixups__");
local_fixup_phandles(dti, "__local_fixups__");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, not sure if it only makes sense to do this with -L.

}

if (sort)
Expand Down
3 changes: 3 additions & 0 deletions dtc.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,11 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
struct reserve_info *reservelist,
struct node *tree, uint32_t boot_cpuid_phys);
void sort_tree(struct dt_info *dti);
void generate_labels_from_tree(struct dt_info *dti, const char *name);
void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
void generate_fixups_tree(struct dt_info *dti, const char *name);
void generate_local_fixups_tree(struct dt_info *dti, const char *name);
void local_fixup_phandles(struct dt_info *dti, const char *name);

/* Checks */

Expand All @@ -354,6 +356,7 @@ struct dt_info *dt_from_blob(const char *fname);

/* Tree source */

void add_phandle_marker(struct dt_info *dti, struct property *prop, unsigned int offset);
void dt_to_source(FILE *f, struct dt_info *dti);
struct dt_info *dt_from_source(const char *f);

Expand Down
4 changes: 2 additions & 2 deletions dtdiff
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ source_and_sort () {
*.dts)
IFORMAT=dts
;;
*.dtb)
*.dtb|*.dtbo)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even something this simple needs some sort of commit message: why is this change desirable? (I can see why, but someone reading the commit log in the future might not).

IFORMAT=dtb
;;
esac
Expand All @@ -28,7 +28,7 @@ source_and_sort () {
exit 2
fi

$DTC -I $IFORMAT -O dts -qq -f -s -o - "$DT"
$DTC -@ -L -I $IFORMAT -O dts -qq -f -s -o - "$DT"
}

if [ $# != 2 ]; then
Expand Down
80 changes: 80 additions & 0 deletions livetree.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* (C) Copyright David Gibson <[email protected]>, IBM Corporation. 2005.
*/

#include <libfdt/libfdt.h>

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. We don't currently use libfdt within dtc, which is sort-of deliberate (they provide a cross-check to each other). There's a case to be made to drop that policy and instead use libfdt for much of the flat tree logic within dtc. However, I'd really prefer to do that as an overall change, using it for as much as we can, rather than adding small pieces of libfdt usage ad-hoc.

..also, I can't actually see what you're using this for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is only a development relict, will try without.

#include "dtc.h"
#include "srcpos.h"

Expand Down Expand Up @@ -1048,6 +1050,28 @@ static void generate_local_fixups_tree_internal(struct dt_info *dti,
generate_local_fixups_tree_internal(dti, lfn, c);
}

void generate_labels_from_tree(struct dt_info *dti, const char *name)
{
struct node *an;
struct property *p;

an = get_subnode(dti->dt, name);
if (!an)
return;

for_each_property(an, p) {
struct node *labeled_node;

labeled_node = get_node_by_path(dti->dt, p->val.val);
if (labeled_node)
add_label(&labeled_node->labels, p->name);
else if (quiet < 1)
fprintf(stderr, "Warning: Path %s referenced in %s missing",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest suggest an explicit "referenced in symbol %s" to make it clearer what's going on without context.

p->val.val, name);

}
}

void generate_label_tree(struct dt_info *dti, const char *name, bool allocph)
{
if (!any_label_tree(dti, dti->dt))
Expand All @@ -1071,3 +1095,59 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
generate_local_fixups_tree_internal(dti, build_root_node(dti->dt, name),
dti->dt);
}

static void local_fixup_phandles_node(struct dt_info *dti, struct node *lf, struct node *n)
{
struct property *lfp;
struct node *lfsubnode;

for_each_property(lf, lfp) {
struct property *p = get_property(n, lfp->name);
size_t i;

if (!p) {
if (quiet < 1)
fprintf(stderr, "Warning: Property %s in %s referenced in __local_fixups__ missing\n",
lfp->name, n->fullpath);
continue;
}

/*
* Each property in the __local_fixup__ tree is a concatination
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits: 'concatenation' and __local_fixups__ .

* of offsets, so it must be a multiple of sizeof(fdt32_t).
*/
if (lfp->val.len % sizeof(fdt32_t)) {
if (quiet < 1)
fprintf(stderr, "Warning: property %s in /__local_fixups__%s malformed\n",
lfp->name, n->fullpath);
continue;
}

for (i = 0; i < lfp->val.len; i += sizeof(fdt32_t))
add_phandle_marker(dti, p, fdt32_ld((fdt32_t *)(lfp->val.val + i)));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but I'd prefer to use i += 1 with the loop limit decreased correspondingly. Then you can cast lfp->val.val to fdt32_t * once and just index into it with i.

}

for_each_child(lf, lfsubnode) {
struct node *subnode = get_subnode(n, lfsubnode->name);

if (!subnode) {
if (quiet < 1)
fprintf(stderr, "Warning: subnode %s in %s referenced in __local_fixups__ missing\n",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For properties you do need to give the property name and path separately. However for subnodes you might as well just stick them together "node %s/%s referenced in local_fixups ...".

lfsubnode->name, n->fullpath);
continue;
}

local_fixup_phandles_node(dti, lfsubnode, subnode);
}
}

void local_fixup_phandles(struct dt_info *dti, const char *name)
{
struct node *an;

an = get_subnode(dti->dt, name);
if (!an)
return;

local_fixup_phandles_node(dti, an, dti->dt);
}
32 changes: 32 additions & 0 deletions treesource.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* (C) Copyright David Gibson <[email protected]>, IBM Corporation. 2005.
*/

#include <libfdt/libfdt.h>

#include "dtc.h"
#include "srcpos.h"

Expand Down Expand Up @@ -161,6 +163,36 @@ static void add_string_markers(struct property *prop)
}
}

void add_phandle_marker(struct dt_info *dti, struct property *prop, unsigned int offset)
{
struct marker *m, **mi;
cell_t phandle;
struct node *refn;

m = xmalloc(sizeof(*m));
m->offset = offset;
m->type = REF_PHANDLE;

phandle = fdt32_ld((fdt32_t *)(prop->val.val + m->offset));
refn = get_node_by_phandle(dti->dt, phandle);

if (refn) {
if (refn->labels)
m->ref = refn->labels->label;
else
m->ref = refn->fullpath;
} else if (quiet < 1) {
fprintf(stderr, "Warning: node referenced by phandle 0x%x in property %s not found\n",
phandle, prop->name);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you do need to consider __fixups__ as well. Without that, this error will be triggered if you're decompiling a dtbo with any external phandle references.

}

for (mi = &prop->val.markers; *mi && (*mi)->offset < m->offset;)
mi = &(*mi)->next;

m->next = *mi;
*mi = m;
}

static enum markertype guess_value_type(struct property *prop)
{
int len = prop->val.len;
Expand Down