From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b
 PVH guests
Date: Thu, 23 Jul 2015 08:21:16 -0600
Message-ID: <55B1147D0200007800094A8C@prv-mh.provo.novell.com>
References: <1436566853-8444-1-git-send-email-boris.ostrovsky@oracle.com>
	<1436566853-8444-4-git-send-email-boris.ostrovsky@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1436566853-8444-4-git-send-email-boris.ostrovsky@oracle.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: elena.ufimtseva@oracle.com, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, roger.pau@citrix.com
List-Id: xen-devel@lists.xenproject.org

>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> Changes in v3:
> * Defined compat_mmuext_op(). (XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t)
>   is not defined in header files so I used 'void' type.

How is it not? It's in compat/xen.h (which is a generated header).

> @@ -4951,6 +4950,29 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
>      [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
>  };
>  
> +extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
> +                            unsigned int count,
> +                            XEN_GUEST_HANDLE_PARAM(uint) pdone,
> +                            unsigned int foreigndom);
> +static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
> +    HYPERCALL(platform_op),
> +    COMPAT_CALL(memory_op),
> +    HYPERCALL(xen_version),
> +    HYPERCALL(console_io),
> +    [ __HYPERVISOR_grant_table_op ]  =
> +        (hvm_hypercall_t *)hvm_grant_table_op_compat32,
> +    COMPAT_CALL(vcpu_op),
> +    COMPAT_CALL(mmuext_op),
> +    HYPERCALL(xsm_op),
> +    COMPAT_CALL(sched_op),
> +    HYPERCALL(event_channel_op),
> +    [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
> +    HYPERCALL(hvm_op),
> +    HYPERCALL(sysctl),
> +    HYPERCALL(domctl),
> +    [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation

Looks like you didn't fully sync with staging - did you forget that it
was you who added xenpmu_op to the 64-bit counterpart?
Without that ...

> @@ -4981,7 +5003,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>          return viridian_hypercall(regs);
>  
>      if ( (eax >= NR_hypercalls) ||
> -         (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
> +         (is_pvh_domain(currd) ? !pvh_hypercall32_table[eax]
>                                 : !hvm_hypercall32_table[eax]) )

... this will break (as we're assuming 32- and 64-bit tables to be fully
in sync here; there's still the pending work item of constructing these
tables so that this has a better chance of not getting broken).

Also, since you're touching this here anyway, could I ask you to
adjust this along the lines of the change you do further down,
i.e. to become

         !(is_pvh_domain(currd) ? pvh_hypercall32_table
                                : hvm_hypercall32_table)[eax] )

Thanks, Jan