Re: [Xen-ia64-devel] [Patch] iosapic virtualization again

[Tristan Gingold <Tristan.Gingold@xxxxxxxx>]

Le Mercredi 22 Février 2006 17:34, Tian, Kevin a écrit :
> Several coding style comments:
>
> 1.
> +     if (vector == IA64_TIMER_VECTOR || vector == IA64_IPI_VECTOR)
> +             return 0;
>
> For this point, I second Eddie to have irq descriptor add a flag to
> indicate whether belonging to xen or guest, instead of hardcode here. You
> can grep IRQ_GUEST in xen/arch/x86. Though guest SMP and driver domain is
> not ready yet, interface should be designed clean and well to allow better
> cooperation between guest and xen.
>
> 2. Not sure why you pull in iosapic.h into your patch. Seems no
> modification there which just need copy from linux source at compile time.
> If you really want to include this file, you can avoid adding
> xen_iosapic_write in c file and instead move its content to iosapic_write
> defined in iosapic.h
I suppose you speak about the iosapic.h for linux.
I have removed the iosapic_version() declaration, since it has been modified 
and is never used outside iosapic.

> 3. Why ifdef XEN but nothing changed:
> +#ifdef XEN
> +     vector = assign_irq_vector(AUTO_ASSIGN);
> +#else
> +     /* If vector is running out, we try to find a sharable vector */
> +#endif
> +     vector = assign_irq_vector(AUTO_ASSIGN);
Correct.

> 4. It's ugly to see:
> +#define VCPU_XEN ((struct vcpu *)1)
> Also no place to init vcpu with this value, however later it's checked when
> reflecting interrupt
Maybe I should remove this ?

Tristan.


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