Re: [PATCH RFC] xen: arm: Log a warning message when a deprecated hypercall is used

[Julien Grall <julien.grall@citrix.com>]

Hi Ian,

Do you plan to resend this patch?

Regards,

On 20/01/2015 11:52, Ian Campbell wrote:
> A few folks have been caught out by OSes which call e.g.
> HYPERVISOR_event_channel_op_compat which has been deprecated since
> 3.2.2 (i.e. long before Xen on ARM). Existing x86 code can still
> safely and quietly using those calls, waiting for an unsuspecting ARM
> porter to turn up and trip over it. This turns out to be rather
> perplexing when it happens, since it can be obscured e.g. by various
> conditionals like __XEN_INTERFACE_VERSION__ what is actually being
> called.
>
> Note that I'm making a distinction here between hypercalls which are
> simply not used/implemented on arm (yet) and those which were
> deprecated and replaced by a newer variant prior to Xen on ARM even
> being invented.  The latter will never be implemented on ARM and have
> non-deprecated aliases leading to confusion so those are the ones for
> which a warning is useful.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> ---
> RFC since I'm not sure how extreme our reaction ought to be here, e.g.
> I considered domain_crash() or even panic() when in a debug build. A
> XENLOG_DEBUG message is about the most benign of the options.
>
> Jan/Keir, although this is ARM specific I'd welcome your views as
> x86/REST maintainers.
>
> Ard, I've not actually run this -- any chance you could re-b0rk your
> Tianocore image and give it a go?
> ---
>   xen/arch/arm/traps.c |   30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index ad046e8..89cbde6 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1148,6 +1148,22 @@ die:
>   }
>   #endif
>
> +static register_t do_deprecated_hypercall(void)
> +{
> +    struct cpu_user_regs *regs = guest_cpu_user_regs();
> +    const register_t op =
> +#ifdef CONFIG_ARM_64
> +        !is_32bit_domain(current->domain) ?
> +            regs->x16
> +        :
> +#endif
> +            regs->r12;
> +
> +    gdprintk(XENLOG_DEBUG, "%pv: deprecated hypercall %ld\n",
> +             current, (unsigned long)op);
> +    return -ENOSYS;
> +}
> +
>   typedef register_t (*arm_hypercall_fn_t)(
>       register_t, register_t, register_t, register_t, register_t);
>
> @@ -1167,15 +1183,29 @@ typedef struct {
>           .fn = (arm_hypercall_fn_t) &do_arm_ ## _name,                \
>           .nr_args = _nr_args,                                         \
>       }
> +/*
> + * Only use this for hypercalls which were deprecated (i.e. replaced
> + * by something else) before Xen on ARM was created, i.e. *not* for
> + * hypercalls which are simply not yet used on ARM.
> + */
> +#define HYPERCALL_DEPRECATED(_name, _nr_args)                   \
> +    [ __HYPERVISOR_##_name ] = {                                \
> +        .fn = (arm_hypercall_fn_t) &do_deprecated_hypercall,    \
> +        .nr_args = _nr_args,                                    \
> +    }
> +
>   static arm_hypercall_t arm_hypercall_table[] = {
>       HYPERCALL(memory_op, 2),
>       HYPERCALL(domctl, 1),
>       HYPERCALL(sched_op, 2),
> +    HYPERCALL_DEPRECATED(sched_op_compat, 2),
>       HYPERCALL(console_io, 3),
>       HYPERCALL(xen_version, 2),
>       HYPERCALL(xsm_op, 1),
>       HYPERCALL(event_channel_op, 2),
> +    HYPERCALL_DEPRECATED(event_channel_op_compat, 1),
>       HYPERCALL(physdev_op, 2),
> +    HYPERCALL_DEPRECATED(physdev_op_compat, 1),
>       HYPERCALL(sysctl, 2),
>       HYPERCALL(hvm_op, 2),
>       HYPERCALL(grant_table_op, 3),
>

-- 
Julien Grall