On Tue, 17 Jan 2017, Borislav Petkov wrote:
> void load_ucode_amd_ap(unsigned int cpuid_1_eax)
> {
> struct equiv_cpu_entry *eq;
> struct microcode_amd *mc;
> + struct cont_desc *desc;
> u16 eq_id;
>
> + if (IS_ENABLED(CONFIG_X86_32)) {
> + mc = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
> + desc = (struct cont_desc *)__pa_nodebug(&cont);
> + } else {
> + mc = (struct microcode_amd *)amd_ucode_patch;
> + desc = &cont;
Bah! Now I realize that 'cont' is not a local variable as I assumed when
looking at the other patch. 'cont' is a pretty bad name for a (file) global
variable. Can we please use a more obvious name ?
While at it please make that thing static as there cant be a user outside
of that file.
> + }
> +
> /* First AP hasn't cached it yet, go through the blob. */
> - if (!cont.data) {
> - struct cpio_data cp;
> + if (!desc->data) {
> + struct cpio_data cp = { };
>
> - if (cont.size == -1)
> + if (desc->size == -1)
> return;
I'm not really fond of abusing the size member for this. And that '-1'
seems to have different meanings depending on other members. Really not
intuitive.
Please introduce a proper state member which tells what the descriptor
struct contains, i.e. EMPTY, VALID, INVALID ....
Thanks,
tglx
