help-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Custom libre kernel configuration devolving into Anbox review I gues


From: Tobias Geerinckx-Rice
Subject: Re: Custom libre kernel configuration devolving into Anbox review I guess
Date: Sat, 13 Nov 2021 13:23:47 +0100

Petr,

phodina 写道:
Though the issue seems to come from the Archlinux Wiki[1] where they supply wrong CONFIG options.

Well… yes, it's a wiki. The Arch one in particular has a reputation to uphold.

Still, it[1] doesn't suggest either of the problematic

+   ("CONFIG_ASHMEM" . m)
+   ("CONFIG_ANDROID_BINDER_IPC" . m)

values.  These can't work:

 config ASHMEM
       bool "Enable the Anonymous Shared Memory Subsystem"
 config ANDROID_BINDER_IPC
       bool "Android Binder IPC Driver"

So don't waste time hunting down dependencies which don't exist. The third-party modules[2] were never part of Linux and are by now well obsolete.

From the same wiki:

CONFIG_ANDROID_BINDER_DEVICES="binder,hwbinder,vndbinder"
[lalala lala]
With your new kernel, you will need to append the following to your
boot arguments:
binder.devices=binder,hwbinder,vndbinder,anbox-binder,\
  anbox-hwbinder,anbox-vndbinder

Why does it first recommend a different value from what ‘you will need’ to boot with later? Merely because it's the Kconfig default? This does not fill me with confidence.

Later(!) on, it suggests yet a third, seemingly preferred, option:

scripts/config --set-str CONFIG_ANDROID_BINDER_DEVICES ""
[because]
Not everybody was happy with the binder module in Linux. To address the issues, binderfs was created. One has to choose between the old and the new way when compiling the kernel. With the options [above],
one will use binderfs instead.

So… maybe that's the cool (and more secure) new thing and we should be using binderfs without any DEVICES instead? Is binderfs some kind of /dev/pts for them? Have you tested Waydroid without any?

I'm picky because I want to suggest the following, which makes it important that at least one person understands these changes and that we get them right:

Do we really need yet another kernel variant? Building nearly-identical kernels on CI is quite expensive, especially on ARM. Let's not add them lightly.

Is this code so dubious — compared to the rest of CONFIG_STAGING, which we already enable — that it must be quarantined in a separate kernel? If so, why?

Do these options make practical sense to enable on non-ARM kernels? Does Android run on them?

Why was CONFIG_ANDROID_BINDERFS set to #f in your previous patch[0]? Typo? Evolved knowledge?

I often forget to do so myself but still recommend adding a human-readable ‘v1 -> v2’ changelog (that won't be added to the git commit message) to explain such changes and catch any unintented ones.

Kind regards,

T G-R

PS: Another nitpick, but as CONFIG_ANDROID ‘unlocks’ CONFIG_ASHMEM, please move the latter to the end of the list.

[0]: https://issues.guix.gnu.org/51771#1
[1]: https://wiki.archlinux.org/title/Waydroid
[2]: https://github.com/anbox/anbox-modules/issues/75

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]