From: Michael Ellerman Date: Fri, 5 Oct 2012 01:15:18 +0000 (+1000) Subject: kvm tools: Fix segfault on powerpc in xics_register() X-Git-Tag: next-20121008~7^2~1 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=ea9b027e890785a5ddcc15c3b1d25e383c0f31e8;p=karo-tx-linux.git kvm tools: Fix segfault on powerpc in xics_register() 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 Signed-off-by: Pekka Enberg --- diff --git a/tools/kvm/powerpc/irq.c b/tools/kvm/powerpc/irq.c index 6d134c516128..e89fa3b11ad0 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 6aaf424e5b8c..8fce121705c8 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 1cf95581ac98..d4b5caae8af7 100644 --- a/tools/kvm/powerpc/xics.c +++ b/tools/kvm/powerpc/xics.c @@ -18,6 +18,8 @@ #include #include +#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 144915ba890e..d5bc6f92fa82 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