On 19.08.2013, at 07:55, Alexey Kardashevskiy wrote:
> Before XICS-KVM comes, it makes sense to clean up the emulated XICS a bit.
>
> This does:
> 1. add assert in ics_realize()
> 2. change variable names from "k" to more informative ones
> 3. add "const" to every TypeInfo
> 4. replace fprintf(stderr, ..."\n") with error_report
> 5. replace old style qdev_init_nofail() with new style
> object_property_set_bool().
>
> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
> Changes:
> v3:
> * ics_realize() fixed to be actual realize callback rather than initfn
> * asserts replaced with Error**
> ---
> hw/intc/xics.c | 42 ++++++++++++++++++++++++++++++------------
> 1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index c80fa80..4d08c58 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -29,6 +29,7 @@
> #include "trace.h"
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/xics.h"
> +#include "qemu/error-report.h"
>
> /*
> * ICP: Presentation layer
> @@ -211,7 +212,7 @@ static void icp_class_init(ObjectClass *klass, void *data)
> dc->vmsd = &vmstate_icp_server;
> }
>
> -static TypeInfo icp_info = {
> +static const TypeInfo icp_info = {
> .name = TYPE_ICP,
> .parent = TYPE_DEVICE,
> .instance_size = sizeof(ICPState),
> @@ -442,15 +443,17 @@ static const VMStateDescription vmstate_ics = {
> },
> };
>
> -static int ics_realize(DeviceState *dev)
> +static void ics_realize(DeviceState *dev, Error **errp)
> {
> ICSState *ics = ICS(dev);
>
> + if (!ics->nr_irqs) {
> + error_setg(errp, "Number of interrupts needs to be greater 0");
> + return;
> + }
> ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
> ics->islsi = g_malloc0(ics->nr_irqs * sizeof(bool));
> ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs);
> -
> - return 0;
> }
>
> static void ics_class_init(ObjectClass *klass, void *data)
> @@ -458,13 +461,14 @@ static void ics_class_init(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> ICSStateClass *isc = ICS_CLASS(klass);
>
> - dc->init = ics_realize;
> + dc->realize = ics_realize;
> dc->vmsd = &vmstate_ics;
> dc->reset = ics_reset;
> isc->post_load = ics_post_load;
> + isc->post_load = ics_post_load;
Same line twice?
> }
>
> -static TypeInfo ics_info = {
> +static const TypeInfo ics_info = {
> .name = TYPE_ICS,
> .parent = TYPE_DEVICE,
> .instance_size = sizeof(ICSState),
> @@ -680,8 +684,8 @@ static void xics_cpu_setup(XICSState *icp, PowerPCCPU
> *cpu)
> break;
>
> default:
> - fprintf(stderr, "XICS interrupt controller does not support this CPU
> "
> - "bus model\n");
> + error_report("XICS interrupt controller does not support this CPU "
> + "bus model");
> abort();
> }
> }
> @@ -690,8 +694,14 @@ static void xics_realize(DeviceState *dev, Error **errp)
> {
> XICSState *icp = XICS(dev);
> ICSState *ics = icp->ics;
> + Error *error = NULL;
> int i;
>
> + if (!icp->nr_servers) {
> + error_setg(errp, "Number of servers needs to be greater 0");
> + return;
> + }
> +
> /* Registration of global state belongs into realize */
> spapr_rtas_register("ibm,set-xive", rtas_set_xive);
> spapr_rtas_register("ibm,get-xive", rtas_get_xive);
> @@ -706,7 +716,11 @@ static void xics_realize(DeviceState *dev, Error **errp)
> ics->nr_irqs = icp->nr_irqs;
> ics->offset = XICS_IRQ_BASE;
> ics->icp = icp;
> - qdev_init_nofail(DEVICE(ics));
> + object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
Are you sure this is correct? So you are trying to force set the ics to
realize? Is this the right way to do this? Andreas?
Alex
