thr3ads.net - Xen devel - [Xen-devel] [GIT PULL REPOST] xen/dom0/apic-ops: Xen dom0 APIC changes [May 2009]

If this information is useful, please help other people find it:
Share via:

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

[Xen-devel] [GIT PULL REPOST] xen/dom0/apic-ops: Xen dom0 APIC changes

Hi Ingo,

This is a repost of the changes for Xen''s apic hooks.  This is
unchanged
from the last time I posted it.

These changes allow us to route interrupts via Xen''s event channel
mechanism, so that hardware interrupts can be distributed to the
appropriate guest domain.

It adds a minimal ioapic_ops interface to allow us to transform the
low-level IO-APIC accesses from mmio into hypercalls.  This is sufficient
for our needs because the accesses themselves are going to the real
hardware IO APICs, and we want all the other logic above generating
the requests.  You initially approved of this approach.

More recently, you proposed that we add a higher-level ioapic driver
abstraction to encompass not only Xen''s current needs, but also to
address
KVM possible use for such an interface.  This seems like a good idea in
general, but not something we can do in time for the next merge window.

I propose that you take the current minimal approach, and we can
develop something fuller when we can see what KVM''s needs are, and have
enough time to do a proper job.

There are no other outstanding issues.

Please pull it into tip.git x86/xen/dom0/apic

Thanks,
	J

The following changes since commit ce791368bb4a53d05e78e1588bac0aacde8db84c:
  Jeremy Fitzhardinge (1):
        xen/i386: make sure initial VGA/ISA mappings are not overridden

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
for-ingo/xen/dom0/apic-ops

Gerd Hoffmann (2):
      xen: set pirq name to something useful.
      xen: fix legacy irq setup, make ioapic-less machines work.

Ian Campbell (1):
      xen: pre-initialize legacy irqs early

Jeremy Fitzhardinge (14):
      xen/dom0: handle acpi lapic parsing in Xen dom0
      x86: add io_apic_ops to allow interception
      xen: implement io_apic_ops
      xen: create dummy ioapic mapping
      xen: implement pirq type event channels
      x86/io_apic: add get_nr_irqs_gsi()
      xen/apic: identity map gsi->irqs
      xen: direct irq registration to pirq event channels
      xen: bind pirq to vector and event channel
      xen: don''t setup acpi interrupt unless there is one
      xen: use acpi_get_override_irq() to get triggering for legacy irqs
      xen: initialize irq 0 too
      xen: dynamically allocate irq & event structures
      xen: disable MSI

 arch/x86/include/asm/io_apic.h |   10 ++
 arch/x86/include/asm/xen/pci.h |   13 ++
 arch/x86/kernel/acpi/boot.c    |   18 +++-
 arch/x86/kernel/apic/io_apic.c |   55 ++++++++-
 arch/x86/xen/Kconfig           |   11 ++
 arch/x86/xen/Makefile          |    3 +-
 arch/x86/xen/apic.c            |   69 ++++++++++
 arch/x86/xen/enlighten.c       |    2 +
 arch/x86/xen/mmu.c             |   10 ++
 arch/x86/xen/pci.c             |   86 +++++++++++++
 arch/x86/xen/xen-ops.h         |    6 +
 drivers/pci/pci.h              |    2 -
 drivers/xen/events.c           |  273 ++++++++++++++++++++++++++++++++++++++--
 include/linux/pci.h            |    6 +
 include/xen/events.h           |   19 +++
 15 files changed, 568 insertions(+), 15 deletions(-)
 create mode 100644 arch/x86/include/asm/xen/pci.h
 create mode 100644 arch/x86/xen/apic.c
 create mode 100644 arch/x86/xen/pci.c


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0

When running in Xen dom0, we still want to parse the ACPI tables to
find out about local and IO apics, but we don''t want to actually use
the lapics.

Put a couple of tests for Xen to prevent lapics from being mapped or
accessed.  This is very Xen-specific behaviour, so there didn''t seem to
be any point in adding more indirection.

[ Impact: ignore local apics, which are not usable under Xen ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Reviewed-by: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kernel/acpi/boot.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 723989d..4147e0c 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -41,6 +41,8 @@
 #include <asm/mpspec.h>
 #include <asm/smp.h>
 
+#include <asm/xen/hypervisor.h>
+
 static int __initdata acpi_force = 0;
 u32 acpi_rsdt_forced;
 #ifdef	CONFIG_ACPI
@@ -218,6 +220,10 @@ static void __cpuinit acpi_register_lapic(int id, u8
enabled)
 {
 	unsigned int ver = 0;
 
+	/* We don''t want to register lapics when in Xen dom0 */
+	if (xen_initial_domain())
+		return;
+
 	if (!enabled) {
 		++disabled_cpus;
 		return;
@@ -802,6 +808,10 @@ static int __init acpi_parse_fadt(struct acpi_table_header
*table)
 
 static void __init acpi_register_lapic_address(unsigned long address)
 {
+	/* Xen dom0 doesn''t have usable lapics */
+	if (xen_initial_domain())
+		return;
+
 	mp_lapic_addr = address;
 
 	set_fixmap_nocache(FIX_APIC_BASE, address);
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 02/17] x86: add io_apic_ops to allow interception

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Xen dom0 needs to paravirtualize IO operations to the IO APIC, so add
a io_apic_ops for it to intercept.  Do this as ops structure because
there''s at least some chance that another paravirtualized environment
may want to intercept these.

[ Impact: indirect IO APIC access via io_apic_ops ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/io_apic.h |    9 +++++++
 arch/x86/kernel/apic/io_apic.c |   50 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 9d826e4..8cbfe73 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -21,6 +21,15 @@
 #define IO_APIC_REDIR_LEVEL_TRIGGER	(1 << 15)
 #define IO_APIC_REDIR_MASKED		(1 << 16)
 
+struct io_apic_ops {
+	void (*init)(void);
+	unsigned int (*read)(unsigned int apic, unsigned int reg);
+	void (*write)(unsigned int apic, unsigned int reg, unsigned int value);
+	void (*modify)(unsigned int apic, unsigned int reg, unsigned int value);
+};
+
+void __init set_io_apic_ops(const struct io_apic_ops *);
+
 /*
  * The structure of the IO-APIC:
  */
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 30da617..c24f116 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -66,6 +66,25 @@
 
 #define __apicdebuginit(type) static type __init
 
+static void __init __ioapic_init_mappings(void);
+static unsigned int __io_apic_read(unsigned int apic, unsigned int reg);
+static void __io_apic_write(unsigned int apic, unsigned int reg,
+			    unsigned int val);
+static void __io_apic_modify(unsigned int apic, unsigned int reg,
+			     unsigned int val);
+
+static struct io_apic_ops io_apic_ops = {
+	.init = __ioapic_init_mappings,
+	.read = __io_apic_read,
+	.write = __io_apic_write,
+	.modify = __io_apic_modify,
+};
+
+void __init set_io_apic_ops(const struct io_apic_ops *ops)
+{
+	io_apic_ops = *ops;
+}
+
 /*
  *      Is the SiS APIC rmw bug present ?
  *      -1 = don''t know, 0 = no, 1 = yes
@@ -385,6 +404,24 @@ set_extra_move_desc(struct irq_desc *desc, const struct
cpumask *mask)
 }
 #endif
 
+static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
+{
+	return io_apic_ops.read(apic, reg);
+}
+
+static inline void io_apic_write(unsigned int apic, unsigned int reg,
+				 unsigned int value)
+{
+	io_apic_ops.write(apic, reg, value);
+}
+
+static inline void io_apic_modify(unsigned int apic, unsigned int reg,
+				  unsigned int value)
+{
+	io_apic_ops.modify(apic, reg, value);
+}
+
+
 struct io_apic {
 	unsigned int index;
 	unsigned int unused[3];
@@ -405,14 +442,15 @@ static inline void io_apic_eoi(unsigned int apic, unsigned
int vector)
 	writel(vector, &io_apic->eoi);
 }
 
-static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
+static unsigned int __io_apic_read(unsigned int apic, unsigned int reg)
 {
 	struct io_apic __iomem *io_apic = io_apic_base(apic);
 	writel(reg, &io_apic->index);
 	return readl(&io_apic->data);
 }
 
-static inline void io_apic_write(unsigned int apic, unsigned int reg, unsigned
int value)
+static void __io_apic_write(unsigned int apic, unsigned int reg,
+			    unsigned int value)
 {
 	struct io_apic __iomem *io_apic = io_apic_base(apic);
 	writel(reg, &io_apic->index);
@@ -425,7 +463,8 @@ static inline void io_apic_write(unsigned int apic, unsigned
int reg, unsigned i
  *
  * Older SiS APIC requires we rewrite the index register
  */
-static inline void io_apic_modify(unsigned int apic, unsigned int reg, unsigned
int value)
+static void __io_apic_modify(unsigned int apic, unsigned int reg,
+			     unsigned int value)
 {
 	struct io_apic __iomem *io_apic = io_apic_base(apic);
 
@@ -4141,6 +4180,11 @@ static struct resource * __init
ioapic_setup_resources(void)
 
 void __init ioapic_init_mappings(void)
 {
+	io_apic_ops.init();
+}
+
+static void __init __ioapic_init_mappings(void)
+{
 	unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
 	struct resource *ioapic_res;
 	int i;
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 03/17] xen: implement io_apic_ops

Writes to the IO APIC are paravirtualized via hypercalls, so implement
the appropriate operations.

[ Impact: implement Xen interface for io_apic_ops ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/Makefile    |    2 +-
 arch/x86/xen/apic.c      |   64 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/xen/enlighten.c |    2 +
 arch/x86/xen/xen-ops.h   |    6 ++++
 4 files changed, 73 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/xen/apic.c

diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index c4cda96..73ecb74 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -11,4 +11,4 @@ obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 
 obj-$(CONFIG_SMP)		+= smp.o spinlock.o
 obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
-obj-$(CONFIG_XEN_DOM0)		+= vga.o
+obj-$(CONFIG_XEN_DOM0)		+= vga.o apic.o
diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
new file mode 100644
index 0000000..8ae563c
--- /dev/null
+++ b/arch/x86/xen/apic.c
@@ -0,0 +1,64 @@
+#include <linux/kernel.h>
+#include <linux/threads.h>
+#include <linux/bitmap.h>
+
+#include <asm/io_apic.h>
+#include <asm/acpi.h>
+
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+
+#include <xen/interface/xen.h>
+#include <xen/interface/physdev.h>
+
+static void __init xen_io_apic_init(void)
+{
+}
+
+static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
+{
+	struct physdev_apic apic_op;
+	int ret;
+
+	apic_op.apic_physbase = mp_ioapics[apic].apicaddr;
+	apic_op.reg = reg;
+	ret = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, &apic_op);
+	if (ret)
+		BUG();
+	return apic_op.value;
+}
+
+
+static void xen_io_apic_write(unsigned int apic, unsigned int reg, unsigned int
value)
+{
+	struct physdev_apic apic_op;
+
+	apic_op.apic_physbase = mp_ioapics[apic].apicaddr;
+	apic_op.reg = reg;
+	apic_op.value = value;
+	if (HYPERVISOR_physdev_op(PHYSDEVOP_apic_write, &apic_op))
+		BUG();
+}
+
+static struct io_apic_ops __initdata xen_ioapic_ops = {
+	.init = xen_io_apic_init,
+	.read = xen_io_apic_read,
+	.write = xen_io_apic_write,
+	.modify = xen_io_apic_write,
+};
+
+void xen_init_apic(void)
+{
+	if (!xen_initial_domain())
+		return;
+
+	set_io_apic_ops(&xen_ioapic_ops);
+
+#ifdef CONFIG_ACPI
+	/*
+	 * Pretend ACPI found our lapic even though we''ve disabled it,
+ 	 * to prevent MP tables from setting up lapics.
+ 	 */
+	acpi_lapic = 1;
+#endif
+}
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 12e4d9c..3a4932a 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1085,6 +1085,8 @@ asmlinkage void __init xen_start_kernel(void)
 		set_iopl.iopl = 1;
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_set_iopl, &set_iopl) == -1)
 			BUG();
+
+		xen_init_apic();
 	}
 
 	/* set the limit of our address space */
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 40abcef..0853949 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -76,13 +76,19 @@ struct dom0_vga_console_info;
 
 #ifdef CONFIG_XEN_DOM0
 void xen_init_vga(const struct dom0_vga_console_info *, size_t size);
+void xen_init_apic(void);
 #else
 static inline void xen_init_vga(const struct dom0_vga_console_info *info,
 				size_t size)
 {
 }
+
+static inline void xen_init_apic(void)
+{
+}
 #endif
 
+
 /* Declare an asm function, along with symbols needed to make it
    inlineable */
 #define DECL_ASM(ret, name, ...)		\
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 04/17] xen: create dummy ioapic mapping

We don''t allow direct access to the IO apic, so make sure that any
request to map it just "maps" non-present pages.  We should see any
attempts at direct access explode nicely.

[ Impact: debuggability (make failures obvious) ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/mmu.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 331e52d..139c8de 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1919,6 +1919,16 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t
phys, pgprot_t prot)
 		pte = pfn_pte(phys, prot);
 		break;
 
+#ifdef CONFIG_X86_IO_APIC
+	case FIX_IO_APIC_BASE_0 ... FIX_IO_APIC_BASE_END:
+		/*
+		 * We just don''t map the IO APIC - all access is via
+		 * hypercalls.  Keep the address in the pte for reference.
+		 */
+		pte = pfn_pte(phys, PAGE_NONE);
+		break;
+#endif
+
 	case FIX_PARAVIRT_BOOTMAP:
 		/* This is an MFN, but it isn''t an IO mapping from the
 		   IO domain */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 05/17] xen: implement pirq type event channels

A privileged PV Xen domain can get direct access to hardware.  In
order for this to be useful, it must be able to get hardware
interrupts.

Being a PV Xen domain, all interrupts are delivered as event channels.
PIRQ event channels are bound to a pirq number and an interrupt
vector.  When a IO APIC raises a hardware interrupt on that vector, it
is delivered as an event channel, which we can deliver to the
appropriate device driver(s).

This patch simply implements the infrastructure for dealing with pirq
event channels.

[ Impact: integrate hardware interrupts into Xen''s event scheme ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 drivers/xen/events.c |  245 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/xen/events.h |   11 +++
 2 files changed, 253 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 97f4b39..fd98c19 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -16,7 +16,7 @@
  *    (typically dom0).
  * 2. VIRQs, typically used for timers.  These are per-cpu events.
  * 3. IPIs.
- * 4. Hardware interrupts. Not supported at present.
+ * 4. PIRQs - Hardware interrupts.
  *
  * Jeremy Fitzhardinge <jeremy@xensource.com>, XenSource Inc, 2007
  */
@@ -40,6 +40,9 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/event_channel.h>
 
+/* Leave low irqs free for identity mapping */
+#define LEGACY_IRQS	16
+
 /*
  * This lock protects updates to the following mapping and reference-count
  * arrays. The lock does not need to be acquired to read the mapping tables.
@@ -83,10 +86,12 @@ struct irq_info
 		enum ipi_vector ipi;
 		struct {
 			unsigned short gsi;
-			unsigned short vector;
+			unsigned char vector;
+			unsigned char flags;
 		} pirq;
 	} u;
 };
+#define PIRQ_NEEDS_EOI	(1 << 0)
 
 static struct irq_info irq_info[NR_IRQS];
 
@@ -106,6 +111,7 @@ static inline unsigned long *cpu_evtchn_mask(int cpu)
 #define VALID_EVTCHN(chn)	((chn) != 0)
 
 static struct irq_chip xen_dynamic_chip;
+static struct irq_chip xen_pirq_chip;
 
 /* Constructor for packed IRQ information. */
 static struct irq_info mk_unbound_info(void)
@@ -218,6 +224,15 @@ static unsigned int cpu_from_evtchn(unsigned int evtchn)
 	return ret;
 }
 
+static bool pirq_needs_eoi(unsigned irq)
+{
+	struct irq_info *info = info_for_irq(irq);
+
+	BUG_ON(info->type != IRQT_PIRQ);
+
+	return info->u.pirq.flags & PIRQ_NEEDS_EOI;
+}
+
 static inline unsigned long active_evtchns(unsigned int cpu,
 					   struct shared_info *sh,
 					   unsigned int idx)
@@ -334,7 +349,7 @@ static int find_unbound_irq(void)
 	int irq;
 	struct irq_desc *desc;
 
-	for (irq = 0; irq < nr_irqs; irq++)
+	for (irq = LEGACY_IRQS; irq < nr_irqs; irq++)
 		if (irq_info[irq].type == IRQT_UNBOUND)
 			break;
 
@@ -350,6 +365,210 @@ static int find_unbound_irq(void)
 	return irq;
 }
 
