On Thu, Feb 16, 2017 at 02:47:25PM +0100, Cédric Le Goater wrote: > Today, the ICP (Interrupt Controller Presenter) objects are created by > the 'nr_servers' property handler of the XICS object and a class > handler. They are realized in the XICS object realize routine. > > Let's simplify the process by creating the ICP objects along with the > XICS object at the machine level. > > Signed-off-by: Cédric Le Goater <c...@kaod.org>
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
>
> Changes since v1:
>
> - added a XICS link to the ICS object
> - removed xics_set_nr_servers() routine
> - removed set_nr_servers() class handler
>
> hw/intc/xics.c | 74
> +++++++++++++--------------------------------------
> hw/intc/xics_kvm.c | 21 +--------------
> hw/intc/xics_spapr.c | 26 ------------------
> hw/ppc/spapr.c | 30 ++++++++++++++++-----
> include/hw/ppc/xics.h | 3 ---
> 5 files changed, 42 insertions(+), 112 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index e70d3b8b1095..9f22814815c9 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -151,67 +151,11 @@ static void xics_common_reset(DeviceState *d)
> }
> }
>
> -void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> - const char *typename, Error **errp)
> -{
> - int i;
> -
> - xics->nr_servers = nr_servers;
> -
> - xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
> - for (i = 0; i < xics->nr_servers; i++) {
> - char name[32];
> - ICPState *icp = &xics->ss[i];
> -
> - object_initialize(icp, sizeof(*icp), typename);
> - snprintf(name, sizeof(name), "icp[%d]", i);
> - object_property_add_child(OBJECT(xics), name, OBJECT(icp), errp);
> - icp->xics = xics;
> - }
> -}
> -
> -static void xics_prop_get_nr_servers(Object *obj, Visitor *v,
> - const char *name, void *opaque,
> - Error **errp)
> -{
> - XICSState *xics = XICS_COMMON(obj);
> - int64_t value = xics->nr_servers;
> -
> - visit_type_int(v, name, &value, errp);
> -}
> -
> -static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
> - const char *name, void *opaque,
> - Error **errp)
> -{
> - XICSState *xics = XICS_COMMON(obj);
> - XICSStateClass *xsc = XICS_COMMON_GET_CLASS(xics);
> - Error *error = NULL;
> - int64_t value;
> -
> - visit_type_int(v, name, &value, &error);
> - if (error) {
> - error_propagate(errp, error);
> - return;
> - }
> - if (xics->nr_servers) {
> - error_setg(errp, "Number of servers is already set to %u",
> - xics->nr_servers);
> - return;
> - }
> -
> - assert(xsc->set_nr_servers);
> - xsc->set_nr_servers(xics, value, errp);
> -}
> -
> static void xics_common_initfn(Object *obj)
> {
> XICSState *xics = XICS_COMMON(obj);
>
> QLIST_INIT(&xics->ics);
> - object_property_add(obj, "nr_servers", "int",
> - xics_prop_get_nr_servers, xics_prop_set_nr_servers,
> - NULL, NULL, NULL);
> }
>
> static void xics_common_class_init(ObjectClass *oc, void *data)
> @@ -450,12 +394,30 @@ static void icp_reset(DeviceState *dev)
> qemu_set_irq(icp->output, 0);
> }
>
> +static void icp_realize(DeviceState *dev, Error **errp)
> +{
> + ICPState *icp = ICP(dev);
> + Object *obj;
> + Error *err = NULL;
> +
> + obj = object_property_get_link(OBJECT(dev), "xics", &err);
> + if (!obj) {
> + error_setg(errp, "%s: required link 'xics' not found: %s",
> + __func__, error_get_pretty(err));
> + return;
> + }
> +
> + icp->xics = XICS_COMMON(obj);
> +}
> +
> +
> static void icp_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> dc->reset = icp_reset;
> dc->vmsd = &vmstate_icp_server;
> + dc->realize = icp_realize;
> }
>
> static const TypeInfo icp_info = {
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 4a6c0522feb6..6cabc11f6be1 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -368,12 +368,6 @@ static void xics_kvm_cpu_setup(XICSState *xics,
> PowerPCCPU *cpu)
> ss->cap_irq_xics_enabled = true;
> }
>
> -static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> - Error **errp)
> -{
> - xics_set_nr_servers(xics, nr_servers, TYPE_KVM_ICP, errp);
> -}
> -
> static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> uint32_t token,
> uint32_t nargs, target_ulong args,
> @@ -386,9 +380,7 @@ static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState
> *spapr,
> static void xics_kvm_realize(DeviceState *dev, Error **errp)
> {
> KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
> - XICSState *xics = XICS_COMMON(dev);
> - int i, rc;
> - Error *error = NULL;
> + int rc;
> struct kvm_create_device xics_create_device = {
> .type = KVM_DEV_TYPE_XICS,
> .flags = 0,
> @@ -438,16 +430,6 @@ static void xics_kvm_realize(DeviceState *dev, Error
> **errp)
>
> xicskvm->kernel_xics_fd = xics_create_device.fd;
>
> - assert(xics->nr_servers);
> - for (i = 0; i < xics->nr_servers; i++) {
> - object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
> - &error);
> - if (error) {
> - error_propagate(errp, error);
> - goto fail;
> - }
> - }
> -
> kvm_kernel_irqchip = true;
> kvm_msi_via_irqfd_allowed = true;
> kvm_gsi_direct_mapping = true;
> @@ -468,7 +450,6 @@ static void xics_kvm_class_init(ObjectClass *oc, void
> *data)
>
> dc->realize = xics_kvm_realize;
> xsc->cpu_setup = xics_kvm_cpu_setup;
> - xsc->set_nr_servers = xics_kvm_set_nr_servers;
> }
>
> static const TypeInfo xics_spapr_kvm_info = {
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 03e42a866603..859b5675e175 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -239,23 +239,8 @@ static void rtas_int_on(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> }
>
> -static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> - Error **errp)
> -{
> - xics_set_nr_servers(xics, nr_servers, TYPE_ICP, errp);
> -}
> -
> static void xics_spapr_realize(DeviceState *dev, Error **errp)
> {
> - XICSState *xics = XICS_SPAPR(dev);
> - Error *error = NULL;
> - int i;
> -
> - if (!xics->nr_servers) {
> - error_setg(errp, "Number of servers needs to be greater 0");
> - return;
> - }
> -
> /* Registration of global state belongs into realize */
> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
> @@ -268,24 +253,13 @@ static void xics_spapr_realize(DeviceState *dev, Error
> **errp)
> spapr_register_hypercall(H_XIRR_X, h_xirr_x);
> spapr_register_hypercall(H_EOI, h_eoi);
> spapr_register_hypercall(H_IPOLL, h_ipoll);
> -
> - for (i = 0; i < xics->nr_servers; i++) {
> - object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
> - &error);
> - if (error) {
> - error_propagate(errp, error);
> - return;
> - }
> - }
> }
>
> static void xics_spapr_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
> - XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
>
> dc->realize = xics_spapr_realize;
> - xsc->set_nr_servers = xics_spapr_set_nr_servers;
> }
>
> static const TypeInfo xics_spapr_info = {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 94b1e8e3227a..5d7c35de8cd9 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -96,17 +96,17 @@
> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift))
>
> static XICSState *try_create_xics(const char *type, const char *type_ics,
> - int nr_servers, int nr_irqs, Error **errp)
> + const char *type_icp, int nr_servers,
> + int nr_irqs, Error **errp)
> {
> Error *err = NULL, *local_err = NULL;
> XICSState *xics;
> ICSState *ics = NULL;
> + int i;
>
> xics = XICS_COMMON(object_new(type));
> qdev_set_parent_bus(DEVICE(xics), sysbus_get_default());
> - object_property_set_int(OBJECT(xics), nr_servers, "nr_servers", &err);
> - object_property_set_bool(OBJECT(xics), true, "realized", &local_err);
> - error_propagate(&err, local_err);
> + object_property_set_bool(OBJECT(xics), true, "realized", &err);
> if (err) {
> goto error;
> }
> @@ -122,6 +122,22 @@ static XICSState *try_create_xics(const char *type,
> const char *type_ics,
> }
> QLIST_INSERT_HEAD(&xics->ics, ics, list);
>
> + xics->ss = g_malloc0(nr_servers * sizeof(ICPState));
> + xics->nr_servers = nr_servers;
> +
> + for (i = 0; i < nr_servers; i++) {
> + ICPState *icp = &xics->ss[i];
> +
> + object_initialize(icp, sizeof(*icp), type_icp);
> + object_property_add_child(OBJECT(xics), "icp[*]", OBJECT(icp), NULL);
> + object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xics),
> NULL);
> + object_property_set_bool(OBJECT(icp), true, "realized", &err);
> + if (err) {
> + goto error;
> + }
> + object_unref(OBJECT(icp));
> + }
> +
> return xics;
>
> error:
> @@ -143,7 +159,7 @@ static XICSState *xics_system_init(MachineState *machine,
>
> if (machine_kernel_irqchip_allowed(machine)) {
> xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM,
> - nr_servers, nr_irqs, &err);
> + TYPE_KVM_ICP, nr_servers, nr_irqs, &err);
> }
> if (machine_kernel_irqchip_required(machine) && !xics) {
> error_reportf_err(err,
> @@ -154,8 +170,8 @@ static XICSState *xics_system_init(MachineState *machine,
> }
>
> if (!xics) {
> - xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, nr_servers,
> - nr_irqs, errp);
> + xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, TYPE_ICP,
> + nr_servers, nr_irqs, errp);
> }
>
> return xics;
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 8fe3c4d2ceab..fc4abcd4e796 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -74,7 +74,6 @@ struct XICSStateClass {
> DeviceClass parent_class;
>
> void (*cpu_setup)(XICSState *icp, PowerPCCPU *cpu);
> - void (*set_nr_servers)(XICSState *icp, uint32_t nr_servers, Error
> **errp);
> };
>
> struct XICSState {
> @@ -189,8 +188,6 @@ void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t
> phandle);
>
> void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
> void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
> -void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> - const char *typename, Error **errp);
>
> /* Internal XICS interfaces */
> int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
