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

[Grzegorz Milos <gm281@cam.ac.uk>]

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@fujitsu-siemens.com
> # 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@fujitsu-siemens.com>
> 
> 
> 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);
> +        }
> +    }
>  }