[Xen-devel] Re: [PATCH]mini-os: Using new hypercall interfaces

[Grzegorz Milos <gm281@xxxxxxxxx>]

You've dropped 'data' from bind_evtchn() call in your changes to 
events.c. That breaks our API, where event handler gets void* to some 
piece of data supplied when binding a channel. It looks to me that no 
changes to events.c are required whatsoever.
Apart of that everything looks fine. Could you resend the patch?

Cheers
Gregor

Dietmar Hahn wrote:
> Hi,
>
> this patch switches to the newer interfaces for
> HYPERVISOR_sched_op() and HYPERVISOR_event_channel_op().
> I testet it only on x86_32 because I have no x86_64 machine!
> Please have a look!
> Thanks.
>
> Dietmar.
>
>
> ------------------------------------------------------------------------
>
> # HG changeset patch
> # User dietmar.hahn@xxxxxxxxxxxxxxxxxxx
> # Date 1164206513 -3600
> # Node ID 74abfaeeb7ba15d7ca366c814c412d1ccc594532
> # Parent  1ef9954a26686b35b946d889726d4d35c283c2a0
> Switched to new interfaces HYPERVISOR_sched_op() and
> HYPERVISOR_event_channel_op().
>
> Signed-off-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxxxxxxx>
>
>
> diff -r 1ef9954a2668 -r 74abfaeeb7ba extras/mini-os/events.c
> --- a/extras/mini-os/events.c   Wed Nov 22 09:51:20 2006 +0000
> +++ b/extras/mini-os/events.c   Wed Nov 22 15:41:53 2006 +0100
> @@ -88,18 +88,18 @@ void unbind_evtchn(evtchn_port_t port )
>  
>  int bind_virq(uint32_t virq, evtchn_handler_t handler, void *data)
>  {
> -       evtchn_bind_virq_t op;
> -
> -       /* Try to bind the virq to a port */
> -       op.virq = virq;
> -       op.vcpu = smp_processor_id();
> -
> -       if ( HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &op) != 0 )
> +       struct evtchn_bind_virq send =
> +       {
> +               .virq = virq,
> +               .vcpu = smp_processor_id()
> +       };
> +       if(HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &send) != 0)
>         {
>                 printk("Failed to bind virtual IRQ %d\n", virq);
>                 return 1;
> -    }
> -    bind_evtchn(op.port, handler, data);
> +       }
> +       bind_evtchn(send.port, handler, NULL);
> +
>         return 0;
>  }
>  
> diff -r 1ef9954a2668 -r 74abfaeeb7ba extras/mini-os/include/x86/x86_32/hypercall-x86_32.h
> --- a/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h      Wed Nov 22 
> 09:51:20 2006 +0000
> +++ b/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h      Wed Nov 22 
> 15:41:53 2006 +0100
> @@ -165,9 +165,16 @@ HYPERVISOR_fpu_taskswitch(
>         return _hypercall1(int, fpu_taskswitch, set);
>  }
>  
> + static inline int
> +HYPERVISOR_sched_op_compat(
> +       int cmd, unsigned long arg)
> +{
> +       return _hypercall2(int, sched_op_compat, cmd, arg);
> +}
> +
>  static inline int
>  HYPERVISOR_sched_op(
> -       int cmd, unsigned long arg)
> +       int cmd, void *arg)
>  {
>         return _hypercall2(int, sched_op, cmd, arg);
>  }
> @@ -238,9 +245,21 @@ HYPERVISOR_update_va_mapping(
>  
>  static inline int
>  HYPERVISOR_event_channel_op(
> -       int cmd, void *op)
> -{
> -       return _hypercall2(int, event_channel_op, cmd, op);
> +       int cmd, void *arg)
> +{
> +       int rc = _hypercall2(int, event_channel_op, cmd, arg);
> +
> +#ifdef CONFIG_XEN_COMPAT_030002
> +       if (unlikely(rc == -ENOSYS)) {
> +               struct evtchn_op op;
> +               op.cmd = cmd;
> +               memcpy(&op.u, arg, sizeof(op.u));
> +               rc = _hypercall1(int, event_channel_op_compat, &op);
> +               memcpy(arg, &op.u, sizeof(op.u));
> +       }
> +#endif
> +
> +       return rc;
>  }
>  
>  static inline int
> diff -r 1ef9954a2668 -r 74abfaeeb7ba 
> extras/mini-os/include/x86/x86_64/hypercall-x86_64.h
> --- a/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h      Wed Nov 22 
> 09:51:20 2006 +0000
> +++ b/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h      Wed Nov 22 
> 15:41:53 2006 +0100
> @@ -170,8 +170,15 @@ HYPERVISOR_fpu_taskswitch(
>  }
>  
>  static inline int
> +HYPERVISOR_sched_op_compat(
> +       int cmd, unsigned long arg)
> +{
> +       return _hypercall2(int, sched_op_compat, cmd, arg);
> +}
> +
> +static inline int
>  HYPERVISOR_sched_op(
> -       int cmd, unsigned long arg)
> +       int cmd, void *arg)
>  {
>         return _hypercall2(int, sched_op, cmd, arg);
>  }
> @@ -235,9 +242,21 @@ HYPERVISOR_update_va_mapping(
>  
>  static inline int
>  HYPERVISOR_event_channel_op(
> -       int cmd, void *op)
> -{
> -    return _hypercall2(int, event_channel_op, cmd, op);
> +       int cmd, void *arg)
> +{
> +       int rc = _hypercall2(int, event_channel_op, cmd, arg);
> +
> +#ifdef CONFIG_XEN_COMPAT_030002
> +       if (unlikely(rc == -ENOSYS)) {
> +               struct evtchn_op op;
> +               op.cmd = cmd;
> +               memcpy(&op.u, arg, sizeof(op.u));
> +               rc = _hypercall1(int, event_channel_op_compat, &op);
> +               memcpy(arg, &op.u, sizeof(op.u));
> +       }
> +#endif
> +
> +       return rc;
>  }
>  
>  static inline int
> diff -r 1ef9954a2668 -r 74abfaeeb7ba extras/mini-os/kernel.c
> --- a/extras/mini-os/kernel.c   Wed Nov 22 09:51:20 2006 +0000
> +++ b/extras/mini-os/kernel.c   Wed Nov 22 15:41:53 2006 +0100
> @@ -28,6 +28,7 @@
>   */
>  
>  #include <os.h>
> +#include <errno.h>
>  #include <hypervisor.h>
>  #include <mm.h>
>  #include <events.h>
> @@ -159,5 +160,14 @@ void do_exit(void)
>  void do_exit(void)
>  {
>      printk("Do_exit called!\n");
> -    for ( ;; ) HYPERVISOR_sched_op(SCHEDOP_shutdown, SHUTDOWN_crash);
> +    for( ;; )
> +    {
> +        struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_crash };
> +        HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
> +        int rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
> +        if(rc == -ENOSYS)
> +        {
> +            HYPERVISOR_sched_op_compat(SCHEDOP_shutdown, SHUTDOWN_crash);
> +        }
> +    }
>  }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel