from:"Nicolai Hähnle"

Re: [PATCH xserver v2 1/4] os: move xf86PrivsElevated here

2018-03-14 Thread Nicolai Hähnle


On 14.03.2018 14:36, Emil Velikov wrote:

On 13 March 2018 at 21:46, Ben Crocker  wrote:


--- a/include/os.h
+++ b/include/os.h
@@ -368,6 +368,9 @@ System(const char *cmdline);
  #define Fclose(a) fclose(a)
  #endif

+extern _X_EXPORT Bool
+PrivsElevated(void);
+

Any particular reason why this is exported?
Is it simply mimicking the surrounding code, or there's a genuine reason for it?


I believe I just mimicked the surrounding code, but it's been a while...

Cheers,
Nicolai



Not an issue either way, the series is
Reviewed-by: Emil Velikov 

Somewhat unrelated:
Seems like commit 49f77fff1495c0a2050fb18f9b1fc627839bbfc2 exported
1000+ symbols as part of (?) folding the extension modules in core.
Yet it never followed to hide the internal functions as things were done.

-Emil



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC xserver v3 00/11] DRI3 v1.2: DMA fences

2017-11-09 Thread Nicolai Hähnle


Will this also support the wait-fence part of PresentPixmap?

Having an explicit wait-fence in PresentPixmap could potentially help 
with OpenGL driver multi-threading. We currently have to sync with the 
driver thread before sending PresentPixmap, to ensure that the necessary 
command submissions have completed in the kernel and an implicit fence 
added to the buffer.


It would be better if we could just create a syncobj in the 
application/API thread, add that as a wait-fence for the present, and 
hand it off to the driver thread to be signaled once previous GL 
commands have completed.


Cheers,
Nicolai


On 06.11.2017 22:42, Louis-Francis Ratté-Boulianne wrote:

Hello,

This patchset implements of what we would like to see become DRI3 v1.2,
that is the implementation of DMA fences.
For some context, please see previous submissions:

https://lists.x.org/archives/xorg-devel/2017-August/054439.html
https://lists.x.org/archives/xorg-devel/2017-September/054770.html

The main change in this iteration is:

  - Make SetTriggered and Reset return a result instead of adding
a new 'type' field for fences.

Here are the repositories:

https://gitlab.collabora.com/lfrb/dri3proto/commits/rfc/2017-10/x11-fences
https://gitlab.collabora.com/lfrb/presentproto/commits/rfc/2017-11/x11-fences
https://gitlab.collabora.com/lfrb/xserver/commits/rfc/2017-11/x11-fences

Thanks,

Louis-Francis

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-28 Thread Nicolai Hähnle


Hi Daniel,

On 28.07.2017 12:46, Daniel Stone wrote:

On 28 July 2017 at 10:24, Nicolai Hähnle <nhaeh...@gmail.com> wrote:

On 28.07.2017 09:44, Daniel Stone wrote:

No, I don't think it is. Tiled layouts still have a stride: if you
look at i915 X/Y/Yf/Y_CCS/Yf_CCS (the latter two containing an
auxiliary compression/fast-clear buffer), iMX/etnaviv
tiled/supertiled, or VC4 T-tiled modifiers and how they're handled
both for DRIImage and KMS interchange, they all specify a stride which
is conceptually the same as linear, if you imagine linear to be 1x1
tiled.

Most tiling users accept any integer units of tiles (buffer width
aligned to tile width), but not always. The NV12MT format used by
Samsung media decoders (also shipped in some Qualcomm SoCs) is
particularly special, IIRC requiring height to be aligned to a
multiple of two tiles.


Fair enough, but I think you need to distinguish between the chosen stride
and stride *requirements*. I do think it makes sense to consider the stride
requirement as part of the format/layout description, but more below.


Right. Stride is a property of one buffer, stride requirements are a
property of the users of that buffer (GPU, display control, media
encode, etc). The requirements also depend on use, e.g. trying to do
rotation through your scanout engine can change those requirements.


Right.



It definitely seems attractive to kill two birds with one stone, but
I'd really much rather not conflate format description/advertisement,
and allocation restriction, into one enum. I'm still on the side of
saying that this is a problem modifiers do not solve, deferring to the
allocator we need anyway in order to determine things like placement
and global optimality (e.g. rotated scanout placing further
restrictions on allocation).


Okay, the original issue here is that the allocator *cannot* determine the
alignment requirement in the use case that prompted this sub-thread.

The use case is PRIME off-loading, where the rendering GPU supports linear
layouts with a 64 byte stride, while the display GPU requires a 256 byte
stride.

The allocator *cannot* solve this issue, because the allocation happens on
the rendering GPU. We need to communicate somehow what the display GPU's
stride requirements are.

How do you propose to do that?


The allocator[0] in itself can't magically reach across processes to
determine desired usage and resolve dependencies. But the entire
design behind it was to be able to solve cross-device usage: between
GPU and scanout, between both of those and media encode/decode, etc.
Obviously it can't do that without help, so winsys will need to gain
protocol in order to express those in terms the allocator will
understand.

