Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
On Friday, 2019-05-24 07:49:37 +0300, Tapani Pälli wrote: > > > On 5/23/19 8:22 PM, Sumit Semwal wrote: > > Hi Eric, > > > > On Thu, 23 May 2019 at 20:25, Eric Engestrom > > 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 > > > > > 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 > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > On Tue, 14 May 2019 at 08:18, Tapani Pälli > > > > > > > > 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
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
On 5/23/19 8:22 PM, Sumit Semwal wrote: Hi Eric, On Thu, 23 May 2019 at 20:25, Eric Engestrom 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 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 wrote: Hi all, On Tue, 14 May 2019 at 08:18, Tapani Pälli 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
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
Hi Eric, On Thu, 23 May 2019 at 20:25, Eric Engestrom 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 > > > 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 > > > > > wrote: > > > > > > > > > > > > Hi all, > > > > > > > > > > > > On Tue, 14 May 2019 at 08:18, Tapani Pälli > > > > > > 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 > > >
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
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 > > 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 > > > > wrote: > > > > > > > > > > Hi all, > > > > > > > > > > On Tue, 14 May 2019 at 08:18, Tapani Pälli > > > > > 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. > >
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
Hi; On 5/22/19 9:20 PM, Alistair Strachan wrote: On Tue, May 21, 2019 at 10:10 PM Tapani Pälli 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 wrote: Hi all, On Tue, 14 May 2019 at 08:18, Tapani Pälli 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,
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
On Tue, May 21, 2019 at 10:10 PM Tapani Pälli 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 wrote: > >> > >> Hi all, > >> > >> On Tue, 14 May 2019 at 08:18, Tapani Pälli 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
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
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 wrote: Hi all, On Tue, 14 May 2019 at 08:18, Tapani Pälli 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. 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? // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
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 wrote: > > Hi all, > > On Tue, 14 May 2019 at 08:18, Tapani Pälli 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. 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? > -Emil Best, Sumit. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
Hi all, On Tue, 14 May 2019 at 08:18, Tapani Pälli 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. 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 -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
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? Cheers, Nicolai On 10.05.19 08:02, Sumit Semwal wrote: Mesa uses libexpat for many configuration parsing needs; however some userspaces like Android may not want to use libexpat for various reasons - eg some might restrict reading of any config xml files from filesystems. This patchset proposes a simple lookup mechanism for the default values as per current core mesa, keeping the same mesa-internal API as existing xmlconfig.c. Note: This RFC doesn't change mesa drivers that directly use libexpat API - vc4 and intel gen decoder. If these drivers need Android to be enabled for them, I request help from the experts there. For building and testing this on current AOSP/master, I have two hack patches - one provides empty dummy declarations for the XML* API in use in gen_decoder, while the other disables vc4 decoder functionality. These can be found at [1]. These have been built and boot-tested to UI on dragonboard. [1]: https://git.linaro.org/people/sumit.semwal/aosp/external/mesa3d.git/log/?h=expat_wip Sumit Semwal (2): mesa: utils: provide alternate default config mechanism mesa: Android: enable altxmlconfig for O+ src/gallium/targets/dri/Android.mk | 8 +- src/mesa/drivers/dri/Android.mk| 12 +- src/util/Android.mk| 12 +- src/util/Makefile.sources | 2 +- src/util/altxmlconfig.c| 261 + 5 files changed, 275 insertions(+), 20 deletions(-) create mode 100644 src/util/altxmlconfig.c ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism
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. Cheers, Nicolai On 10.05.19 08:02, Sumit Semwal wrote: > Mesa uses libexpat for many configuration parsing needs; however some > userspaces like Android may not want to use libexpat for various reasons - > eg some might restrict reading of any config xml files from filesystems. > > This patchset proposes a simple lookup mechanism for the default values > as per current core mesa, keeping the same mesa-internal API as existing > xmlconfig.c. > > Note: > This RFC doesn't change mesa drivers that directly use libexpat API - vc4 > and intel gen decoder. If these drivers need Android to be enabled for > them, I request help from the experts there. > > For building and testing this on current AOSP/master, I have two hack patches > - one provides empty dummy declarations for the XML* API in use in > gen_decoder, while the other disables vc4 decoder functionality. These can be > found at [1]. > > These have been built and boot-tested to UI on dragonboard. > > [1]: > https://git.linaro.org/people/sumit.semwal/aosp/external/mesa3d.git/log/?h=expat_wip > > Sumit Semwal (2): >mesa: utils: provide alternate default config mechanism >mesa: Android: enable altxmlconfig for O+ > > src/gallium/targets/dri/Android.mk | 8 +- > src/mesa/drivers/dri/Android.mk| 12 +- > src/util/Android.mk| 12 +- > src/util/Makefile.sources | 2 +- > src/util/altxmlconfig.c| 261 + > 5 files changed, 275 insertions(+), 20 deletions(-) > create mode 100644 src/util/altxmlconfig.c > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 0/2] Alternate default config mechanism
Mesa uses libexpat for many configuration parsing needs; however some userspaces like Android may not want to use libexpat for various reasons - eg some might restrict reading of any config xml files from filesystems. This patchset proposes a simple lookup mechanism for the default values as per current core mesa, keeping the same mesa-internal API as existing xmlconfig.c. Note: This RFC doesn't change mesa drivers that directly use libexpat API - vc4 and intel gen decoder. If these drivers need Android to be enabled for them, I request help from the experts there. For building and testing this on current AOSP/master, I have two hack patches - one provides empty dummy declarations for the XML* API in use in gen_decoder, while the other disables vc4 decoder functionality. These can be found at [1]. These have been built and boot-tested to UI on dragonboard. [1]: https://git.linaro.org/people/sumit.semwal/aosp/external/mesa3d.git/log/?h=expat_wip Sumit Semwal (2): mesa: utils: provide alternate default config mechanism mesa: Android: enable altxmlconfig for O+ src/gallium/targets/dri/Android.mk | 8 +- src/mesa/drivers/dri/Android.mk| 12 +- src/util/Android.mk| 12 +- src/util/Makefile.sources | 2 +- src/util/altxmlconfig.c| 261 + 5 files changed, 275 insertions(+), 20 deletions(-) create mode 100644 src/util/altxmlconfig.c -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
