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

[Tapani Pälli]

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.

// Tapani