Re: [Xen-devel] [PATCH V2] common/mem_access: merged mem_access setting interfaces

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.03.17 at 21:20, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 03/20/2017 06:54 PM, Jan Beulich wrote:
>>>>> On 20.03.17 at 17:48, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> On 03/20/2017 06:40 PM, Jan Beulich wrote:
>>>>>>> On 20.03.17 at 17:16, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> On 03/20/2017 06:14 PM, Razvan Cojocaru wrote:
>>>>>> In any case, back when I've added xc_set_mem_access_multi() I've also
>>>>>> modified struct xen_mem_access_op in the same manner:
>>>>>>
>>>>>>
>>>>> 
> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commitdiff;h=1ef5056bd6274e 
> 
>>>>> cbe065387b6cf45657d6d700cd
>>>>>
>>>>> Oh, nevermind, I think you're referring to the fact that I had back then
>>>>> added members to the end of the structure, and so the old layout had
>>>>> remained compatible. Point taken.
>>>>
>>>> Not just that - there you've also introduced a new sub-op.
>>>
>>> Yes, but by modifying the structure for use with
>>> XENMEM_access_op_set_access_multi, it's now also changed for, e.g.,
>>> XENMEM_access_op_set_access - since it's common to them. Other than the
>>> place where the new data has been added, I believe that the same
>>> critique would apply to the old patch, unless I'm misunderstanding
>>> something.
>> 
>> Indeed, strictly speaking that change wasn't really okay either,
>> as someone passing the smaller structure near the end of a page
>> might get -EFAULT back. So we're trying to do better this time ...
> 
> Two question remain to be answered then:
> 
> 1. How should this proceed? Andrew's comment, taken together with Tamas'
> last patch ("altp2m: Allow specifying external-only use-case") would
> seem to imply that the best way forward is to revert to the previous
> patch which duplicates the xc_set_mem_access_multi()'s mem-op with
> xc_altp2m_set_mem_access_multi()'s HVMOP (with the compat code to be
> worked out). Is there a consensus on this?

To reach consensus we first need to get a firm understanding of
the requirements of all interested parties. With Ravi not
responding, I've added Paul in the hopes that he may be able to
track down who in Intel is currently able to represent their altp2m
intentions.

> 2. What is the best way to avoid incompatibilities such as the one
> mentioned? For example, in this case, should I have deprecated
> XENMEM_access_op_set_access and XENMEM_access_op_set_access_multi and
> added XENMEM_access_op_set_access2 and XENMEM_access_op_set_access2? Or
> do you have something different in mind?

Indeed it looks like there would be no way around introducing a
new sub-op. We should then take the opportunity and make it
extensible from the beginning, unless we all agree to it not
being a problem to possibly have to introduce further new sub-
ops down the road. Whether such introduction implies
deprecation of the old ones is a separate decision.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel