Re: [PATCH] docs, amd_ucode: Add AMD container file format notes

[Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>]

On 7/17/2014 12:06 PM, Boris Ostrovsky wrote:
>
>> +
>> +The microcode container file format is as follows-
>> +-------------+-----------------------------------------------+-------------------+------------------+------------------------------------------------------------+ 
>>
>> +Name         | Byte Offset                                   | 
>> Length            |  Value           | 
>> Description                                                |
>> +-------------+-----------------------------------------------+-------------------+------------------+------------------------------------------------------------+ 
>>
>> +UCODE_MAGIC  | 0                                             | 
>> 4                 | MAGIC VAL        | AMD UCODE MAGIC 
>> ID                                         |
>> +Type         | 4                                             | 
>> 4                 | 0                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>
> Description should say something like  "Equivalence Table Type" (i.e. 
> since value is zero, this is not a patch)
>

Ok

>> +Length       | 8                                             | 
>> 4                 | equiv_table_len1 | Length of equivalence table(1) 
>> bytes                       |
>> + | Includes a terminating entry with all                      |
>> + | zeroes (1 entry = 16 bytes)                                |
>
> Please describe somewhere equivalence table entry's format.
>

Pinged linux-firmware folks about including container format documentation.
If that is OK with them, then maybe I should move this, and can also 
extend it
to include equiv_table format..

>> +Equiv_table  | 12                                            | 
>> equiv_table_len1  | -                | Equivalence 
>> table                                          |
>> +Type         | 12 + equiv_table_len1                         | 
>> 4                 | 1                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>
> "Microcode Patch Type" for description?
>

Ok

>> +Length       | 16 + equiv_table_len1                         | 
>> 4                 | m(1)             | Length of 1st ucode patch in 
>> bytes                         |
>> +Patch        | 20 + equiv_table_len1                         | 
>> m(1)              | -                | 1st ucode 
>> patch                                            |
>
> Maybe mention somewhere patch's header format (i.e. struct 
> microcode_header_amd)?
>
>
>> + . /
>> + . \
>> + . /
>> +Type         | 20 + equiv_table_len1 + m(1) + ... + m(x-1)   | 
>> 4                 | 1                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>> +Length       | 24 + equiv_table_len1 + m(1) + ... + m(x-1)   | 
>> 4                 | m(x)             | Length of last ucode patch of 
>> this container#1 in bytes    |
>
> "last ucode patch" -> "Xth ucode patch".
>

Ok. (Although both pretty much convey same meaning in my head)

>> +Patch        | 28 + equiv_table_len1 + m(1) + ... + m(x-1)   | 
>> m(x)              | -                | last ucode patch of 
>> container#1                            |
>> +-------------+-----------------------------------------------+-------------------+------------------+------------------------------------------------------------+ 
>>
>> +UCODE_MAGIC  | 28 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | MAGIC VAL        | AMD UCODE MAGIC ID 
>> (container#2 starts here)               |
>> +Type         | 32 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | 0                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>> +Length       | 36 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | equiv_table_len2 | Length of equivalence table(2) 
>> in bytes                    |
>> + | Includes a terminating entry with all                      |
>> + | zeroes (1 entry = 16 bytes)                                |
>> +Equiv_table  | 40 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> equiv_table_len2  | -                | Equivalence 
>> table                                          |
>> +Type         | 40 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | 1                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>> +                  + equiv_table_len2 |
>> +Length       | 44 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | n(1)             | Length of 1st ucode patch in 
>> bytes                         |
>> +                  + equiv_table_len2 |
>> +Patch        | 48 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> n(1)              | -                | 1st ucode 
>> patch                                            |
>> +                  + equiv_table_len2 |
>> + . /
>> + . \
>> + . /
>> +Type         | 48 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | 1                | 0 - Equivalence table, 1- 
>> Microcode patch                  |
>> +                  + equiv_table_len2 + ... + n(y-1) |
>> +Length       | 52 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> 4                 | n(y)             | Length of last ucode patch of 
>> this container in bytes      |
>> +                  + equiv_table_len2 + ... + n(y-1) |
>> +Patch        | 56 + equiv_table_len1 + m(1) + ... + m(x)     | 
>> n(y)              | -                | last ucode patch of 
>> container#2                            |
>> +                  + equiv_table_len2 + ... + n(y) |
>> +-------------+-----------------------------------------------+-------------------+------------------+------------------------------------------------------------+ 
>>
>> + . /
>> + . \
>> + . /
>> +-------------+-----------------------------------------------+-------------------+------------------+------------------------------------------------------------+ 
>>
>> +
>>
>> +Misc Notes:
>> +-----------
>> +It is not recommended to concatenate two(or more) container files of
>> +the same kind. (two microcode_amd_fam15h.bin for example)
>> +This is because:
>> +There is no check in Xen currently to verify this.
>> +Now, given a situation where
>> +1. An earlier container has a patch that 'fits' the processor you are
>> +   currently on,
>> +2. There is a subsequent container that *also* has a patch that 'fits',
>> +3. The second patch happens to be an update over the patch found on the
>> +   first container file.
>> +
>> +In such a case, only the patch from the first container is applied.
>> +This is because Xen assumes that the the containers (if concatenated
>> +together) are different kinds AND the 'Mutual Exclusivity' rule is
>> +always true.
>> +
>> +On such cases of deadlock, it is preferable to update the microcode 
>> container
>
> "Deadlock" doesn't sound like the best term here. You probably meant 
> that in cases where users are not sure about provenance of containers 
> they should obtain a "good" set since it's not guaranteed that the 
> latest patch will be applied?
>
>

Yep. You provided a more accurate description here I think, so I shall 
blatantly copy your words if you don't have any objections:)

-Aravind.