AOSPAlliance / android-prepare-vendor

Set of scripts to automate AOSP compatible vendor blobs generation from factory images
24 stars 8 forks source link

Redfin (Pixel 5) support for Android 11 #47

Closed sdh4 closed 3 years ago

sdh4 commented 3 years ago

Here is a first cut at support for Redfin (Pixel 5), both base and full configurations.

Both boot sucessfully and at least basic functionality is working (e.g. camera, boot with locked bootloader). I also have confirmed that calling and SMS work at least in the full configuration with Verizon (US)

chirayudesai commented 3 years ago

This is looking nice.

If you want to compare to another known working config, we have https://github.com/Flex1911/android-prepare-vendor/commits/11

First thing that stands out in the diff is that you added two more partitions, featenabler and vendor_boot. This also misses the rename from sm6150 to lito for bootctrl.

chirayudesai commented 3 years ago

You're also missing changes to scripts/generate-vendor.sh, and spacing in the full config looks off.

danielfullmer commented 3 years ago

Thanks for the contribution! I was able to successfully build an image for redfin using sdh4's branch, but I had to add an additional fix for including the BoardConfigVendor.mk in scripts/generate-vendor.sh (that's in Flex1911's version). (I don't currently have a Pixel 5 to test this with)

Here's the file list differences between Google's image and one built using this branch, for the system, system_ext, and vendor partitions, using the naked configuration: https://gist.github.com/danielfullmer/07e522de589b3b616bfa9157cefdec06

system_ext appears to be missing a number of qualcomm-related files. One good place to check are those listed under device/google/redfin/self-extractors/qcom/staging, specifically device-partial.mk.

chirayudesai commented 3 years ago

Flex1911's branch boots for here, r22. (I was already building that, saw this after, happy coincidence really)

@danielfullmer which Google image is that diff against? Stock or AOSP driver binaries? If it's the latter then that already differs from stock on it's own, at least it has in the past releases.

danielfullmer commented 3 years ago

@chirayudesai It's the diff against Google's stock image, not one built with their driver binaries.

chirayudesai commented 3 years ago

Additionally, I have been told that the redfin config works on bramble as well, which makes sense given that two devices of the same family usually have identical configs (and even coral/flame/sunfish are all identical)

chirayudesai commented 3 years ago

This builds once you make the scripts/generate-vendor.sh change to create proprietary/BoardConfigVendor.mk, and boots.

AOSP r22, RQ1A.201205.011 (and blobs extracted from the same build)

Just need some minor cleanup, testing on bramble and then this is probably good to go. I'll create issues for testing.

sdh4 commented 3 years ago

I've added the scripts/generate-vendor.sh change (whoops!) I've fixed a typo "libavservices_minijail_vendor" vs. "libavservices_minijail.vendor" I've fixed the sm6150 to lito I've fixed the secure_element version from 1.0 to1.2 in the vendor_skip_files I've fixed the version number for the vibrator service, which is not present in the vendor file I've untabified the full section, which I think fixes the formatting.

Here are other differences to Flex1911 (happy to change, but not sure what is best).

Since both configs are known working, and including more is probably safe than including less, I would propose in general using the union of the file sets and the intersection of the exclusions, unless there are specific reasons to do otherwise.

Thoughts?

sdh4 commented 3 years ago

Added an additional commit with union of file sets and intersection of exclusions as suggested above

danielfullmer commented 3 years ago

Here's the new diff for the output of the latest commit (4173123b77ede5bbc00d478199565afaae75afe5): https://gist.github.com/b34d4ac09ceb45d0c0a6adfd38c377e4

sdh4 commented 3 years ago

I am not the right person to know which of these are important. Obviously the "naked" dist is supposed to be kept fairly slim. I could see bulk-adding the various qualcomm libraries, though. Thought/suggestions?

chirayudesai commented 3 years ago

I've added the scripts/generate-vendor.sh change (whoops!) I've fixed a typo "libavservices_minijail_vendor" vs. "libavservices_minijail.vendor" I've fixed the sm6150 to lito I've fixed the secure_element version from 1.0 to1.2 in the vendor_skip_files I've fixed the version number for the vibrator service, which is not present in the vendor file I've untabified the full section, which I think fixes the formatting.

Here are other differences to Flex1911 (happy to change, but not sure what is best).

  • I have an extra product/etc/apns-conf.xml in product-other.

Keep this for sure.

  • I'm missing "system_ext/framework/com.qualcomm.qti.uceservice-V2.0-java.jar", in system_ext-bytecode

Should add this, we have it on others.

  • I have an extra system_ext/lib/com.quicinc.cne.api@1.1.so in system_ext-other

Not sure if cne works here, but we have these on other devices anyway.

  • I am missing "system_ext/lib/libqmi_cci_system.so", "system_ext/lib/libqmi_encdec_system.so", "system_ext/lib/libsdm-disp-apis.qti.so", "system_ext/lib/vendor.qti.imsrtpservice@3.0.so", "system_ext/lib64/libqmi_cci_system.so", "system_ext/lib64/libqmi_encdec_system.so", "system_ext/lib64/libsdm-disp-apis.qti.so", and "system_ext/lib64/vendor.qti.imsrtpservice@3.0.so" in system_ext-other

If you grep for these in tree here, you'll see some of them are present in other devices. I'd say let's just match and keeps this consistent for now?

That is not in flame/coral, but in only in sunfish for some reason. forced-modules is to force AOSP to build this particular module explicitly, in case it isn't getting built.

Should add this.

  • There is a smattering of differences in vendor-skip-files.

Since both configs are known working, and including more is probably safe than including less, I would propose in general using the union of the file sets and the intersection of the exclusions, unless there are specific reasons to do otherwise.

Thoughts?

I'd agree. I'd also say let's try and match existing devices as much as we can. Some of this is a mess, but if it's the same mess everywhere it's a lot easier to clean up / verify fixes. coral/flame/sunfish are all known 100% working at this point.

I am not the right person to know which of these are important. Obviously the "naked" dist is supposed to be kept fairly slim. I could see bulk-adding the various qualcomm libraries, though. Thought/suggestions?

The main idea behind the "naked" dist has always been to avoid any dependencies on Google Play Services, and also any extra carrier-specific apps (some of which may depend on Play Services anyway)

"full" is meant to be used with Google Apps (works with microG too for the most part), and "naked" with plain AOSP.

Qualcomm libraries, especially if needed for hardware functionality, definitely belong in "naked"

From what I can see, some of the files in danielfullmer's diff are present on other devices, but not in the newer devices, must have missed those when moving to 11. And then some of them I think we can build in AOSP, which is what we use forced-modules for.

To figure that things out we have sort of a semi-scripted approach, https://freenode.logbot.info/aospalliance/20200519#c3917655 - the lineage commit that adds 'modinstalled'

sdh4 commented 3 years ago

I just added one more commit (6f6caca) with additional libraries (mainly Qualcomm) in system_ext. The prior commit ( 4173123 ) was mostly lined up with libraries in sunfish, as that was the template I started with.

sdh4 commented 3 years ago

Removed the selinux entries. I can confirm that redfin seems to be generally working with commit e81c9d8

chris42 commented 3 years ago

Sorry for a little side question: I stumbled here while seeing to get a Pixel 5 running for grapheneOS. So far I am building a regular AOSP with the driver binaries. Is there any information why you are not using the driver binaries, but extract things out of the factory image? The README is a bit vague and states that the binaries are actually not being provided?

sdh4 commented 3 years ago

chris42: My understanding is that there are two reasons: First, way the driver binaries are provided its's not possible to use them in an OS with the bootloader locked. Second, there are a lot of components that are missing, so a lot of functionality doesn't work and there are compatibility issues with some carriers.

sdh4 commented 3 years ago

So I have been struggling with adbd crashing on the Pixel 5, which is obviously hard to troubleshoot. I'm not sure whether this is related to the vendor or not. I My first builds had no trouble.

Every time I try to do an eng build or similar to get debug messages at boot up, rather than getting the the messages adb is entirely non functional. Toggling adb in the dev ops seems make adbd try to restart but it still doesn't respond. The critical parameteers are like things should be -- ro.secure=0; ro.debuggable=true. I've reconfigured everything exactly like those early builds, tried clearing out /out, etc. and I still can't get adbd to work on bootup anymore.

Any ideas? Anybody seen anything like this?

chris42 commented 3 years ago

In some other device builds I had troubles with adb when it was already connected and listening during boot. Needed to wait a while then connect for it to work.

chris42 commented 3 years ago

I just tried to use this config with an AOSP build and not even get a proper vendor image build. This is my first time using android-prepare-vendor, is there any other documentation or examples I can follow?

hackoder commented 3 years ago

@sdh4 what does adbd crashing look like? I have AOSP running with your changes for vendor and left adb logcat running for 20 or so minutes without issues.

Also went through the checklist in #48 and can confirm that everything works, other than Verified boot, wifi calling, and VoLTE which I haven't tested yet.