The idea was to split information into positive capabilities and
negative constraints. Modifier queries fall into the same boat as
format queries: you're expressing an additional capability ('I can
speak tiled'). Stride alignment, for me, falls into a negative
constraint ('linear allocations must have stride aligned to 256
bytes'). Similarly, placement constraints (VRAM possibly only
accessible to SLI-type paired GPU vs. GTT vs. pure system RAM, etc)
are in the same boat AFAICT. So this helps solve one side of the
equation, but not the other.


I've been thinking about this some more, and I can see now that the 
changed modifier scheme that I originally proposed does not fit well 
into places where modifiers are used to express buffer properties (e.g. 
DRI3PixmapFromBuffers, DRI3BuffersFromPixmap).


But I see no proposal on how to fix the issue so far. You cannot fully 
separate capabilities from constraints. As is, we (AMD) cannot properly 
implement the proposed DRI3 v1.1: what would we return in 
DRI3GetSupportedModifiers?


The natural option is to return (at least) DRM_FORMAT_MOD_LINEAR, but 
that would be a lie, because we *don't* speak arbitrary linear formats.


I don't think this is difficult to fix in terms of protocol, although 
there's plenty of opportunity for bike-shedding :)


I see roughly two options:

1. Make the constraints per-modifier, and add a "constraints: 
ListOfCard32" (or 64) to the response to DRI3GetSupportedModifiers. We 
can then reserve some bits for global constraints (e.g. placement) and 
some bits on a per-modifier basis (e.g. stride alignment for linear). 
You could build constraints like DRM_CONSTRAINT_PLACEMENT_SYSTEM | 
DRM_CONSTRAINT_LINEAR_STRIDE_256B.


2. Make the constraints global, and add a DRI3GetConstraints protocol 
with the same signature as DRI3GetSupportedModifiers. We'd need vendor 
namespaces for the constraint defines, to support constraints that are 
specific to vendor-specific modifiers. You could have entries like 
DRM_CONSTRAINT_PLACEMENT(DRM_CONSTRAINT_PLACEMENT_SYSTEM) and, as a 
separate list entry, DRM_CONSTRAINT_LINEAR_STRIDE(256).


Cheers,
Nicolai
___
xorg-devel@lists.x.org: X.Org development
Archives: http://list

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-28 Thread Nicolai Hähnle


On 28.07.2017 09:44, Daniel Stone wrote:

Hi Nicolai,
Trying to tackle the stride subthread in one go ...

On 25 July 2017 at 09:28, Nicolai Hähnle <nhaeh...@gmail.com> wrote:

On 22.07.2017 14:00, Daniel Stone wrote:

On 21 July 2017 at 18:32, Michel Dänzer <mic...@daenzer.net> wrote:

We just ran into an issue which might mean that there's still something
missing in this v2 proposal:

The context is DRI3 PRIME render offloading of glxgears (not useful in
practice, but bear with me). The display GPU is Raven Ridge, which
requires
that the stride even of linear textures is a multiple of 256 bytes. The
renderer GPU is Polaris12, which still supports smaller alignment of the
stride. With the glxgears window of width 300, the renderer GPU driver
chooses a stride of 304 (* 4 / 256 = 4.75), whereas the display GPU would
require 320 (* 4 / 256 = 5). This cannot work.



The obvious answer is just to increase padding on external/winsys
surfaces? Increasing it for all allocations would probably be a
non-starter, but winsys surfaces are rare enough that you could
probably afford to take the hit, I guess.


I see two basic approaches to solve this:
[...]
Maybe there are other possible approaches I'm missing? Other comments?


I don't have any great solution off the top of my head, but I'd be
inclined to bundle stride in with placement. TTBOMK (from having
looked at radv), buffers shared cross-GPU also need to be allocated
from a separate externally-visible memory heap. And at the moment,
lacking placement information at allocation time (at least for EGL
allocations, via DRIImage), that happens via transparent migration at
import time I think. Placement restrictions would probably also
involve communicating base address alignment requirements.


Stride isn't really in the same category as placement and base address
alignment, though.

Placement and base address alignment requirements can apply to all possible
texture layouts, while the concept of stride is specific to linear layouts.


No, I don't think it is. Tiled layouts still have a stride: if you
look at i915 X/Y/Yf/Y_CCS/Yf_CCS (the latter two containing an
auxiliary compression/fast-clear buffer), iMX/etnaviv
tiled/supertiled, or VC4 T-tiled modifiers and how they're handled
both for DRIImage and KMS interchange, they all specify a stride which
is conceptually the same as linear, if you imagine linear to be 1x1
tiled.

Most tiling users accept any integer units of tiles (buffer width
aligned to tile width), but not always. The NV12MT format used by
Samsung media decoders (also shipped in some Qualcomm SoCs) is
particularly special, IIRC requiring height to be aligned to a
multiple of two tiles.


Fair enough, but I think you need to distinguish between the chosen 
stride and stride *requirements*. I do think it makes sense to consider 
the stride requirement as part of the format/layout description, but 
more below.




Given that, I'm fairly inclined to punt those until we have the grand
glorious allocator, rather than trying to add it to EGL/GBM
separately. The modifiers stuff was a fairly obvious augmentation -
EGL already had no-modifier format import but no query as to which
formats it would accept, and modifiers are a logical extension of
format - but adding the other restrictions is a bigger step forward.


Perhaps a third option would be to encode the required pitch_in_bytes
alignment as part of the modifier?

So DRI3GetSupportedModifiers would return DRM_FORMAT_MOD_LINEAR_256B when
the display GPU is a Raven Ridge.

More generally, we could say that fourcc_mod_code(NONE, k) means that the
pitch_in_bytes has to be a multiple of 2**k for e.g. k <= 31. Or if you
prefer, we could have a stride requirement in elements or pixels instead of
bytes.


I've been thinking this over for the past couple of days, and we could
make it work in clients. But we already have DRM_FORMAT_MOD_LINEAR
with a well-defined meaning, implemented in quite a few places. I
think special-casing linear to encode stride alignment is something
we'd regret in the long run, especially given that some hardware
_does_ have more strict alignment requirements for tiled modes than
just an integer number of tiles allocated.

AFAICT, both AMD and NVIDIA are both going to use a fair bit of the
tiling enum space to encode tile size as well as layout. If allocation
alignment requirements (in both dimensions) needed to be added to
that, it's entirely likely that there wouldn't be enough space and
you'd need to put it somewhere else than the modifier, in which case
we've not even really solved the problem.


At least for AMD, the alignment requirements are de facto part of the 
tiling description, so I don't think it makes a difference.




It definitely seems attractive to kill two birds with one stone, but
I'd really much rather not conflate format description/advertisement,
and allocation restriction, into one enum. I'm still on the side of
saying that this is a problem modifie

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-27 Thread Nicolai Hähnle


On 27.07.2017 03:14, Michel Dänzer wrote:

On 26/07/17 09:15 PM, Nicolai Hähnle wrote:

On 26.07.2017 08:29, Michel Dänzer wrote:

On 25/07/17 05:28 PM, Nicolai Hähnle wrote:

On 22.07.2017 14:00, Daniel Stone wrote:


Given that, I'm fairly inclined to punt those until we have the grand
glorious allocator, rather than trying to add it to EGL/GBM
separately. The modifiers stuff was a fairly obvious augmentation -
EGL already had no-modifier format import but no query as to which
formats it would accept, and modifiers are a logical extension of
format - but adding the other restrictions is a bigger step forward.


Perhaps a third option would be to encode the required pitch_in_bytes
alignment as part of the modifier?

So DRI3GetSupportedModifiers would return DRM_FORMAT_MOD_LINEAR_256B
when the display GPU is a Raven Ridge.

More generally, we could say that fourcc_mod_code(NONE, k) means that
the pitch_in_bytes has to be a multiple of 2**k for e.g. k <= 31. Or if
you prefer, we could have a stride requirement in elements or pixels
instead of bytes.


Interesting idea. FWIW, I suspect we'd need to support specifying the
requirement in both bytes or pixels, since one or the other alone may
not be sufficient to describe the constraints of all hardware.


 From what I've seen, modifiers are always specified together with one
specific format, so the bytes-per-pixel are always known, so I don't
think we need both.


The proposal adds two DRI3 extension requests for querying the list of
supported formats and modifiers, respectively. This suggests that the
supported formats and modifiers can be freely combined.


Which are these? I only saw DRI3GetSupportedModifiers, which takes both 
a window and a format argument.


Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-26 Thread Nicolai Hähnle


On 26.07.2017 19:42, Marek Olšák wrote:

On Wed, Jul 26, 2017 at 7:05 PM, Alex Deucher <alexdeuc...@gmail.com> wrote:

On Wed, Jul 26, 2017 at 8:15 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:

On 26.07.2017 08:29, Michel Dänzer wrote:


On 25/07/17 05:28 PM, Nicolai Hähnle wrote:


On 22.07.2017 14:00, Daniel Stone wrote:



I don't have any great solution off the top of my head, but I'd be
inclined to bundle stride in with placement. TTBOMK (from having
looked at radv), buffers shared cross-GPU also need to be allocated
from a separate externally-visible memory heap. And at the moment,
lacking placement information at allocation time (at least for EGL
allocations, via DRIImage), that happens via transparent migration at
import time I think. Placement restrictions would probably also
involve communicating base address alignment requirements.



Stride isn't really in the same category as placement and base address
alignment, though.

Placement and base address alignment requirements can apply to all
possible texture layouts, while the concept of stride is specific to
linear layouts.



Also, the starting address of shareable buffers is generally aligned to
at least the CPU page size anyway. Do we know of any cases requiring
higher alignment than that, and if so, which address space does the
requirement apply to?



Only tiling modes, as Marek mentioned. We don't do tiling shares across
different GPUs right now.

Maybe we can do it in the future with gfx9 GPUs. But then the alignment
requirements should be known on both sides based on the tiling mode anyway
-- if they even apply for non-VRAM textures.


