On Mon, Mar 20, 2017 at 2:20 PM, Razvan Cojocaru <rcojoc...@bitdefender.com>
wrote:

> On 03/20/2017 06:54 PM, Jan Beulich wrote:
> >>>> On 20.03.17 at 17:48, <rcojoc...@bitdefender.com> wrote:
> >> On 03/20/2017 06:40 PM, Jan Beulich wrote:
> >>>>>> On 20.03.17 at 17:16, <rcojoc...@bitdefender.com> 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?
>
> 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?
>

IMHO deprecating the old memops shouldn't be needed. The code on the
hypervisor side all lands in the same spot, so keeping both memops (and the
hvmop) in place should add no extra work from a maintenance perspective.

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

Reply via email to