sdh4 commented 3 years ago

@hackoder: The symptoms of (presumed) adbd crashing are that it is not possible to connect via adb. I have been able to get back to a working configuration by going back to a prior commit (8cc66129343102df25fcc6bed1ba8bcdddede085). The problem may be specific to eng builds (where adbd runs with root permissions) or the full config. When it occurs, turning off usb debugging in developer options may help allow other USB functions to work.

I am now in the process of bisecting to determine the problematic commit/config. As you can imagine it may be a slow process @chris42: I have a procedure published at https://thermal.cnde.iastate.edu/aosp_build_instructions.xhtml but it hasn't yet been updated to Android 11

chris42 commented 3 years ago

@sdh4 Thanks for sharing the guide, it is pretty much what I do in my builds. However running the script with your config and compiling will not generate a vendor.img. All other .img seem to be there.

Also trying to understand what all this scripts are doing/what they are for. I compared a vendor factory image and the vendor image that is created using drivers and binaries. Not sure if that helps in any way, but found the following files missing in the regular aosp build compared to the factory one: https://pastebin.com/ViSgUDUY

sdh4 commented 3 years ago

@chris42 android-prepare-vendor does not generate a vendor image; they generate a vendor tree to place in your android build directory. Then the android build process creates the vendor image from the vendor tree. It is necessary if you are planning on signing your build so that it can boot with the bootloader locked.

chris42 commented 3 years ago

@sdh4 Yes, that is why I wrote "... and compiling will not generate a vendor.img". As usual it was some stupid mistake I made before. Will come back if I find something useful.

sdh4 commented 3 years ago

I figured out the adb problem. It was specific to the full configuration and probably eng (or otherwise rootable) builds, likely resulting from an incorrect selinux configuration that was blocking adbd from accesses that should have been allowed.

Specifically there are three files: "etc/selinux/plat_sepolicy_vers.txt", "etc/selinux/precompiled_sepolicy", and "etc/selinux/vndservice_contexts" that were getting filtered out in the naked configuration but not the full configuration, because of VENDOR_SKIP_FILES_NAKED entries in scripts/constants.sh.

I added these three files explicitly to vendor-skip-files for the full configuration and that has solved my problems. It would appear that these same files have caused problems in other configurations (#43). Would it make sense to transfer them from VENDOR_SKIP_FILES_NAKED to VENDOR_SKIP_FILES in scripts/constants.sh?

I can confirm with these changes that verified boot, wifi, bluetooth, camera, sound, fingerprint unlock, flashlight, GPS, hotspot, calling, and text messaging all work, at least superficially, with the latest commit ( 2477c47 ) . I should be able to confirm VoLTE soon as well.

danielfullmer commented 3 years ago

Seeing that there were dynamic linking issues with lib-imsvt.so, I had a thought: We could check the DT_NEEDED entries in the ELF shared libraries of our built image, and report if the library needs another library that is provided by the upstream image, but is not present in our image. This should catch some of these simple errors with missing libraries.

I have a small snippet of bash that does this: https://gist.github.com/danielfullmer/a582fa467a681f69a27ae88b6168f95d It outputs the following for redfin:

system_ext/lib/libqcc_file_agent_sys.so needed by system_ext/lib/vendor.qti.hardware.qccsyshal@1.0-halimpl.so
system_ext/lib/libhoaeffects_csim.so needed by system_ext/lib/libhoaeffects.qti.so
system_ext/lib64/libhoaeffects_csim.so needed by system_ext/lib64/libhoaeffects.qti.so
system_ext/lib64/libqcc_file_agent_sys.so needed by system_ext/lib64/vendor.qti.hardware.qccsyshal@1.0-halimpl.so

I intend to check the rest of the devices to see if we're missing anything else like this.

Also, for this PR, there remain some relevant differences in vendor/build.prop. I believe we should at least ensure that these are set:

ro.hardware.egl=adreno
ro.hardware.vulkan=adreno
sdh4 commented 3 years ago

Latest commit (e7262d9) adds the libraries and build.prop entries identified by @danielfullmer .... Thanks!

chirayudesai commented 3 years ago

Given that this seems to be working well in general, we should merge this soon after a final review, and any additional fixes if needed can come later.

One thing is that it's a lot of commits now, and generally in the past we've squashed but in this case some of it seems useful to keep around, so I don't have a preference. @danielfullmer @dan-v do either of you prefer squashing or just merging as-is?

Lastly let's also just copy this to bramble, make any needed changes (if some file specifically has redfin in it's name), and that should for the most part work just as well.