Re: [Xen-devel] [PATCH 2/5] x86/ioapic: introduce helper to fetch triggering mode of GSI

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Apr 18, 2017 at 06:19:34AM -0600, Jan Beulich wrote:
> >>> On 27.03.17 at 12:44, <roger.pau@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/io_apic.c
> > +++ b/xen/arch/x86/io_apic.c
> > @@ -2261,6 +2261,28 @@ int io_apic_set_pci_routing (int ioapic, int pin, 
> > int irq, int edge_level, int a
> >      return 0;
> >  }
> >  
> > +unsigned int io_apic_get_gsi_trigger(unsigned int gsi)
> 
> bool
> 
> > +{
> > +    struct IO_APIC_route_entry entry;
> > +    unsigned int ioapic, base_gsi;
> > +
> > +    ASSERT(gsi < nr_irqs_gsi);
> > +
> > +    /* For GSI type find if the GSI is level or edge triggered */
> 
> The comment looks like it wants to go ahead of the function instead
> of here. And what's "GSI type"?

GSI type of interrupt, better worded as "For a given GSI find...". But I'm
thinking that it might be better to fetch this from the Dom0 vIO APIC instead.

> > +    for ( ioapic = 0; ioapic < nr_ioapics; ioapic++ )
> > +    {
> > +        base_gsi = io_apic_gsi_base(ioapic);
> > +
> > +        if ( gsi >= base_gsi && gsi < base_gsi + nr_ioapic_entries[ioapic] 
> > )
> > +            break;
> > +    }
> > +    ASSERT(ioapic < nr_ioapics);
> > +
> > +    entry = ioapic_read_entry(ioapic, gsi - base_gsi, 0);
> > +
> > +    return entry.trigger;
> > +}
> 
> It is presumably not entirely without reason that you add this function
> without having an immediate user (in existing code) for it: The trigger
> bit is entirely meaningless in a masked RTE.

As said above, it might be better to fetch this from the Dom0 vIO APIC. The
caller is added in the next patch, but if I fetch this from the vIO APIC I
don't need this patch anymore.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel