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

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

On 10/6/2014 11:50 AM, Boris Ostrovsky wrote:
> On 10/03/2014 11:40 AM, Aravind Gopalakrishnan wrote:
>> This patch introduces documentation notes about AMD container
>> file formats and where to obtain latest container files from.
>>
>> Also, We provide a how-to for updating patch level by
>> concatenating container files along with initrd images.
>> Misc notes about how Xen handles two containers of same
>> kind (if/when) they are concatenated together are also included.
>>
>> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
>> ---
>> Changes in V2:
>>   - Boris suggested the format part itself is probably better suited
>>     in linux-firmware tree. There was also another idea about getting
>>     it out as a separate vendor doc rather than hanging it off of a
>>     repository. Since this is still WIP, I have removed the ascii art
>>     about the container file format itself here.
>>     - I shall update docs with links to relevant documentation as and
>>       when it is available.
>>   - Including patch header format (as suggested by Boris)
>>   - Provide more accurate description of what was meant by "deadlock"
>>     (as suggested by Boris)
>>
>>   docs/misc/amd-ucode-container.txt | 101 
>> ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 101 insertions(+)
>>   create mode 100644 docs/misc/amd-ucode-container.txt
>>
>> diff --git a/docs/misc/amd-ucode-container.txt 
>> b/docs/misc/amd-ucode-container.txt
>> new file mode 100644
>> index 0000000..872b5f5
>> --- /dev/null
>> +++ b/docs/misc/amd-ucode-container.txt
>> @@ -0,0 +1,101 @@
>> +-------------------------------------------------
>> +AMD Microcode Container File format
>> +-------------------------------------------------
>> +Author:
>> +  Aravind [dot] Gopalakrishnan [at] amd [dot] com
>> +Initial version:
>> +  July 2014
>> +Updated:
>> +  October 2014
>> +-------------------------------------------------
>> +
>> +Intro to AMD Container Files:
>> +-----------------------------
>> +
>> +* AMD provides microcode patch support for processors belonging to AMD
>> +  processor families 10h, 11h, 12h, 14h, and 15h.
>
> What about 16h?
>

There isn't a 16h container file released yet..
But-

> Would it be fair to say that beginning with family 10h AMD provides 
> microcode patch support?
>

Yes, I think that's a fair statement.
So, I'll modify it as you stated.

>> +* There is one single file (container file) containing all microcode 
>> patches
>> +  for AMD families 10h - 14h processors. [microcode_amd.bin]
>> +* For AMD family 15h processors there is a separate container file. 
>> [microcode_amd_fam15h.bin]
>> +* Microcode patches are not incremental, therefore you only need to 
>> make
>> +  sure you have the latest container file for your AMD processor 
>> family.
>> +* One can find the latest AMD microcode containers from [1], [2]
>> +
>> +Mutual Exclusivity Rule of AMD containers:
>> +* The patches for families 10h - 14h are guaranteed to be only on
>> +  microcode_amd.bin
>> +* Similarly, patches for family 15h will only be on 
>> microcode_amd_fam15h.bin
>> +* This is because, the processes and scripts used to create 
>> container files
>> +  ensure that there is no mix-up
>> +
>> +Microcode patch header structure:
>> +---------------------------------
>> +struct __packed microcode_header_amd {
>> +    uint32_t data_code;
>> +    uint32_t patch_id;
>> +    uint8_t  mc_patch_data_id[2];
>> +    uint8_t  mc_patch_data_len;
>> +    uint8_t  init_flag;
>> +    uint32_t mc_patch_data_checksum;
>> +    uint32_t nb_dev_id;
>> +    uint32_t sb_dev_id;
>> +    uint16_t processor_rev_id;
>> +    uint8_t  nb_rev_id;
>> +    uint8_t  sb_rev_id;
>> +    uint8_t  bios_api_rev;
>> +    uint8_t  reserved1[3];
>> +    uint32_t match_reg[8];
>> +}
>> +More details about microcode patch header are typically not exposed 
>> to public.
>> +
>> +Apply microcode updates using initrd:
>> +-------------------------------------
>> +Initrd images can be modified to contain AMD microcode containers in 
>> cpio
>> +format at the start of the image.
>> +
>> +Following example shows how to generate a combined initrd
>> +Note: initrd-<val> could be different on your machine. Substitute 
>> accordingly
>> +Example System base: Ubuntu 13.04 with 3.8.0-30-generic kernel
>> +
>> +1.  mkdir initrd-for-xen-with_append
>> +2.  cd initrd-for-xen-with_append
>> +3.  mkdir -p kernel/x86/microcode
>> +4.  cp /lib/firmware/amd-ucode/microcode_amd.bin .
>> +5.  cp /lib/firmware/amd-ucode/microcode_amd_fam15h.bin .
>> +6.  cat microcode_amd.bin microcode_amd_fam15h.bin > 
>> microcode_concatenated.bin
>> +7.  mv microcode_concatenated.bin kernel/x86/microcode/AuthenticAMD.bin
>> +8.  rm microcode_amd.bin microcode_amd_fam15h.bin
>
> Steps 4-8 can be written as
>
>   cat /lib/firmware/amd-ucode/microcode_amd.bin \
>         /lib/firmware/amd-ucode/microcode_amd_fam15h.bin > \
>         kernel/x86/microcode/AuthenticAMD.bin
>

Agree. Shall fix this.

>
>> +9.  find . | cpio -o -H newc > ucode.cpio
>> +10.  cp /boot/initrd.img-3.8.0-30-generic .
>> +11. cat ucode.cpio initrd.img-3.8.0-30-generic > 
>> initrd_for_xen_with_ucode
>> +12. cp initrd_for_xen_with_ucode /boot/
>
> 10-12 can be
>
>     cat ucode.cpio /boot/initrd.img-3.8.0-30-generic > 
> /boot/initrd_for_xen_with_ucode
>

Yes, Will fix.

>> +13. On grub.cfg, provide the above initrd name as module.
>
> i.e. 'module /boot/initrd_for_xen_with_ucode'
>
>> +14. Use 'ucode=scan' option as Xen boot parameter.
>> +
>> +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.
>
> I think this whole section can be shortened to:
>     "It is not recommended to concatenate  multiple container files of 
> the
>      same kind (e.g. two microcode_amd_fam15h.bin) since the 
> hypervisor will
>      apply only the first matching patch. If the subsequent file has a 
> newer update
>      that update will be ignored."
>
> Or something like that.
>

Yeah, that makes sense.
Shall resend with your suggested modifications.

Thanks,
-Aravind.