[virglrenderer-devel] [RFC PATCH] vrend and build: Obtain version information and add it to the caps v2
Gert Wollny
gert.wollny at collabora.com
Tue Jun 5 06:18:02 UTC 2018
Am Montag, den 04.06.2018, 10:57 -0700 schrieb Gurchetan Singh:
> The caps structure is supposed to all 32-bit integers, otherwise we
> might run into compiler padding/alignment issues (specifically, with
> "char host_virglrenderer_version[30]").
I see, but padding and alignment issues can probably be alliviated by
using the right packing pragma on the host and guest side and by
changing the strings to a size that is a multiple of four.
> IMO this is not necessary. Most virglrenderer users know (or should
> know) how to get information
> about the host environment.
The reason we were talking about to add this here was to make it
possible to extract this information automatically for recording it
when the test suites are run.
Currently, there is no clean way to extract the version information
about virgilrenderer (it's a git hash that can only be extracted from
the the source directory), and hence, to record this information any
test environment has to be specifically tailored to the build
environment.
Also people usually have more then one mesa installed (e.g. the one
provided by the distribution that is pulled in automatically and the
one they are working with to develop), so by recording the information
from inside the guest we can make sure that we refer to the right
version. In summary, I think that having this version information
available inside the guest would be a good thing for debugging
purposes, at least for now, when we have to change both, host and guest
libraries.
Anyway, I send this as an RFC because I had my doubts about whether the
caps structure would be the right place to add this, and if my
arguments for adding the possibility to pass this information to the
guest are in any way convincing, I'm also open for suggestions on how
to add this in another way,
best,
Gert
> On Mon, Jun 4, 2018 at 6:44 AM Gert Wollny <gert.wollny at collabora.com
> > wrote:
> >
> > In order to make it easier to reproduce and analyse failures in the
> > guest
> > system add information about the host system to the caps v2 info.
> > Specifically, add the host OpenGL Version string and a version
> > string of
> > virglrenderer to the caps that can then be read by the guest.
> >
> > This patch requires another patch against mesa/virgl to actually
> > read this
> > information. Then the information ahout the host system will be
> > presented as
> > part of the renderer string, e.g.
> >
> > "OpenGL renderer string: virgl (Host: 3.3 (Core Profile) \
> > Mesa 18.2.0-devel (git-bccce60b8a), vrend: 0.6.0 (git-
> > 8a78fc59))"
> >
> > The python script git_sha1_gen.py has been adapted from Mesa. I
> > send this as RFC
> > because I'm not sure whether the caps is really the right place to
> > add this
> > information or whether it would be better to add another command
> > that would be
> > used to pass this information (and possible other things) around. I
> > also didn't
> > send the mesa/virgl patch yet, because there the caps.v2 structure
> > is currently
> > out of sync with virglrenderer (missing tgsi_invariant and
> > shader_buffer_offset_alignment.
> >
> > Thanks for any comments,
> > Gert
> >
> > ---
> > Makefile.am | 2 +-
> > bin/git_sha1_gen.py | 46
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > src/Makefile.am | 16 +++++++++++++++-
> > src/git_sha1.c.in | 8 ++++++++
> > src/git_sha1.h | 1 +
> > src/virgl_hw.h | 2 ++
> > src/vrend_renderer.c | 8 ++++++++
> > 7 files changed, 81 insertions(+), 2 deletions(-)
> > create mode 100644 bin/git_sha1_gen.py
> > create mode 100644 src/git_sha1.c.in
> > create mode 100644 src/git_sha1.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index b0121f2..7c4d999 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -29,7 +29,7 @@ pkgconfigdir = $(libdir)/pkgconfig
> > pkgconfig_DATA = virglrenderer.pc
> >
> > EXTRA_DIST = \
> > - virglrenderer.pc.in
> > + virglrenderer.pc.in bin
> >
> > @CODE_COVERAGE_RULES@
> >
> > diff --git a/bin/git_sha1_gen.py b/bin/git_sha1_gen.py
> > new file mode 100644
> > index 0000000..0c87be5
> > --- /dev/null
> > +++ b/bin/git_sha1_gen.py
> > @@ -0,0 +1,46 @@
> > +#!/usr/bin/env python
> > +
> > +"""
> > +Generate the contents of the git_sha1.h file.
> > +The output of this script goes to stdout.
> > +"""
> > +
> > +
> > +import argparse
> > +import os
> > +import os.path
> > +import subprocess
> > +import sys
> > +
> > +
> > +def get_git_sha1():
> > + """Try to get the git SHA1 with git rev-parse."""
> > + git_dir = os.path.join(os.path.dirname(sys.argv[0]), '..',
> > '.git')
> > + try:
> > + git_sha1 = "(git-"+ subprocess.check_output([
> > + 'git',
> > + '--git-dir=' + git_dir,
> > + 'rev-parse',
> > + 'HEAD',
> > + ], stderr=open(os.devnull, 'w')).decode("ascii")[:8] + ")"
> > + except:
> > + # don't print anything if it fails
> > + git_sha1 = ''
> > + return git_sha1
> > +
> > +parser = argparse.ArgumentParser()
> > +parser.add_argument('--output', help='File to write the #define
> > in',
> > + required=True)
> > +args = parser.parse_args()
> > +
> > +git_sha1 = os.environ.get('VIRGL_GIT_SHA1_OVERRIDE',
> > get_git_sha1())
> > +git_sha1_c_in_path = os.path.join(os.path.dirname(sys.argv[0]),
> > + '..', 'src', 'git_sha1.c.in')
> > +with open(git_sha1_c_in_path , 'r') as git_sha1_c_in:
> > + new_sha1 = git_sha1_c_in.read().replace('@VCS_TAG@', git_sha1)
> > + if os.path.isfile(args.output):
> > + with open(args.output, 'r') as git_sha1_c:
> > + if git_sha1_c.read() == new_sha1:
> > + quit()
> > + with open(args.output, 'w') as git_sha1_c:
> > + git_sha1_c.write(new_sha1)
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 76f4162..f120fe8 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -1,4 +1,8 @@
> > SUBDIRS := gallium/auxiliary
> > +
> > +BUILT_SOURCES = git_sha1.c
> > +CLEANFILES = $(BUILT_SOURCES)
> > +
> > AM_LDFLAGS = -lm \
> > $(GBM_LIBS) \
> > $(EPOXY_LIBS) \
> > @@ -21,6 +25,7 @@ libvrend_la_SOURCES = \
> > virgl_hw.h \
> > virgl_protocol.h \
> > vrend_iov.h \
> > + git_sha1.c \
> > vrend_renderer.c \
> > vrend_renderer.h \
> > vrend_shader.c \
> > @@ -45,6 +50,7 @@ libvrend_la_SOURCES += \
> > virgl_glx_context.c
> > endif
> >
> > +
> > lib_LTLIBRARIES = libvirglrenderer.la
> > noinst_LTLIBRARIES = libvrend.la
> >
> > @@ -58,6 +64,14 @@ libvirglrenderer_la_LDFLAGS = $(GM_LDFLAGS)
> > $(EPOXY_LDFLAGS) $(X11_LDFLAGS)
> > libvirglrendererincludedir = ${includedir}/virgl
> > libvirglrendererinclude_HEADERS = virglrenderer.h
> >
> > -EXTRA_DIST = gallium/include
> > +.PHONY: git_version
> > +
> > +git_sha1.c: git_version
> > +
> > +git_version:
> > + @echo "updating $@"
> > + $(PYTHON) $(top_srcdir)/bin/git_sha1_gen.py --output
> > git_sha1.c
> > +
> > +EXTRA_DIST = gallium/include git_sha1.c.in git_sha1.h
> >
> > -include $(top_srcdir)/git.mk
> > diff --git a/src/git_sha1.c.in b/src/git_sha1.c.in
> > new file mode 100644
> > index 0000000..ebf3a5c
> > --- /dev/null
> > +++ b/src/git_sha1.c.in
> > @@ -0,0 +1,8 @@
> > +
> > +#include "git_sha1.h"
> > +
> > +const char *get_git_sha1(void)
> > +{
> > + return "@VCS_TAG@";
> > +}
> > +
> > diff --git a/src/git_sha1.h b/src/git_sha1.h
> > new file mode 100644
> > index 0000000..74ef5c8
> > --- /dev/null
> > +++ b/src/git_sha1.h
> > @@ -0,0 +1 @@
> > +extern const char *get_git_sha1(void);
> > diff --git a/src/virgl_hw.h b/src/virgl_hw.h
> > index a430fae..de0391e 100644
> > --- a/src/virgl_hw.h
> > +++ b/src/virgl_hw.h
> > @@ -298,6 +298,8 @@ struct virgl_caps_v2 {
> > uint32_t uniform_buffer_offset_alignment;
> > uint32_t tgsi_invariant;
> > uint32_t shader_buffer_offset_alignment;
> > + char host_gl_version_string[60]; /* OpenGL (ES) profile
> > version string */
> > + char host_virglrenderer_version[30]; /* our version */
> > };
> >
> > union virgl_caps {
> > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > index 4398830..ac4a088 100644
> > --- a/src/vrend_renderer.c
> > +++ b/src/vrend_renderer.c
> > @@ -49,6 +49,7 @@
> > #include "vrend_shader.h"
> >
> > #include "vrend_renderer.h"
> > +#include "git_sha1.h"
> >
> > #include "virgl_hw.h"
> >
> > @@ -6683,10 +6684,17 @@ static bool
> > vrend_renderer_fill_caps_common(uint32_t set, uint32_t version,
> > } else if (set == 2) {
> > memset(caps, 0, sizeof(*caps));
> > caps->max_version = 2;
> > +
> > + snprintf(caps->v2.host_gl_version_string, 59, "%s",
> > glGetString(GL_VERSION));
> > + snprintf(caps->v2.host_virglrenderer_version, 29, "%s %s",
> > + PACKAGE_VERSION, get_git_sha1());
> > + debug_printf("Host: %s, vrend: %s\n", caps-
> > >v2.host_gl_version_string,
> > + caps->v2.host_virglrenderer_version);
> > }
> >
> > gl_ver = epoxy_gl_version();
> >
> > +
> > /*
> > * We can't fully support this feature on GLES,
> > * but it is needed for OpenGL 2.1 so lie.
> > --
> > 2.16.4
> >
> > _______________________________________________
> > virglrenderer-devel mailing list
> > virglrenderer-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
More information about the virglrenderer-devel
mailing list