Patchwork [2/3] kvm tools: Fix segfault on powerpc in xics_register()

login
register
mail settings
Submitter Michael Ellerman
Date Oct. 5, 2012, 1:15 a.m.
Message ID <1349399719-9317-2-git-send-email-michael@ellerman.id.au>
Download mbox | patch
Permalink /patch/189387/
State New
Headers show

Comments

Michael Ellerman - Oct. 5, 2012, 1:15 a.m.
In commit 06e6648 "move kvm_cpus into struct kvm", kvm_cpu__init() became
kvm_cpu__arch_init() called from a new kvm_cpu__init(), and the call was moved
from the end of the init sequence to much earlier, and in particular prior to
irq__init().

This leads to a segfault on powerpc, because kvm_cpu__arch_init() calls into
xics_cpu_register(), which dereferences vcpu->kvm.icp which is uninitialised
until irq__init().

Later in commit a48488d "use init/exit where possible", irq__init() was pulled
out of the init sequence and made a dev_base_init() routine, on x86. On powerpc
the call to irq__init() was dropped entirely.

Finally, we now have a circular dependency between kvm_cpu__init() (which needs
kvm->arch.icp), and irq__init() (which needs kvm->nrcpus). This is caused by
the combination of commit 89f40a7 "move nrcpus into struct kvm_config",
which moved the global nrcpus into kvm->cfg, and commit 06e6648 "move kvm_cpus
into struct kvm", which moved the setup of kvm->nrcpus from kvm->cfg into
kvm_cpu__init().

To fix it we drop irq__init() entirely, if we ever have a non xics irq option
we can bring it back. We turn xics_system_init() into xics_init(), and have it
do the allocation and setup of the icp/ics, including the per-vcpu setup,
removing the dependency from kvm_cpu__init() (via kvm_cpu__arch_init()).

xics_init() is a base_init() routine, it can't be core, which should be early
enough, fingers crossed.

Finally drop irq__exit(), it does nothing and is never called.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/powerpc/irq.c     |   19 -------------------
 tools/kvm/powerpc/kvm-cpu.c |    3 ---
 tools/kvm/powerpc/xics.c    |   38 +++++++++++++++++++++++---------------
 tools/kvm/powerpc/xics.h    |    5 -----
 4 files changed, 23 insertions(+), 42 deletions(-)

Patch

diff --git a/tools/kvm/powerpc/irq.c b/tools/kvm/powerpc/irq.c
index 6d134c5..e89fa3b 100644
--- a/tools/kvm/powerpc/irq.c
+++ b/tools/kvm/powerpc/irq.c
@@ -26,8 +26,6 @@ 
 #include "xics.h"
 #include "spapr_pci.h"
 
-#define XICS_IRQS               1024
-
 /*
  * FIXME: The code in this file assumes an SPAPR guest, using XICS.  Make
  * generic & cope with multiple PPC platform types.
@@ -51,23 +49,6 @@  int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 	return 0;
 }
 
-int irq__init(struct kvm *kvm)
-{
-	/*
-	 * kvm->nr_cpus is now valid; for /now/, pass
-	 * this to xics_system_init(), which assumes servers
-	 * are numbered 0..nrcpus.  This may not really be true,
-	 * but it is OK currently.
-	 */
-	kvm->arch.icp = xics_system_init(XICS_IRQS, kvm->nrcpus);
-	return 0;
-}
-
-int irq__exit(struct kvm *kvm)
-{
-	return 0;
-}
-
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
 {
 	die(__FUNCTION__);
diff --git a/tools/kvm/powerpc/kvm-cpu.c b/tools/kvm/powerpc/kvm-cpu.c
index 6aaf424..8fce121 100644
--- a/tools/kvm/powerpc/kvm-cpu.c
+++ b/tools/kvm/powerpc/kvm-cpu.c
@@ -93,9 +93,6 @@  struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 	 */
 	vcpu->is_running = true;
 
-	/* Register with IRQ controller (FIXME, assumes XICS) */
-	xics_cpu_register(vcpu);
-
 	return vcpu;
 }
 
