Re: ioapic: adaption required?

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

On Monday 08 October 2007 16:49:51 Manuel Bouyer wrote:
> On Mon, Oct 08, 2007 at 02:09:23PM +0200, Christoph Egger wrote:
> > On Monday 08 October 2007 13:33:38 Manuel Bouyer wrote:
> > > On Mon, Oct 08, 2007 at 01:22:32PM +0200, Christoph Egger wrote:
> > > > > xen/include/i82093var.h Or maybe you mean to use APIC operations to
> > > > > query/ack the interrupts instead of these hypercalls ?
> > > >
> > > > I'm talking about the hypercall used in pirq_establish() in
> > > > xen/evtchn.c.
> > >
> > > Do you know what should be used instead now ?
> >
> > The information is in  include/xen3-public/physdev.h.
> >
> > To acknowledge an interrupt use:
> >
> > /*
> >  * Notify end-of-interrupt (EOI) for the specified IRQ.
> >  * @arg == pointer to physdev_eoi structure.
> >  */
> > #define PHYSDEVOP_eoi                   12
> > struct physdev_eoi {
> >     /* IN */
> >     uint32_t irq;
> > };
> >
> > To query for an interrupt use:
> >
> > /*
> >  * Query the status of an IRQ line.
> >  * @arg == pointer to physdev_irq_status_query structure.
> >  */
> > #define PHYSDEVOP_irq_status_query       5
> > struct physdev_irq_status_query {
> >     /* IN */
> >     uint32_t irq;
> >     /* OUT */
> >     uint32_t flags; /* XENIRQSTAT_* */
> > };
> >
> > /* Need to call PHYSDEVOP_eoi when the IRQ has been serviced? */
> > #define _XENIRQSTAT_needs_eoi   (0)
> > #define  XENIRQSTAT_needs_eoi   (1U<<_XENIRQSTAT_needs_eoi)
> >
> > /* IRQ shared by multiple guests? */
> > #define _XENIRQSTAT_shared      (1)
> > #define  XENIRQSTAT_shared      (1U<<_XENIRQSTAT_shared)
> >
> > To set VCPU's iopl use:
> >
> > /*
> >  * Set the current VCPU's I/O privilege level.
> >  * @arg == pointer to physdev_set_iopl structure.
> >  */
> > #define PHYSDEVOP_set_iopl               6
> > struct physdev_set_iopl {
> >     /* IN */
> >     uint32_t iopl;
> > };
> >
> > To allocate/free an interrupt use:
> >
> > /*
> >  * Allocate or free a physical upcall vector for the specified IRQ line.
> >  * @arg == pointer to physdev_irq structure.
> >  */
> > #define PHYSDEVOP_alloc_irq_vector      10
> > #define PHYSDEVOP_free_irq_vector       11
> > struct physdev_irq {
> >     /* IN */
> >     uint32_t irq;
> >     /* IN or OUT */
> >     uint32_t vector;
> > };
> >
> >
> > The new physdev_op collects all these structs in an union:
> >
> >
> > struct physdev_op {
> >     uint32_t cmd;
> >     union {
> >         struct physdev_irq_status_query      irq_status_query;
> >         struct physdev_set_iopl              set_iopl;
> >         struct physdev_set_iobitmap          set_iobitmap;
> >         struct physdev_apic                  apic_op;
> >         struct physdev_irq                   irq_op;
> >     } u;
> > };
> >
> > So, fill out the structure, set cmd to one of the above's #define,
> > then do the hypercall.
>
> PHYSDEVOP_eoi and PHYSDEVOP_irq_status_query looks a lot like what we're
> doing now with the old call;

Yes, that is right, but ...

> as long as we're not sharing IRQs with multiple guests, I'm not sure it
> makes a difference. 
> Maybe PHYSDEVOP_alloc_irq_vector/PHYSDEVOP_free_irq_vector could make a
> difference with what we do now (PHYSDEVOP_ASSIGN_VECTOR), although I
> don't seee in which way for now (vectors have not been alloctated at
> this point, PHYSDEVOP_free_irq_vector isn't that usefull for us).

... the difference comes when a PCI device is mapped into a DomU.
The Dom0 never knows when a PV DomU or HVM DomU is doing that.

For that reason the new way works in the way to always allocate
the irq vector before using it.

Christoph