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

[Tobias Geerinckx-Rice <me@tobias.gr>]

[-- Attachment #1: Type: text/plain, Size: 2974 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]