Re: [Qemu-devel] [PATCH v2 2/3] raspi: Raspberry Pi 3 support

[bzt]

Dear Phil,

As you can see, the board ids are quite controversial, and bcm2837 is
not listed at all. Therefore 0xc44 is just a constant to identify qemu
"board". Feel free to change the id if you'd like.

About the r1 register, you've liked the wrong firmware. That's for
AArch32 mode, not for AArch64 mode which the raspi3 machine type uses.
In AArch64 mode, the firmware
https://github.com/raspberrypi/tools/blob/920c7ed2ee/armstubs/armstub8.S
passes only the device tree pointer in x0 and only for the primary
CPU, no board id of any kind.

Yes I know that the real firmware can start a hw raspi3 board in 32
bit mode, but it's better to have 64 bit as default, as raspi2 is 32
bit only anyway. Unfortunately there's no way to autodetect if the
image passed to the -kernel argument was compiled for AArch32 or
AArch64 mode, the instructions are way to similarly encoded and
there's no magic in raw binaries. Real hw board differentiates by
checking the file name of the kernel file on the boot partition (named
kernel7.img vs. kernel8.img) and parsing config.txt for arm specific
options. This is not an option for qemu as it does not emulate VC GPU
bootcode nor does it receive an entire boot partition with the -kernel
argument. Another reason to use AArch64 for raspi3 machine is if a
real board finds both kernel7.img and kernel8.img on the boot
partition, then it will load the AArch64 kernel.

Cheers,
bzt

On 9/3/19, Philippe Mathieu-Daudé <address@hidden> wrote:
> Hi Pekka and Zoltán,
>
> On Fri, Feb 16, 2018 at 8:04 AM Pekka Enberg <address@hidden> wrote:
>>
>> This patch adds Raspberry Pi 3 support to hw/arm/raspi.c. The
>> differences to Pi 2 are:
>>
>>  - Firmware address
>>  - Board ID
>>  - Board revision
>>
>> The CPU is different too, but that's going to be configured as part of
>> the machine default CPU when we introduce a new machine type.
>>
>> The patch was written from scratch by me but the logic is similar to
>> Zoltán Baldaszti's previous work, which I used as a reference (with
>> permission from the author):
>>
>>   https://github.com/bztsrc/qemu-raspi3
>>
>> Signed-off-by: Pekka Enberg <address@hidden>
>> ---
>>  hw/arm/raspi.c | 31 +++++++++++++++++++++----------
>>  1 file changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index c24a4a1b14..66fe10e376 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -5,6 +5,9 @@
>>   * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
>>   * Written by Andrew Baumann
>>   *
>> + * Raspberry Pi 3 emulation Copyright (c) 2018 Zoltán Baldaszti
>> + * Upstream code cleanup (c) 2018 Pekka Enberg
>> + *
>>   * This code is licensed under the GNU GPLv2 and later.
>>   */
>>
>> @@ -22,10 +25,11 @@
>>  #define SMPBOOT_ADDR    0x300 /* this should leave enough space for ATAGS
>> */
>>  #define MVBAR_ADDR      0x400 /* secure vectors */
>>  #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
>> -#define FIRMWARE_ADDR   0x8000 /* Pi loads kernel.img here by default */
>> +#define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default
>> */
>> +#define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default
>> */
>>
>>  /* Table of Linux board IDs for different Pi versions */
>> -static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
>> +static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] =
>> 0xc44};
>
> Where does the value 0xc44 comes from?
>
> I can only find 0xc42/0xc43 defined:
> https://github.com/raspberrypi/linux/blob/rpi-3.18.y/arch/arm/tools/mach-types#L525
>
> 0xc43 seems controversial, see
> http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-February/001268.html
> addition:
> https://github.com/raspberrypi/linux/commit/d9fac63adac#diff-6722037d79570df5b392a49e0e006573R526
>
> Looking at the firmware source, the r1 register is always set to the
> BCM2708 machine id before calling the kernel:
> https://github.com/raspberrypi/tools/blob/920c7ed2ee/armstubs/armstub7.S#L133
>
> Thanks,
>
> Phil.
>