[igt-dev] [PATCH i-g-t v4 1/1] tools/generate_cb_buffer: Add script to assemble CB kernel

[Abodunrin, Akeem G]

> On Mon, Feb 24, 2020 at 11:48:08AM +0200, Petri Latvala wrote:
> > On Fri, Feb 21, 2020 at 12:22:43AM -0800, Akeem G Abodunrin wrote:
> > > This patch adds script and applicable assembly sources, so that we
> > > can use igt to assemble Clear Batch Buffer kernel for gen7 and
> > > gen7.5 devices - Resultant header files would be imported to i915, and
> used as they are...
> > >
> > > With this patch, user need to have mesa configured on their
> > > platform, before igt could be used to achieve the purpose of
> > > assembling the kernel from source.
> > >
> > > This is needed for "Security mitigation for Intel Gen7/7.5 HWs"
> > > Intel ID: PSIRT-TA-201910-001/CVEID: CVE-2019-14615
> > >
> > > v2: Addressed formatting, -g option and other minor issues (Petri)
> > > V3: Update script due to suggested changes in i915, and Mesa tool
> > > v4: Update help comment with Mesa build option with meson (Petri)
> > >
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > Cc: Petri Latvala <petri.latvala at intel.com>
> > > Cc: Bloomfield Jon <jon.bloomfield at intel.com>
> > > Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin at intel.com>
> > > ---
> > >  scripts/asm_eu_kernel.sh                | 153 ++++++++++++++++++++++++
> > >  tools/assembly_source/hsw_clear_buf.asm | 141
> > > ++++++++++++++++++++++  tools/assembly_source/ivb_clear_buf.asm |
> > > 139 +++++++++++++++++++++
> > >  3 files changed, 433 insertions(+)
> > >  create mode 100755 scripts/asm_eu_kernel.sh  create mode 100644
> > > tools/assembly_source/hsw_clear_buf.asm
> > >  create mode 100644 tools/assembly_source/ivb_clear_buf.asm
> > >
> > > diff --git a/scripts/asm_eu_kernel.sh b/scripts/asm_eu_kernel.sh new
> > > file mode 100755 index 00000000..1958829c
> > > --- /dev/null
> > > +++ b/scripts/asm_eu_kernel.sh
> > > @@ -0,0 +1,153 @@
> > > +#!/bin/bash
> > > +#
> > > +# SPDX-License-Identifier: MIT
> > > +#
> > > +# Copyright © 2020 Intel Corporation # # Permission is hereby
> > > +granted, free of charge, to any person obtaining a # copy of this
> > > +software and associated documentation files (the "Software"), # to
> > > +deal in the Software without restriction, including without
> > > +limitation # the rights to use, copy, modify, merge, publish,
> > > +distribute, sublicense, # and/or sell copies of the Software, and
> > > +to permit persons to whom the # Software is furnished to do so, subject
> to the following conditions:
> > > +#
> > > +# The above copyright notice and this permission notice (including
> > > +the next # paragraph) shall be included in all copies or
> > > +substantial portions of the # Software.
> > > +#
> > > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> > > +EXPRESS OR # IMPLIED, INCLUDING BUT NOT LIMITED TO THE
> WARRANTIES
> > > +OF MERCHANTABILITY, # FITNESS FOR A PARTICULAR PURPOSE AND
> > > +NONINFRINGEMENT.  IN NO EVENT SHALL # THE AUTHORS OR
> COPYRIGHT
> > > +HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER # LIABILITY,
> > > +WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> #
> > > +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> OR OTHER DEALINGS # IN THE SOFTWARE.
> > > +
> > > +export ASSEMBLY_SOURCE=./tools/assembly_source
> > > +
> > > +function get_help {
> > > +        echo "Usage:    asm_eu_kernel.sh [options]"
> > > +        echo "Note: hsw_clear_kernel.c/ivb_clear_kernel.c automatically
> generated by this script should never be modified - it would be imported to
> i915, to use as it is..."
> > > +        echo " "
> > > +        echo "Please make sure your Mesa tool is compiled with "-
> Dtools=intel" and "-Ddri-drivers=i965", and run this script from IGT source
> root directory"
> > > +        echo " "
> > > +        echo "Options are:"
> > > +        echo " -h                       display this help message, and exit"
> > > +        echo " -g=platform              generation of device: use "hsw" for
> gen7.5, and "ivb" for gen7 devices"
> > > +        echo " -o=name_of_file          output file to store Mesa assembled c-
> literal for the device - If none specified, default file will be used - ivb/hsw-
> cb_assembled"
> > > +        echo " -m=mesa                  Mesa source directory"
As pointed out here, user only specify Mesa source directory...

> > > +	esac
> > > +done
> > > +shift $(($OPTIND-1))
> > > +
> > > +if [ "x$1" != "x" ]; then
> > > +	echo -e "Unknown option: $1\n"
> > > +	get_help
> > > +	exit 1
> > > +fi
> > > +
> > > +I965_ASM="$mesa_dir/build/debug/src/intel/tools/i965_asm"
> >
> > Should this be $mesa_dir/build/src/intel/tools/i965_asm instead?
 
> Both assume the user specied the build directly exactly like so when building
> Mesa. We should probably ask the user to point at either the binary itself or
> the build directory.

Yes, we could do that - but that would be a long "complex" command line option for the script 😊
May be we can check for various options where binary could be in the script - I don't know how many other build options Mesa has, but we know that this is common " src/intel/tools/i965_asm", what precedes that is the optional part now. 

That being said, we can always update this script, especially if anything changes in Mesa - but since this address security issue, I honestly don't know how long we can afford to delay this, and other related patches.

Thanks,
~Akeem