We should be able to do some 1D tiling modes.  That doesn't have any
per sku alignment dependencies.


Yeah, I think 1D tiling for displayable 32bpp is compatible across all
radeon GPUs newer than R600.

All non-X non-VAR tiling modes on Radeon/GFX9 (Vega, Raven) are the
same on all GFX9 GPUs and might be the same on all future products.
The only catch is that X modes are better optimized for the memory
config, so non-X modes can be slower. I think the non-X modes might
also be compatible with Intel (the first 12 at least), so some
cross-vendor interface might be possible. All GFX9 tiling modes:


Right. It might be worth it to try to use some of these tiling modes to 
make PRIME a bit more efficient in some cases. Non-X modes may be 
non-optimal, but certainly they're better than linear :)


Cheers,
Nicolai




SW_LINEAR (256B pitch alignment)
SW_256B_S
SW_256B_D (compatible with older Radeons if bpp == 32)
SW_256B_R (compatible with older Radeons if bpp == 32)
SW_4KB_Z (Z = depth/stencil sample order)
SW_4KB_S (S = standard)
SW_4KB_D (D = displayable)
SW_4KB_R (R = displayable rotated)
SW_64KB_Z
SW_64KB_S
SW_64KB_D
SW_64KB_R
SW_VAR_Z (VAR = tile size depends on memory config)
SW_VAR_S
SW_VAR_D
SW_VAR_R
SW_64KB_Z_T
SW_64KB_S_T
SW_64KB_D_T
SW_64KB_R_T
SW_4KB_Z_X (X = optimized for memory config)
SW_4KB_S_X
SW_4KB_D_X
SW_4KB_R_X
SW_64KB_Z_X
SW_64KB_S_X
SW_64KB_D_X
SW_64KB_R_X
SW_VAR_Z_X
SW_VAR_S_X
SW_VAR_D_X
SW_VAR_R_X

Marek




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-26 Thread Nicolai Hähnle


On 26.07.2017 08:29, Michel Dänzer wrote:

On 25/07/17 05:28 PM, Nicolai Hähnle wrote:

On 22.07.2017 14:00, Daniel Stone wrote:


I don't have any great solution off the top of my head, but I'd be
inclined to bundle stride in with placement. TTBOMK (from having
looked at radv), buffers shared cross-GPU also need to be allocated
from a separate externally-visible memory heap. And at the moment,
lacking placement information at allocation time (at least for EGL
allocations, via DRIImage), that happens via transparent migration at
import time I think. Placement restrictions would probably also
involve communicating base address alignment requirements.


Stride isn't really in the same category as placement and base address
alignment, though.

Placement and base address alignment requirements can apply to all
possible texture layouts, while the concept of stride is specific to
linear layouts.


Also, the starting address of shareable buffers is generally aligned to
at least the CPU page size anyway. Do we know of any cases requiring
higher alignment than that, and if so, which address space does the
requirement apply to?


Only tiling modes, as Marek mentioned. We don't do tiling shares across 
different GPUs right now.


Maybe we can do it in the future with gfx9 GPUs. But then the alignment 
requirements should be known on both sides based on the tiling mode 
anyway -- if they even apply for non-VRAM textures.




Given that, I'm fairly inclined to punt those until we have the grand
glorious allocator, rather than trying to add it to EGL/GBM
separately. The modifiers stuff was a fairly obvious augmentation -
EGL already had no-modifier format import but no query as to which
formats it would accept, and modifiers are a logical extension of
format - but adding the other restrictions is a bigger step forward.


Perhaps a third option would be to encode the required pitch_in_bytes
alignment as part of the modifier?

So DRI3GetSupportedModifiers would return DRM_FORMAT_MOD_LINEAR_256B
when the display GPU is a Raven Ridge.

More generally, we could say that fourcc_mod_code(NONE, k) means that
the pitch_in_bytes has to be a multiple of 2**k for e.g. k <= 31. Or if
you prefer, we could have a stride requirement in elements or pixels
instead of bytes.


Interesting idea. FWIW, I suspect we'd need to support specifying the
requirement in both bytes or pixels, since one or the other alone may
not be sufficient to describe the constraints of all hardware.


From what I've seen, modifiers are always specified together with one 
specific format, so the bytes-per-pixel are always known, so I don't 
think we need both. Specifying it in bytes is a bit more natural for our 
hardware, that's all.


Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

2017-07-25 Thread Nicolai Hähnle


On 22.07.2017 14:00, Daniel Stone wrote:

On 21 July 2017 at 18:32, Michel Dänzer  wrote:

On 20/07/17 01:08 PM, Daniel Stone wrote:

DRI3 version 1.1 adds support for explicit format modifiers, including
multi-planar buffers.


Adding mesa-dev, Nicolai and Marek.

We just ran into an issue which might mean that there's still something
missing in this v2 proposal:

The context is DRI3 PRIME render offloading of glxgears (not useful in
practice, but bear with me). The display GPU is Raven Ridge, which requires
that the stride even of linear textures is a multiple of 256 bytes. The
renderer GPU is Polaris12, which still supports smaller alignment of the
stride. With the glxgears window of width 300, the renderer GPU driver
chooses a stride of 304 (* 4 / 256 = 4.75), whereas the display GPU would
require 320 (* 4 / 256 = 5). This cannot work.


The obvious answer is just to increase padding on external/winsys
surfaces? Increasing it for all allocations would probably be a
non-starter, but winsys surfaces are rare enough that you could
probably afford to take the hit, I guess.


I see two basic approaches to solve this:

1. A protocol request for the client to retrieve the display
GPU constraints on the stride (and possibly other parameters) for a
given format and modifier.


+ corresponding new EGL request and new GBM/KMS API :\


2. A protocol request which allows the creation of a pixmap with
given format and modifier. The renderer GPU driver needs to pass in
the stride it would choose, then the display GPU driver can choose a
stride satisfying the constraints on both sides.


Heh, that sounds familiar - DRI2!


Maybe there are other possible approaches I'm missing? Other comments?


I don't have any great solution off the top of my head, but I'd be
inclined to bundle stride in with placement. TTBOMK (from having
looked at radv), buffers shared cross-GPU also need to be allocated
from a separate externally-visible memory heap. And at the moment,
lacking placement information at allocation time (at least for EGL
allocations, via DRIImage), that happens via transparent migration at
import time I think. Placement restrictions would probably also
involve communicating base address alignment requirements.


Stride isn't really in the same category as placement and base address 
alignment, though.


Placement and base address alignment requirements can apply to all 
possible texture layouts, while the concept of stride is specific to 
linear layouts.




Given that, I'm fairly inclined to punt those until we have the grand
glorious allocator, rather than trying to add it to EGL/GBM
separately. The modifiers stuff was a fairly obvious augmentation -
EGL already had no-modifier format import but no query as to which
formats it would accept, and modifiers are a logical extension of
format - but adding the other restrictions is a bigger step forward.


Perhaps a third option would be to encode the required pitch_in_bytes 
alignment as part of the modifier?


So DRI3GetSupportedModifiers would return DRM_FORMAT_MOD_LINEAR_256B 
when the display GPU is a Raven Ridge.


More generally, we could say that fourcc_mod_code(NONE, k) means that 
the pitch_in_bytes has to be a multiple of 2**k for e.g. k <= 31. Or if 
you prefer, we could have a stride requirement in elements or pixels 
instead of bytes.


Cheers,
Nicolai






Cheers,
Daniel




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/4] os: move xf86PrivsElevated here

2017-01-27 Thread Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haeh...@amd.com>

Having different types of code all trying to check for elevated privileges
is a bad idea. This implementation is the most thorough one.

