Hi Bin,

On 27 January 2015 at 01:59, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Tue, Jan 27, 2015 at 9:22 AM, Simon Glass <s...@chromium.org> wrote:
>> Sometimes microcode is delivered as a header file. Allow the tool to
>> support this as well as collecting multiple microcode blocks into a
>> single update.
>>
>> Signed-off-by: Simon Glass <s...@chromium.org>
>> ---
>
> Tested-by: Bin Meng <bmeng...@gmail.com>
>
> But please see my comments below.
>
>>  tools/microcode-tool.py | 90 
>> ++++++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 70 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/microcode-tool.py b/tools/microcode-tool.py
>> index 003716d..71c2e91 100755
>> --- a/tools/microcode-tool.py
>> +++ b/tools/microcode-tool.py
>> @@ -76,6 +76,35 @@ def ParseFile(fname):
>>          microcodes[name] = Microcode(name, data)
>>      return date, license_text, microcodes
>>
>> +def ParseHeaderFiles(fname_list):
>> +    """Parse a list of header files and return the component parts
>> +
>> +    Args:
>> +        fname_list: List of files to parse
>> +    Returns:
>> +            date:         String containing date from the file's header
>> +            license_text: List of text lines for the license file
>> +            microcodes:   List of Microcode objects from the file
>> +    """
>> +    microcodes = {}
>> +    license_text = []
>> +    date = ''
>> +    name = None
>> +    for fname in fname_list:
>> +        name = os.path.basename(fname).lower()
>> +        name = os.path.splitext(name)[0]
>> +        data = []
>> +        with open(fname) as fd:
>> +            for line in fd:
>> +                line = line.rstrip()
>> +
>> +                # Omit anything after the last comma
>> +                words = line.split(',')[:-1]
>> +                data += [word + ',' for word in words]
>> +        microcodes[name] = Microcode(name, data)
>> +    return date, license_text, microcodes
>> +
>> +
>>  def List(date, microcodes, model):
>>      """List the available microcode chunks
>>
>> @@ -129,13 +158,13 @@ def FindMicrocode(microcodes, model):
>>              break
>>      return found, tried
>>
>> -def CreateFile(date, license_text, mcode, outfile):
>> +def CreateFile(date, license_text, mcodes, outfile):
>>      """Create a microcode file in U-Boot's .dtsi format
>>
>>      Args:
>>          date:       String containing date of original microcode file
>>          license:    List of text lines for the license file
>> -        mcode:      Microcode object to write
>> +        mcodes:      Microcode objects to write (normally only 1)
>>          outfile:    Filename to write to ('-' for stdout)
>>      """
>>      out = '''/*%s
>> @@ -159,15 +188,22 @@ intel,processor-flags = <%#x>;
>>  data = <%s
>>  \t>;'''
>>      words = ''
>> -    for i in range(len(mcode.words)):
>> -        if not (i & 3):
>> -            words += '\n'
>> -        val = mcode.words[i]
>> -        # Change each word so it will be little-endian in the FDT
>> -        # This data is needed before RAM is available on some platforms so 
>> we
>> -        # cannot do an endianness swap on boot.
>> -        val = struct.unpack("<I", struct.pack(">I", val))[0]
>> -        words += '\t%#010x' % val
>> +    add_comments = len(mcodes) > 1
>> +    for mcode in mcodes:
>> +        if add_comments:
>> +            words += '\n/* %s */' % mcode.name
>> +        for i in range(len(mcode.words)):
>> +            if not (i & 3):
>> +                words += '\n'
>> +            val = mcode.words[i]
>> +            # Change each word so it will be little-endian in the FDT
>> +            # This data is needed before RAM is available on some platforms 
>> so
>> +            # we cannot do an endianness swap on boot.
>> +            val = struct.unpack("<I", struct.pack(">I", val))[0]
>> +            words += '\t%#010x' % val
>> +
>> +    # Use the first microcode for the headers
>> +    mcode = mcodes[0]
>>
>>      # Take care to avoid adding a space before a tab
>>      text = ''
>> @@ -187,8 +223,8 @@ data = <%s
>>                  print >> sys.stderr, "Creating directory '%s'" % 
>> MICROCODE_DIR
>>                  os.makedirs(MICROCODE_DIR)
>>              outfile = os.path.join(MICROCODE_DIR, mcode.name + '.dtsi')
>> -            print >> sys.stderr, "Writing microcode for '%s' to '%s'" % (
>> -                     mcode.name, outfile)
>> +        print >> sys.stderr, "Writing microcode for '%s' to '%s'" % (
>> +                ', '.join([mcode.name for mcode in mcodes]), outfile)
>>          with open(outfile, 'w') as fd:
>>              print >> fd, out % tuple(args)
>>
>> @@ -198,8 +234,12 @@ def MicrocodeTool():
>>      parser = OptionParser()
>>      parser.add_option('-d', '--mcfile', type='string', action='store',
>>                      help='Name of microcode.dat file')
>> +    parser.add_option('-H', '--headerfile', type='string', action='append',
>> +                    help='Name of .h file containing microcode')
>>      parser.add_option('-m', '--model', type='string', action='store',
>> -                    help='Model name to extract')
>> +                    help="Model name to extract ('all' for all)")
>> +    parser.add_option('-M', '--multiple', type='string', action='store',
>> +                    help="Allow output of multiple models")
>>      parser.add_option('-o', '--outfile', type='string', action='store',
>>                      help='Filename to use for output (- for stdout), 
>> default is'
>>                      ' %s/<name>.dtsi' % MICROCODE_DIR)
>> @@ -224,9 +264,14 @@ def MicrocodeTool():
>>      if cmd not in commands:
>>          parser.error("Unknown command '%s'" % cmd)
>>
>> -    if not options.mcfile:
>> -        parser.error('You must specify a microcode file')
>> -    date, license_text, microcodes = ParseFile(options.mcfile)
>> +    if (not not options.mcfile) != (not not options.mcfile):
>> +        parser.error("You must specify either header files or a microcode 
>> file, not both")
>> +    if options.headerfile:
>> +        date, license_text, microcodes = 
>> ParseHeaderFiles(options.headerfile)
>> +    elif options.mcfile:
>> +        date, license_text, microcodes = ParseFile(options.mcfile)
>> +    else:
>> +        parser.error('You must specify a microcode file (or header files)')
>>
>>      if cmd == 'list':
>>          List(date, microcodes, options.model)
>> @@ -236,16 +281,21 @@ def MicrocodeTool():
>>          if not options.model:
>>              parser.error('You must specify a model to create')
>>          model = options.model.lower()
>> -        mcode_list, tried = FindMicrocode(microcodes, model)
>> +        if options.model == 'all':
>> +            options.multiple = True
>> +            mcode_list = microcodes.values()
>> +            tried = []
>> +        else:
>> +            mcode_list, tried = FindMicrocode(microcodes, model)
>>          if not mcode_list:
>>              parser.error("Unknown model '%s' (%s) - try 'list' to list" %
>>                          (model, ', '.join(tried)))
>> -        if len(mcode_list) > 1:
>> +        if not options.multiple and len(mcode_list) > 1:
>>              parser.error("Ambiguous model '%s' (%s) matched %s - try 'list' 
>> "
>>                          "to list or specify a particular file" %
>>                          (model, ', '.join(tried),
>>                          ', '.join([m.name for m in mcode_list])))
>> -        CreateFile(date, license_text, mcode_list[0], options.outfile)
>> +        CreateFile(date, license_text, mcode_list, options.outfile)
>>      else:
>>          parser.error("Unknown command '%s'" % cmd)
>>
>> --
>
> My testing results show that, compared to '-d microcode.dat' approach,
> generated microcode.dtsi using '-H' does not have license header and
> date set. Is this intentional?

Not really, but the header I tested with has no license info.

I suspect we could enable use of -d also to pick this up. But we still
wouldn't get the date. I'm not sure what automation we should have for
this case.

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to