Re: [V10 PATCH 18/23] PVH xen: add hypercall support for PVH

["Jan Beulich" <JBeulich@suse.com>]

>>> On 09.08.13 at 02:55, Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
> On Thu, 08 Aug 2013 08:41:04 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> >>> On 08.08.13 at 04:12, Mukesh Rathor <mukesh.rathor@oracle.com>
>> >>> wrote:
>> > On Wed, 7 Aug 2013 17:43:54 +0100
>> > George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>> >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor
>> >> <mukesh.rathor@oracle.com> wrote:
>> >> > +static hvm_hypercall_t *const
>> >> > pvh_hypercall64_table[NR_hypercalls] = {
>> >> > +    HYPERCALL(platform_op),
>> >> > +    HYPERCALL(memory_op),
>> >> > +    HYPERCALL(xen_version),
>> >> > +    HYPERCALL(console_io),
>> >> > +    [ __HYPERVISOR_grant_table_op ]  = (hvm_hypercall_t
>> >> > *)hvm_grant_table_op,
>> >> > +    [ __HYPERVISOR_vcpu_op ]         = (hvm_hypercall_t
>> >> > *)hvm_vcpu_op,
>> >> > +    HYPERCALL(mmuext_op),
>> >> > +    HYPERCALL(xsm_op),
>> >> > +    HYPERCALL(sched_op),
>> >> > +    HYPERCALL(event_channel_op),
>> >> > +    [ __HYPERVISOR_physdev_op ]      = (hvm_hypercall_t
>> >> > *)hvm_physdev_op,
>> >> > +    HYPERCALL(hvm_op),
>> >> > +    HYPERCALL(sysctl),
>> >> > +    HYPERCALL(domctl)
>> >> > +};
>> >> 
>> >> It would be nice if this list were in the same order as the other
>> >> lists, so that it is easy to figure out what calls are common and
>> >> what calls are different.
>> > 
>> > These are ordered by the hcall number, and assists in the debug.
>> 
>> But with George asking, do you now understand a little better
>> why on a very early revision I had asked to copy either the
>> HVM or PV hypercall table, and override just the entries that
>> need overrides (making it very clear which ones differ)?
> 
> Like I've said before, I believe that is a poor and obfuscating way of doing
> it, and I don't want my name on something I completely disagree with. It
> makes code harder to read IMO. I'm adding such a
> small extension to the existing HVM code, that I believe its hardly
> reaching a tipping point. PVH is still evolving, this is first patch,
> again, minimal changes to make a guest boot and come up in PVH mode.
> Over time we'll come to understand more what other hcalls need to be
> added and to what extent. At that point further enhancements can be
> made... 

And I didn't mean to ask that you change your patch in this regard,
I just wanted to point out that I'm not the only one thinking
differently than you.

In any event, I guess once your code is in I'll try to remember to
follow IanC's suggestion and script the hypercall table generation.

Jan