Re: [PATCH for-4.15 v2] VMX: use a single, global APIC access page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.02.2021 11:26, George Dunlap wrote:
> 
> 
>> On Feb 22, 2021, at 7:51 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>
>> On 19.02.2021 18:05, Ian Jackson wrote:
>>> Jan Beulich writes ("Re: [PATCH v2] VMX: use a single, global APIC access 
>>> page"):
>>>> While this patch was triggered not just by Julien's observation of
>>>> the early p2m insertion being a problem, but also many earlier
>>>> times of running into this odd code, it is - especially at this
>>>> stage - perhaps a possible option to split the change into just
>>>> the movement of the set_mmio_p2m_entry() invocation and all the
>>>> rest, in order to defer that rest until after 4.15.
>>>
>>> I infer that this contains a bugfix, but perhaps other
>>> changes/improvements too.
>>>
>>> George, I think you're our expert on this refcounting stuff - what do
>>> you think of this ?
>>>
>>> I guess my key question is whether this change will introduce risk by
>>> messing with the complex refcounting machineryt - or remove it by
>>> removing an interaction with the refcounting.
>>
>> If anything, then the latter, but largely neither afaict
> 
> Does it actually contain a bugfix?  It’s not at all clear to me from reading 
> the description that it’s anything other than a clean-up.  If there’s 
> something else that needs to be called out explicitly.

Hmm, yes. The change wanted making anyway, for a long time imo. Hence
when putting together the patch I forgot to call out that as a side
effect it addresses a memory leak, as reported by Julien. With the
splitting of the two changes that'll be necessarily mentioned. I'm
about to submit v3.

Jan

> It should indeed theoretically make things safer long-term; the current 
> vlapic_page allocation is using special-case of the refcounting rules, making 
> it much more prone to being the subject of an “oversight”.  But at this point 
> in the release we don’t have much time at all to shake out any potential bugs 
> in the new implementation; as such I’d consider anything other than the 
> minimum necessary to fix a bug to be not worth it.
> 
>  -George
>