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

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

[Ian Campbell]

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>
Tested-by: Ard Biesheuvel <ard@linaro.org>
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>
---
v2:
   - Use correct format code (I didn't yet shave the yakk of providing
     PRIxREG et al).
   - Settle on just a debug message for now.

Ard tested the last version, but this one is unchanged enough that
I've retained it.
---
 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 258d4c5..06fb40f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1171,6 +1171,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 %lu\n",
+             current, (unsigned long)op);
+    return -ENOSYS;
+}
+
 typedef register_t (*arm_hypercall_fn_t)(
     register_t, register_t, register_t, register_t, register_t);
 
@@ -1190,15 +1206,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),
-- 
1.7.10.4

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

[Julien Grall]

Hi Ian,

On 26/06/2015 13:39, 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>
> Tested-by: Ard Biesheuvel <ard@linaro.org>
> 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>
> ---
> v2:
>     - Use correct format code (I didn't yet shave the yakk of providing
>       PRIxREG et al).
>     - Settle on just a debug message for now.
>
> Ard tested the last version, but this one is unchanged enough that
> I've retained it.
> ---
>   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 258d4c5..06fb40f 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1171,6 +1171,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 %lu\n",
> +             current, (unsigned long)op);
> +    return -ENOSYS;
> +}
> +
>   typedef register_t (*arm_hypercall_fn_t)(
>       register_t, register_t, register_t, register_t, register_t);
>
> @@ -1190,15 +1206,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)                   \

I guess the parameter __nr_args is just here for completeness? If so:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

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

[Ian Campbell]

On Fri, 2015-06-26 at 16:43 +0200, Julien Grall wrote:
> Hi Ian,
> 
> On 26/06/2015 13:39, 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>
> > Tested-by: Ard Biesheuvel <ard@linaro.org>
> > 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>
> > ---
> > v2:
> >     - Use correct format code (I didn't yet shave the yakk of providing
> >       PRIxREG et al).
> >     - Settle on just a debug message for now.
> >
> > Ard tested the last version, but this one is unchanged enough that
> > I've retained it.
> > ---
> >   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 258d4c5..06fb40f 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -1171,6 +1171,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 %lu\n",
> > +             current, (unsigned long)op);
> > +    return -ENOSYS;
> > +}
> > +
> >   typedef register_t (*arm_hypercall_fn_t)(
> >       register_t, register_t, register_t, register_t, register_t);
> >
> > @@ -1190,15 +1206,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)                   \
> 
> I guess the parameter __nr_args is just here for completeness?

It goes into the hypercall table and is used to clobber the arguments in
debug mode, just like for a non-deprecated hypercall.

>  If so:
> 
> Reviewed-by: Julien Grall <julien.grall@citrix.com>
> 
> Regards,
> 

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

[Julien Grall]

On 26/06/2015 16:55, Ian Campbell wrote:
>>> + * 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)                   \
>>
>> I guess the parameter __nr_args is just here for completeness?
>
> It goes into the hypercall table and is used to clobber the arguments in
> debug mode, just like for a non-deprecated hypercall.

Thanks, you can keep the Reviewed-by then.

Regards,

-- 
Julien Grall

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

[Ian Campbell]

On Fri, 2015-06-26 at 17:00 +0200, Julien Grall wrote:
> 
> On 26/06/2015 16:55, Ian Campbell wrote:
> >>> + * 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)                   \
> >>
> >> I guess the parameter __nr_args is just here for completeness?
> >
> > It goes into the hypercall table and is used to clobber the arguments in
> > debug mode, just like for a non-deprecated hypercall.
> 
> Thanks, you can keep the Reviewed-by then.

Applied, thanks.