+static bool identity_mapped_irq(unsigned irq)
+{
+	/* only identity map legacy irqs */
+	return irq < LEGACY_IRQS;
+}
+
+static void pirq_unmask_notify(int irq)
+{
+	struct physdev_eoi eoi = { .irq = irq };
+
+	if (unlikely(pirq_needs_eoi(irq))) {
+		int rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
+		WARN_ON(rc);
+	}
+}
+
+static void pirq_query_unmask(int irq)
+{
+	struct physdev_irq_status_query irq_status;
+	struct irq_info *info = info_for_irq(irq);
+
+	BUG_ON(info->type != IRQT_PIRQ);
+
+	irq_status.irq = irq;
+	if (HYPERVISOR_physdev_op(PHYSDEVOP_irq_status_query, &irq_status))
+		irq_status.flags = 0;
+
+	info->u.pirq.flags &= ~PIRQ_NEEDS_EOI;
+	if (irq_status.flags & XENIRQSTAT_needs_eoi)
+		info->u.pirq.flags |= PIRQ_NEEDS_EOI;
+}
+
+static bool probing_irq(int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	return desc && desc->action == NULL;
+}
+
+static unsigned int startup_pirq(unsigned int irq)
+{
+	struct evtchn_bind_pirq bind_pirq;
+	struct irq_info *info = info_for_irq(irq);
+	int evtchn = evtchn_from_irq(irq);
+
+	BUG_ON(info->type != IRQT_PIRQ);
+
+	if (VALID_EVTCHN(evtchn))
+		goto out;
+
+	bind_pirq.pirq = irq;
+	/* NB. We are happy to share unless we are probing. */
+	bind_pirq.flags = probing_irq(irq) ? 0 : BIND_PIRQ__WILL_SHARE;
+	if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &bind_pirq) != 0) {
+		if (!probing_irq(irq))
+			printk(KERN_INFO "Failed to obtain physical IRQ %d\n",
+			       irq);
+		return 0;
+	}
+	evtchn = bind_pirq.port;
+
+	pirq_query_unmask(irq);
+
+	evtchn_to_irq[evtchn] = irq;
+	bind_evtchn_to_cpu(evtchn, 0);
+	info->evtchn = evtchn;
+
+ out:
+	unmask_evtchn(evtchn);
+	pirq_unmask_notify(irq);
+
+	return 0;
+}
+
+static void shutdown_pirq(unsigned int irq)
+{
+	struct evtchn_close close;
+	struct irq_info *info = info_for_irq(irq);
+	int evtchn = evtchn_from_irq(irq);
+
+	BUG_ON(info->type != IRQT_PIRQ);
+
+	if (!VALID_EVTCHN(evtchn))
+		return;
+
+	mask_evtchn(evtchn);
+
+	close.port = evtchn;
+	if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
+		BUG();
+
+	bind_evtchn_to_cpu(evtchn, 0);
+	evtchn_to_irq[evtchn] = -1;
+	info->evtchn = 0;
+}
+
+static void enable_pirq(unsigned int irq)
+{
+	startup_pirq(irq);
+}
+
+static void disable_pirq(unsigned int irq)
+{
+}
+
+static void ack_pirq(unsigned int irq)
+{
+	int evtchn = evtchn_from_irq(irq);
+
+	move_native_irq(irq);
+
+	if (VALID_EVTCHN(evtchn)) {
+		mask_evtchn(evtchn);
+		clear_evtchn(evtchn);
+	}
+}
+
+static void end_pirq(unsigned int irq)
+{
+	int evtchn = evtchn_from_irq(irq);
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (WARN_ON(!desc))
+		return;
+
+	if ((desc->status & (IRQ_DISABLED|IRQ_PENDING)) =+	   
(IRQ_DISABLED|IRQ_PENDING)) {
+		shutdown_pirq(irq);
+	} else if (VALID_EVTCHN(evtchn)) {
+		unmask_evtchn(evtchn);
+		pirq_unmask_notify(irq);
+	}
+}
+
+static int find_irq_by_gsi(unsigned gsi)
+{
+	int irq;
+
+	for (irq = 0; irq < NR_IRQS; irq++) {
+		struct irq_info *info = info_for_irq(irq);
+
+		if (info == NULL || info->type != IRQT_PIRQ)
+			continue;
+
+		if (gsi_from_irq(irq) == gsi)
+			return irq;
+	}
+
+	return -1;
+}
+
+/*
+ * Allocate a physical irq, along with a vector.  We don''t assign an
+ * event channel until the irq actually started up.  Return an
+ * existing irq if we''ve already got one for the gsi.
+ */
+int xen_allocate_pirq(unsigned gsi)
+{
+	int irq;
+	struct physdev_irq irq_op;
+
+	spin_lock(&irq_mapping_update_lock);
+
+	irq = find_irq_by_gsi(gsi);
+	if (irq != -1) {
+		printk(KERN_INFO "xen_allocate_pirq: returning irq %d for gsi
%u\n",
+		       irq, gsi);
+		goto out;	/* XXX need refcount? */
+	}
+
+	if (identity_mapped_irq(gsi)) {
+		irq = gsi;
+		dynamic_irq_init(irq);
+	} else
+		irq = find_unbound_irq();
+
+	set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
+				      handle_level_irq, "pirq");
+
+	irq_op.irq = irq;
+	if (HYPERVISOR_physdev_op(PHYSDEVOP_alloc_irq_vector, &irq_op)) {
+		dynamic_irq_cleanup(irq);
+		irq = -ENOSPC;
+		goto out;
+	}
+
+	irq_info[irq] = mk_pirq_info(0, gsi, irq_op.vector);
+
+out:
+	spin_unlock(&irq_mapping_update_lock);
+
+	return irq;
+}
+
+int xen_vector_from_irq(unsigned irq)
+{
+	return vector_from_irq(irq);
+}
+
+int xen_gsi_from_irq(unsigned irq)
+{
+	return gsi_from_irq(irq);
+}
+
 int bind_evtchn_to_irq(unsigned int evtchn)
 {
 	int irq;
@@ -922,6 +1141,26 @@ static struct irq_chip xen_dynamic_chip __read_mostly = {
 	.retrigger	= retrigger_dynirq,
 };
 
+static struct irq_chip xen_pirq_chip __read_mostly = {
+	.name		= "xen-pirq",
+
+	.startup	= startup_pirq,
+	.shutdown	= shutdown_pirq,
+
+	.enable		= enable_pirq,
+	.unmask		= enable_pirq,
+
+	.disable	= disable_pirq,
+	.mask		= disable_pirq,
+
+	.ack		= ack_pirq,
+	.end		= end_pirq,
+
+	.set_affinity	= set_affinity_irq,
+
+	.retrigger	= retrigger_dynirq,
+};
+
 void __init xen_init_IRQ(void)
 {
 	int i;
diff --git a/include/xen/events.h b/include/xen/events.h
index 9f24b64..e5b541d 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -58,4 +58,15 @@ void xen_poll_irq(int irq);
 /* Determine the IRQ which is bound to an event channel */
 unsigned irq_from_evtchn(unsigned int evtchn);
 
+/* Allocate an irq for a physical interrupt, given a gsi.  "Legacy"
+   GSIs are identity mapped; others are dynamically allocated as
+   usual. */
+int xen_allocate_pirq(unsigned gsi);
+
+/* Return vector allocated to pirq */
+int xen_vector_from_irq(unsigned pirq);
+
+/* Return gsi allocated to pirq */
+int xen_gsi_from_irq(unsigned pirq);
+
 #endif	/* _XEN_EVENTS_H */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 06/17] x86/io_apic: add get_nr_irqs_gsi()

From: Jeremy Fitzhardinge <jeremy@f9-builder.(none)>

Add get_nr_irqs_gsi() to return nr_irqs_gsi.  Xen will use this to
determine how many irqs it needs to reserve for hardware irqs.

[ Impact: new interface to get max GSI ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Reviewed-by: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/include/asm/io_apic.h |    1 +
 arch/x86/kernel/apic/io_apic.c |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 8cbfe73..e33ccb7 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -181,6 +181,7 @@ extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
 #endif
 
 extern void probe_nr_irqs_gsi(void);
+extern int get_nr_irqs_gsi(void);
 
 extern int setup_ioapic_entry(int apic, int irq,
 			      struct IO_APIC_route_entry *entry,
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index c24f116..07dc530 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3917,6 +3917,11 @@ void __init probe_nr_irqs_gsi(void)
 	printk(KERN_DEBUG "nr_irqs_gsi: %d\n", nr_irqs_gsi);
 }
 
+int get_nr_irqs_gsi(void)
+{
+	return nr_irqs_gsi;
+}
+
 #ifdef CONFIG_SPARSE_IRQ
 int __init arch_probe_nr_irqs(void)
 {
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 07/17] xen/apic: identity map gsi->irqs

From: Jeremy Fitzhardinge <jeremy@f9-builder.(none)>

Reserve the lower irq range for use for hardware interrupts so we
can identity-map them.

[ Impact: preserve compat with native ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 drivers/xen/events.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index fd98c19..88395bb 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -31,6 +31,7 @@
 #include <asm/ptrace.h>
 #include <asm/irq.h>
 #include <asm/idle.h>
+#include <asm/io_apic.h>
 #include <asm/sync_bitops.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
@@ -40,9 +41,6 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/event_channel.h>
 
-/* Leave low irqs free for identity mapping */
-#define LEGACY_IRQS	16
-
 /*
  * This lock protects updates to the following mapping and reference-count
  * arrays. The lock does not need to be acquired to read the mapping tables.
@@ -344,12 +342,24 @@ static void unmask_evtchn(int port)
 	put_cpu();
 }
 
+static int get_nr_hw_irqs(void)
+{
+	int ret = 1;
+
+#ifdef CONFIG_X86_IO_APIC
+	ret = get_nr_irqs_gsi();
+#endif
+
+	return ret;
+}
+
 static int find_unbound_irq(void)
 {
 	int irq;
 	struct irq_desc *desc;
+	int start = get_nr_hw_irqs();
 
-	for (irq = LEGACY_IRQS; irq < nr_irqs; irq++)
+	for (irq = start; irq < nr_irqs; irq++)
 		if (irq_info[irq].type == IRQT_UNBOUND)
 			break;
 
@@ -367,8 +377,8 @@ static int find_unbound_irq(void)
 
 static bool identity_mapped_irq(unsigned irq)
 {
-	/* only identity map legacy irqs */
-	return irq < LEGACY_IRQS;
+	/* identity map all the hardware irqs */
+	return irq < get_nr_hw_irqs();
 }
 
 static void pirq_unmask_notify(int irq)
@@ -537,6 +547,7 @@ int xen_allocate_pirq(unsigned gsi)
 
 	if (identity_mapped_irq(gsi)) {
 		irq = gsi;
+		irq_to_desc_alloc_cpu(irq, 0);
 		dynamic_irq_init(irq);
 	} else
 		irq = find_unbound_irq();
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 08/17] xen: direct irq registration to pirq event channels

This patch puts the hooks into place so that when the interrupt
subsystem registers an irq, it gets routed via Xen (if we''re running
under Xen).

The first step is to get a gsi for a particular device+pin.  We use
the normal acpi interrupt routing to do the mapping.

We reserve enough irq space to fit the hardware interrupt sources in,
so we can allocate the irq == gsi, as we do in the native case;
software events will get allocated irqs above that.

Having allocated an irq, we ask Xen to allocate a vector, and then
bind that pirq/vector to an event channel.  When the hardware raises
an interrupt on a vector, Xen signals us on the corresponding event
channel, which gets routed to the irq and delivered to the appropriate
device driver.

This patch does everything except set up the IO APIC pin routing to
the vector.

[ Impact: route hardware interrupts via Xen ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/xen/pci.h |   13 +++++++++++
 arch/x86/kernel/acpi/boot.c    |    8 ++++++-
 arch/x86/xen/Kconfig           |   11 +++++++++
 arch/x86/xen/Makefile          |    1 +
 arch/x86/xen/pci.c             |   47 ++++++++++++++++++++++++++++++++++++++++
 drivers/xen/events.c           |    6 ++++-
 include/xen/events.h           |    8 ++++++
 7 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/include/asm/xen/pci.h
 create mode 100644 arch/x86/xen/pci.c

diff --git a/arch/x86/include/asm/xen/pci.h b/arch/x86/include/asm/xen/pci.h
new file mode 100644
index 0000000..0563fc6
--- /dev/null
+++ b/arch/x86/include/asm/xen/pci.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_X86_XEN_PCI_H
+#define _ASM_X86_XEN_PCI_H
+
+#ifdef CONFIG_XEN_DOM0_PCI
+int xen_register_gsi(u32 gsi, int triggering, int polarity);
+#else
+static inline int xen_register_gsi(u32 gsi, int triggering, int polarity)
+{
+	return -1;
+}
+#endif
+
+#endif	/* _ASM_X86_XEN_PCI_H */
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 4147e0c..d4de1c2 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -41,6 +41,8 @@
 #include <asm/mpspec.h>
 #include <asm/smp.h>
 
+#include <asm/xen/pci.h>
+
 #include <asm/xen/hypervisor.h>
 
 static int __initdata acpi_force = 0;
@@ -530,9 +532,13 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
  */
 int acpi_register_gsi(u32 gsi, int triggering, int polarity)
 {
-	unsigned int irq;
+	int irq;
 	unsigned int plat_gsi = gsi;
 
+	irq = xen_register_gsi(gsi, triggering, polarity);
+	if (irq >= 0)
+		return irq;
+
 #ifdef CONFIG_PCI
 	/*
 	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index fe69286..42e9f0a 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -37,6 +37,17 @@ config XEN_DEBUG_FS
 	  Enable statistics output and various tuning options in debugfs.
 	  Enabling this option may incur a significant performance overhead.
 
+config XEN_PCI_PASSTHROUGH
+       bool #"Enable support for Xen PCI passthrough devices"
+       depends on XEN && PCI
+       help
+         Enable support for passing PCI devices through to
+	 unprivileged domains. (COMPLETELY UNTESTED)
+
+config XEN_DOM0_PCI
+       def_bool y
+       depends on XEN_DOM0 && PCI
+
 config XEN_DOM0
 	bool "Enable Xen privileged domain support"
 	depends on XEN && X86_IO_APIC && ACPI
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 73ecb74..639965a 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -12,3 +12,4 @@ obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 obj-$(CONFIG_SMP)		+= smp.o spinlock.o
 obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
 obj-$(CONFIG_XEN_DOM0)		+= vga.o apic.o
+obj-$(CONFIG_XEN_DOM0_PCI)	+= pci.o
\ No newline at end of file
diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
new file mode 100644
index 0000000..f450007
--- /dev/null
+++ b/arch/x86/xen/pci.c
@@ -0,0 +1,47 @@
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+
+#include <asm/pci_x86.h>
+
+#include <asm/xen/hypervisor.h>
+
+#include <xen/interface/xen.h>
+#include <xen/events.h>
+
+#include "xen-ops.h"
+
+int xen_register_gsi(u32 gsi, int triggering, int polarity)
+{
+	int irq;
+
+	if (!xen_domain())
+		return -1;
+
+	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity
%d\n",
+	       gsi, triggering, polarity);
+
+	irq = xen_allocate_pirq(gsi);
+
+	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
+
+	return irq;
+}
+
+void __init xen_setup_pirqs(void)
+{
+#ifdef CONFIG_ACPI
+	int irq;
+
+	/*
+	 * Set up acpi interrupt in acpi_gbl_FADT.sci_interrupt.
+	 */
+	irq = xen_allocate_pirq(acpi_gbl_FADT.sci_interrupt);
+
+	printk(KERN_INFO "xen: allocated irq %d for acpi %d\n",
+	       irq, acpi_gbl_FADT.sci_interrupt);
+
+	/* Blerk. */
+	acpi_gbl_FADT.sci_interrupt = irq;
+#endif
+}
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 88395bb..968e927 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -419,6 +419,7 @@ static unsigned int startup_pirq(unsigned int irq)
 	struct evtchn_bind_pirq bind_pirq;
 	struct irq_info *info = info_for_irq(irq);
 	int evtchn = evtchn_from_irq(irq);
+	int rc;
 
 	BUG_ON(info->type != IRQT_PIRQ);
 
@@ -428,7 +429,8 @@ static unsigned int startup_pirq(unsigned int irq)
 	bind_pirq.pirq = irq;
 	/* NB. We are happy to share unless we are probing. */
 	bind_pirq.flags = probing_irq(irq) ? 0 : BIND_PIRQ__WILL_SHARE;
-	if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &bind_pirq) != 0) {
+	rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &bind_pirq);
+	if (rc != 0) {
 		if (!probing_irq(irq))
 			printk(KERN_INFO "Failed to obtain physical IRQ %d\n",
 			       irq);
@@ -1187,4 +1189,6 @@ void __init xen_init_IRQ(void)
 		mask_evtchn(i);
 
 	irq_ctx_init(smp_processor_id());
+
+	xen_setup_pirqs();
 }
diff --git a/include/xen/events.h b/include/xen/events.h
index e5b541d..6fe4863 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -69,4 +69,12 @@ int xen_vector_from_irq(unsigned pirq);
 /* Return gsi allocated to pirq */
 int xen_gsi_from_irq(unsigned pirq);
 
+#ifdef CONFIG_XEN_DOM0_PCI
+void xen_setup_pirqs(void);
+#else
+static inline void xen_setup_pirqs(void)
+{
+}
+#endif
+
 #endif	/* _XEN_EVENTS_H */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 09/17] xen: bind pirq to vector and event channel

Having converting a dev+pin to a gsi, and that gsi to an irq, and
allocated a vector for the irq, we must program the IO APIC to deliver
an interrupt on a pin to the vector, so Xen can deliver it as an event
channel.

Given the pirq, we can get the gsi and vector.  We map the gsi to a
specific IO APIC''s pin, and set the routing entry.

(We were passing the ACPI triggering and polarity levels directly into
the apic - but they have reversed values.  The result was that
all the level-triggered interrupts were edge, and vice-versa.
It''s surprising that anything worked at all, but now AHCI works
for me.

Thanks for Gerd Hoffmann for noticing this.)

[ Impact: program IO APICs under Xen ]

Diagnosed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/apic.c |    2 ++
 arch/x86/xen/pci.c  |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index 8ae563c..35a8af7 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -4,6 +4,7 @@
 
 #include <asm/io_apic.h>
 #include <asm/acpi.h>
+#include <asm/hw_irq.h>
 
 #include <asm/xen/hypervisor.h>
 #include <asm/xen/hypercall.h>
@@ -13,6 +14,7 @@
 
 static void __init xen_io_apic_init(void)
 {
+	enable_IO_APIC();
 }
 
 static unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index f450007..af4e898 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -2,6 +2,8 @@
 #include <linux/acpi.h>
 #include <linux/pci.h>
 
+#include <asm/mpspec.h>
+#include <asm/io_apic.h>
 #include <asm/pci_x86.h>
 
 #include <asm/xen/hypervisor.h>
@@ -11,6 +13,32 @@
 
 #include "xen-ops.h"
 
+static void xen_set_io_apic_routing(int irq, int trigger, int polarity)
+{
+	int ioapic, ioapic_pin;
+	int vector, gsi;
+	struct IO_APIC_route_entry entry;
+
+	gsi = xen_gsi_from_irq(irq);
+	vector = xen_vector_from_irq(irq);
+
+	ioapic = mp_find_ioapic(gsi);
+	if (ioapic == -1) {
+		printk(KERN_WARNING "xen_set_ioapic_routing: irq %d gsi %d ioapic
%d\n",
+			irq, gsi, ioapic);
+		return;
+	}
+
+	ioapic_pin = mp_find_ioapic_pin(ioapic, gsi);
+
+	printk(KERN_INFO "xen_set_ioapic_routing: irq %d gsi %d vector %d ioapic
%d pin %d triggering %d polarity %d\n",
+		irq, gsi, vector, ioapic, ioapic_pin, trigger, polarity);
+
+	setup_ioapic_entry(ioapic, -1, &entry, ~0, trigger, polarity, vector,
+			   ioapic_pin);
+	ioapic_write_entry(ioapic, ioapic_pin, entry);
+}
+
 int xen_register_gsi(u32 gsi, int triggering, int polarity)
 {
 	int irq;
@@ -25,6 +53,11 @@ int xen_register_gsi(u32 gsi, int triggering, int polarity)
 
 	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
 
+	if (irq > 0)
+		xen_set_io_apic_routing(irq,
+					triggering == ACPI_EDGE_SENSITIVE ? 0 : 1,
+					polarity == ACPI_ACTIVE_HIGH ? 0 : 1);
+
 	return irq;
 }
 
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 10/17] xen: pre-initialize legacy irqs early

From: Ian Campbell <ian.campbell@citrix.com>

Various legacy devices, such as IDE, assume their legacy interrupts are
already initialized and are immediately usable.  Pre-initialize all the
legacy interrupts.

[ Impact: ISA/legacy device compat ]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index af4e898..402a5bd 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -63,9 +63,9 @@ int xen_register_gsi(u32 gsi, int triggering, int polarity)
 
 void __init xen_setup_pirqs(void)
 {
-#ifdef CONFIG_ACPI
 	int irq;
 
+#ifdef CONFIG_ACPI
 	/*
 	 * Set up acpi interrupt in acpi_gbl_FADT.sci_interrupt.
 	 */
@@ -77,4 +77,8 @@ void __init xen_setup_pirqs(void)
 	/* Blerk. */
 	acpi_gbl_FADT.sci_interrupt = irq;
 #endif
+
+	/* Pre-allocate legacy irqs */
+	for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
+		xen_allocate_pirq(irq);
 }
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 11/17] xen: don''t setup acpi interrupt unless there is one

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

If the SCI hasn''t been set, then presumably we''re not running
with acpi, don''t bother setting up the interrupt.

[ Impact: compatibility with pre-ACPI machines ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index 402a5bd..00ad6df 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -69,13 +69,12 @@ void __init xen_setup_pirqs(void)
 	/*
 	 * Set up acpi interrupt in acpi_gbl_FADT.sci_interrupt.
 	 */
-	irq = xen_allocate_pirq(acpi_gbl_FADT.sci_interrupt);
+	if (acpi_gbl_FADT.sci_interrupt > 0) {
+		irq = xen_allocate_pirq(acpi_gbl_FADT.sci_interrupt);
 
-	printk(KERN_INFO "xen: allocated irq %d for acpi %d\n",
-	       irq, acpi_gbl_FADT.sci_interrupt);
-
-	/* Blerk. */
-	acpi_gbl_FADT.sci_interrupt = irq;
+		printk(KERN_INFO "xen: allocated irq %d for acpi %d\n",
+		       irq, acpi_gbl_FADT.sci_interrupt);
+	}
 #endif
 
 	/* Pre-allocate legacy irqs */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 12/17] xen: use acpi_get_override_irq() to get triggering for legacy irqs

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

We need to set up proper IO apic entries for legacy irqs, which are
not normally configured by either normal acpi interrupt routing or
PNP.

This also generalizes the acpi interrupt setup, so we can remove it
as a special case.

[ Impact: compatibility with legacy/ISA hardware ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c |   24 ++++++++++--------------
 1 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index 00ad6df..db0c74c 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -65,19 +65,15 @@ void __init xen_setup_pirqs(void)
 {
 	int irq;
 
-#ifdef CONFIG_ACPI
-	/*
-	 * Set up acpi interrupt in acpi_gbl_FADT.sci_interrupt.
-	 */
-	if (acpi_gbl_FADT.sci_interrupt > 0) {
-		irq = xen_allocate_pirq(acpi_gbl_FADT.sci_interrupt);
-
-		printk(KERN_INFO "xen: allocated irq %d for acpi %d\n",
-		       irq, acpi_gbl_FADT.sci_interrupt);
-	}
-#endif
-
 	/* Pre-allocate legacy irqs */
-	for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
-		xen_allocate_pirq(irq);
+	for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
+		int trigger, polarity;
+
+		if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
+			continue;
+
+		xen_register_gsi(irq,
+			trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE,
+			polarity ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH);
+	}
 }
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 13/17] xen: initialize irq 0 too

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

IRQ 0 is valid, so make sure it gets initialized properly too.
(Though in practice it doesn''t matter, because its the timer
interrupt we don''t use under Xen.)

[ Impact: theoretical bugfix, cleanup ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index db0c74c..381b7ab 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -53,7 +53,7 @@ int xen_register_gsi(u32 gsi, int triggering, int polarity)
 
 	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
 
-	if (irq > 0)
+	if (irq >= 0)
 		xen_set_io_apic_routing(irq,
 					triggering == ACPI_EDGE_SENSITIVE ? 0 : 1,
 					polarity == ACPI_ACTIVE_HIGH ? 0 : 1);
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 14/17] xen: dynamically allocate irq & event structures

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Dynamically allocate the irq_info and evtchn_to_irq arrays, so that
1) the irq_info array scales to the actual number of possible irqs,
and 2) we don''t needlessly increase the static size of the kernel
when we aren''t running under Xen.

Derived on patch from Mike Travis <travis@sgi.com>.

[ Impact: reduce memory usage ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 drivers/xen/events.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 968e927..e6ddf78 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -27,6 +27,7 @@
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/bootmem.h>
+#include <linux/irqnr.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq.h>
@@ -91,11 +92,9 @@ struct irq_info
 };
 #define PIRQ_NEEDS_EOI	(1 << 0)
 
-static struct irq_info irq_info[NR_IRQS];
+static struct irq_info *irq_info;
 
-static int evtchn_to_irq[NR_EVENT_CHANNELS] = {
-	[0 ... NR_EVENT_CHANNELS-1] = -1
-};
+static int *evtchn_to_irq;
 struct cpu_evtchn_s {
 	unsigned long bits[NR_EVENT_CHANNELS/BITS_PER_LONG];
 };
@@ -515,7 +514,7 @@ static int find_irq_by_gsi(unsigned gsi)
 {
 	int irq;
 
-	for (irq = 0; irq < NR_IRQS; irq++) {
+	for (irq = 0; irq < nr_irqs; irq++) {
 		struct irq_info *info = info_for_irq(irq);
 
 		if (info == NULL || info->type != IRQT_PIRQ)
@@ -1180,7 +1179,11 @@ void __init xen_init_IRQ(void)
 	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
 
 	cpu_evtchn_mask_p = alloc_bootmem(size);
-	BUG_ON(cpu_evtchn_mask_p == NULL);
+	irq_info = alloc_bootmem(nr_irqs * sizeof(*irq_info));
+
+	evtchn_to_irq = alloc_bootmem(NR_EVENT_CHANNELS * sizeof(*evtchn_to_irq));
+	for (i = 0; i < NR_EVENT_CHANNELS; i++)
+		evtchn_to_irq[i] = -1;
 
 	init_evtchn_cpu_bindings();
 
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 15/17] xen: set pirq name to something useful.

From: Gerd Hoffmann <kraxel@xeni.home.kraxel.org>

Make pirq show useful information in /proc/interrupts

[ Impact: better output in /proc/interrupts ]

Signed-off-by: Gerd Hoffmann <kraxel@xeni.home.kraxel.org>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c   |    3 ++-
 drivers/xen/events.c |    4 ++--
 include/xen/events.h |    2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index 381b7ab..4b286f1 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -49,7 +49,8 @@ int xen_register_gsi(u32 gsi, int triggering, int polarity)
 	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity
%d\n",
 	       gsi, triggering, polarity);
 
-	irq = xen_allocate_pirq(gsi);
+	irq = xen_allocate_pirq(gsi, (triggering == ACPI_EDGE_SENSITIVE)
+				     ? "ioapic-edge" : "ioapic-level");
 
 	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
 
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index e6ddf78..f84d13b 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -532,7 +532,7 @@ static int find_irq_by_gsi(unsigned gsi)
  * event channel until the irq actually started up.  Return an
  * existing irq if we''ve already got one for the gsi.
  */
-int xen_allocate_pirq(unsigned gsi)
+int xen_allocate_pirq(unsigned gsi, char *name)
 {
 	int irq;
 	struct physdev_irq irq_op;
@@ -554,7 +554,7 @@ int xen_allocate_pirq(unsigned gsi)
 		irq = find_unbound_irq();
 
 	set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
-				      handle_level_irq, "pirq");
+				      handle_level_irq, name);
 
 	irq_op.irq = irq;
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_alloc_irq_vector, &irq_op)) {
diff --git a/include/xen/events.h b/include/xen/events.h
index 6fe4863..4b19b9c 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -61,7 +61,7 @@ unsigned irq_from_evtchn(unsigned int evtchn);
 /* Allocate an irq for a physical interrupt, given a gsi.  "Legacy"
    GSIs are identity mapped; others are dynamically allocated as
    usual. */
-int xen_allocate_pirq(unsigned gsi);
+int xen_allocate_pirq(unsigned gsi, char *name);
 
 /* Return vector allocated to pirq */
 int xen_vector_from_irq(unsigned pirq);
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 16/17] xen: fix legacy irq setup, make ioapic-less machines work.

From: Gerd Hoffmann <kraxel@xeni.home.kraxel.org>

If the machine has no IO APICs, then just allocate a set of legacy
interrupts.

[ Impact: fix Xen compatibility with old machines ]

Signed-off-by: Gerd Hoffmann <kraxel@xeni.home.kraxel.org>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/pci.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c
index 4b286f1..07b59fe 100644
--- a/arch/x86/xen/pci.c
+++ b/arch/x86/xen/pci.c
@@ -66,6 +66,12 @@ void __init xen_setup_pirqs(void)
 {
 	int irq;
 
+	if (0 == nr_ioapics) {
+		for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
+			xen_allocate_pirq(irq, "xt-pic");
+		return;
+	}
+
 	/* Pre-allocate legacy irqs */
 	for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
 		int trigger, polarity;
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-27 07:37 UTC

head link

[Xen-devel] [PATCH 17/17] xen: disable MSI

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Disable MSI until we support it properly.

[ Impact: prevent MSI subsystem from crashing ]

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/apic.c |    3 +++
 drivers/pci/pci.h   |    2 --
 include/linux/pci.h |    6 ++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index 35a8af7..fece57a 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -1,6 +1,7 @@
 #include <linux/kernel.h>
 #include <linux/threads.h>
 #include <linux/bitmap.h>
+#include <linux/pci.h>
 
 #include <asm/io_apic.h>
 #include <asm/acpi.h>
@@ -54,6 +55,8 @@ void xen_init_apic(void)
 	if (!xen_initial_domain())
 		return;
 
+	pci_no_msi();
+
 	set_io_apic_ops(&xen_ioapic_ops);
 
 #ifdef CONFIG_ACPI
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d03f6b9..79ada7b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -111,10 +111,8 @@ extern struct rw_semaphore pci_bus_sem;
 extern unsigned int pci_pm_d3_delay;
 
 #ifdef CONFIG_PCI_MSI
-void pci_no_msi(void);
 extern void pci_msi_init_pci_dev(struct pci_dev *dev);
 #else
-static inline void pci_no_msi(void) { }
 static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 #endif
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 72698d8..724d030 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1253,5 +1253,11 @@ static inline irqreturn_t pci_sriov_migration(struct
pci_dev *dev)
 }
 #endif
 
+#ifdef CONFIG_PCI_MSI
+void pci_no_msi(void);
+#else
+static inline void pci_no_msi(void) { }
+#endif
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */
-- 
1.6.0.6


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

Linus Torvalds

2009-May-27 15:34 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

On Wed, 27 May 2009, Jeremy Fitzhardinge wrote:> 
> [ Impact: prevent MSI subsystem from crashing ]
Grr.

I looked at several of these impact lines, and they were _all_ totally 
misleading.

Please, guys. Stop with the F*CKING impact lines already. Add them if they 
are obvious, but don''t make them this idiotic "do an impact line
whether
or not it makes sense".

I hate them. They are stupid. 90% of all the impact lines I see are either 
(a) misleading or (b) totally inane and pointless. 

In this case, we would have been a LOT BETTER OFF with having just a 
better header line that said "xen: disable MSI to avoid crash" or 
something like that. But no. THAT F*CKING IMPACT LINE apparently meant 
that Jeremy just turned off his brain, and made _both_ the header line and 
the Impact: line be non-descriptive.

Really. Stop it. Ingo, start saying "no" to people, at least for
impact
lines that do not make sense. It adds _nothing_, and it actually detracts 
from real content, because just the inanity of them delutes the whole 
meaning of it.

		Linus

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

David Miller

2009-May-27 20:40 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 27 May 2009 08:34:22 -0700 (PDT)
> I hate them. They are stupid. 90% of all the impact lines I see are either 
> (a) misleading or (b) totally inane and pointless. 
I just delete any Impact lines I see in patches sent to me, and I
honestly suggest you do so as well.

Really.

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

Ingo Molnar

2009-May-27 21:18 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, 27 May 2009, Jeremy Fitzhardinge wrote:
> > 
> > [ Impact: prevent MSI subsystem from crashing ]
> 
> Grr.
> 
> I looked at several of these impact lines, and they were _all_ totally 
> misleading.
> 
> Please, guys. Stop with the F*CKING impact lines already. Add them 
> if they are obvious, but don''t make them this idiotic "do an 
> impact line whether or not it makes sense".
> 
> I hate them. They are stupid. 90% of all the impact lines I see 
> are either (a) misleading or (b) totally inane and pointless.
> 
> In this case, we would have been a LOT BETTER OFF with having just 
> a better header line that said "xen: disable MSI to avoid crash" 
> or something like that. But no. THAT F*CKING IMPACT LINE 
> apparently meant that Jeremy just turned off his brain, and made 
> _both_ the header line and the Impact: line be non-descriptive.
> 
> Really. Stop it. Ingo, start saying "no" to people, at least for 
> impact lines that do not make sense. It adds _nothing_, and it 
> actually detracts from real content, because just the inanity of 
> them delutes the whole meaning of it.
hm, i have to concur. Too often it ends up splitting attention away 
from the title of the commit. I do reject (or fix up) bad impact 
lines - will stop doing them altogether if you think there''s a net 
downside to them ...

	Ingo

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

Linus Torvalds

2009-May-27 21:46 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

On Wed, 27 May 2009, Ingo Molnar wrote:> 
> hm, i have to concur. Too often it ends up splitting attention away 
> from the title of the commit. I do reject (or fix up) bad impact 
> lines - will stop doing them altogether if you think there''s a net
> downside to them ...
I actually think that if there is a good reason for them, they can stay. 

Just don''t make it one of those "every commit that goes through me
has to
have one".

Pu another way: if they actually add value in highlighting the commits 
that _should_ stand out, then hey, by all means, keep such ones. I would 
not at all object if it was an issue of 

 [ Impact: fix bugzilla entry 455123 ]

or

 [ Impact: fix user-triggerable oops ]

or something that actually matters, and that you _want_ to stand out, and 
that you may well _want_ to grep for.

It''s when the whole series has them, and they don''t add
anything that
isn''t better said in the summary line, _that''s_ what I
dislike.

So to take the above bugzilla example: it really wouldn''t be a good 
summary line (because the summary line should describe what the commit 
does, not point to some bugzilla entry), but at the same time it''s
clearly
something that I do think we might want to automate the logs for.

IOW, that is something even I personally wouldn''t mind adding to a
commit,
to help people like Rafael that track bugzilla. It makes sense as a 
special marker, even though it clearly _shoudln''t_ be the summary. See?

Similarly, the "user-triggerable oops" might well be worth
high-lighting
in some manner. Now, the summary _might_ talk about it, but equally well 
the summary might be more specific in the actual implementation issue, and 
then perhaps the impact line is worth it.

But if all commits have them (at least for the x86-tip), then it''s not
a
really highlight event any more, is it? At that point, anything it says is 
probably just as well described by the summary line - at least for any 
"regular" commits that aren''t in some way worth the extra
attention.

			Linus

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

Ingo Molnar

2009-May-27 22:01 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, 27 May 2009, Ingo Molnar wrote:
> > 
> > hm, i have to concur. Too often it ends up splitting attention 
> > away from the title of the commit. I do reject (or fix up) bad 
> > impact lines - will stop doing them altogether if you think 
> > there''s a net downside to them ...
> 
> I actually think that if there is a good reason for them, they can 
> stay.
> 
> Just don''t make it one of those "every commit that goes
through me
> has to have one".
> 
> Pu another way: if they actually add value in highlighting the 
> commits that _should_ stand out, then hey, by all means, keep such 
> ones. I would not at all object if it was an issue of
> 
>  [ Impact: fix bugzilla entry 455123 ]
> 
> or
> 
>  [ Impact: fix user-triggerable oops ]
> 
> or something that actually matters, and that you _want_ to stand 
> out, and that you may well _want_ to grep for.
> 
> It''s when the whole series has them, and they don''t add
anything
> that isn''t better said in the summary line, _that''s_ what
I
> dislike.
> 
> So to take the above bugzilla example: it really wouldn''t be a 
> good summary line (because the summary line should describe what 
> the commit does, not point to some bugzilla entry), but at the 
> same time it''s clearly something that I do think we might want to 
> automate the logs for.
> 
> IOW, that is something even I personally wouldn''t mind adding to a
> commit, to help people like Rafael that track bugzilla. It makes 
> sense as a special marker, even though it clearly _shoudln''t_ be 
> the summary. See?
> 
> Similarly, the "user-triggerable oops" might well be worth 
> high-lighting in some manner. Now, the summary _might_ talk about 
> it, but equally well the summary might be more specific in the 
> actual implementation issue, and then perhaps the impact line is 
> worth it.
> 
> But if all commits have them (at least for the x86-tip), then it''s
> not a really highlight event any more, is it? At that point, 
> anything it says is probably just as well described by the summary 
> line - at least for any "regular" commits that aren''t in
some way
> worth the extra attention.
ok.

Beyond impact lines for bugfixes, there''s one other
''bulk'' impact
line that i find pretty important - the most boring and most 
repetitive ones:

   Impact: cleanup

Sometimes also in the form of:

   Impact: refactor code

It signals a conscious "this is not intended to have direct side 
effects" marker. It''s mis-used sometimes - but the ones i add tend
to be very specific (and common) type of patches.

Obviously for any buggy commit that designation will be patently 
false: but then again every commit in the kernel claims and intends 
to be bug free - still a significant proportion, 2-3% of all 
upstream kernel commits are buggy ;-)

So later on it makes it easy to see how much of an known impact a 
commit was supposed to have - and whether a badness/misbehavior was 
intended or not. (It also makes it easier for me to chastise repeat 
offenders who send ''cleanup'' patches which are all but.)

The ''cleanups which are not'' tend to contain the most
surprising
bugs (because those tend to be the most unexpected bugs - commits 
marked known-dangerous tend not to surprise anyone if they break), 
so i think it makes sense to delineate that category sharply, and 
observe (and enforce) safe coding techniques for 
cleanup/code-preparation patches.

That concept works great for us in arch/x86, we tend to be less and 
less surprised about what kind of commits produce what kinds of 
bugs.

The impact-line quality of non-cleanup and non-bugfix patches tends 
to be the poorest. And for them there''s no surprise generally if 
there''s some unexpected impact.

	Ingo

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

Roland Dreier

2009-May-27 22:03 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

> Pu another way: if they actually add value in highlighting the commits  > that _should_ stand out, then hey, by all means, keep such ones. I would 
 > not at all object if it was an issue of 
 > 
 >  [ Impact: fix bugzilla entry 455123 ]

I wonder if it''s really worth having such a visually distinctive style
for tagging things that fix bugzilla entries.  I''ve been just writing
out in English the bug information -- eg a recent changelog contains

    This patch fixes <https://bugs.openfabrics.org/show_bug.cgi?id=1571>,
    an NFS/RDMA server crash.

I could see adding a tag along the lines of tested-by, reported-by,
reviewed-by, etc.  Maybe something like

    Closes-bug: <URL>

so the above language would become

    Closes-bug: https://bugs.openfabrics.org/show_bug.cgi?id=1571

And then "git log|grep ''Closes-bug:''" or "git
log|grep ''<bug URL>''"
becomes interesting...

 >  [ Impact: fix user-triggerable oops ]

This I think gets close to the never-ending argument about tagging
"security" bugs.  It might not be obvious immediately that a given
change fixes a user-triggerable oops and grepping the log for commits
that claim to fix a certain type of problem is quite likely to miss some
such fixes.

In the case where I know that a commit *does* fix a user-triggerable
oops, I try to note it in the changelog by saying, "This fixes an oops
that can be triggered by a user passing in garbage input xyz..." but
I''m
not sure if we want to put that in a standardized greppable form.

 - R.

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

Linus Torvalds

2009-May-27 22:13 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

On Wed, 27 May 2009, Roland Dreier wrote:> 
> I could see adding a tag along the lines of tested-by, reported-by,
> reviewed-by, etc.  Maybe something like
> 
>     Closes-bug: <URL>
I agree that that would work, but that way also lies endless other tags. 
People seem to always want to have some magic tag to grep for, if we can 
make ''impact:'' be it rather than have potentially lots of
random ones,
maybe impact: is simply better?

		Linus

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

Roland Dreier

2009-May-27 22:20 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

> I agree that that would work, but that way also lies endless other tags.  > People seem to always want to have some magic tag to grep for, if we can 
 > make ''impact:'' be it rather than have potentially lots
of random ones,
 > maybe impact: is simply better?

Dunno... maybe just "Tag:"?  ;)

Maybe just "git log|grep ''bug url''" is good enough
without having to
make any special format?

 - R.

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

Ingo Molnar

2009-May-27 22:25 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

* David Miller <davem@davemloft.net> wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed, 27 May 2009 08:34:22 -0700 (PDT)
> 
> > I hate them. They are stupid. 90% of all the impact lines I see 
> > are either (a) misleading or (b) totally inane and pointless.
> 
> I just delete any Impact lines I see in patches sent to me, and I 
> honestly suggest you do so as well.
Still you committed a fair number of them already:

earth4:~/tip> git log net/ | grep Impact:
    [...]
    Impact: Attribute functions with __acquires(...) resp. __releases(...).
    Impact: Attribute function with __releases(...)
    Impact: Remove redundant variable declarations, resp. rename
    Impact: Attribute functions with __acquires(...) resp. __releases(...).
    Impact: Include header file.
    Impact: Use ''static const char[]'' instead of
''static char[]'', and
    Impact: Trust in the comment and add ''__force'' to the
cast.
    Impact: Attribute function with __acquires(...) resp. __releases(...).

btw., these are one of the weirdest impact lines i''ve ever seen.

You seem to dismiss them unconditionally and indiscriminately, 
without giving any thought to why good impact lines might be useful. 

Good impact lines _are_ really useful to me in my everyday 
maintenance workflow - while bad impact lines indeed are not.
So i try to commit and pull good ones only.

I challenge you to count the number of bad impact lines in the 
current pending x86-next tree, compared to the number of good impact 
lines there.

	Ingo

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

Ingo Molnar

2009-May-27 22:26 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

* Roland Dreier <rdreier@cisco.com> wrote:
>  > I agree that that would work, but that way also lies endless 
>  > other tags. People seem to always want to have some magic tag 
>  > to grep for, if we can make ''impact:'' be it rather
than have
>  > potentially lots of random ones, maybe impact: is simply 
>  > better?
> 
> Dunno... maybe just "Tag:"?  ;)
> 
> Maybe just "git log|grep ''bug url''" is good
enough without having
> to make any special format?
There''s various other categories beyond bugfixes with urls where 
impact lines are useful in practice.

	Ingo

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

Alan Cox

2009-May-28 06:28 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

> I could see adding a tag along the lines of tested-by, reported-by,
> reviewed-by, etc.  Maybe something like
> 
>     Closes-bug: <URL>
That would be useful and speed up closing bugs from changelogs. Right now
thats a grep and human task.

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

Andrew Morton

2009-May-28 06:38 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

On Thu, 28 May 2009 07:28:44 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk>
wrote:
> > I could see adding a tag along the lines of tested-by, reported-by,
> > reviewed-by, etc.  Maybe something like
> > 
> >     Closes-bug: <URL>
> 
> That would be useful and speed up closing bugs from changelogs. Right now
> thats a grep and human task.
Yup.  I presently use "Addresses: <url>" in the changelog.

y:/usr/src/git26> git-log | grep ''^    Addresses.*bugzilla''
| wc -l
36

Formalising that a bit more would be good.

Not all patches literally "close" a bugzilla report though.  Sometimes
there''s related info in bugzilla but the bug remains.  But
"closes-bug:" is close enough.


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

Jeff Garzik

2009-May-28 19:51 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