Signed-off-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
---
 hw/xfree86/common/xf86Init.c | 59 +
 include/os.h |  3 +++
 os/utils.c   | 63 
 3 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index e61fe66..758b926 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -230,78 +230,21 @@ xf86PrintBanner(void)
 xf86ErrorFVerb(0, "Current version of pixman: %s\n",
pixman_version_string());
 xf86ErrorFVerb(0, "\tBefore reporting problems, check "
"" __VENDORDWEBSUPPORT__ "\n"
"\tto make sure that you have the latest version.\n");
 }
 
 Bool
 xf86PrivsElevated(void)
 {
-static Bool privsTested = FALSE;
-static Bool privsElevated = TRUE;
-
-if (!privsTested) {
-#if defined(WIN32)
-privsElevated = FALSE;
-#else
-if ((getuid() != geteuid()) || (getgid() != getegid())) {
-privsElevated = TRUE;
-}
-else {
-#if defined(HAVE_ISSETUGID)
-privsElevated = issetugid();
-#elif defined(HAVE_GETRESUID)
-uid_t ruid, euid, suid;
-gid_t rgid, egid, sgid;
-
-if ((getresuid(, , ) == 0) &&
-(getresgid(, , ) == 0)) {
-privsElevated = (euid != suid) || (egid != sgid);
-}
-else {
-printf("Failed getresuid or getresgid");
-/* Something went wrong, make defensive assumption */
-privsElevated = TRUE;
-}
-#else
-if (getuid() == 0) {
-/* running as root: uid==euid==0 */
-privsElevated = FALSE;
-}
-else {
-/*
- * If there are saved ID's the process might still be 
privileged
- * even though the above test succeeded. If issetugid() and
- * getresgid() aren't available, test this by trying to set
- * euid to 0.
- */
-unsigned int oldeuid;
-
-oldeuid = geteuid();
-
-if (seteuid(0) != 0) {
-privsElevated = FALSE;
-}
-else {
-if (seteuid(oldeuid) != 0) {
-FatalError("Failed to drop privileges.  Exiting\n");
-}
-privsElevated = TRUE;
-}
-}
-#endif
-}
-#endif
-privsTested = TRUE;
-}
-return privsElevated;
+return PrivsElevated();
 }
 
 static void
 InstallSignalHandlers(void)
 {
 /*
  * Install signal handler for unexpected signals
  */
 xf86Info.caughtSignal = FALSE;
 if (!xf86Info.notrapSignals) {
diff --git a/include/os.h b/include/os.h
index d2c41b4..686f6d6 100644
--- a/include/os.h
+++ b/include/os.h
@@ -355,20 +355,23 @@ Fclose(void *);
 extern const char *
 Win32TempDir(void);
 
 extern int
 System(const char *cmdline);
 
 #define Fopen(a,b) fopen(a,b)
 #define Fclose(a) fclose(a)
 #endif
 
+extern _X_EXPORT Bool
+PrivsElevated(void);
+
 extern _X_EXPORT void
 CheckUserParameters(int argc, char **argv, char **envp);
 extern _X_EXPORT void
 CheckUserAuthorization(void);
 
 extern _X_EXPORT int
 AddHost(ClientPtr /*client */ ,
 int /*family */ ,
 unsigned /*length */ ,
 const void * /*pAddr */ );
diff --git a/os/utils.c b/os/utils.c
index ac55cd7..024989e 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1717,20 +1717,83 @@ System(const char *cmdline)
 
 /* Close process and thread handles. */
 CloseHandle(pi.hProcess);
 CloseHandle(pi.hThread);
 free(cmd);
 
 return dwExitCode;
 }
 #endif
 
+Bool
+PrivsElevated(void)
+{
+static Bool privsTested = FALSE;
+static Bool privsElevated = TRUE;
+
+if (!privsTested) {
+#if defined(WIN32)
+privsElevated = FALSE;
+#else
+if ((getuid() != geteuid()) || (getgid() != getegid())) {
+privsElevated = TRUE;
+}
+else {
+#if defined(HAVE_ISSETUGID)
+privsElevated = issetugid();
+#elif defined(HAVE_GETRESUID)
+uid_t ruid, euid, suid;
+gid_t rgid, egid, sgid;
+
+if ((getresuid(, , ) == 0) &&
+(getresgid(, , ) == 0)) {
+privsElevated = (euid != suid) || (egid != sgid);
+}
+else {
+printf("Failed getresuid or getresgid");
+/* Something went wrong, make defensi

[PATCH xserver 3/4] xfree86: replace all uses of xf86PrivsElevated with PrivsElevated

2017-01-27 Thread Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haeh...@amd.com>

Signed-off-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
---
 hw/xfree86/common/xf86Config.c |  2 +-
 hw/xfree86/common/xf86Init.c   | 12 +++-
 hw/xfree86/common/xf86Priv.h   |  2 --
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index f03acf3..5d2cf0a 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -2309,21 +2309,21 @@ xf86HandleConfigFile(Bool autoconfig)
 Bool singlecard = 0;
 Bool implicit_layout = FALSE;
 XF86ConfLayoutPtr layout;
 
 if (!autoconfig) {
 char *filename, *dirname, *sysdirname;
 const char *filesearch, *dirsearch;
 MessageType filefrom = X_DEFAULT;
 MessageType dirfrom = X_DEFAULT;
 
-if (!xf86PrivsElevated()) {
+if (!PrivsElevated()) {
 filesearch = ALL_CONFIGPATH;
 dirsearch = ALL_CONFIGDIRPATH;
 }
 else {
 filesearch = RESTRICTED_CONFIGPATH;
 dirsearch = RESTRICTED_CONFIGDIRPATH;
 }
 
 if (xf86ConfigFile)
 filefrom = X_CMDLINE;
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 758b926..deed91f 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -227,26 +227,20 @@ xf86PrintBanner(void)
 #if defined(BUILDERSTRING)
 xf86ErrorFVerb(0, "%s \n", BUILDERSTRING);
 #endif
 xf86ErrorFVerb(0, "Current version of pixman: %s\n",
pixman_version_string());
 xf86ErrorFVerb(0, "\tBefore reporting problems, check "
"" __VENDORDWEBSUPPORT__ "\n"
"\tto make sure that you have the latest version.\n");
 }
 
-Bool
-xf86PrivsElevated(void)
-{
-return PrivsElevated();
-}
-
 static void
 InstallSignalHandlers(void)
 {
 /*
  * Install signal handler for unexpected signals
  */
 xf86Info.caughtSignal = FALSE;
 if (!xf86Info.notrapSignals) {
 OsRegisterSigWrapper(xf86SigWrapper);
 }
@@ -886,21 +880,21 @@ OsVendorInit(void)
 /* Set stderr to non-blocking. */
 #ifndef O_NONBLOCK
 #if defined(FNDELAY)
 #define O_NONBLOCK FNDELAY
 #elif defined(O_NDELAY)
 #define O_NONBLOCK O_NDELAY
 #endif
 
 #ifdef O_NONBLOCK
 if (!beenHere) {
-if (xf86PrivsElevated()) {
+if (PrivsElevated()) {
 int status;
 
 status = fcntl(fileno(stderr), F_GETFL, 0);
 if (status != -1) {
 fcntl(fileno(stderr), F_SETFL, status | O_NONBLOCK);
 }
 }
 }
 #endif
 #endif
@@ -1041,21 +1035,21 @@ xf86PrintDefaultModulePath(void)
 
 static void
 xf86PrintDefaultLibraryPath(void)
 {
 ErrorF("%s\n", DEFAULT_LIBRARY_PATH);
 }
 
 static void
 xf86CheckPrivs(const char *option, const char *arg)
 {
-if (xf86PrivsElevated() && !xf86PathIsSafe(arg)) {
+if (PrivsElevated() && !xf86PathIsSafe(arg)) {
 FatalError("\nInvalid argument for %s - \"%s\"\n"
 "\tWith elevated privileges %s must specify a relative 
path\n"
 "\twithout any \"..\" elements.\n\n", option, arg, option);
 }
 }
 
 /*
  * ddxProcessArgument --
  * Process device-dependent command line args. Returns 0 if argument is
  *  not device dependent, otherwise Count of number of elements of argv
@@ -1342,21 +1336,21 @@ ddxProcessArgument(int argc, char **argv, int i)
  * Print out correct use of device dependent commandline options.
  *  Maybe the user now knows what really to do ...
  */
 
 void
 ddxUseMsg(void)
 {
 ErrorF("\n");
 ErrorF("\n");
 ErrorF("Device Dependent Usage\n");
-if (!xf86PrivsElevated()) {
+if (!PrivsElevated()) {
 ErrorF("-modulepath paths  specify the module search path\n");
 ErrorF("-logfile file  specify a log file name\n");
 ErrorF("-configure probe for devices and write an "
__XCONFIGFILE__ "\n");
 ErrorF
 ("-showopts  print available options for all installed 
drivers\n");
 }
 ErrorF
 ("-config file   specify a configuration file, relative to 
the\n");
 ErrorF("   " __XCONFIGFILE__
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index c1f8a18..d78e8f6 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -154,16 +154,14 @@ xf86CloseLog(enum ExitCode error);
 
 /* xf86Init.c */
 extern _X_EXPORT Bool
 xf86LoadModules(const char **list, void **optlist);
 extern _X_EXPORT int
 xf86SetVerbosity(int verb);
 extern _X_EXPORT int
 xf86SetLogVerbosity(int verb)

[PATCH xserver 0/4] os, glx: honor LIBGL_DRIVERS_PATH, and privilege checks

2017-01-27 Thread Nicolai Hähnle

Hi all,

the main point of this series is to take LIBGL_DRIVERS_PATH into account
when loading a DRI driver for libglx.

This is convenient for testing, and Glamor already does this anyway.
Unsurprisingly, dynamic linker mayhem tends to ensue when Glamor and libglx
load different drivers (or rather, different versions of the same driver),
so this is even somewhat of a bug fix.

Obviously, trusting LIBGL_DRIVERS_PATH is a bad idea when the X server is
installed setuid. While looking into how to test for that, I noticed that
different parts of the xserver do this test in different ways. So the first
three patches are about settling on the most thorough test that I found in
the current code.

Please review!

Thanks,
Nicolai
--
 glx/glxdricommon.c | 38 --
 hw/xfree86/common/xf86Config.c |  2 +-
 hw/xfree86/common/xf86Init.c   | 69 ++--
 hw/xfree86/common/xf86Priv.h   |  2 -
 include/os.h   |  3 ++
 os/utils.c | 65 +-
 6 files changed, 105 insertions(+), 74 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 4/4] glx: honor LIBGL_DRIVERS_PATH when loading DRI drivers

2017-01-27 Thread Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haeh...@amd.com>

Allow switching to another driver build without a full installation.

Glamor already takes LIBGL_DRIVERS_PATH into account, so this change
makes sure that the same driver is used in both parts of the server.

Signed-off-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
---
 glx/glxdricommon.c | 38 ++
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
index f6c6fcd..a370845 100644
--- a/glx/glxdricommon.c
+++ b/glx/glxdricommon.c
@@ -241,28 +241,58 @@ static const char dri_driver_path[] = DRI_DRIVER_PATH;
 void *
 glxProbeDriver(const char *driverName,
void **coreExt, const char *coreName, int coreVersion,
void **renderExt, const char *renderName, int renderVersion)
 {
 int i;
 void *driver;
 char filename[PATH_MAX];
 char *get_extensions_name;
 const __DRIextension **extensions = NULL;
+const char *path = NULL;
+
+/* Search in LIBGL_DRIVERS_PATH if we're not setuid. */
+if (!PrivsElevated())
+path = getenv("LIBGL_DRIVERS_PATH");
+
+if (!path)
+path = dri_driver_path;
+
+do {
+const char *next;
+int path_len;
+
+next = strchr(path, ':');
+if (next) {
+path_len = next - path;
+next++;
+} else {
+path_len = strlen(path);
+next = NULL;
+}
 
-snprintf(filename, sizeof filename, "%s/%s_dri.so",
- dri_driver_path, driverName);
+snprintf(filename, sizeof filename, "%.*s/%s_dri.so", path_len, path,
+ driverName);
+
+driver = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
+if (driver != NULL)
+break;
 
-driver = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
-if (driver == NULL) {
 LogMessage(X_ERROR, "AIGLX error: dlopen of %s failed (%s)\n",
filename, dlerror());
+
+path = next;
+} while (path);
+
+if (driver == NULL) {
+LogMessage(X_ERROR, "AIGLX error: unable to load driver %s\n",
+  driverName);
 goto cleanup_failure;
 }
 
 if (asprintf(_extensions_name, "%s_%s",
  __DRI_DRIVER_GET_EXTENSIONS, driverName) != -1) {
 const __DRIextension **(*get_extensions)(void);
 
 get_extensions = dlsym(driver, get_extensions_name);
 if (get_extensions)
 extensions = get_extensions();
-- 
2.7.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/4] os: use PrivsElevated instead of a manual check

2017-01-27 Thread Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haeh...@amd.com>

Signed-off-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
---
 os/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/os/utils.c b/os/utils.c
index 024989e..05733b0 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1861,21 +1861,21 @@ enum BadCode {
 #endif
 
 void
 CheckUserParameters(int argc, char **argv, char **envp)
 {
 enum BadCode bad = NotBad;
 int i = 0, j;
 char *a, *e = NULL;
 
 #if CHECK_EUID
-if (geteuid() == 0 && getuid() != geteuid())
+if (PrivsElevated())
 #endif
 {
 /* Check each argv[] */
 for (i = 1; i < argc; i++) {
 if (strcmp(argv[i], "-fp") == 0) {
 i++;/* continue with next argument. skip the 
length check */
 if (i >= argc)
 break;
 }
 else {
-- 
2.7.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Time to update GSoC/EVoC ideas page

2017-01-20 Thread Nicolai Hähnle


Hi Rob,

On 19.01.2017 23:32, Rob Clark wrote:

Just a friendly reminder that now would be a good time to update the
wiki page for GSoC/EVoC ideas:

  https://www.x.org/wiki/SummerOfCodeIdeas/

There are currently still some stale ideas there (and probably plenty
of missing good ideas).

Also, I've added a "Potential Mentors" section.  Please add yourself
if you are willing to be a mentor (and what sorts of projects you
would be willing to mentor)


I'd be happy to be listed as a possible mentor on the DriConf 
replacement project (nha on IRC/freenode), but I either don't have a 
Wiki account or forgot it a long time ago. Could you put my name down on 
the page?


Thanks,
Nicolai



BR,
-R
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] A few build.sh features

2009-02-17 Thread Nicolai Hähnle

Am Dienstag 17 Februar 2009 15:25:44 schrieb Dan Nicholson:
 On Mon, Feb 16, 2009 at 11:15 AM, Ian Romanick i...@freedesktop.org wrote:
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
 
  Brian Rogers wrote:
  Here are a few patches that do things I found useful.
 
 
  1. Specify -p to run 'git pull' on each component before building it.
 
  NAK.  You really want 'git fetch ; git rebase origin/current branch'.
   git-pull really is the wrong thing to do here!  I have a separate
  script that does this for everything in the tree.  The function that
  does the fetch / rebase is:

 I don't know what version of git this showed up in, but you can just
 run git pull --rebase so it runs rebase instead of merge. So, you
 could probably just use that.

 On the other hand, I suspect that most people using build.sh are
 people that are just trying to keep up with xorg git and don't have
 patches that they're trying to keep on top of master. In that case, a
 simple pull would probably be fine for build.sh. I think anyone
 developing X probably has their own build setup not using build.sh.

Please never, ever use this kind of logic. The people who want to start with 
developing X have just as little clue about the intricacies of the build 
system as the people who are only trying to track xorg git. So they will look 
in the same places for tools to help with their needs.

In a very real way, it is *more* important for those helper scripts to work 
well for people who are actually doing the development, because having good 
development tools helps attracts developers.

And if the tools work well for developers, they very likely will work well for 
people who are only tracking, too.

By the way, I work on Mesa (and occasionally fiddle with the corresponding 
DDX), and I use this build.sh, or rather a very hacked up version of it, 
tailored to my needs. But I started out with a helper script that I just 
downloaded off the internets.

cu,
Nicolai
___
xorg-devel mailing list
xorg-devel@lists.x.org
http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2 1/4] os: move xf86PrivsElevated here

Re: [RFC xserver v3 00/11] DRI3 v1.2: DMA fences

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

Re: [Mesa-dev] [PATCH dri3proto v2] Add modifier/multi-plane requests, bump to v1.1

[PATCH xserver 1/4] os: move xf86PrivsElevated here

[PATCH xserver 3/4] xfree86: replace all uses of xf86PrivsElevated with PrivsElevated

[PATCH xserver 0/4] os, glx: honor LIBGL_DRIVERS_PATH, and privilege checks

[PATCH xserver 4/4] glx: honor LIBGL_DRIVERS_PATH when loading DRI drivers

[PATCH xserver 2/4] os: use PrivsElevated instead of a manual check

Re: Time to update GSoC/EVoC ideas page

Re: [PATCH] A few build.sh features

15 matches

Site Navigation

Mail list logo

Footer information