[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ppc/xics: fix XICSStateClass parent class
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH] ppc/xics: fix XICSStateClass parent class |
Date: |
Tue, 14 Feb 2017 07:25:07 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/14/2017 02:58 AM, David Gibson wrote:
> On Mon, Feb 13, 2017 at 10:33:24AM +0100, Cédric Le Goater wrote:
>> XICSState inherits from SysBusDevice and so the object class should
>> inherit from SysBusDeviceClass.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>
> That's definitely a bug, but I don't think this is the right fix.
>
> XICS was put on sysbus back when I thought that was where you put any
> "system" devices that didn't belong on a more specific bus. I've
> since discovered that's not the case, and - particularly since xics
> doesn't expose any mmio - it shouldn't be a sysbus device at all.
>
> I've instead made a patch changing it to a direct child of
> TYPE_DEVICE. Since the device shouldn't ever be command line
> instantiated, and it has no migratable state I believe this should be
> a safe change.
>
> This is in ppc-for-2.9, tentatively; please review:
>
> From eae21fcf2430588f17e6a58d505aa8eaa580b507 Mon Sep 17 00:00:00 2001
> From: David Gibson <address@hidden>
> Date: Tue, 14 Feb 2017 12:58:05 +1100
> Subject: [PATCH] xics: XICS should not be a SysBusDevice
>
> Currently xics - the component of the IBM POWER interrupt controller
> representing the overall interrupt fabric / architecture is represented as
> a descedent of SysBusDevice. However, this is not really correct - the
descendant ?
> xics presents nothing in MMIO space so it should be an "unattached" device
> in the current QOM model.
>
> Since this device will always be created by the machine type, not created
> specifically from the command line, and because it has no migrated state
> it should be safe to move it around the device composition tree.
>
> Therefore this patch changes it to a descendent of TYPE_DEVICE, and makes
> it an unattached device.
>
> Signed-off-by: David Gibson <address@hidden>
Reviewed-by: Cédric Le Goater <address@hidden>
Thanks,
C.
> ---
> hw/intc/xics.c | 2 +-
> hw/ppc/spapr.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 095c16a..372b831 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -260,7 +260,7 @@ static void xics_common_class_init(ObjectClass *oc, void
> *data)
>
> static const TypeInfo xics_common_info = {
> .name = TYPE_XICS_COMMON,
> - .parent = TYPE_SYS_BUS_DEVICE,
> + .parent = TYPE_DEVICE,
> .instance_size = sizeof(XICSState),
> .class_size = sizeof(XICSStateClass),
> .instance_init = xics_common_initfn,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6f37288..9b6ad0f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -101,7 +101,7 @@ static XICSState *try_create_xics(const char *type, int
> nr_servers,
> Error *err = NULL;
> DeviceState *dev;
>
> - dev = qdev_create(NULL, type);
> + dev = DEVICE(object_new(type));
> qdev_prop_set_uint32(dev, "nr_servers", nr_servers);
> qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs);
> object_property_set_bool(OBJECT(dev), true, "realized", &err);
>