[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 20 Mar 2020 16:48:26 +0000
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@xxxxxxxxxx; spf=Pass smtp.mailfrom=Andrew.Cooper3@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Fri, 20 Mar 2020 16:48:50 +0000
  • Ironport-sdr: zbsT37etKLZOSWJ4qpC7y7z7Vf9p9unrfvLjR8aPDYb6d12Pklr1E0eNFgSvwqagmzROXhLneC ncGrxyZQyHyD7R3Cw+2iqk+Wiwz425VIg0jDkNvOpuTQCYdkghddKv/mETGzN+1N3vCEKJEnRv Nwz4mTNKCs3cUBxKZgWx6MZtAmgD54s6802h1szNldV83t7L2P7vrYv76ozAtBAloFLuuDRxae Nca6DzoHwFQRJvkR7IG/Ia0o79mlqHK6RKINLBD4BtxkTOZqLw4VoLD0OXHdfbDyvSHmmQuvY9 W1U=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.


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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.