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

[Android 10] [TESTME] Use ro.hardware.<class>=qcom to point to harware libraries #642

Closed
wants to merge 2 commits into from

Conversation

MarijnS95
Copy link
Contributor

This fixes the camera that fails to start on Tama.

Together with sonyxperiadev/device-sony-kumano#4 and sonyxperiadev/device-sony-tama#64

Android 10 adds a nice feature that verifies whether the realpath of a
loaded module starts with the searched path the module came from. For
symlinks this is not true (for example the realpath of the camera
library is /odm/lib/hw/camera.qcom.so: this does not start with
/vendor/lib/hw where the search originated from).

See the relevant change in libhardware:
https://android.googlesource.com/platform/hardware/libhardware/+/e448a05fecab601c59b89a2f948e1ad18050e40f%5E%21/#F0

Instead of creating a symlink on the odm partition, set
ro.hardware.camera=qcom in the relevant platform repositories (currently
only Tama and Kumano). This ensures libhardware also looks for
camera.qcom.so in all search paths (vendor and odm libraries).

Do the same for vulkan symlinks (while not strictly necessary) to clean up.

TODO:

Check other symlinks for removal. In principle, library folders in /odm are included in the search path (and VNDK rules) so there should be no problem loading these directly.

While the vulkan link can safely be removed on all platforms the link (without name change) for camera cannot, because "new" camera blobs try to load them explicitly from /vendor: CHIUSECASE: chxutils.cpp:526 LibMap() Failed to load library /vendor/lib/hw/camera.qcom.so error dlopen failed: library "/vendor/lib/hw/camera.qcom.so" not found

Tested on Mermaid and Akatsuki. Please test other devices!

Android 10 adds a nice feature that verifies whether the realpath of a
loaded module starts with the searched path the module came from. For
symlinks this is not true (for example the realpath of the camera
library is /odm/lib/hw/camera.qcom.so: this does _not_ start with
/vendor/lib/hw where the search originated from).

See the relevant change in libhardware:
https://android.googlesource.com/platform/hardware/libhardware/+/e448a05fecab601c59b89a2f948e1ad18050e40f%5E%21/#F0

Instead of creating a symlink on the odm partition, set
ro.hardware.camera=qcom in the relevant platform repositories (currently
only Tama and Kumano). This ensures libhardware also looks for
camera.qcom.so in all search paths (vendor and odm libraries).

Signed-off-by: MarijnS95 <marijns95@gmail.com>
Following the change on the camera, convert vulkan-specific symlinks to
properties as well. While vulkan works fine without this change it is
mostly just a correctness cleanup.

Signed-off-by: MarijnS95 <marijns95@gmail.com>
@jerpelea
Copy link
Collaborator

i will check if it is possible to load them from odm

@ix5
Copy link
Contributor

ix5 commented Sep 14, 2019

I concur with Marijn, going with Google's way will prove less painful for the future.

@jerpelea I remember you said no last time this was discussed for Pie #565
Symlinks are an ugly hack and tend to require extra sepolicy annoyances.

@MarijnS95 just as a refresher, there's a difference between searchable and permitted paths in ld.config.$(VERSION).txt which I always forget.
But I just looked it up and if the symlink source is in searchable paths you're good, but the target needs to be in both searchable and permitted paths.

@jerpelea
Copy link
Collaborator

for O it may be ok but not for O
I will check again

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Sep 14, 2019

@ix5 That doesn't entirely sound like the difference between search and permitted. In part because the meaning of the former also depends on isolated. At least as far as I understand the documentation.
But let's continue this elsewhere, if you want.

@jerpelea Unfortunately the answer is no. Maybe we can trim off some symlinks, but the ones to egl must remain because the loader only looks in /vendor 🤦‍♂️
https://android.googlesource.com/platform/frameworks/native/+/android-10.0.0_r3/opengl/libs/EGL/Loader.cpp#163

After some quick testing Android and Vulkan run fine when removing everything but egl. However most of the files are related to RS which I did not test yet.

The same holds for the camera. As shown in the PR under # TODO one of the camera libraries explicitly loads from /vendor. If you can solve this in the blobs it might be possible. Perhaps we can also try to remove as much symlinks as possible.

@ix5
Copy link
Contributor

ix5 commented Sep 15, 2019

Gerrit CL time I'd say... maybe we can get rid of the hardcoded /vendor location until R.

Imo we can merge this already and get rid of the symlinks in one fell swoop once we've gotten AOSP to look in /odm as well.

@MarijnS95
Copy link
Contributor Author

Let's find some time to CL that stuff, and see about fixing the hardcoded path in the camera blobs.

For now I suggest to leave this PR open, as a point of discussion and because removing the unused links is not vital (but merging might definitely help with testing).
We should however merge
sonyxperiadev/device-sony-kumano#4
sonyxperiadev/device-sony-tama#64
As these are vital to get a working camera on Android 10.

@sraase
Copy link
Contributor

sraase commented Sep 25, 2019

Unfortunately, camera.qcom.so has the component pathes fixed to /vendor/lib64/, and it is a prebuilt.

@MarijnS95
Copy link
Contributor Author

@sraase Yeah, I figured com.qti.chi.override.so is a prebuilt to you guys. Pity but we'll have to deal with it. I'll see (at some point in time...) whether the other symlinks can go.

@sraase
Copy link
Contributor

sraase commented Sep 25, 2019

The com.qti.chi.override.so is not a prebuilt, but camera.qcom.so is.

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Sep 25, 2019

If it isn't then it should be fixable, as the hardcoded string is in there (unless it comes from a static linked lib).
Or, unless camera.qcom.so also expects itself to be in /vendor and does all kinds of craziness...
Edit: Yeah, there are quite a few strings pointing to /vendor/lib 😶

@sraase
Copy link
Contributor

sraase commented Sep 25, 2019

Well, com.qti.chi.override.so (similar to other modules) is loaded from camera.qcom.so - with fixed pathes. I don't think camera.qcom.so expects itself to be in /vendor, but I haven't verified.

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Sep 25, 2019

That's interesting, because com.qti.chi.override.so also seems to load camera.qcom.so 😁 but indeed the passthrough HAL starts with camera.qcom.so.
Anyway it doesn't really matter, we can live with the symlinks.

@sraase
Copy link
Contributor

sraase commented Sep 25, 2019

You are right. So that path is fixed as well. Sigh.
edit: Patching the chi override can be done, so that is not really an issue. Doesn't help with the other modules and drivers, though.

@jerpelea jerpelea force-pushed the master branch 3 times, most recently from 699ce56 to 5d1178b Compare October 3, 2019 13:16
@jerpelea
Copy link
Collaborator

we can drop this commit since the symlinks were dropped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants