Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Wed, Apr 11, 2012 at 12:49:12PM +0100, Ian Campbell wrote:
> On Wed, 2012-04-11 at 12:42 +0100, Stefano Stabellini wrote:
> > On Wed, 11 Apr 2012, Zhang, Xiantao wrote:
> > > > -----Original Message-----
> > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > > Sent: Wednesday, April 11, 2012 12:21 AM
> > > > To: Lin Ming
> > > > Cc: Jan Beulich; Konrad Rzeszutek Wilk; Zhang, Xiantao; xen-devel
> > > > Subject: Re: [Xen-devel] [RFC PATCH] x86: Add a new physdev_op
> > > > PHYSDEVOP_nr_irqs_gsi
> > > > 
> > > > On Tue, 10 Apr 2012, Lin Ming wrote:
> > > > > On Tue, 2012-04-10 at 16:33 +0100, Jan Beulich wrote:
> > > > > > >>> On 10.04.12 at 17:13, Lin Ming <mlin@ss.pku.edu.cn> wrote:
> > > > > > > This new physdev_op is added for Linux guest kernel to get the
> > > > > > > correct nr_irqs_gsi value.
> > > > > >
> > > > > > I'm not convinced this is really needed - the kernel can work out
> > > > > > the right number without any hypercall afaict.
> > > > >
> > > > > In Linux kernel:
> > > > >
> > > > > mp_register_ioapic(...):
> > > > >
> > > > >         entries = io_apic_get_redir_entries(idx);
> > > > >         gsi_cfg = mp_ioapic_gsi_routing(idx);
> > > > >         gsi_cfg->gsi_base = gsi_base;
> > > > >         gsi_cfg->gsi_end = gsi_base + entries - 1;
> > > > >
> > > > >         /*
> > > > >          * The number of IO-APIC IRQ registers (== #pins):
> > > > >          */
> > > > >         ioapics[idx].nr_registers = entries;
> > > > >
> > > > >         if (gsi_cfg->gsi_end >= gsi_top)
> > > > >                 gsi_top = gsi_cfg->gsi_end + 1;
> > > > >
> > > > > io_apic_get_redir_entries calls io_apic_read(), which returns wrong
> > > > > value(0xFFFFFFFF) on Xen Dom0 kernel.$

It is actually now 0xfd.

> > > > >
> > > > > How can we get the correct gsi_top value, which is used to set
> > > > > nr_irqs_gsi, without hypercall?

So, why is this important?

> > > > >
> > > > > The problem here is we don't have a Xen version of io_apic_read in
> > > > > Linux kernel.
> > > > 
> > > > Actually we do: http://marc.info/?l=linux-kernel&m=133295662314519
> 
> Why was this patch never seen on xen-devel?

Probably b/c I forgot to put the CC on it. Either way it first
has to go through Ingo's tree for 3.5.

>  
> > > This is not enough, seems fixed value are returned for each read ?
> > 
> > Yes, it looks like we are always returning 0x00170020, that means 24
> > entries that I guess is correct for the vast majority of io_apics.
> > 
> > We could limit ourselves to adding a comment saying "we assume the
> > io_apic has 24 redir entries".
> > 
> > Or we could use PHYSDEVOP_apic_read to do an actual io_apic read for
> > reg 0x1.
> 
> This seems like the obviously right thing to do, compared with
> hardcoding 24...

Could do as well.
> 
> Ian.
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel