> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 23 January 2020 13:30 > To: Durrant, Paul <pdurr...@amazon.co.uk> > Cc: Kevin Tian <kevin.t...@intel.com>; Wei Liu <w...@xen.org>; Andrew Cooper > <andrew.coop...@citrix.com>; Jun Nakajima <jun.nakaj...@intel.com>; xen- > de...@lists.xenproject.org; Roger Pau Monné <roger....@citrix.com> > Subject: Re: [PATCH v2 1/3] x86 / vmx: make apic_access_mfn type-safe > > On 23.01.2020 14:09, Durrant, Paul wrote: > >> -----Original Message----- > >> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of > Jan > >> Beulich > >> Sent: 23 January 2020 12:45 > >> To: Durrant, Paul <pdurr...@amazon.co.uk> > >> Cc: Kevin Tian <kevin.t...@intel.com>; Wei Liu <w...@xen.org>; Andrew > Cooper > >> <andrew.coop...@citrix.com>; Jun Nakajima <jun.nakaj...@intel.com>; > xen- > >> de...@lists.xenproject.org; Roger Pau Monné <roger....@citrix.com> > >> 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 <pdurr...@amazon.com> > >>> Acked-by: Kevin Tian <kevin.t...@intel.com> > >>> Reviewed-by: Jan Beulich <jbeul...@suse.com> > >> > >> 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. >
Oh, I see. That implication was not at all obvious to me. I thought he was just after being able to 'destroy' as many times as it took to finish, in which case our choice for the value of INVALID_MFN is indeed unfortunate. If that's the goal I'll switch to use _mfn(0) as a comparator. Paul _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel