Re: [Xen-devel] [PATCH v2 1/3] x86 / vmx: make apic_access_mfn type-safe

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.01.2020 14:09, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan
>> Beulich
>> Sent: 23 January 2020 12:45
>> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
>> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; xen-
>> devel@xxxxxxxxxxxxxxxxxxxx; Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> Subject: Re: [Xen-devel] [PATCH v2 1/3] x86 / vmx: make apic_access_mfn
>> type-safe
>>
>> On 23.01.2020 13:21, Paul Durrant wrote:
>>> Use mfn_t rather than unsigned long and change previous tests against 0
>> to
>>> tests against INVALID_MFN (also introducing initialization to that
>> value).
>>>
>>> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
>>> Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> No, this isn't what the R-b was given for.
> 
> Oh, sorry, I misunderstood; I thought the R-b was good as long as idempotency 
> was ensured.
> 
>>
>>> v2:
>>>  - Set apic_access_mfn to INVALID_MFN in vmx_free_vlapic_mapping() to
>> make
>>>    the function idempotent
>>
>> Andrew had suggested to use 0 instead of INVALID_MFN. I don't see
>> how you achieved idempotency with this adjustment. Aiui
>> vmx_free_vlapic_mapping() is supposed to also run correctly if
>> vmx_alloc_vlapic_mapping() was never called.
> 
> It will. vmx_domain_initialise() will set apic_access_mfn to INVALID_MFN
> so vmx_free_vlapic_mapping() will do nothing.

I'm sorry, it was implied that it also needs to work if
vmx_domain_initialise() was never called. Andrew's goal after
all is, aiui, to be able to call "destroy" functions on error
paths irrespective of how far "create" had managed to progress.

Jan

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