Re: [Xen-ia64-devel] pv_ops: imntrinsic pv_ops

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Wed, Apr 02, 2008 at 01:51:28PM +0800, Dong, Eddie wrote:
> Current definition of intrinsic APIs seems to be too expansive, this one
> 
> give alternative way to do simply and reduce some changes.
> If this applies, further simplification can be applied.
> Thx, eddie

Interesting approach.
If we can replace most of them, I'll apply.
But half converted state is inconsistent.

Please see some comments below.

>     Simplify intrinsic API handling.
> 
> Signed-off-by: Yaozu (Eddie) Dong <eddie.dong@xxxxxxxxx>
> 
> diff --git a/arch/ia64/kernel/paravirt.c b/arch/ia64/kernel/paravirt.c
> index 4b01c44..6ce4f60 100644
> --- a/arch/ia64/kernel/paravirt.c
> +++ b/arch/ia64/kernel/paravirt.c
> @@ -3,6 +3,7 @@
>   *
>   * Copyright (c) 2008 Isaku Yamahata <yamahata at valinux co jp>
>   *                    VA Linux Systems Japan K.K.
> + *   Yaozu (Eddie) Dong <eddie.dong@xxxxxxxxx>
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -53,29 +54,18 @@ struct pv_init_ops pv_init_ops;
>  
>  /* ia64_native_xxx are macros so that we have to make them real
> functions */
>  
> -static void
> -ia64_native_fc_func(unsigned long addr)
> -{
> -     ia64_native_fc(addr);
> -}
> -
> -static unsigned long
> -ia64_native_thash_func(unsigned long addr)
> -{
> -     return ia64_native_thash(addr);
> +#define NATIVE_INTRINSIC_API1(type, name, para1)             \
> +static type native_ ## name(unsigned long para1) {   \
> +     return name(para1);                                     \
>  }
>  
> -static unsigned long
> -ia64_native_get_cpuid_func(int index)
> -{
> -     return ia64_native_get_cpuid(index);
> -}
> -
> -static unsigned long
> -ia64_native_get_pmd_func(int index)
> -{
> -     return ia64_native_get_pmd(index);
> -}
> +#undef CONFIG_PARAVIRT
> +NATIVE_INTRINSIC_API1(void, ia64_fc, addr)
> +NATIVE_INTRINSIC_API1(unsigned long, ia64_thash, addr)
> +NATIVE_INTRINSIC_API1(unsigned long, ia64_get_cpuid, addr)
> +NATIVE_INTRINSIC_API1(unsigned long, ia64_get_pmd, index)
> +NATIVE_INTRINSIC_API1(void, ia64_intrin_local_irq_restore, flags)
> +#define CONFIG_PARAVIRT

Defining those function by macro is a good idea.
But, undef/redefine CONFIG_PARAVIRT looks ugly and
defining conflicting name would be confusing.

I guess your concern is removing bunch of #define ia64_xxx ...
(And yes, I agree with you to clean them up.)
So how about something like the following?

in intrinsic.h

#ifdef CONFIG_APRAVIRT
#define IA64_INTRINSIC_API(name)        paravirt. ## name
#else
#define IA64_INTRINSIC_API(name)        ia64_native_ ## name
#endif

#define ia64_fc                         IA64_INTRINSIC(fc)
...

and keep ia64_native_xxx definitions.
This doesn't depend on the number of arguments.

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel