From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4DB0FC32771 for ; Sat, 18 Jan 2020 22:29:24 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F3C3E2468D for ; Sat, 18 Jan 2020 22:29:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DO8pQ0L4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F3C3E2468D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:45388 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iswao-0006kZ-Sf for qemu-devel@archiver.kernel.org; Sat, 18 Jan 2020 17:29:23 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:37195) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iswa9-0006K6-QG for qemu-devel@nongnu.org; Sat, 18 Jan 2020 17:28:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iswa7-0004jq-UF for qemu-devel@nongnu.org; Sat, 18 Jan 2020 17:28:41 -0500 Received: from mail-io1-xd42.google.com ([2607:f8b0:4864:20::d42]:45816) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iswa7-0004jY-Ng; Sat, 18 Jan 2020 17:28:39 -0500 Received: by mail-io1-xd42.google.com with SMTP id i11so29768521ioi.12; Sat, 18 Jan 2020 14:28:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=walFZZcTozveskYiOXwPzNEmRLzkjzl/4sHRkc1mPeg=; b=DO8pQ0L41BJDxjeIzzLEec8abCk2v3AIZ97+eFAVH6MXoRpvxTpy43PAFFSHJUNTFP dGQd3O5djvSXVQLuOjGsirXhepGQH8NQP7OrU2HyZflJY3sG2KR1tPwDfsWIaWPG0rDE SFly7RMmL29V+CypzrmxtG0PPjCKol/da/YFWvJN1NJRPmuZjKvynmS/04XDS4nuqJXL zI3tfCSLcWlYzXEzKFmCugh1YaRmKBIf97IhYr36Qse/r2grC1gLF5VFQAtoQ8TuX1ud mo25bcob9VWzuNOROYcUQL8NnNfeHIwQ7VZPhav6B6yDSNV/3eb/eVf5P8PZIDgOZlSa EQ4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=walFZZcTozveskYiOXwPzNEmRLzkjzl/4sHRkc1mPeg=; b=uR9AZv9FeMC0qqkGOnVgJ0oYIlLMgf3CGzSrWNCotuGBO3obgBB7kIXVIvDCxFnyoG Zy2TKCWrJys+oyhx0/s68mtqUDPjGlwl5mCJcmkTckdabciRqTaa2rEicu4t7Z/60VKy VrD84dOa0aLOcZ/g5BOjO34XNjHjfonqtAzTTfxIfxc5sfAq59Mzvhz3Qg7Eb9ix21kd RCLUpbmHFyIXnTJ30aZWmtW72pedHX4zJP7FcQ2RcjKKeBX6EoyzOuQ6RZbFFIhjAT0r Gsry3Z750VyvEmVDi7s2fzGMVZ1lqW2igZDdudwZEF5ugT8tbGR+KcJJJ/mTHJz29D9y DKVQ== X-Gm-Message-State: APjAAAVk7UScrHB0iSSJ8L9nOi8Aq6fb7dTwLksKYqLKF+IitypXexgt ZS+tdC/KlqosHtW+DLgeIBONHTlnLSZZ+tKQY44= X-Google-Smtp-Source: APXvYqyKvpeP+CXwTj6S1ykg2HTeM+G+C2hHtEIUhAAXOZ8PKuUCDf73tjvWQPaSZdJDBp5I0UWIwvbYC5qlAqxBp9o= X-Received: by 2002:a02:bb02:: with SMTP id y2mr37488577jan.99.1579386518755; Sat, 18 Jan 2020 14:28:38 -0800 (PST) MIME-Version: 1.0 References: <20200108200020.4745-1-nieklinnenbank@gmail.com> <20200108200020.4745-11-nieklinnenbank@gmail.com> <9c878f16-b18c-cc1a-9e82-40dbdd31b823@redhat.com> <316c94b7-fea2-f44e-6d31-10fb41b09be3@redhat.com> In-Reply-To: <316c94b7-fea2-f44e-6d31-10fb41b09be3@redhat.com> From: Niek Linnenbank Date: Sat, 18 Jan 2020 23:28:27 +0100 Message-ID: Subject: Re: [PATCH v3 10/17] hw/arm/allwinner-h3: add Boot ROM support To: =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: multipart/alternative; boundary="00000000000069def1059c7194a3" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::d42 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , qemu-arm , QEMU Developers Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000069def1059c7194a3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, Jan 18, 2020 at 10:09 AM Philippe Mathieu-Daud=C3=A9 wrote: > On 1/15/20 12:10 AM, Niek Linnenbank wrote: > > On Tue, Jan 14, 2020 at 12:28 AM Philippe Mathieu-Daud=C3=A9 > > > wrote: > > > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: > > > A real Allwinner H3 SoC contains a Boot ROM which is the > > > first code that runs right after the SoC is powered on. > > > The Boot ROM is responsible for loading user code (e.g. a > bootloader) > > > from any of the supported external devices and writing the > downloaded > > > code to internal SRAM. After loading the SoC begins executing th= e > > code > > > written to SRAM. This commits adds emulation of the Boot ROM > firmware > > > setup functionality by loading user code from SD card. > > > > > > Signed-off-by: Niek Linnenbank > > > > > --- > > > include/hw/arm/allwinner-h3.h | 23 +++++++++++++++++++++++ > > > hw/arm/allwinner-h3.c | 28 +++++++++++++++++++++++++++= + > > > hw/arm/orangepi.c | 3 +++ > > > 3 files changed, 54 insertions(+) > > > > > > diff --git a/include/hw/arm/allwinner-h3.h > > b/include/hw/arm/allwinner-h3.h > > > index 5d74cca28e..4b66227ac4 100644 > > > --- a/include/hw/arm/allwinner-h3.h > > > +++ b/include/hw/arm/allwinner-h3.h > > > @@ -50,6 +50,7 @@ > > > #include "hw/sd/allwinner-sdhost.h" > > > #include "hw/net/allwinner-sun8i-emac.h" > > > #include "target/arm/cpu.h" > > > +#include "sysemu/block-backend.h" > > > > > > /** > > > * Allwinner H3 device list > > > @@ -130,4 +131,26 @@ typedef struct AwH3State { > > > MemoryRegion sram_c; > > > } AwH3State; > > > > > > +/** > > > + * Emulate Boot ROM firmware setup functionality. > > > + * > > > + * A real Allwinner H3 SoC contains a Boot ROM > > > + * which is the first code that runs right after > > > + * the SoC is powered on. The Boot ROM is responsible > > > + * for loading user code (e.g. a bootloader) from any > > > + * of the supported external devices and writing the > > > + * downloaded code to internal SRAM. After loading the SoC > > > + * begins executing the code written to SRAM. > > > + * > > > + * This function emulates the Boot ROM by copying 32 KiB > > > + * of data from the given block device and writes it to > > > + * the start of the first internal SRAM memory. > > > + * > > > + * @s: Allwinner H3 state object pointer > > > + * @blk: Block backend device object pointer > > > + * @errp: Error object pointer for raising errors > > > + */ > > > +void allwinner_h3_bootrom_setup(AwH3State *s, BlockBackend *blk= , > > > + Error **errp); > > > + > > > #endif /* HW_ARM_ALLWINNER_H3_H */ > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > > index e692432b4e..e7b768ad5b 100644 > > > --- a/hw/arm/allwinner-h3.c > > > +++ b/hw/arm/allwinner-h3.c > > > @@ -27,6 +27,7 @@ > > > #include "hw/char/serial.h" > > > #include "hw/misc/unimp.h" > > > #include "hw/usb/hcd-ehci.h" > > > +#include "hw/loader.h" > > > #include "sysemu/sysemu.h" > > > #include "hw/arm/allwinner-h3.h" > > > > > > @@ -168,6 +169,33 @@ enum { > > > AW_H3_GIC_NUM_SPI =3D 128 > > > }; > > > > > > +void allwinner_h3_bootrom_setup(AwH3State *s, BlockBackend *blk= , > > Error **errp) > > > +{ > > > + uint8_t *buffer; > > > + int64_t rom_size =3D 32 * KiB; > > > > Why restrict to 32K? The A1 SRAM is 64K. > > > > > > The reason is that the actual Boot ROM on the H3 also uses 32K: > > https://linux-sunxi.org/BROM > > > > See the 'U-Boot SPL Limitations' table at the end of the page. > > > > You can see the comment in the table there regarding the 32 KiB: > > "Sizes larger than 32 KiB are rejected by the BROM. Exactly 32 KiB i= s > > fine, as verified by writing a special pattern at the end of the SPL an= d > > checking it in the SRAM." > > Probably it would not harm to increase it to the full size of the SRAM, > > but I tried to model > > the behavior as close to the real hardware as possible. > > OK, then please document this difference in the commit description - > such "While the A1 SRAM is 64K, we limit to 32K because ..." - and add a > reference to https://linux-sunxi.org/BROM#U-Boot_SPL_limitations > > Agreed, I'll add this to the commit message. --=20 Niek Linnenbank --00000000000069def1059c7194a3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sat, Jan 18, 2020 at 10:09 AM Phil= ippe Mathieu-Daud=C3=A9 <philmd@red= hat.com> wrote:
On 1/15/20 12:10 AM, Niek Linnenbank wrote:
> On Tue, Jan 14, 2020 at 12:28 AM Philippe Mathieu-Daud=C3=A9
> <philmd@redh= at.com <mailto:philmd@redhat.com>> wrote:
>
>=C2=A0 =C2=A0 =C2=A0On 1/8/20 9:00 PM, Niek Linnenbank wrote:
>=C2=A0 =C2=A0 =C2=A0 > A real Allwinner H3 SoC contains a Boot ROM w= hich is the
>=C2=A0 =C2=A0 =C2=A0 > first code that runs right after the SoC is p= owered on.
>=C2=A0 =C2=A0 =C2=A0 > The Boot ROM is responsible for loading user = code (e.g. a bootloader)
>=C2=A0 =C2=A0 =C2=A0 > from any of the supported external devices an= d writing the downloaded
>=C2=A0 =C2=A0 =C2=A0 > code to internal SRAM. After loading the SoC = begins executing the
>=C2=A0 =C2=A0 =C2=A0code
>=C2=A0 =C2=A0 =C2=A0 > written to SRAM. This commits adds emulation = of the Boot ROM firmware
>=C2=A0 =C2=A0 =C2=A0 > setup functionality by loading user code from= SD card.
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.co= m
>=C2=A0 =C2=A0 =C2=A0<mailto:nieklinnenbank@gmail.com>>
>=C2=A0 =C2=A0 =C2=A0 > ---
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0include/hw/arm/allwinner-h3.h | 2= 3 +++++++++++++++++++++++
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0hw/arm/allwinner-h3.c=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0| 28 ++++++++++++++++++++++++++++
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0hw/arm/orangepi.c=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 3 +++
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A03 files changed, 54 insertions(+)=
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > diff --git a/include/hw/arm/allwinner-h3.h >=C2=A0 =C2=A0 =C2=A0b/include/hw/arm/allwinner-h3.h
>=C2=A0 =C2=A0 =C2=A0 > index 5d74cca28e..4b66227ac4 100644
>=C2=A0 =C2=A0 =C2=A0 > --- a/include/hw/arm/allwinner-h3.h
>=C2=A0 =C2=A0 =C2=A0 > +++ b/include/hw/arm/allwinner-h3.h
>=C2=A0 =C2=A0 =C2=A0 > @@ -50,6 +50,7 @@
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/sd/allwinner-sd= host.h"
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/net/allwinner-s= un8i-emac.h"
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "target/arm/cpu.h&q= uot;
>=C2=A0 =C2=A0 =C2=A0 > +#include "sysemu/block-backend.h"<= br> >=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0/**
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0 * Allwinner H3 device list
>=C2=A0 =C2=A0 =C2=A0 > @@ -130,4 +131,26 @@ typedef struct AwH3State= {
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0MemoryRegion sram_c= ;
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0} AwH3State;
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > +/**
>=C2=A0 =C2=A0 =C2=A0 > + * Emulate Boot ROM firmware setup functiona= lity.
>=C2=A0 =C2=A0 =C2=A0 > + *
>=C2=A0 =C2=A0 =C2=A0 > + * A real Allwinner H3 SoC contains a Boot R= OM
>=C2=A0 =C2=A0 =C2=A0 > + * which is the first code that runs right a= fter
>=C2=A0 =C2=A0 =C2=A0 > + * the SoC is powered on. The Boot ROM is re= sponsible
>=C2=A0 =C2=A0 =C2=A0 > + * for loading user code (e.g. a bootloader)= from any
>=C2=A0 =C2=A0 =C2=A0 > + * of the supported external devices and wri= ting the
>=C2=A0 =C2=A0 =C2=A0 > + * downloaded code to internal SRAM. After l= oading the SoC
>=C2=A0 =C2=A0 =C2=A0 > + * begins executing the code written to SRAM= .
>=C2=A0 =C2=A0 =C2=A0 > + *
>=C2=A0 =C2=A0 =C2=A0 > + * This function emulates the Boot ROM by co= pying 32 KiB
>=C2=A0 =C2=A0 =C2=A0 > + * of data from the given block device and w= rites it to
>=C2=A0 =C2=A0 =C2=A0 > + * the start of the first internal SRAM memo= ry.
>=C2=A0 =C2=A0 =C2=A0 > + *
>=C2=A0 =C2=A0 =C2=A0 > + * @s: Allwinner H3 state object pointer
>=C2=A0 =C2=A0 =C2=A0 > + * @blk: Block backend device object pointer=
>=C2=A0 =C2=A0 =C2=A0 > + * @errp: Error object pointer for raising e= rrors
>=C2=A0 =C2=A0 =C2=A0 > + */
>=C2=A0 =C2=A0 =C2=A0 > +void allwinner_h3_bootrom_setup(AwH3State *s= , BlockBackend *blk,
>=C2=A0 =C2=A0 =C2=A0 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Error= **errp);
>=C2=A0 =C2=A0 =C2=A0 > +
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#endif /* HW_ARM_ALLWINNER_H3_H *= /
>=C2=A0 =C2=A0 =C2=A0 > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/a= llwinner-h3.c
>=C2=A0 =C2=A0 =C2=A0 > index e692432b4e..e7b768ad5b 100644
>=C2=A0 =C2=A0 =C2=A0 > --- a/hw/arm/allwinner-h3.c
>=C2=A0 =C2=A0 =C2=A0 > +++ b/hw/arm/allwinner-h3.c
>=C2=A0 =C2=A0 =C2=A0 > @@ -27,6 +27,7 @@
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/char/serial.h&q= uot;
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/misc/unimp.h&qu= ot;
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/usb/hcd-ehci.h&= quot;
>=C2=A0 =C2=A0 =C2=A0 > +#include "hw/loader.h"
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "sysemu/sysemu.h&qu= ot;
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0#include "hw/arm/allwinner-h= 3.h"
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > @@ -168,6 +169,33 @@ enum {
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0AW_H3_GIC_NUM_SPI= =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D 128
>=C2=A0 =C2=A0 =C2=A0 >=C2=A0 =C2=A0};
>=C2=A0 =C2=A0 =C2=A0 >
>=C2=A0 =C2=A0 =C2=A0 > +void allwinner_h3_bootrom_setup(AwH3State *s= , BlockBackend *blk,
>=C2=A0 =C2=A0 =C2=A0Error **errp)
>=C2=A0 =C2=A0 =C2=A0 > +{
>=C2=A0 =C2=A0 =C2=A0 > +=C2=A0 =C2=A0 uint8_t *buffer;
>=C2=A0 =C2=A0 =C2=A0 > +=C2=A0 =C2=A0 int64_t rom_size =3D 32 * KiB;=
>
>=C2=A0 =C2=A0 =C2=A0Why restrict to 32K? The A1 SRAM is 64K.
>
>
> The reason is that the actual Boot ROM on the H3 also uses 32K:
> https://linux-sunxi.org/BROM
>
> See the 'U-Boot SPL Limitations' table at the end of the page.=
>
> You can see the comment in the table there regarding the 32 KiB:
>=C2=A0 =C2=A0 "Sizes larger than 32 KiB are rejected by the BROM. = Exactly 32 KiB is
> fine, as verified by writing a special pattern at the end of the SPL a= nd
> checking it in the SRAM."
> Probably it would not harm to increase it to the full size of the SRAM= ,
> but I tried to model
> the behavior as close to the real hardware as possible.

OK, then please document this difference in the commit description -
such "While the A1 SRAM is 64K, we limit to 32K because ..." - an= d add a
reference to https://linux-sunxi.org/BROM#U-Boot_= SPL_limitations

Agreed, I'll add this to the commit message.

--
Niek Linnenbank

--00000000000069def1059c7194a3--