On Tue, Jul 23, 2013 at 06:57:12PM +0200, Torsten Kaiser wrote:
> >> * Save the amd_bsp_mpb on apply, not on load. Otherwise someone could
> >> later load an older microcode file that would overwrite amd_bsp_mpb,
> >> but would not be applied to the CPUs
> >
> > See the patch id check in apply_ucode_in_initrd()?
> >
> > if (eq_id == mc->hdr.processor_rev_id && rev < mc->hdr.patch_id)
>
> I meant with "load" load_microcode_amd() not the loading of the
> microcode into the CPU.
>
> 1.: load microcode rev X into CPU (early or normal is not important)
> 2.: get older microcode file that only contains rev Y with Y<X
> 3.: trigger load_microcode_amd() with a corrupt file: This will call
> cleanup() and empty pcache.
Ok, that's actually a good catch. So I wonder: why in hell would we
flush the pcache if some of the blobs we're loading are corrupted. So
what?! Jacob, what were you thinking - I'd be very interested to know
what the idea behind this was.
So, just to refresh everybody: the idea of the pcache is to keep all
patches for the current family in memory so that we can support all
sorts of hotplug and cpu mixed stepping diddling.
> 4.: trigger load_microcode_amd() with the older file:
> * this will now load rev Y into pcache
> * rev Y will be returned by find_patch and copied into amd_bsp_mpb
> * any try to apply rev Y will be skipped in apply_microcode_amd()
>
> So now the CPU still correctly has rev X, but amd_bsp_mpb will contain
> the wrong rev Y.
Right, so this shouldn't happen - what should happen is, pcache would
hold both X and Y and find_patch would automatically give you the right
one.
And this is guaranteed since we keep the patches in a sorted linked list
by ->patch_id which is guaranteed to be increasing.
So actually load_microcode_amd() shouldn't be doing cleanup() but simply
return ret upwards.
> That copying already in load_microcode also is suspicious if someone
> would only load the microcode but not apply it. But I did not find
> a codepath in arch/x86/kernel/microcode_core.c to load it without a
> followup apply.
Yeah, we always load and apply.
So now back to the original problem - load_microcode_amd() shouldn't
clear the pcache and, in that case, a subsequent find_patch() would
always give the right patch.
Ok?
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
