From 46aff099d2e8587f94dfa0de1eb47a75a9067b60 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 28 Oct 2020 13:50:47 -0700 Subject: [PATCH 1/5] Do not assume GKI just with vendor_boot. This change ensures changes to GENERIC_KERNEL_CMDLINE only affects devices that explicitly says it uses GKI/generic boot image. In details, if the device has vendor_boot, but does not explicitly specify that it uses GKI/generic boot image, do not include GENERIC_KERNEL_CMDLINE in boot. boot cmdline is left empty in this case. The old logic: - If device uses GKI *OR* has vendor_boot: boot uses GENERIC_KERNEL_CMDLINE, and do not include kernel base and pagesize. - If device has vendor_boot, INTERNAL_KERNEL_CMDLINE, kernel base and pagesize goes in vendor_boot. - If device does not use GKI nor have vendor_boot: boot uses INTERNAL_KERNEL_CMDLINE, and includes kernel base and pagesize. The new logic: - If using GKI, boot uses GENERIC_KERNEL_CMDLINE. Remove kernel base and pagesize because they are device-specific. - If not using GKI: - If building vendor_boot, INTERNAL_KERNEL_CMDLINE, base and pagesize goes in vendor_boot; boot does not have cmdline, base or pagesize. - Otherwise, put them in boot Comparison of the code before and after: - If device uses GKI, - For boot partition: - cmdline continues to be GENERIC_KERNEL_CMDLINE - kernel base and pagesize continues to be excluded - For vendor_boot partition: - cmdline continues to be INTERNAL_KERNEL_CMDLINE - kernel base and pagesize continues to be included - If device does not use GKI: - If device has a vendor_boot partition: - For boot partition: * cmdline changes from GENERIC_KERNEL_CMDLINE to empty - kernel base and pagesize continues to be excluded - For vendor_boot partition: - cmdline continues to be INTERNAL_KERNEL_CMDLINE - kernel base and pagesize continues to be included - If device does not have a vendor_boot partition: - For boot partition: - cmdline continues to be INTERNAL_KERNEL_CMDLINE - kernel base and pagesize continues to be included Test: builds Bug: 171512004 Change-Id: I4feac435698f43ac299b430bff66147057865a62 --- core/Makefile | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/core/Makefile b/core/Makefile index 8ca00ed82..83328bb5b 100644 --- a/core/Makefile +++ b/core/Makefile @@ -754,30 +754,27 @@ endif INTERNAL_KERNEL_CMDLINE := $(strip $(INTERNAL_KERNEL_CMDLINE) buildvariant=$(TARGET_BUILD_VARIANT) $(VERITY_KEYID)) -boot_uses_generic_kernel_image := -ifdef BUILDING_VENDOR_BOOT_IMAGE - # building vendor boot image, dtb/base/pagesize go there - boot_uses_generic_kernel_image := true -else ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) - boot_uses_generic_kernel_image := true -endif - -ifeq (true,$(boot_uses_generic_kernel_image)) +# kernel cmdline/base/pagesize in boot. +# - If using GKI, use GENERIC_KERNEL_CMDLINE. Remove kernel base and pagesize because they are +# device-specific. +# - If not using GKI: +# - If building vendor_boot, INTERNAL_KERNEL_CMDLINE, base and pagesize goes in vendor_boot. +# - Otherwise, put them in boot. +ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) ifdef GENERIC_KERNEL_CMDLINE INTERNAL_BOOTIMAGE_ARGS += --cmdline "$(GENERIC_KERNEL_CMDLINE)" endif -else # boot_uses_generic_kernel_image != true -ifdef BOARD_KERNEL_BASE - INTERNAL_BOOTIMAGE_ARGS += --base $(BOARD_KERNEL_BASE) -endif -ifdef BOARD_KERNEL_PAGESIZE - INTERNAL_BOOTIMAGE_ARGS += --pagesize $(BOARD_KERNEL_PAGESIZE) -endif -ifdef INTERNAL_KERNEL_CMDLINE - INTERNAL_BOOTIMAGE_ARGS += --cmdline "$(INTERNAL_KERNEL_CMDLINE)" -endif -endif # boot_uses_generic_kernel_image == true -boot_uses_generic_kernel_image := +else ifndef BUILDING_VENDOR_BOOT_IMAGE # && BOARD_USES_GENERIC_KERNEL_IMAGE != true + ifdef INTERNAL_KERNEL_CMDLINE + INTERNAL_BOOTIMAGE_ARGS += --cmdline "$(INTERNAL_KERNEL_CMDLINE)" + endif + ifdef BOARD_KERNEL_BASE + INTERNAL_BOOTIMAGE_ARGS += --base $(BOARD_KERNEL_BASE) + endif + ifdef BOARD_KERNEL_PAGESIZE + INTERNAL_BOOTIMAGE_ARGS += --pagesize $(BOARD_KERNEL_PAGESIZE) + endif +endif # BUILDING_VENDOR_BOOT_IMAGE == "" && BOARD_USES_GENERIC_KERNEL_IMAGE != true INTERNAL_MKBOOTIMG_VERSION_ARGS := \ --os_version $(PLATFORM_VERSION_LAST_STABLE) \ @@ -4541,11 +4538,9 @@ endif ifdef INSTALLED_KERNEL_TARGET $(hide) cp $(INSTALLED_KERNEL_TARGET) $(zip_root)/BOOT/ endif -ifdef INSTALLED_VENDOR_BOOTIMAGE_TARGET +ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) echo "$(GENERIC_KERNEL_CMDLINE)" > $(zip_root)/BOOT/cmdline -else ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) - echo "$(GENERIC_KERNEL_CMDLINE)" > $(zip_root)/BOOT/cmdline -else # INSTALLED_VENDOR_BOOTIMAGE_TARGET == "" && BOARD_USES_GENERIC_KERNEL_IMAGE != true +else ifndef INSTALLED_VENDOR_BOOTIMAGE_TARGET # && BOARD_USES_GENERIC_KERNEL_IMAGE != true echo "$(INTERNAL_KERNEL_CMDLINE)" > $(zip_root)/BOOT/cmdline ifdef INSTALLED_2NDBOOTLOADER_TARGET cp $(INSTALLED_2NDBOOTLOADER_TARGET) $(zip_root)/BOOT/second From 3941a876decdf86db442df36d16e4090e636d8af Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 28 Oct 2020 16:33:06 -0700 Subject: [PATCH 2/5] Remove device-specific bits if recovery_as_boot On legacy devices (launched with R and below), if device: - has a vendor_boot partition, and - uses recovery_as_boot Then, when building the recovery/boot partition, the device-specific bits, including dtb/kernel base/pagesize should be moved to vendor_boot. Previously, it is incorrectly assumed that A/B => recovery_as_boot. In reality, we do have A/B devices with a dedicated recovery partition. Note that for devices that uses GKI (BOARD_USES_GENERIC_KERNEL_IMAGE), recovery_as_boot is never set to true. Instead, recovery resources are moved to vendor_boot. On these devices, the conditional 'vendor_boot && recovery-as-boot' is always false. Hence: - If the device has a dedicated recovery partition, it should use V3 header, and dtb/base/pagesize won't be in recovery header. - If device does not have a dedicated recovery partition, the recovery image won't be built. Test: builds Change-Id: I0db2af20470cbe8a21044a984cccf264590aaccf --- core/Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/Makefile b/core/Makefile index 83328bb5b..7d0212049 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1890,7 +1890,7 @@ $(INSTALLED_RECOVERY_RAMDISK_BUILD_PROP_TARGET): $(INSTALLED_RAMDISK_BUILD_PROP_ $(copy-file-to-target) endif -ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(AB_OTA_UPDATER))) +ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) INTERNAL_RECOVERYIMAGE_ARGS := --ramdisk $(recovery_ramdisk) ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) @@ -1899,7 +1899,7 @@ ifdef GENERIC_KERNEL_CMDLINE endif # GENERIC_KERNEL_CMDLINE != "" endif # BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE != true -else # not (BUILDING_VENDOR_BOOT_IMAGE and AB_OTA_UPDATER) +else # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) INTERNAL_RECOVERYIMAGE_ARGS := \ $(addprefix --second ,$(INSTALLED_2NDBOOTLOADER_TARGET)) \ --ramdisk $(recovery_ramdisk) @@ -1928,7 +1928,7 @@ endif ifdef BOARD_INCLUDE_DTB_IN_BOOTIMG INTERNAL_RECOVERYIMAGE_ARGS += --dtb $(INSTALLED_DTBIMAGE_TARGET) endif -endif # INSTALLED_VENDOR_BOOTIMAGE_TARGET not defined +endif # (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) ifndef BOARD_RECOVERY_MKBOOTIMG_ARGS BOARD_RECOVERY_MKBOOTIMG_ARGS := $(BOARD_MKBOOTIMG_ARGS) endif @@ -4490,11 +4490,11 @@ else ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) cp $(firstword $(INSTALLED_KERNEL_TARGET)) $(zip_root)/$(PRIVATE_RECOVERY_OUT)/kernel endif endif -ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(AB_OTA_UPDATER))) +ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) echo "$(GENERIC_KERNEL_CMDLINE)" > $(zip_root)/$(PRIVATE_RECOVERY_OUT)/cmdline endif # BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE != true -else # not (BUILDING_VENDOR_BOOT_IMAGE and AB_OTA_UPDATER) +else # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) ifdef INSTALLED_2NDBOOTLOADER_TARGET cp $(INSTALLED_2NDBOOTLOADER_TARGET) $(zip_root)/$(PRIVATE_RECOVERY_OUT)/second endif @@ -4522,7 +4522,7 @@ endif ifdef BOARD_KERNEL_PAGESIZE echo "$(BOARD_KERNEL_PAGESIZE)" > $(zip_root)/$(PRIVATE_RECOVERY_OUT)/pagesize endif -endif # not (BUILDING_VENDOR_BOOT_IMAGE and AB_OTA_UPDATER) +endif # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) endif # INSTALLED_RECOVERYIMAGE_TARGET defined or BOARD_USES_RECOVERY_AS_BOOT is true @# Components of the boot image $(hide) mkdir -p $(zip_root)/BOOT From c56931c1739ef0cbc75fd41e3da9699e5509e1e6 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 28 Oct 2020 16:35:19 -0700 Subject: [PATCH 3/5] Move common code out of the conditional. Test: builds Change-Id: I75e13b929dbdb31785a59898579ed14bcc3eef60 --- core/Makefile | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/Makefile b/core/Makefile index 7d0212049..6b1206e9c 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1890,8 +1890,9 @@ $(INSTALLED_RECOVERY_RAMDISK_BUILD_PROP_TARGET): $(INSTALLED_RAMDISK_BUILD_PROP_ $(copy-file-to-target) endif +INTERNAL_RECOVERYIMAGE_ARGS := --ramdisk $(recovery_ramdisk) + ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) - INTERNAL_RECOVERYIMAGE_ARGS := --ramdisk $(recovery_ramdisk) ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) ifdef GENERIC_KERNEL_CMDLINE @@ -1900,9 +1901,7 @@ endif # GENERIC_KERNEL_CMDLINE != "" endif # BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE != true else # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) - INTERNAL_RECOVERYIMAGE_ARGS := \ - $(addprefix --second ,$(INSTALLED_2NDBOOTLOADER_TARGET)) \ - --ramdisk $(recovery_ramdisk) +INTERNAL_RECOVERYIMAGE_ARGS += $(addprefix --second ,$(INSTALLED_2NDBOOTLOADER_TARGET)) # Assumes this has already been stripped ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) ifdef INTERNAL_KERNEL_CMDLINE From 78860164d08cdc7b938bde0496a5a53122c6373e Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 28 Oct 2020 14:15:21 -0700 Subject: [PATCH 4/5] Do not put GENERIC_KERNEL_CMDLINE in recovery image. The GENERIC_KERNEL_CMDLINE should only be in the generic boot image. If device uses recovery-as-boot, it never uses generic boot image because on devices with generic boot image, recovery resources are moved to vendor_boot instead. Bug: 171512004 Test: builds Change-Id: Ia84e604a8ded28af39c7f1861ff5d3b3af55849f --- core/Makefile | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/core/Makefile b/core/Makefile index 6b1206e9c..2b09c09e0 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1892,15 +1892,7 @@ endif INTERNAL_RECOVERYIMAGE_ARGS := --ramdisk $(recovery_ramdisk) -ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) - -ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) -ifdef GENERIC_KERNEL_CMDLINE - INTERNAL_RECOVERYIMAGE_ARGS += --cmdline "$(GENERIC_KERNEL_CMDLINE)" -endif # GENERIC_KERNEL_CMDLINE != "" -endif # BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE != true - -else # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) +ifneq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) INTERNAL_RECOVERYIMAGE_ARGS += $(addprefix --second ,$(INSTALLED_2NDBOOTLOADER_TARGET)) # Assumes this has already been stripped ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) @@ -4489,11 +4481,7 @@ else ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) cp $(firstword $(INSTALLED_KERNEL_TARGET)) $(zip_root)/$(PRIVATE_RECOVERY_OUT)/kernel endif endif -ifeq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) -ifneq (true,$(BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE)) - echo "$(GENERIC_KERNEL_CMDLINE)" > $(zip_root)/$(PRIVATE_RECOVERY_OUT)/cmdline -endif # BOARD_EXCLUDE_KERNEL_FROM_RECOVERY_IMAGE != true -else # not (BUILDING_VENDOR_BOOT_IMAGE and BOARD_USES_RECOVERY_AS_BOOT) +ifneq (truetrue,$(strip $(BUILDING_VENDOR_BOOT_IMAGE))$(strip $(BOARD_USES_RECOVERY_AS_BOOT))) ifdef INSTALLED_2NDBOOTLOADER_TARGET cp $(INSTALLED_2NDBOOTLOADER_TARGET) $(zip_root)/$(PRIVATE_RECOVERY_OUT)/second endif From 78b55b224408dc956f64f826fc666bc8df70e61d Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 28 Oct 2020 14:19:05 -0700 Subject: [PATCH 5/5] Mount generic ramdisk as readwrite. With this change, first stage init can prepare and move resources to accomodate devices with and without a dedicated recovery partition. Test: build with and without recovery partition, and manually inspect Bug: 171512004 Change-Id: I7bd61f74c16ee77f3f05dc208e0f3cfe81e302b0 --- core/Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/Makefile b/core/Makefile index 2b09c09e0..9335fc5b1 100644 --- a/core/Makefile +++ b/core/Makefile @@ -711,6 +711,10 @@ else BUILT_BOOTIMAGE_TARGET := $(PRODUCT_OUT)/boot.img endif +# kernel cmdline for GKI +GENERIC_KERNEL_CMDLINE := rw +.KATI_READONLY := GENERIC_KERNEL_CMDLINE + # $1: boot image target # returns the kernel used to make the bootimage define bootimage-to-kernel