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.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.
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? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
