[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS |
Date: |
Mon, 08 Jul 2013 13:22:07 -0500 |
User-agent: |
Notmuch/0.15.2+202~g0c4b8aa (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Alexey Kardashevskiy <address@hidden> 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 <address@hidden>
> ---
> 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?
> +
> +struct ics_irq_state {
> + uint32_t 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;
> +};
>
> qemu_irq xics_get_qirq(struct icp_state *icp, int irq);
> void xics_set_irq_type(struct icp_state *icp, int irq, bool lsi);
>
> -struct icp_state *xics_system_init(int nr_servers, int nr_irqs);
> +void xics_common_init(struct icp_state *icp, qemu_irq_handler handler);
> +void xics_common_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu);
> +void xics_common_reset(struct icp_state *icp);
> +
> void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu);
>
> +extern const VMStateDescription vmstate_icp_server;
> +extern const VMStateDescription vmstate_ics;
This is the wrong way of doing whatever you're trying to do.
Regards,
Anthony Liguori
> +
> #endif /* __XICS_H__ */
> --
> 1.7.10.4
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Anthony Liguori, 2013/07/08
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS,
Anthony Liguori <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Alexey Kardashevskiy, 2013/07/08
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Benjamin Herrenschmidt, 2013/07/09
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Anthony Liguori, 2013/07/09
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Alexey Kardashevskiy, 2013/07/09
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Benjamin Herrenschmidt, 2013/07/09
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/17] pseries: rework XICS, Anthony Liguori, 2013/07/10