Skip to content

Building related enhancements, and try to match compatible on kernel dtbs #570

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.

Sign up for GitHub

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

Open
me-cafebabe wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Building related enhancements, and try to match compatible on kernel dtbs #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a5c812
lk2nd: Fix ABOOT_STANDALONE code guards
SebaUbuntu Aug 13, 2025
7db21a5
lk2nd: Make dtc command overrideable
me-cafebabe Aug 17, 2025
e012550
lk2nd: dev_tree: Mark as best dtb node if compatible matches with lk2…
me-cafebabe Aug 7, 2025
8f33290
dts: msm8953-xiaomi-common: sakura: This device uses xiaomi-daisy dtb…
me-cafebabe Aug 7, 2025
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
4 changes: 4 additions & 0 deletions app/aboot/aboot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ extern int get_target_boot_params(const char *cmdline, const char *part,
char **buf);

void *info_buf;
#if !ABOOT_STANDALONE
void write_device_info_mmc(device_info *dev);
void write_device_info_flash(device_info *dev);
#endif
static int aboot_save_boot_hash_mmc(uint32_t image_addr, uint32_t image_size);
static int aboot_frp_unlock(char *pname, void *data, unsigned sz);
static inline uint64_t validate_partition_size(struct ptentry *ptn);
Expand Down Expand Up @@ -2513,11 +2515,13 @@ int boot_linux_from_flash(void)

if(target_use_signed_kernel() && (!device.is_unlocked))
{
#if !ABOOT_STANDALONE
/* Make sure everything from scratch address is read before next step!*/
if(device.is_tampered)
{
write_device_info_flash(&device);
}
#endif
#if USE_PCOM_SECBOOT
set_tamper_flag(device.is_tampered);
#endif
Expand Down
2 changes: 2 additions & 0 deletions app/aboot/recovery.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ static const int MISC_COMMAND_PAGE = 1; // bootloader command is this page
static char buf[4096];

extern uint32_t get_page_size(void);
#if !ABOOT_STANDALONE
extern void reset_device_info(void);
extern void set_device_root(void);
#endif

int get_recovery_message(struct recovery_message *out)
{
Expand Down
6 changes: 6 additions & 0 deletions include/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ uint32_t platform_get_max_periph(void);
int platform_is_msm8996(void);
int platform_is_apq8096_mediabox(void);
bool platform_use_qmp_misc_settings(void);
#if !ABOOT_STANDALONE
void set_device_unlock_value(int type, bool status);
#endif
void get_product_name(unsigned char *buf);
void get_bootloader_version(unsigned char *buf);
void get_baseband_version(unsigned char *buf);
Expand All @@ -110,11 +112,15 @@ void* get_rpmb_snd_rcv_buff(void);
int LoadImage(char *Pname, void **ImgBuf, uint32_t *ImgSzActual);
void boot_verifier_init(void);
uint32_t get_page_size(void);
#if !ABOOT_STANDALONE
int read_rollback_index(uint32_t loc, uint64_t *roll_back_index);
int write_rollback_index(uint32_t loc, uint64_t roll_back_index);
#if VERIFIED_BOOT_2
int get_userkey(uint8_t **user_key, uint32_t *user_key_size);
int erase_userkey(void);
int store_userkey(uint8_t *user_key, uint32_t user_key_size);
#endif
#endif
bool is_device_locked_critical(void);
bool is_verity_enforcing(void);
#endif
14 changes: 13 additions & 1 deletion lk2nd/device/device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ const char *const *lk2nd_device_get_dtb_hints(void)
return lk2nd_dev.dtbfiles;
}

/**
* lk2nd_device_get_dtb_compatible() - Get a compatible string for matching kernel dtb.
*/
const char *lk2nd_device_get_dtb_compatible(void)
{
return lk2nd_dev.dtb_compatible ? lk2nd_dev.dtb_compatible : lk2nd_dev.compatible;
}

static int find_device_node(const void *dtb)
{
int lk2nd_node, node, ret;
Expand Down Expand Up @@ -106,12 +114,16 @@ static void parse_dtb(const void *dtb)
return;
}

val = fdt_getprop(dtb, node, "compatible", &len);
val = fdt_stringlist_get(dtb, node, "compatible", 0, &len);
if (val && len > 0)
lk2nd_dev.compatible = strndup(val, len);
else
dprintf(CRITICAL, "Failed to read 'compatible': %d\n", len);

val = fdt_stringlist_get(dtb, node, "compatible", 1, &len);
if (val && len > 0)
lk2nd_dev.dtb_compatible = strndup(val, len);

@TravMurav TravMurav Oct 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While I see how it's tempting to just hack things together to make your single usecase work, I think this makes the code more confusing since it's hard to understand what is the meaning of which prop, and it would violate expectations one would have of DT matching.

I.e. you're currently creating two seemingly unrelated fields in the struct, but they now suddenly are an "array" of at most two fields, used for dtb mathing. Except contrary to the usual dtb logic, if the second (less specific, fallback) compatible is present, it would be preferred, even if a more specific compatible is available.

I'd strongly prefer this to align to common dt logic, i.e. I'd expect the lk2nd_dev.compatible be changed to something like lk2nd_dev.compatibles with a setup similar to .dtbfiles, and your selection logic to iterate over all of them to find the best pick.

This will unfortunately require a slight refactoring where all uses of lk2nd_dev.compatible will have to be changed to lk2nd_dev.compatibles[0], with a bit more care taken with assignments to create a correct pointer structure.

What would I do to refactor for this implementation:

  1. Study the usage of lk2nd_dev.compatible, I believe it's mainly used in lk2nd device module for picking the dtb in case of lk1st->lk2nd boot. Based on the usage, I'd change the variable to a char** like dtbfiles, use lkfdt_stringlist_get_all to assign it in the parser, and pointer to a string literal where it's assigned at compile time/from cmdline. Then refactor all read usages like suggested above. (I'd put it as a separate refactor commit)
  2. Based on that work of refactoring lk2nd compatible list, I'd create a new function int lk2nd_is_compatible(char *compatible) that would compare a provided compatible against a list and return either -1 (no match) or an index of the provided compatible (i.e. sakura=0, daisy=1), which could be used to rank dtbs in a container by best match (i.e. loop over them and find the lowest index - that's the one to boot). This would allow you to add sakura.dtb later if you have to and keep the logic working. This funciton could also be reusable later if we ever want to implement other system containers like BLS#2 (UKI) for example.


val = fdt_getprop(dtb, node, "model", &len);
if (val && len > 0)
lk2nd_dev.model = strndup(val, len);
Expand Down
1 change: 1 addition & 0 deletions lk2nd/device/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ struct lk2nd_device {
const char *model;
const char *battery;

const char *dtb_compatible;
const char *const *dtbfiles;
bool single_key;
struct lk2nd_menu_keys menu_keys;
Expand Down
2 changes: 1 addition & 1 deletion lk2nd/device/dts/msm8953/msm8953-xiaomi-common.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
sakura {
/* Xiaomi Redmi 6 Pro (sakura) and Xiaomi Mi A2 Lite (daisy) are software compatible */
model = "Xiaomi Redmi 6 Pro";
compatible = "xiaomi,sakura";
compatible = "xiaomi,sakura", "xiaomi,daisy";
lk2nd,match-bootloader = "*DAISY*";

lk2nd,dtb-files = "msm8953-xiaomi-daisy";
Expand Down
2 changes: 2 additions & 0 deletions lk2nd/include/lk2nd/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ void lk2nd_device2nd_copy_atags(void *tags, const char *cmdline,

#if WITH_LK2ND_DEVICE
const char *const *lk2nd_device_get_dtb_hints(void);
const char *lk2nd_device_get_dtb_compatible(void);
#else
static inline const char *const *lk2nd_device_get_dtb_hints(void) { return NULL; };
static inline const char *lk2nd_device_get_dtb_compatible(void) { return NULL; };
#endif

#endif /* LK2ND_DEVICE_H */
2 changes: 1 addition & 1 deletion make/build.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ $(BUILDDIR)/%.dtb: %.dts
$(NOECHO)$(TOOLCHAIN_PREFIX)cpp -nostdinc -undef -x assembler-with-cpp \
-D_DTB_NAME_=\"$(basename $(notdir $<))\" \
$(DT_INCLUDES) $< -MD -MT $@ -MF $@.d -o $@.dts
$(NOECHO)dtc -O dtb -I dts --align 16 -o $@ $@.dts
$(NOECHO)$(DTC) -O dtb -I dts --align 16 -o $@ $@.dts
1 change: 1 addition & 0 deletions makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ DEPS := $(ALLOBJS:%o=%d) $(addsuffix .d,$(ALLDTBS))
# default to no ccache
CCACHE ?=
CC := $(CCACHE) $(TOOLCHAIN_PREFIX)gcc
DTC ?= dtc
ifeq ($(LD),ld)
LD := $(TOOLCHAIN_PREFIX)ld
endif
Expand Down
37 changes: 37 additions & 0 deletions platform/msm_shared/dev_tree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
#endif
#include <boot_stats.h>
#include <verifiedboot.h>
#if WITH_LK2ND_DEVICE
#include <lk2nd/device.h>
#endif

#define NODE_PROPERTY_MAX_LEN 64
#define ADD_OF(a, b) (UINT_MAX - b > a) ? (a + b) : UINT_MAX
Expand Down Expand Up @@ -401,6 +404,15 @@ static boolean dtb_read_find_match(dt_info *current_dtb_info, dt_info *best_dtb_
return false;
}

#if WITH_LK2ND_DEVICE
if (fdt_node_check_compatible(dtb, root_offset, lk2nd_device_get_dtb_compatible()) == 0) {
dprintf(INFO, "Compatible %s matches, mark as best dtb node\n", lk2nd_device_get_dtb_compatible());
current_dtb_info->dt_match_val = UINT_MAX;
current_dtb_info->dt_match_val &= ~exact_match;
goto cleanup;
}
#endif

/* Get the msm-id prop from DTB and find best match */
platform_prop = (const char *)fdt_getprop(dtb, root_offset, "qcom,msm-id", &platform_id_len);
if (platform_prop && (platform_id_len > 0) && (!(platform_id_len % PLAT_ID_SIZE))) {
Expand Down Expand Up @@ -823,6 +835,7 @@ static int dev_tree_compatible(const void *dtb, const void *real_dtb, uint32_t d
uint32_t board_data_count;
uint32_t pmic_data_count;
uint32_t dtb_count = 0;;
bool set_dt_entry_is_best = false;

if (!root_offset)
root_offset = fdt_path_offset(dtb, "/");
Expand All @@ -840,6 +853,13 @@ if (DEBUGLEVEL >= SPEW) {
}
}

#if WITH_LK2ND_DEVICE
if (fdt_node_check_compatible(dtb, root_offset, lk2nd_device_get_dtb_compatible()) == 0) {
dprintf(INFO, "Compatible %s matches, mark as best dtb node\n", lk2nd_device_get_dtb_compatible());
set_dt_entry_is_best = true;
}
#endif

/* Find the pmic-id prop from DTB , if pmic-id is present then
* the DTB is version 3, otherwise find the board-id prop from DTB ,
* if board-id is present then the DTB is version 2 */
Expand Down Expand Up @@ -921,6 +941,7 @@ if (DEBUGLEVEL >= SPEW) {
cur_dt_entry->pmic_rev[3] = board_pmic_target(3);
cur_dt_entry->offset = (uint32_t)real_dtb;
cur_dt_entry->size = dtb_size;
cur_dt_entry->is_best = set_dt_entry_is_best;

if (min_plat_id_len == DT_ENTRY_LGE8974_SIZE)
cur_dt_entry->board_hw_subtype = fdt32_to_cpu(((const struct dt_entry_v1 *)plat_prop)->offset);
Expand Down Expand Up @@ -1076,6 +1097,7 @@ if (DEBUGLEVEL >= SPEW) {
dt_entry_array[k].pmic_rev[3]= pmic_data[n].pmic_version[3];
dt_entry_array[k].offset = (uint32_t)real_dtb;
dt_entry_array[k].size = dtb_size;
dt_entry_array[k].is_best = set_dt_entry_is_best;
k++;
}

Expand All @@ -1090,6 +1112,7 @@ if (DEBUGLEVEL >= SPEW) {
dt_entry_array[k].pmic_rev[3]= board_pmic_target(3);
dt_entry_array[k].offset = (uint32_t)real_dtb;
dt_entry_array[k].size = dtb_size;
dt_entry_array[k].is_best = set_dt_entry_is_best;
k++;
}
}
Expand Down Expand Up @@ -1829,6 +1852,20 @@ static struct dt_entry *platform_dt_match_best(struct dt_entry_node *dt_list)
if (!update_dtb_entry_node(dt_list, DTB_PMIC3))
return NULL;

#if WITH_LK2ND_DEVICE
list_for_every_entry(&dt_list->node, dt_node_tmp1, dt_node, node) {
if (!dt_node_tmp1)
break;

if (!dt_node_tmp1->dt_entry_m)
continue;

if (dt_node_tmp1->dt_entry_m->is_best) {
return dt_node_tmp1->dt_entry_m;
}
}
#endif

list_for_every_entry(&dt_list->node, dt_node_tmp1, dt_node, node) {
if (!dt_node_tmp1) {
dprintf(CRITICAL, "ERROR: Couldn't find the suitable DTB!\n");
Expand Down
1 change: 1 addition & 0 deletions platform/msm_shared/include/dev_tree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ struct dt_entry
uint32_t offset;
uint32_t size;
uint32_t idx;
bool is_best;
};

struct dt_table
Expand Down