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

Thu May 23 17:22:46 UTC 2019

Hi Eric,

On Thu, 23 May 2019 at 20:25, Eric Engestrom <eric.engestrom at intel.com> wrote:
>
> 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?
So, I am assuming you mean the altxmlconfig.c that's part of this
patchset, rather than the existing xmlconfig.c which depends on expat.
If yes, then I feel this could be a good solution.

Best,
Sumit.