From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33046) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwOnJ-0008Ne-VB for qemu-devel@nongnu.org; Mon, 08 Jul 2013 23:40:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UwOnH-0004ee-Km for qemu-devel@nongnu.org; Mon, 08 Jul 2013 23:40:49 -0400 Received: from mail-yh0-f51.google.com ([209.85.213.51]:45913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwOnH-0004eW-3b for qemu-devel@nongnu.org; Mon, 08 Jul 2013 23:40:47 -0400 Received: by mail-yh0-f51.google.com with SMTP id l109so2208969yhq.10 for ; Mon, 08 Jul 2013 20:40:46 -0700 (PDT) Message-ID: <51DB8639.9050906@ozlabs.ru> Date: Tue, 09 Jul 2013 13:40:41 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1372315560-5478-1-git-send-email-aik@ozlabs.ru> <1372315560-5478-3-git-send-email-aik@ozlabs.ru> <87fvvo29ps.fsf@codemonkey.ws> In-Reply-To: <87fvvo29ps.fsf@codemonkey.ws> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/17] pseries: rework XICS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, Alexander Graf , qemu-ppc@nongnu.org, Paolo Bonzini , Paul Mackerras , David Gibson On 07/09/2013 04:22 AM, Anthony Liguori wrote: > Alexey Kardashevskiy writes: > >> Currently XICS interrupt controller is not a QEMU device. As we are going >> to support in-kernel emulated XICS which is a part of KVM, it make >> sense not to extend the existing XICS and have multiple KVM stub functions >> but to create yet another device and share pieces between fully emulated >> XICS and in-kernel XICS. >> >> The rework includes: >> * port to QOM >> * made few functions public to use from in-kernel XICS implementation >> * made VMStateDescription public to be used for in-kernel XICS migration >> * move xics_system_init() to spapr.c, it tries creating fully-emulated >> XICS now and will try in-kernel XICS in upcoming patches. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/intc/xics.c | 109 ++++++++++++++++++++++++++----------------------- >> hw/ppc/spapr.c | 28 +++++++++++++ >> include/hw/ppc/xics.h | 59 ++++++++++++++++++++++++-- >> 3 files changed, 141 insertions(+), 55 deletions(-) >> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >> index 091912e..0e374c8 100644 >> --- a/hw/intc/xics.c >> +++ b/hw/intc/xics.c >> @@ -34,13 +34,6 @@ >> * ICP: Presentation layer >> */ >> >> -struct icp_server_state { >> - uint32_t xirr; >> - uint8_t pending_priority; >> - uint8_t mfrr; >> - qemu_irq output; >> -}; >> - >> #define XISR_MASK 0x00ffffff >> #define CPPR_MASK 0xff000000 >> >> @@ -49,12 +42,6 @@ struct icp_server_state { >> >> struct ics_state; >> >> -struct icp_state { >> - long nr_servers; >> - struct icp_server_state *ss; >> - struct ics_state *ics; >> -}; >> - >> static void ics_reject(struct ics_state *ics, int nr); >> static void ics_resend(struct ics_state *ics); >> static void ics_eoi(struct ics_state *ics, int nr); >> @@ -171,27 +158,6 @@ static void icp_irq(struct icp_state *icp, int server, int nr, uint8_t priority) >> /* >> * ICS: Source layer >> */ >> - >> -struct ics_irq_state { >> - int server; >> - uint8_t priority; >> - uint8_t saved_priority; >> -#define XICS_STATUS_ASSERTED 0x1 >> -#define XICS_STATUS_SENT 0x2 >> -#define XICS_STATUS_REJECTED 0x4 >> -#define XICS_STATUS_MASKED_PENDING 0x8 >> - uint8_t status; >> -}; >> - >> -struct ics_state { >> - int nr_irqs; >> - int offset; >> - qemu_irq *qirqs; >> - bool *islsi; >> - struct ics_irq_state *irqs; >> - struct icp_state *icp; >> -}; >> - >> static int ics_valid_irq(struct ics_state *ics, uint32_t nr) >> { >> return (nr >= ics->offset) >> @@ -506,9 +472,8 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> rtas_st(rets, 0, 0); /* Success */ >> } >> >> -static void xics_reset(void *opaque) >> +void xics_common_reset(struct icp_state *icp) >> { >> - struct icp_state *icp = (struct icp_state *)opaque; >> struct ics_state *ics = icp->ics; >> int i; >> >> @@ -527,7 +492,12 @@ static void xics_reset(void *opaque) >> } >> } >> >> -void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu) >> +static void xics_reset(DeviceState *d) >> +{ >> + xics_common_reset(XICS(d)); >> +} >> + >> +void xics_common_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu) >> { >> CPUState *cs = CPU(cpu); >> CPUPPCState *env = &cpu->env; >> @@ -551,37 +521,72 @@ void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu) >> } >> } >> >> -struct icp_state *xics_system_init(int nr_servers, int nr_irqs) >> +void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu) >> +{ >> + xics_common_cpu_setup(icp, cpu); >> +} >> + >> +void xics_common_init(struct icp_state *icp, qemu_irq_handler handler) >> { >> - struct icp_state *icp; >> - struct ics_state *ics; >> + struct ics_state *ics = icp->ics; >> >> - icp = g_malloc0(sizeof(*icp)); >> - icp->nr_servers = nr_servers; >> icp->ss = g_malloc0(icp->nr_servers*sizeof(struct icp_server_state)); >> >> ics = g_malloc0(sizeof(*ics)); >> - ics->nr_irqs = nr_irqs; >> + ics->nr_irqs = icp->nr_irqs; >> ics->offset = XICS_IRQ_BASE; >> - ics->irqs = g_malloc0(nr_irqs * sizeof(struct ics_irq_state)); >> - ics->islsi = g_malloc0(nr_irqs * sizeof(bool)); >> + ics->irqs = g_malloc0(ics->nr_irqs * sizeof(struct ics_irq_state)); >> + ics->islsi = g_malloc0(ics->nr_irqs * sizeof(bool)); >> >> icp->ics = ics; >> ics->icp = icp; >> >> - ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, nr_irqs); >> + ics->qirqs = qemu_allocate_irqs(handler, ics, ics->nr_irqs); >> +} >> >> - spapr_register_hypercall(H_CPPR, h_cppr); >> - spapr_register_hypercall(H_IPI, h_ipi); >> - spapr_register_hypercall(H_XIRR, h_xirr); >> - spapr_register_hypercall(H_EOI, h_eoi); >> +static void xics_realize(DeviceState *dev, Error **errp) >> +{ >> + struct icp_state *icp = XICS(dev); >> + >> + xics_common_init(icp, ics_set_irq); >> >> spapr_rtas_register("ibm,set-xive", rtas_set_xive); >> spapr_rtas_register("ibm,get-xive", rtas_get_xive); >> spapr_rtas_register("ibm,int-off", rtas_int_off); >> spapr_rtas_register("ibm,int-on", rtas_int_on); >> >> - qemu_register_reset(xics_reset, icp); >> +} >> + >> +static Property xics_properties[] = { >> + DEFINE_PROP_UINT32("nr_servers", struct icp_state, nr_servers, -1), >> + DEFINE_PROP_UINT32("nr_irqs", struct icp_state, nr_irqs, -1), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> +static void xics_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(oc); >> + >> + dc->realize = xics_realize; >> + dc->props = xics_properties; >> + dc->reset = xics_reset; >> +} >> + >> +static const TypeInfo xics_info = { >> + .name = TYPE_XICS, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(struct icp_state), >> + .class_init = xics_class_init, >> +}; >> + >> +static void xics_register_types(void) >> +{ >> + spapr_register_hypercall(H_CPPR, h_cppr); >> + spapr_register_hypercall(H_IPI, h_ipi); >> + spapr_register_hypercall(H_XIRR, h_xirr); >> + spapr_register_hypercall(H_EOI, h_eoi); >> >> - return icp; >> + type_register_static(&xics_info); >> } >> + >> +type_init(xics_register_types) >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 38c29b7..def3505 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -719,6 +719,34 @@ static int spapr_vga_init(PCIBus *pci_bus) >> } >> } >> >> +static struct icp_state *try_create_xics(const char *type, int nr_servers, >> + int nr_irqs) >> +{ >> + DeviceState *dev; >> + >> + dev = qdev_create(NULL, type); >> + qdev_prop_set_uint32(dev, "nr_servers", nr_servers); >> + qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); >> + if (qdev_init(dev) < 0) { >> + return NULL; >> + } >> + >> + return XICS(dev); >> +} >> + >> +static struct icp_state *xics_system_init(int nr_servers, int nr_irqs) >> +{ >> + struct icp_state *icp = NULL; >> + >> + icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs); >> + if (!icp) { >> + perror("Failed to create XICS\n"); >> + abort(); >> + } >> + >> + return icp; >> +} >> + >> /* pSeries LPAR / sPAPR hardware init */ >> static void ppc_spapr_init(QEMUMachineInitArgs *args) >> { >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index 6bce042..3f72806 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -27,15 +27,68 @@ >> #if !defined(__XICS_H__) >> #define __XICS_H__ >> >> +#include "hw/sysbus.h" >> + >> +#define TYPE_XICS "xics" >> +#define XICS(obj) OBJECT_CHECK(struct icp_state, (obj), TYPE_XICS) >> + >> #define XICS_IPI 0x2 >> -#define XICS_IRQ_BASE 0x10 >> +#define XICS_BUID 0x1 >> +#define XICS_IRQ_BASE (XICS_BUID << 12) >> + >> +/* >> + * We currently only support one BUID which is our interrupt base >> + * (the kernel implementation supports more but we don't exploit >> + * that yet) >> + */ >> >> -struct icp_state; >> +struct icp_state { >> + /*< private >*/ >> + SysBusDevice parent_obj; >> + /*< public >*/ >> + uint32_t nr_servers; >> + uint32_t nr_irqs; >> + struct icp_server_state *ss; >> + struct ics_state *ics; >> +}; >> + >> +struct icp_server_state { >> + uint32_t xirr; >> + uint8_t pending_priority; >> + uint8_t mfrr; >> + qemu_irq output; >> +}; > > If you're exposing all of this, please fix coding style while you're at > it. >> + >> +struct ics_state { >> + uint32_t nr_irqs; >> + uint32_t offset; >> + qemu_irq *qirqs; >> + bool *islsi; >> + struct ics_irq_state *irqs; >> + struct icp_state *icp; >> +}; > > Shouldn't this be a device too? No, why? It is a per CPU state of XICS controller, never exists apart from XICS. -- Alexey