Re: [Xen-devel] [PATCH 5/6] x86/ucode: Alter ops->free_patch() to free the entire patch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.03.2020 17:10, Andrew Cooper wrote:
> On 20/03/2020 15:15, Jan Beulich wrote:
>> On 20.03.2020 15:50, Andrew Cooper wrote:
>>> On 20/03/2020 13:51, Jan Beulich wrote:
>>>> On 19.03.2020 16:26, Andrew Cooper wrote:
>>>>> The data layout for struct microcode_patch is extremely poor, and
>>>>> unnecessarily complicated.  Almost all of it is opaque to core.c, with the
>>>>> exception of free_patch().
>>>>>
>>>>> Move the responsibility for freeing the patch into the free_patch() hook,
>>>>> which will allow each driver to do a better job.
>>>> But that wrapper structure is something common, i.e. to be
>>>> allocated as well as to be freed (preferably) by common code.
>>>> We did specifically move there during review of the most
>>>> recent re-work.
>>> The current behaviour of having it allocated by the request() hook, but
>>> "freed" in a mix of common code and a free() hook, cannot possibly have
>>> been an intended consequence from moving it.
>>>
>>> The free() hook is currently necessary, as is the vendor-specific
>>> allocation logic, so splitting freeing responsibility with the common
>>> code is wrong.
>> Hmm, yes, with the allocation done in vendor code, the freeing
>> could be, too. But the wrapper struct gets allocated last in
>> cpu_request_microcode() (for both Intel and AMD), and hence ought
>> to be relatively easy to get rid of, instead of moving around
>> the freeing (the common code part of the freeing would then
>> simply go away).
> 
> I am working on removing all unnecessary allocations, including folding
> microcode_{intel,amd} into microcode_patch, but I'm still confident this
> wants to be done with microcode_patch being properly opaque to core.c

Oh, sure - I didn't mean to put this under question. It just seems
to me the the route there may better be somewhat different from this
and the following patch.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel