Re: [Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Thomas Huth <thuth@redhat.com>]

On 13.03.2018 16:34, Peter Maydell wrote:
> The bcm2837 is pretty similar to the bcm2836, but it does have
> some differences. Notably, the MPIDR affinity aff1 values it
> sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
> uses, and if this is wrong Linux will not boot.
> 
> Rather than trying to have one device with properties that
> configure it differently for the two cases, create two
> separate QOM devices for the two SoCs. We use the same approach
> as hw/arm/aspeed_soc.c and share code and have a data table
> that might differ per-SoC. For the moment the two types don't
> actually have different behaviour.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/bcm2836.h | 19 +++++++++++++++++++
>  hw/arm/bcm2836.c         | 37 ++++++++++++++++++++++++++++++++-----
>  hw/arm/raspi.c           |  3 ++-
>  3 files changed, 53 insertions(+), 6 deletions(-)

 Hi Peter,

if I've bisected it right, this patch introduced a new way to crash QEMU:

$ aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest \
    -nographic
QEMU 2.11.50 monitor - type 'help' for more information
(qemu) device_add bcm2837
Device 'bcm2837' can not be hotplugged on this machine
(qemu) device_add corgi-ssp
Segmentation fault (core dumped)

Not sure why this is happening, though, I can't see anything obvious
wrong with your patch.

 Thomas



> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 9a10a76631..93248399ba 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -20,6 +20,13 @@
>  
>  #define BCM283X_NCPUS 4
>  
> +/* These type names are for specific SoCs; other than instantiating
> + * them, code using these devices should always handle them via the
> + * BCM283x base class, so they have no BCM2836(obj) etc macros.
> + */
> +#define TYPE_BCM2836 "bcm2836"
> +#define TYPE_BCM2837 "bcm2837"
> +
>  typedef struct BCM283XState {
>      /*< private >*/
>      DeviceState parent_obj;
> @@ -33,4 +40,16 @@ typedef struct BCM283XState {
>      BCM2835PeripheralState peripherals;
>  } BCM283XState;
>  
> +typedef struct BCM283XInfo BCM283XInfo;
> +
> +typedef struct BCM283XClass {
> +    DeviceClass parent_class;
> +    const BCM283XInfo *info;
> +} BCM283XClass;
> +
> +#define BCM283X_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
> +#define BCM283X_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
> +
>  #endif /* BCM2836_H */
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 1d1908654b..07d2705f96 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -23,6 +23,19 @@
>  /* "QA7" (Pi2) interrupt controller and mailboxes etc. */
>  #define BCM2836_CONTROL_BASE    0x40000000
>  
> +struct BCM283XInfo {
> +    const char *name;
> +};
> +
> +static const BCM283XInfo bcm283x_socs[] = {
> +    {
> +        .name = TYPE_BCM2836,
> +    },
> +    {
> +        .name = TYPE_BCM2837,
> +    },
> +};
> +
>  static void bcm2836_init(Object *obj)
>  {
>      BCM283XState *s = BCM283X(obj);
> @@ -156,25 +169,39 @@ static Property bcm2836_props[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> -static void bcm2836_class_init(ObjectClass *oc, void *data)
> +static void bcm283x_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> +    BCM283XClass *bc = BCM283X_CLASS(oc);
>  
> -    dc->props = bcm2836_props;
> +    bc->info = data;
>      dc->realize = bcm2836_realize;
> +    dc->props = bcm2836_props;
>  }
>  
> -static const TypeInfo bcm2836_type_info = {
> +static const TypeInfo bcm283x_type_info = {
>      .name = TYPE_BCM283X,
>      .parent = TYPE_DEVICE,
>      .instance_size = sizeof(BCM283XState),
>      .instance_init = bcm2836_init,
> -    .class_init = bcm2836_class_init,
> +    .class_size = sizeof(BCM283XClass),
> +    .abstract = true,
>  };
>  
>  static void bcm2836_register_types(void)
>  {
> -    type_register_static(&bcm2836_type_info);
> +    int i;
> +
> +    type_register_static(&bcm283x_type_info);
> +    for (i = 0; i < ARRAY_SIZE(bcm283x_socs); i++) {
> +        TypeInfo ti = {
> +            .name = bcm283x_socs[i].name,
> +            .parent = TYPE_BCM283X,
> +            .class_init = bcm283x_class_init,
> +            .class_data = (void *) &bcm283x_socs[i],
> +        };
> +        type_register(&ti);
> +    }
>  }
>  
>  type_init(bcm2836_register_types)
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 58c6e80a17..f588720138 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -136,7 +136,8 @@ static void raspi_init(MachineState *machine, int version)
>      BusState *bus;
>      DeviceState *carddev;
>  
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
> +    object_initialize(&s->soc, sizeof(s->soc),
> +                      version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
>  
>