magiskboot: add basic support for vendor_boot v4

[nilz3000]

This PR addresses Issue #6460 and adds the following functionalities:
* un- and repack multiple ramdisks
* un- and repack bootconfig

There is currently no support for adding or removing ramdisk fragments.

[yujincheng08]

BTW, in the vendor ramdisk table, there is ramdisk_name. We could better use this name instead of indices.

[nilz3000]

BTW, in the vendor ramdisk table, there is ramdisk_name. We could better use this name instead of indices.

I thought about that too, but it isn't mandatory to set. Google doesn't even do it in their own OTAs. Maybe we should use numbers+type.

[yujincheng08]

accroding to the document, vendor ramdisk name must be set and unique.

[yujincheng08]

https://source.android.com/docs/core/architecture/partitions/vendor-boot-partitions

ramdisk_name is an unique name of the ramdisk.

To include multiple vendor ramdisks in vendor_boot:

Set BOARD_BOOT_HEADER_VERSION to 4.
Set BOARD_VENDOR_RAMDISK_FRAGMENTS to a list of logical vendor ramdisk fragment names to be included in vendor_boot.

[nilz3000]

https://source.android.com/docs/core/architecture/partitions/vendor-boot-partitions

ramdisk_name is an unique name of the ramdisk.

To include multiple vendor ramdisks in vendor_boot:

Set BOARD_BOOT_HEADER_VERSION to 4.
Set BOARD_VENDOR_RAMDISK_FRAGMENTS to a list of logical vendor ramdisk fragment names to be included in vendor_boot.

You are right. I was just a bit confused, because the name of the "default" fragment isn't set in the google OTAs, but others are. I will change the naming scheme.

[vvb2060]

There can never be a fragment with default name. Why do you think there needs to be a fragment with such a name?

[vvb2060]

default means all fragments, it is a keyword and cannot be used as a name.
https://source.android.com/docs/core/architecture/bootloader/fastbootd#driver-changes

[yujincheng08]

doubt if ramdisk_name is null-terminated

[nilz3000]

default means all fragments, it is a keyword and cannot be used as a name. https://source.android.com/docs/core/architecture/bootloader/fastbootd#driver-changes

That's what i meant. I think the confusions comes from the example they give in the last paragraph of this, where you have a fragment that isn't explicitly specified.
https://source.android.com/docs/core/architecture/partitions/vendor-boot-partitions

And according to this, it's null-terminated.
https://android.googlesource.com/platform/system/tools/mkbootimg/+/refs/heads/master/include/bootimg/bootimg.h#415

[yujincheng08]

no. its not null-terminated https://android.googlesource.com/platform/system/tools/mkbootimg/+/refs/heads/master/mkbootimg.py#317

[yujincheng08]

https://android.googlesource.com/platform/system/tools/mkbootimg/+/refs/heads/master/include/bootimg/bootimg.h#415

this indicates the name always takes 32 bytes, but it does not mean its null-terminated.

[vvb2060]

That's what i meant. I think the confusions comes from the example they give in the last paragraph of this, where you have a fragment that isn't explicitly specified.

It means the --vendor_ramdisk parameter in the mkbooting.py command line. This default is kept for compatibility with older versions. Will be auto converted to VENDOR_RAMDISK_TYPE_PLATFORM type.

https://cs.android.com/android/platform/superproject/+/master:system/tools/mkbootimg/mkbootimg.py;drc=053c389f03f3c14f86b808608ccb5669ff8b887a;l=454

[yujincheng08]

after disscusion, we decided to add an additional flag to cli to explictly distinguish vendor boot and boot so as to avoid user from accidentially patching vendor boot. In other words, by default, magiskboot should not accept vendor boot unless --vendor is given.

[nilz3000]

after disscusion, we decided to add an additional flag to cli to explictly distinguish vendor boot and boot so as to avoid user from accidentially patching vendor boot. In other words, by default, magiskboot should not accept vendor boot unless --vendor is given.

Sounds good to me. Do you guys want to do this or should I do it?

[yujincheng08]

feel free to implement

[osm0sis]

Why would it need a --vendor switch when the format already uses a different header magic? Shouldn't the return value and boot_patch script just be made to reject VNDRBOOT magic?

Similar things are done to support CHROMEOS magic:
https://github.com/topjohnwu/Magisk/blob/master/scripts/boot_patch.sh#L94-L108

E.g. 3 ) abort "! vendor_boot image is not used by Magisk, try boot/init_boot"

[yujincheng08]

Because we don't actually want to unpack it.

And mostly, returning 3 is not friendly to shell scripts (as we usually command || !abort)

[osm0sis]

I think it's still reasonable where we have a return of 2 signifying CHROMEOS, so catching VNDRBOOT there with a return of 3 and aborting feels like a more consistent approach, but if everyone else has discussed it and agrees that adding some new switch is somehow cleaner instead, well then I guess we'll have to disagree but it is what it is. 👍

[ethaldeman]

Since it looks like the merge conflicts have been resolved, any hope of this getting merged before the next release?

[osm0sis]

@topjohnwu can this please be addressed again? It's going on a year now sitting here approved by @yujincheng08.

[nilz3000]

Whoever wants can grab the patched magiskboot builds here.

[nuschpl]

Besides @topjohnwu personal conflict of interests are there any other obstacles for this being merged ?

[osm0sis]

Besides @topjohnwu personal conflict of interests are there any other obstacles for this being merged ?

There's zero conflict of interest for this one. Not sure why nothing's happening. Hopefully @topjohnwu or someone in the know can comment on if there are more changes wanted before it's ready for merge from their point of view.