[Mesa-dev] [RFC 0/2] Alternate default config mechanism

[Eric Engestrom]

On Thursday, 2019-05-23 08:34:40 +0300, Tapani Pälli wrote:
> Hi;
> 
> On 5/22/19 9:20 PM, Alistair Strachan wrote:
> > On Tue, May 21, 2019 at 10:10 PM Tapani Pälli <tapani.palli at 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.velikov at gmail.com> wrote:
> > > > > 
> > > > > Hi all,
> > > > > 
> > > > > On Tue, 14 May 2019 at 08:18, Tapani Pälli <tapani.palli at 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.

Just FYI, I've started working on this; I should have a branch usable by
tomorrow, but the idea is to use a python script to turn the drirc xml
into a c source containing the build-time values to be used by xmlconfig.c
if the files can't be read (for whatever reason), and then another
commit making expat optional at build time.

I think this should satisfy everyone's desires?