Linus Torvalds wrote:> 
> On Wed, 27 May 2009, Ingo Molnar wrote:
>> hm, i have to concur. Too often it ends up splitting attention away 
>> from the title of the commit. I do reject (or fix up) bad impact 
>> lines - will stop doing them altogether if you think there''s a
net
>> downside to them ...
> 
> I actually think that if there is a good reason for them, they can stay. 
> 
> Just don''t make it one of those "every commit that goes
through me has to
> have one".
> 
> Pu another way: if they actually add value in highlighting the commits 
> that _should_ stand out, then hey, by all means, keep such ones. I would 
> not at all object if it was an issue of 
> 
>  [ Impact: fix bugzilla entry 455123 ]
> 
> or
> 
>  [ Impact: fix user-triggerable oops ]
Ideally "Impact" is pointless if you are otherwise writing a clear, 
concise commit description.  "Fixes user-triggerable oops" on a line
by
itself is clear enough, and is how I''ve been writing descriptions for a
while.

People with the urge to add "Impact:" to every commit wind up either 
being redundant, or for small patches, having the entire ex-Subject 
commit description be "Impact: blah blah blah"

If IOW, if the impact is not already clear, you''re doing something 
wrong, and "Impact" is not necessarily going to fix that.

	Jeff




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

Roland Dreier

2009-May-28 22:21 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

> Yup.  I presently use "Addresses: <url>" in the changelog. > 
 > y:/usr/src/git26> git-log | grep ''^   
Addresses.*bugzilla'' | wc -l
 > 36
 > 
 > Formalising that a bit more would be good.
 > 
 > Not all patches literally "close" a bugzilla report though. 
Sometimes
 > there''s related info in bugzilla but the bug remains.  But
 > "closes-bug:" is close enough.

Yes, I deliberately didn''t suggest "Fixes-bug:" but I like
your
"Addresses:" language even better.  I''ll start using that for
any
commits from now on.

 - R.

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

Jon Masters

2009-Jun-01 02:09 UTC

head link

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

On Thu, 2009-05-28 at 15:21 -0700, Roland Dreier wrote:> > Yup.  I presently use "Addresses: <url>" in the
changelog.
>  > 
>  > y:/usr/src/git26> git-log | grep ''^   
Addresses.*bugzilla'' | wc -l
>  > 36
>  > 
>  > Formalising that a bit more would be good.
>  > 
>  > Not all patches literally "close" a bugzilla report though.
Sometimes
>  > there''s related info in bugzilla but the bug remains.  But
>  > "closes-bug:" is close enough.
> 
> Yes, I deliberately didn''t suggest "Fixes-bug:" but I
like your
> "Addresses:" language even better.  I''ll start using
that for any
> commits from now on.
We use "Resolves" and "Related" within Red Hat and Fedora in
describing
bugs. I might recommend "Related" as it''s already used by a
few groups
here, although it''s simply another way of saying the same.

Jon.



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

Thomas Gleixner

2009-Jun-02 16:58 UTC

head link

[Xen-devel] Re: [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0

On Wed, 27 May 2009, Jeremy Fitzhardinge wrote:
> When running in Xen dom0, we still want to parse the ACPI tables to
> find out about local and IO apics, but we don''t want to actually
use
> the lapics.
Hmm, we parse the tables and discard the information. What''s the point
of this exercise ? Some nice dmesg lines ?
 > Put a couple of tests for Xen to prevent lapics from being mapped or
> accessed.  This is very Xen-specific behaviour, so there didn''t
seem to
> be any point in adding more indirection.
I hate these "if (xen_...)" extra cases even more than the paravirt
misery. They stick Xen dependencies into random places and enforce the
people who want to modify that code to find out why the heck this
needs to be there.

That''s the fundamental design problem with the Dom0 model that you
want just certain parts of Linux and those parts which are in your way
are just hacked out. But this is designed to be a nightmare for
maintainence and development. Are you going to stick more and more of
those "if (xen..)" constructs into places which provide functionality
which is only partially useful to Xen ?

Thanks,

	tglx
> [ Impact: ignore local apics, which are not usable under Xen ]
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Reviewed-by: "H. Peter Anvin" <hpa@zytor.com>
> ---
>  arch/x86/kernel/acpi/boot.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 723989d..4147e0c 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -41,6 +41,8 @@
>  #include <asm/mpspec.h>
>  #include <asm/smp.h>
>  
> +#include <asm/xen/hypervisor.h>
> +
>  static int __initdata acpi_force = 0;
>  u32 acpi_rsdt_forced;
>  #ifdef	CONFIG_ACPI
> @@ -218,6 +220,10 @@ static void __cpuinit acpi_register_lapic(int id, u8
enabled)
>  {
>  	unsigned int ver = 0;
>  
> +	/* We don''t want to register lapics when in Xen dom0 */
> +	if (xen_initial_domain())
> +		return;
> +
>  	if (!enabled) {
>  		++disabled_cpus;
>  		return;
> @@ -802,6 +808,10 @@ static int __init acpi_parse_fadt(struct
acpi_table_header *table)
>  
>  static void __init acpi_register_lapic_address(unsigned long address)
>  {
> +	/* Xen dom0 doesn''t have usable lapics */
> +	if (xen_initial_domain())
> +		return;
> +
>  	mp_lapic_addr = address;
>  
>  	set_fixmap_nocache(FIX_APIC_BASE, address);
> -- 
> 1.6.0.6
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2009-Jun-03 06:38 UTC

head link

[Xen-devel] Re: [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0

Thomas Gleixner wrote:> On Wed, 27 May 2009, Jeremy Fitzhardinge wrote:
>
>   
>> When running in Xen dom0, we still want to parse the ACPI tables to
>> find out about local and IO apics, but we don''t want to
actually use
>> the lapics.
>>     
>
> Hmm, we parse the tables and discard the information. What''s the
point
> of this exercise ? Some nice dmesg lines ?
>   
No, it was to make some highly convoluted logic work.  I''m planning on 
revisting all this to make it so that clearing the APIC cpuid feature 
flag works (ie, the local apics are skipped, but IO-APIC discovery still 
works).

I don''t remember the specific problems I encountered, but it was 
something to do with the fact that CPU discovery is tied up with local 
APIC discovery and some entanglement with how ACPI table parsing works, 
all coupled through some global variables with unclear semantics.
> I hate these "if (xen_...)" extra cases even more than the
paravirt
> misery. They stick Xen dependencies into random places and enforce the
> people who want to modify that code to find out why the heck this
> needs to be there.
>   
I agree.  The if (xen) stuff was there to avoid sugar-coating the 
situation.  I could have prettily hidden things in abstraction layers, 
but if there were no current or even likely non-Xen users, I thought it 
was more honest and direct to just make the situation obvious to the reader.
> That''s the fundamental design problem with the Dom0 model that you
> want just certain parts of Linux and those parts which are in your way
> are just hacked out. But this is designed to be a nightmare for
> maintainence and development. Are you going to stick more and more of
> those "if (xen..)" constructs into places which provide
functionality
> which is only partially useful to Xen ?
>   
No.

My current plan for this apic stuff is to 1) clean up the local apic 
discovery so we can just clear the APIC cpuid flag and have the right 
thing happen (since that''s the truth of the situation: there are no 
local apics available to the kernel), and 2) implement the ioapic driver 
layer so that we can just plug our Xen stuff into that.  That should 
avoid all the explicit if (xen) bits in this part of the code.

    J

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

Xen devel - May 2009 - [GIT PULL REPOST] xen/dom0/apic-ops: Xen dom0 APIC changes

[Xen-devel] [GIT PULL REPOST] xen/dom0/apic-ops: Xen dom0 APIC changes

[Xen-devel] [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0

[Xen-devel] [PATCH 02/17] x86: add io_apic_ops to allow interception

[Xen-devel] [PATCH 03/17] xen: implement io_apic_ops

[Xen-devel] [PATCH 04/17] xen: create dummy ioapic mapping

[Xen-devel] [PATCH 05/17] xen: implement pirq type event channels

[Xen-devel] [PATCH 06/17] x86/io_apic: add get_nr_irqs_gsi()

[Xen-devel] [PATCH 07/17] xen/apic: identity map gsi->irqs

[Xen-devel] [PATCH 08/17] xen: direct irq registration to pirq event channels

[Xen-devel] [PATCH 09/17] xen: bind pirq to vector and event channel

[Xen-devel] [PATCH 10/17] xen: pre-initialize legacy irqs early

[Xen-devel] [PATCH 11/17] xen: don''t setup acpi interrupt unless there is one

[Xen-devel] [PATCH 12/17] xen: use acpi_get_override_irq() to get triggering for legacy irqs

[Xen-devel] [PATCH 13/17] xen: initialize irq 0 too

[Xen-devel] [PATCH 14/17] xen: dynamically allocate irq & event structures

[Xen-devel] [PATCH 15/17] xen: set pirq name to something useful.

[Xen-devel] [PATCH 16/17] xen: fix legacy irq setup, make ioapic-less machines work.

[Xen-devel] [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 17/17] xen: disable MSI

[Xen-devel] Re: [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0

[Xen-devel] Re: [PATCH 01/17] xen/dom0: handle acpi lapic parsing in Xen dom0