Re: [PATCH 02/18] PVH xen: add XENMEM_add_to_physmap_range

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 06 Jun 2013 07:43:09 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 05.06.13 at 22:41, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Wed, 05 Jun 2013 08:32:52 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
..........
> +        if ( rc == -EAGAIN )
> +            rc = hypercall_create_continuation(
> +                __HYPERVISOR_memory_op, "ih", op, arg);
> +
> +        return rc;
> +    }
> +
>      case XENMEM_set_memory_map:
>      {
>          struct xen_foreign_memory_map fmap;
> 
> If the hypercall were handled before, adding a new case statement
> to arch_memory_op() would cause a compilation error. All that was

No, not really. The hcall was handled by xen by returning -ENOSYS. 

> there before this patch was the definition of the hypercall (for ARM),
> but I'm quite strongly opposed to adding x86 support for this
> hypercall only for one half of the possible set of PV (and HVM?)
> guests; the fact that PVH is 64-bit only for the moment has nothing
> to do with this. The only alternative would be to constrain the
> specific sub-hypercall to PVH, but that would be rather contrived,
> so I'm already trying to make clear that I wouldn't accept such a
> solution to the original comment.

Let me add my perpective: 

32bit HVM and PV guest:
   Before my patch, if a call is made with XENMEM_add_to_physmap_range,
   which it can since its an existing hcall, it will come into 
   compat_arch_memory_op() which will return -ENOSYS.

   After my patch, it will do the same, return -ENOSYS. 

So how does this patch cause a regression for existing 32bit 
guests? Moreover, in the past, you've asked to remove even the simplest
benign line space change because it was unrelated to the patch. So changing
something as part of PVH patchset for PV and HVM guests, wouldn't that 
be unrelated and contradictory to the message you've sent? 

You've found some very genunie issues in the patchset, and I really 
appreciate that. But I struggle with this request.

Thanks,
Mukesh