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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/03/2020 16:16, Jan Beulich wrote:
> 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.

How?

We want to remove the pointer from microcode_patch, and don't want the
current contents of microcode_{intel,amd} escaping from their current
source files.

I don't see any option but to rearrange it to be opaque.

~Andrew

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