From: Michael Ellerman <michael@ellerman.id.au>
To: <penberg@kernel.org>
Cc: <kvm@vger.kernel.org>, levinsasha928@gmail.com, <matt@ozlabs.org>,
<kvm-ppc@vger.kernel.org>
Subject: [PATCH 2/3] kvm tools: Fix segfault on powerpc in xics_register()
Date: Fri, 5 Oct 2012 11:15:18 +1000 [thread overview]
Message-ID: <1349399719-9317-2-git-send-email-michael@ellerman.id.au> (raw)
In-Reply-To: <1349399719-9317-1-git-send-email-michael@ellerman.id.au>
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(-)
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
--
1.7.9.5
next prev parent reply other threads:[~2012-10-05 1:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-05 1:15 [PATCH 1/3] kvm tools: Fix powerpc build errors caused by recent changes Michael Ellerman
2012-10-05 1:15 ` Michael Ellerman [this message]
2012-10-05 1:15 ` [PATCH 3/3] kvm tools: Do setup_fdt() later, get powerpc to boot again Michael Ellerman
2012-10-05 6:30 ` [PATCH 1/3] kvm tools: Fix powerpc build errors caused by recent changes Pekka Enberg
2012-10-05 6:33 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1349399719-9317-2-git-send-email-michael@ellerman.id.au \
--to=michael@ellerman.id.au \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=levinsasha928@gmail.com \
--cc=matt@ozlabs.org \
--cc=penberg@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox