Re: [Mesa-dev] [PATCH v2 2/2] egl/surfaceless: Allow DRMless fallback.
On Thu, Jul 26, 2018 at 7:30 AM Emil Velikov
wrote:
> Hi David,
>
> Hi Emil,
Thanks for the comments.
> On 18 July 2018 at 01:12, David Riley wrote:
> > Allow platform_surfaceless to use swrast even if DRM is not available.
> > To be used to allow a fuzzer for virgl to be run on a jailed VM without
> > hardware GL or DRM support.
> >
> > Signed-off-by: David Riley
> > ---
> > src/egl/drivers/dri2/platform_surfaceless.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> > index f5fe7119c6..f4618bfa11 100644
> > --- a/src/egl/drivers/dri2/platform_surfaceless.c
> > +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> > @@ -293,6 +293,7 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool
> swrast)
> > int fd;
> > int i;
> >
> > + /* Attempt to find DRM device. */
> > for (i = 0; i < limit; ++i) {
> >char *card_path;
> >if (asprintf(_path, DRM_RENDER_DEV_NAME, DRM_DIR_NAME, base
> + i) < 0)
> > @@ -327,6 +328,24 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool
> swrast)
> >dri2_dpy->loader_extensions = NULL;
> > }
> >
> > + /* No DRM device, so attempt to fall back to software path w/o DRM.
> */
> > + if (swrast) {
> > + _eglLog(_EGL_DEBUG, "Falling back to surfaceless swrast without
> DRM.");
> > + dri2_dpy->fd = -1;
> > + dri2_dpy->driver_name = strdup("swrast");
> > + if (!dri2_dpy->driver_name) {
> > + return false;
> > + }
> > +
> > + if (dri2_load_driver_swrast(dpy)) {
> > + dri2_dpy->loader_extensions = swrast_loader_extensions;
> > + return true;
> > + }
> > +
> > + free(dri2_dpy->driver_name);
> > + dri2_dpy->driver_name = NULL;
> > + }
>
> Using swrast gives you a fairly different feature-set than kms_swrast.
> Since you're doing virglrenderer fuzzing, you haven't really noticed it.
>
> Yes, the main goal is to be able to get coverage of the virgl APIs and
thus the required GL APIs.
> Regardless, this patch seems like a hack, alike the ones Tomeu did [1].
> There are some ideas, in the comments, how to address this in a better
> fashion.
>
> If you can give it a try, that'll be appreciated. I don't quite have
> the time to pursue it atm.
>
I understand and agree with your comments about kms_swrast requiring KMS
and not wanting to change those semantics.
I'm not quite sure I follow all the rest of your suggestions (new to this
entire code base). You're suggesting that surfaceless move away from
kms_swrast entirely? Or just for the fallback path like I've got in these
changes?
With regards to your other suggestion I also don't follow. Are you're
suggesting that createNewScreen3 for swrast should dispatch between
dri2_init_screen/drisw_init_screen/dri_kms_init_screen based on some enum
instead of a driver that has been loaded and it's vtable?
I'm hoping to make some more targeted changes than major refactoring of
code that I'm quite unfamiliar with.
Thanks,
David
>
> HTH
> Emil
>
> [1]
>
> https://gitlab.collabora.com/tomeu/mesa/commit/4f804ceecd658492234cbf0fa5fb1b156a8fb79c
>
> https://gitlab.collabora.com/tomeu/mesa/commit/54adda6a4d7b5c783d54dfd37d38d1a5a0f3187f
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.
On Thu, Jul 26, 2018 at 7:22 AM Emil Velikov
wrote:
> Hi David,
>
> On 18 July 2018 at 01:12, David Riley wrote:
>
> Commit message here should explain why this is needed. Is the current
> kms_swrast usage failing/crashing somewhere, etc.
>
The change wasn't needed for kms_swrast, just for swrast which added an
dependency on having the loader extension defined with the change
https://github.com/mesa3d/mesa/commit/63c427fa71a07649d5c033a5c6184ef701348590#diff-aa8c7f5cc2f62d6a098b04df0603c87b
I added one to avoid potential issues if other dependencies on a swrast
loader extension are required in the future and to keep the DRM and DRMless
paths be similar. As far as I can tell it's not strictly needed at this
time and I'm okay dropping it.
>
> > Signed-off-by: David Riley
> > ---
> > src/egl/drivers/dri2/platform_surfaceless.c | 28 +
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> > index a0348a5e95..f5fe7119c6 100644
> > --- a/src/egl/drivers/dri2/platform_surfaceless.c
> > +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> > @@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension
> image_loader_extension = {
> > .flushFrontBuffer = surfaceless_flush_front_buffer,
> > };
> >
> > +static const __DRIswrastLoaderExtension swrast_loader_extension = {
> > + .base= { __DRI_SWRAST_LOADER, 1 },
> > + .getDrawableInfo = NULL,
> > + .putImage= NULL,
> > + .getImage= NULL,
> > +};
> > +
> > #define DRM_RENDER_DEV_NAME "%s/renderD%d"
> >
> > static const __DRIextension *image_loader_extensions[] = {
> > @@ -269,6 +276,14 @@ static const __DRIextension
> *image_loader_extensions[] = {
> > NULL,
> > };
> >
> > +static const __DRIextension *swrast_loader_extensions[] = {
> > + _loader_extension.base,
> > + _loader_extension.base,
> > + _lookup_extension.base,
> > + _invalidate.base,
> How did you compiler this list. Gut suggests that at least one of
> those should not be here.
> Doesn't this break the existing kms_swrast usage?
>
> This was the existing list of extensions used prior to this change with a
newly added one for the swrast loader. I ran a basic set of DEQP tests
(dEQP-GLES3.functional.draw.draw_elements.*, dEQP-GLES3.info.*)
>
> >
> >dri2_dpy->fd = fd;
> > - if (dri2_load_driver_dri3(dpy))
> > + if (dri2_load_driver_dri3(dpy)) {
> > return true;
> > + }
> Unnecessary change.
>
I've fixed this for v3.
>
> HTH
> Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/2] Add DRMless surfaceless fallback path.
Ping for this patch set. On Tue, Jul 17, 2018 at 5:12 PM David Riley wrote: > Allow platform_surfaceless to use swrast even if DRM is not available. > To be used to allow a fuzzer for virgl to be run on a jailed VM without > hardware GL or DRM support. > > v2: Comment style fixes and remove redundant assignment. > > David Riley (2): > egl/surfaceless: Define DRI_SWRastLoader extension when using swrast. > egl/surfaceless: Allow DRMless fallback. > > src/egl/drivers/dri2/platform_surfaceless.c | 47 ++--- > 1 file changed, 42 insertions(+), 5 deletions(-) > > -- > 2.18.0.203.gfac676dfb9-goog > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH] gallivm: Clean up llvm data structures upon destruction.
Without this change, dlopen()/dlclose() of any driver results in memory
being leaked which becomes problematic if things are being reinitialized
repeatedly (eg from a fuzzer).
Even with this change, repeated dlopen()/dlclose() results in a single
LLVM mutex being allocated and never freed (used to synchronize
ManagedStatic). I've spoken to some LLVM folks and haven't come up with
a great answer on avoiding that memory leak and not running into issues
with signal handlers and/or global destructors being called after
llvm_shutdown().
With regards to the RFC, there's potentially some issues here with
multiple drivers being loaded with separate LLVM instances and being
shared due to the dlopen() being done with RTLD_GLOBAL, but this falls
squarely outside my realm of confidence.
---
src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 79dbedbb56..d537ae6029 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -68,6 +68,7 @@
#endif
#include
#include
+#include
#include
#include
@@ -813,3 +814,12 @@ lp_is_function(LLVMValueRef v)
return llvm::isa(llvm::unwrap(v));
#endif
}
+
+/*
+ * Attempt to clean up to allow drivers to be loaded/unloaded without
+ * leaking excessive amounts of memory.
+ */
+__attribute__((destructor)) static void llvm_fini()
+{
+ llvm::llvm_shutdown();
+}
--
2.18.0.203.gfac676dfb9-goog
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.
Signed-off-by: David Riley
---
src/egl/drivers/dri2/platform_surfaceless.c | 28 +
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
b/src/egl/drivers/dri2/platform_surfaceless.c
index a0348a5e95..f5fe7119c6 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension
image_loader_extension = {
.flushFrontBuffer = surfaceless_flush_front_buffer,
};
+static const __DRIswrastLoaderExtension swrast_loader_extension = {
+ .base= { __DRI_SWRAST_LOADER, 1 },
+ .getDrawableInfo = NULL,
+ .putImage= NULL,
+ .getImage= NULL,
+};
+
#define DRM_RENDER_DEV_NAME "%s/renderD%d"
static const __DRIextension *image_loader_extensions[] = {
@@ -269,6 +276,14 @@ static const __DRIextension *image_loader_extensions[] = {
NULL,
};
+static const __DRIextension *swrast_loader_extensions[] = {
+ _loader_extension.base,
+ _loader_extension.base,
+ _lookup_extension.base,
+ _invalidate.base,
+ NULL,
+};
+
static bool
surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
{
@@ -288,23 +303,28 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
if (fd < 0)
continue;
- if (swrast)
+ if (swrast) {
dri2_dpy->driver_name = strdup("kms_swrast");
- else
+ dri2_dpy->loader_extensions = swrast_loader_extensions;
+ } else {
dri2_dpy->driver_name = loader_get_driver_for_fd(fd);
+ dri2_dpy->loader_extensions = image_loader_extensions;
+ }
if (!dri2_dpy->driver_name) {
close(fd);
continue;
}
dri2_dpy->fd = fd;
- if (dri2_load_driver_dri3(dpy))
+ if (dri2_load_driver_dri3(dpy)) {
return true;
+ }
close(fd);
dri2_dpy->fd = -1;
free(dri2_dpy->driver_name);
dri2_dpy->driver_name = NULL;
+ dri2_dpy->loader_extensions = NULL;
}
return false;
@@ -338,8 +358,6 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay
*disp)
goto cleanup;
}
- dri2_dpy->loader_extensions = image_loader_extensions;
-
if (!dri2_create_screen(disp)) {
err = "DRI2: failed to create screen";
goto cleanup;
--
2.18.0.203.gfac676dfb9-goog
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/2] egl/surfaceless: Allow DRMless fallback.
Allow platform_surfaceless to use swrast even if DRM is not available.
To be used to allow a fuzzer for virgl to be run on a jailed VM without
hardware GL or DRM support.
Signed-off-by: David Riley
---
src/egl/drivers/dri2/platform_surfaceless.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
b/src/egl/drivers/dri2/platform_surfaceless.c
index f5fe7119c6..f4618bfa11 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -293,6 +293,7 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
int fd;
int i;
+ /* Attempt to find DRM device. */
for (i = 0; i < limit; ++i) {
char *card_path;
if (asprintf(_path, DRM_RENDER_DEV_NAME, DRM_DIR_NAME, base + i) <
0)
@@ -327,6 +328,24 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
dri2_dpy->loader_extensions = NULL;
}
+ /* No DRM device, so attempt to fall back to software path w/o DRM. */
+ if (swrast) {
+ _eglLog(_EGL_DEBUG, "Falling back to surfaceless swrast without DRM.");
+ dri2_dpy->fd = -1;
+ dri2_dpy->driver_name = strdup("swrast");
+ if (!dri2_dpy->driver_name) {
+ return false;
+ }
+
+ if (dri2_load_driver_swrast(dpy)) {
+ dri2_dpy->loader_extensions = swrast_loader_extensions;
+ return true;
+ }
+
+ free(dri2_dpy->driver_name);
+ dri2_dpy->driver_name = NULL;
+ }
+
return false;
}
--
2.18.0.203.gfac676dfb9-goog
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 0/2] Add DRMless surfaceless fallback path.
Allow platform_surfaceless to use swrast even if DRM is not available. To be used to allow a fuzzer for virgl to be run on a jailed VM without hardware GL or DRM support. v2: Comment style fixes and remove redundant assignment. David Riley (2): egl/surfaceless: Define DRI_SWRastLoader extension when using swrast. egl/surfaceless: Allow DRMless fallback. src/egl/drivers/dri2/platform_surfaceless.c | 47 ++--- 1 file changed, 42 insertions(+), 5 deletions(-) -- 2.18.0.203.gfac676dfb9-goog ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.
Signed-off-by: David Riley
---
src/egl/drivers/dri2/platform_surfaceless.c | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
b/src/egl/drivers/dri2/platform_surfaceless.c
index a0348a5..f5fe711 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension
image_loader_extension = {
.flushFrontBuffer = surfaceless_flush_front_buffer,
};
+static const __DRIswrastLoaderExtension swrast_loader_extension = {
+ .base= { __DRI_SWRAST_LOADER, 1 },
+ .getDrawableInfo = NULL,
+ .putImage= NULL,
+ .getImage= NULL,
+};
+
#define DRM_RENDER_DEV_NAME "%s/renderD%d"
static const __DRIextension *image_loader_extensions[] = {
@@ -269,6 +276,14 @@ static const __DRIextension *image_loader_extensions[] = {
NULL,
};
+static const __DRIextension *swrast_loader_extensions[] = {
+ _loader_extension.base,
+ _loader_extension.base,
+ _lookup_extension.base,
+ _invalidate.base,
+ NULL,
+};
+
static bool
surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
{
@@ -288,23 +303,28 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
if (fd < 0)
continue;
- if (swrast)
+ if (swrast) {
dri2_dpy->driver_name = strdup("kms_swrast");
- else
+ dri2_dpy->loader_extensions = swrast_loader_extensions;
+ } else {
dri2_dpy->driver_name = loader_get_driver_for_fd(fd);
+ dri2_dpy->loader_extensions = image_loader_extensions;
+ }
if (!dri2_dpy->driver_name) {
close(fd);
continue;
}
dri2_dpy->fd = fd;
- if (dri2_load_driver_dri3(dpy))
+ if (dri2_load_driver_dri3(dpy)) {
return true;
+ }
close(fd);
dri2_dpy->fd = -1;
free(dri2_dpy->driver_name);
dri2_dpy->driver_name = NULL;
+ dri2_dpy->loader_extensions = NULL;
}
return false;
@@ -338,8 +358,6 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay
*disp)
goto cleanup;
}
- dri2_dpy->loader_extensions = image_loader_extensions;
-
if (!dri2_create_screen(disp)) {
err = "DRI2: failed to create screen";
goto cleanup;
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] egl/surfaceless: Allow DRMless fallback.
Allow platform_surfaceless to use swrast even if DRM is not available.
To be used to allow a fuzzer for virgl to be run on a jailed VM without
hardware GL or DRM support.
Signed-off-by: David Riley
---
src/egl/drivers/dri2/platform_surfaceless.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
b/src/egl/drivers/dri2/platform_surfaceless.c
index f5fe711..3b17e95 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -293,6 +293,7 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
int fd;
int i;
+ // Attempt to find DRM device.
for (i = 0; i < limit; ++i) {
char *card_path;
if (asprintf(_path, DRM_RENDER_DEV_NAME, DRM_DIR_NAME, base + i) <
0)
@@ -327,6 +328,25 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
dri2_dpy->loader_extensions = NULL;
}
+ // No DRM device, so attempt to fall back to software path w/o DRM.
+ if (swrast) {
+ _eglLog(_EGL_DEBUG, "Falling back to surfaceless swrast without DRM.");
+ dri2_dpy->fd = -1;
+ dri2_dpy->driver_name = strdup("swrast");
+ if (!dri2_dpy->driver_name) {
+ return false;
+ }
+
+ if (dri2_load_driver_swrast(dpy)) {
+ dri2_dpy->loader_extensions = swrast_loader_extensions;
+ return true;
+ }
+
+ dri2_dpy->fd = -1;
+ free(dri2_dpy->driver_name);
+ dri2_dpy->driver_name = NULL;
+ }
+
return false;
}
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
