Hi;
On 5/22/19 9:20 PM, Alistair Strachan wrote:
On Tue, May 21, 2019 at 10:10 PM Tapani Pälli <tapani.pa...@intel.com> wrote:
On 5/21/19 4:53 PM, Sumit Semwal wrote:
Hello everyone,
First up, my apologies on not being able to respond earlier; secondly,
thanks very much for your review.
On Wed, 15 May 2019 at 19:27, Emil Velikov <emil.l.veli...@gmail.com> wrote:
Hi all,
On Tue, 14 May 2019 at 08:18, Tapani Pälli <tapani.pa...@intel.com> wrote:
On 5/13/19 6:52 PM, Haehnle, Nicolai wrote:
This approach seems entirely incompatible with si_debug_options.h, and
will be an absolute maintenance nightmare going forward for adding /
removing options, because you're introducing a second location where
options are defined.
Quite frankly, this seems like a terrible idea as-is.
If you really can't use XML for whatever reason, then please find some
way of deriving both the tables here and the XML from the same single
source of truth.
I was looking at this yesterday and came up with same conclusion. We
should have the options in one place. Currently libexpat is statically
linked with Android >=O, maybe for such restricted environments we could
just inline the xml as is at compile time and parse that later or
alternatively (maybe cleaner) parse and generate default option cache
already during compilation?
I realise that jumping the "me too" train does not help much, so here
are some alternative ideas.
How about we first distil the reasons why this is a problem and what
kind. Then explore independent solution for each one - as-is this
seems like a one-size-fits-all approach.
I totally agree that this seems like a rudimentary / ugly approach,
and we can definitely improve upon it once the reasons are discussed.
Some examples:
- XML file may be inaccessible - the in-driver defaults should work(tm)
Yes there are some app specific ones, yet neither(?) of these apps is
present on Android
- libexpat is not available, but libFOO is - investigate into a compat
wrapper
- cannot use external libraries (libexpat or equivalent) - static link
AFAIU, in the Android space, it is a combination of some of the above:
a. current Android doesn't allow GL drivers to access config files
from the vendor partition: this is enforced via selinux policy.
For point a, vendors can (and should) define their own policy rules
regarding what file access and ioctl's are required. This is done by
setting BOARD_SEPOLICY_DIRS in BoardConfig.mk file. That directory then
contains all the necessary rules required for the particular driver to
work. As example:
BOARD_SEPOLICY_DIRS += device/samsung/tuna/sepolicy
If a vendor wanted to use xml based configuration for Mesa it should be
possible by setting a sepolicy rule so that particular library is able
to access such file. Looking at Android Celadon selinux files,
'file_contexts' is probably the place to do it.
The EGL/GLES driver stack is a special kind of HAL in Android
(same-process HAL) so we have to be very careful about expanding the
sepolicy rules to work around unnecessary file accesses. We also have
strict sepolicy "neverallows" for untrusted apps (the processes this
same-process HAL might be loaded into). I strongly disagree with your
suggestion here.
From an Android PoV, the EGL/GLES drivers should minimize their
dependencies so as to not affect other NDK libraries loaded into the
app processes. They should also limit interactions with the rest of
the system, such as opening configuration files. It's clear that Mesa
can work just fine without reading a configuration file, and that the
use case of opening a configuration file should only be necessary for
development and bring-up.
The discussion so far on this thread seems to be optimizing for Mesa's
configuration file, rather than for security and file size. On an
embedded platform such as Android, in cases where Mesa might ship in a
production configuration, there should be no configuration file, and
we would want vendors to optimize for security and file size.
My opinion is that we need Sumit's changes, or something like them.
Pulling in libexpat just to build internal configuration state from a
built-in XML file seems quite over-engineered.
That said, I agree with other feedback on this thread that it should
be possible to derive the baked configuration from the same source of
truth (possibly an XML file) as another platform which might not have
a baked configuration.
b. Also, they had some concerns around how safe libexpat is vis-a-vis
dual-loading, and that's where the concern around static linking came
from.
Alistair, could you please correct me if I am wrong, and if there are
additional details on the need of this?
The concern is basically that libexpat might be "dual loaded" - by the
linker namespace for the sphal, and that of the app itself - and that
there might be global data (like pthread keys, etc.) that could
conflict. The versions of the library needn't be the same, either; the
app and the EGL/GLES stack might be using different versions
(static+static, dynamic+static, etc.)
My concern is more basic though - libexpat is a non-trivial amount of
code, and Mesa only uses it to parse a configuration file that a
production device won't have or want. It won't be allowed to by system
sepolicy. So, we are increasing the size of the graphics stack just to
parse an internally-baked XML file, which seems like a very reasonable
dependency to work on optimizing out.
With selinux suggestion I was mainly trying to balance between having 2
separate solutions against 1. *Ideally* all systems would use same code
for configuration mgmt so that we wouldn't need to support and maintain
2, that comes with a cost.
I don't strongly oppose changes though and maybe some kind of 'default
config generation during compile time' would bring this closer to the goal.
// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
