Lin Ming
2012-Apr-10 15:13 UTC
[RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
This new physdev_op is added for Linux guest kernel to get the correct
nr_irqs_gsi value.
See below Linux kernel patch for detail explanation.
[RFC PATCH] xen: get correct nr_irqs_gsi value from hypervisor
http://marc.info/?l=xen-devel&m=133407004503365&w=2
Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
---
xen/arch/x86/physdev.c | 8 ++++++++
xen/include/public/physdev.h | 6 ++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 05fff9e..0912db0 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -688,6 +688,14 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
setup_gsi.polarity);
break;
}
+ case PHYSDEVOP_nr_irqs_gsi: {
+ struct physdev_nr_irqs_gsi out;
+
+ out.nr_irqs_gsi = nr_irqs_gsi;
+ ret = copy_to_guest(arg, &out, 1) ? -EFAULT : 0;
+
+ break;
+ }
case PHYSDEVOP_get_free_pirq: {
struct physdev_get_free_pirq out;
struct domain *d;
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index b78eeba..7856fc2 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -312,6 +312,12 @@ struct physdev_pci_device {
typedef struct physdev_pci_device physdev_pci_device_t;
DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_t);
+#define PHYSDEVOP_nr_irqs_gsi 29
+struct physdev_nr_irqs_gsi {
+ /* OUT */
+ uint32_t nr_irqs_gsi;
+};
+
/*
* Notify that some PIRQ-bound event channels have been unmasked.
* ** This command is obsolete since interface version 0x00030202 and is **
--
1.7.2.5
Jan Beulich
2012-Apr-10 15:33 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
>>> 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. Jan> See below Linux kernel patch for detail explanation. > > [RFC PATCH] xen: get correct nr_irqs_gsi value from hypervisor > http://marc.info/?l=xen-devel&m=133407004503365&w=2 > > Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn> > --- > xen/arch/x86/physdev.c | 8 ++++++++ > xen/include/public/physdev.h | 6 ++++++ > 2 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c > index 05fff9e..0912db0 100644 > --- a/xen/arch/x86/physdev.c > +++ b/xen/arch/x86/physdev.c > @@ -688,6 +688,14 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg) > setup_gsi.polarity); > break; > } > + case PHYSDEVOP_nr_irqs_gsi: { > + struct physdev_nr_irqs_gsi out; > + > + out.nr_irqs_gsi = nr_irqs_gsi; > + ret = copy_to_guest(arg, &out, 1) ? -EFAULT : 0; > + > + break; > + } > case PHYSDEVOP_get_free_pirq: { > struct physdev_get_free_pirq out; > struct domain *d; > diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h > index b78eeba..7856fc2 100644 > --- a/xen/include/public/physdev.h > +++ b/xen/include/public/physdev.h > @@ -312,6 +312,12 @@ struct physdev_pci_device { > typedef struct physdev_pci_device physdev_pci_device_t; > DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_t); > > +#define PHYSDEVOP_nr_irqs_gsi 29 > +struct physdev_nr_irqs_gsi { > + /* OUT */ > + uint32_t nr_irqs_gsi; > +}; > + > /* > * Notify that some PIRQ-bound event channels have been unmasked. > * ** This command is obsolete since interface version 0x00030202 and is ** > -- > 1.7.2.5 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Lin Ming
2012-Apr-10 15:50 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
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. How can we get the correct gsi_top value, which is used to set nr_irqs_gsi, without hypercall? The problem here is we don''t have a Xen version of io_apic_read in Linux kernel. Thanks, Lin Ming> > Jan
Jan Beulich
2012-Apr-10 16:04 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
>>> On 10.04.12 at 17:50, Lin Ming <mlin@ss.pku.edu.cn> 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.I understand all of the above.> How can we get the correct gsi_top value, which is used to set > nr_irqs_gsi, without hypercall? > > The problem here is we don''t have a Xen version of io_apic_read in Linux > kernel.Meaning you need to craft one if you need one, the more that struct io_apic_ops already exists. The traditional kernel was able to do so quite fine, and hence didn''t need any hypercall. Jan
Stefano Stabellini
2012-Apr-10 16:21 UTC
Re: [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. > > How can we get the correct gsi_top value, which is used to set > nr_irqs_gsi, without hypercall? > > 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
Zhang, Xiantao
2012-Apr-11 00:36 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
> -----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. > > > > How can we get the correct gsi_top value, which is used to set > > nr_irqs_gsi, without hypercall? > > > > 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=133295662314519This is not enough, seems fixed value are returned for each read ? Xiantao
Stefano Stabellini
2012-Apr-11 11:42 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
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. > > > > > > How can we get the correct gsi_top value, which is used to set > > > nr_irqs_gsi, without hypercall? > > > > > > 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 > > 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.
Ian Campbell
2012-Apr-11 11:49 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
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. > > > > > > > > How can we get the correct gsi_top value, which is used to set > > > > nr_irqs_gsi, without hypercall? > > > > > > > > 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=133295662314519Why was this patch never seen on xen-devel?> > 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... Ian.
Konrad Rzeszutek Wilk
2012-Apr-11 13:42 UTC
Re: [RFC PATCH] x86: Add a new physdev_op PHYSDEVOP_nr_irqs_gsi
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