Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell <peter.maydell@linaro.org>]

On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>> From: Qemu-devel <qemu-devel-
>> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
>> Maydell
>> Sent: Tuesday, 13 March 2018 08:35
>>
>> Now we have separate types for BCM2386 and BCM2387, we might as well
>> just hard-code the CPU type they use rather than having it passed
>> through as an object property. This then lets us put the initialization
>> of the CPU object in init rather than realize.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>>  static const BCM283XInfo bcm283x_socs[] = {
>>      {
>>          .name = TYPE_BCM2836,
>> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>
> At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?
>
> (Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)

Yeah, we should do that. I'd forgotten about that, I think
things just got lost in the shuffle of having several
patchsets that tried to change the same things at once.

I guess the simplest thing is to add a patch at the end of
the series that fixes the cpu type for bcm2836.


>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
>> version)
>>      /* Setup the SOC */
>>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>>                                     &error_abort);
>> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
>> -                            &error_abort);
>>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>>                              &error_abort);
>>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;
>
> What about the default_cpu_type field of MachineClass set in
> raspi[23]_machine_init? That seems irrelevant now...

Mmm. It doesn't hurt anything, though.

> Also, if anyone cares (I don't), we also just lost the ability
> to override the CPU type of a raspi model.

Yeah, that's deliberate -- I think that letting the user randomly
plug nonexistent combinations together just confuses people when
they don't work. I guess I should call it out in the commit message
though.

thanks
-- PMM