diff --git a/tools/kvm/powerpc/xics.c b/tools/kvm/powerpc/xics.c
index 1cf9558..d4b5caa 100644
--- a/tools/kvm/powerpc/xics.c
+++ b/tools/kvm/powerpc/xics.c
@@ -18,6 +18,8 @@ 
 #include <stdio.h>
 #include <malloc.h>
 
+#define XICS_NUM_IRQS	1024
+
 
 /* #define DEBUG_XICS yes */
 #ifdef DEBUG_XICS
@@ -441,26 +443,19 @@  static void rtas_int_on(struct kvm_cpu *vcpu, uint32_t token,
 	rtas_st(vcpu->kvm, rets, 0, 0); /* Success */
 }
 
-void xics_cpu_register(struct kvm_cpu *vcpu)
-{
-	if (vcpu->cpu_id < vcpu->kvm->arch.icp->nr_servers)
-		vcpu->kvm->arch.icp->ss[vcpu->cpu_id].cpu = vcpu;
-	else
-		die("Setting invalid server for cpuid %ld\n", vcpu->cpu_id);
-}
-
-struct icp_state *xics_system_init(unsigned int nr_irqs, unsigned int nr_cpus)
+static int xics_init(struct kvm *kvm)
 {
 	int max_server_num;
 	unsigned int i;
 	struct icp_state *icp;
 	struct ics_state *ics;
+	int j;
 
-	max_server_num = nr_cpus;
+	max_server_num = kvm->nrcpus;
 
 	icp = malloc(sizeof(*icp));
 	icp->nr_servers = max_server_num + 1;
-	icp->ss = malloc(icp->nr_servers*sizeof(struct icp_server_state));
+	icp->ss = malloc(icp->nr_servers * sizeof(struct icp_server_state));
 
 	for (i = 0; i < icp->nr_servers; i++) {
 		icp->ss[i].xirr = 0;
@@ -475,14 +470,14 @@  struct icp_state *xics_system_init(unsigned int nr_irqs, unsigned int nr_cpus)
 	 */
 
 	ics = malloc(sizeof(*ics));
-	ics->nr_irqs = nr_irqs;
+	ics->nr_irqs = XICS_NUM_IRQS;
 	ics->offset = XICS_IRQ_OFFSET;
-	ics->irqs = malloc(nr_irqs * sizeof(struct ics_irq_state));
+	ics->irqs = malloc(ics->nr_irqs * sizeof(struct ics_irq_state));
 
 	icp->ics = ics;
 	ics->icp = icp;
 
-	for (i = 0; i < nr_irqs; i++) {
+	for (i = 0; i < ics->nr_irqs; i++) {
 		ics->irqs[i].server = 0;
 		ics->irqs[i].priority = 0xff;
 		ics->irqs[i].saved_priority = 0xff;
@@ -500,8 +495,21 @@  struct icp_state *xics_system_init(unsigned int nr_irqs, unsigned int nr_cpus)
 	spapr_rtas_register("ibm,int-off", rtas_int_off);
 	spapr_rtas_register("ibm,int-on", rtas_int_on);
 
-	return icp;
+	for (j = 0; j < kvm->nrcpus; j++) {
+		struct kvm_cpu *vcpu = kvm->cpus[j];
+
+		if (vcpu->cpu_id >= icp->nr_servers)
+			die("Invalid server number for cpuid %ld\n", vcpu->cpu_id);
+
+		icp->ss[vcpu->cpu_id].cpu = vcpu;
+	}
+
+	kvm->arch.icp = icp;
+
+	return 0;
 }
+base_init(xics_init);
+
 
 void kvm__irq_line(struct kvm *kvm, int irq, int level)
 {
diff --git a/tools/kvm/powerpc/xics.h b/tools/kvm/powerpc/xics.h
index 144915b..d5bc6f9 100644
--- a/tools/kvm/powerpc/xics.h
+++ b/tools/kvm/powerpc/xics.h
@@ -13,11 +13,6 @@ 
 
 #define XICS_IPI        0x2
 
-struct kvm_cpu;
-struct icp_state;
-
-struct icp_state *xics_system_init(unsigned int nr_irqs, unsigned int nr_cpus);
-void xics_cpu_register(struct kvm_cpu *vcpu);
 int xics_alloc_irqnum(void);
 
 #endif