Re: [PATCH RFC 04/13] x86/mm: carve out create_grant_pv_mapping

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Mar 29, 2017 at 11:49:54AM +0100, Andrew Cooper wrote:
> On 29/03/17 11:45, Jan Beulich wrote:
> >>>> On 29.03.17 at 12:27, <andrew.cooper3@citrix.com> wrote:
> >> One idea I had while starting the hypercall work was to introduce a
> >> "struct guest_type_ops" to contain some function pointers for options we
> >> perform on all guests, irrespective of type.  My first candidate for
> >> splitting this way was the hypercall page writing.
> >>
> >> This splitting here is subtly different.  Its more "struct
> >> paging_type_op", but still common operations we would need to perform
> >> for guests.
> >>
> >> I was thinking that ops structures like this would be cleaner to isolate
> >> than needing an explicit dispatch functions such as
> >> create_grant_host_mapping() below.
> >>
> >> Thoughts, (seeing as this is the first time I have floated this idea on
> >> xen-devel) ?
> > I think that's reasonable as long as we don't go too far with this
> > (indirect calls after all generally having a higher overhead than
> > mis-predicted branches).
> 
> Without any #ifdefary in a source tree, that is fine.  It is harder
> however if you want to conditionally compile things out.
> 
> A alternative option would be to have a static inline in a header file
> which contains the appropriate #ifdefary internally.
> 

Both sound reasonable to me. I slightly prefer guest_op_ops because it
seems cleaner.

The overhead concern applies to hot path code -- I don't think most
guest type specific hypercalls qualify.

Wei.

> ~Andrew

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