[Xen-devel] Re: [PATCH 02/18] xen: hook io_apic read/write operations

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

Ingo Molnar wrote:
> * Ingo Molnar <mingo@xxxxxxx> wrote:
>
>   
> > * Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:
> >
> >     
> > > --- a/arch/x86/kernel/apic/io_apic.c
> > > +++ b/arch/x86/kernel/apic/io_apic.c
> > > @@ -62,8 +62,10 @@
> > >  #include <asm/uv/uv_hub.h>
> > >  #include <asm/uv/uv_irq.h>
> > >  
> > > +#include <asm/xen/hypervisor.h>
> > >  #include <asm/apic.h>
> > >  
> > > +
> > >  #define __apicdebuginit(type) static type __init
> > >  
> > >  /*
> > > @@ -407,14 +409,26 @@ static inline void io_apic_eoi(unsigned int apic, 
> > > unsigned int vector)
> > >  
> > >  static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
> > >  {
> > > -       struct io_apic __iomem *io_apic = io_apic_base(apic);
> > > +       struct io_apic __iomem *io_apic;
> > > +
> > > +       if (xen_initial_domain())
> > > +               return xen_io_apic_read(apic, reg);
> > >       
> > hm, any reason why we dont want to create a 'struct io_apic' 
> > driver abstraction instead of spreading xen_initial_domain() 
> > checks all around the code?
> >     
My initial patch did that, and I'm happy to revive it.  But HPA 
preferred this approach, arguing against introducing another layer of 
abstraction for the sake of one user.
> And on a higher level, i still dont see why you dont do the whole 
> Xen thing under an irqchip. There should be no extra crappy checks 
> in native code.
>   
Hm, every time you see this code, you always have this quasi-Pavlovian 
response.  You say "use an irqchip".  I say:
   * We already use irqchip
   * but most of the interesting IO apic accesses (routing) are not
     done via the irqchip interface
   * so irqchip doesn't help

And then you don't reply.  And then you raise it again.

I would *always* prefer to hook into an interface like irqchip rather 
than gouge into the code, but I really think that irqchip isn't that 
interface.  If you have a more specific suggestion or proposal I'll 
happily follow it up, but repeating "you should use an irqchip" isn't 
getting anywhere.
To reiterate:

   * irq_chip is all about interrupt delivery, masking, acking, etc
   * these Xen dom0 apic changes are all about interrupt routing
   * irq_chip doesn't cover routing


   J

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