On Thu, 17 Mar 2016 18:31:42 +0100
Bernhard Nortmann <[email protected]> wrote:

> Am Montag, 25. Januar 2016 05:51:02 UTC+1 schrieb Siarhei Siamashka:
> > Doing certain operations may need uploading and executing code
> > on the device. For example, such operations right now are
> > reading/writing ARM CP15 coprocessor registers. Uploading the
> > code to the device is naturally overwriting some part of SRAM
> > as a side effect. Right now it is not a problem, because the
> > CP15 coprocessor registers are only accessed as part of uploading
> > and executing U-Boot SPL. They are nicely timed not to cause
> > problems and the temporary scratch area gets overwritten by
> > the SPL code anyway.
> >
> > But if we decide to provide access to such operations via
> > command line interface, then any side effects may potentially
> > cause problems for the users. Consider the following scenario:
> >
> >   sunxi-fel clear 0x2000 0x100 \
> >             advanced-command-which-uploads-and-executes-code \
> >             hexdump 0x2000 0x100
> >
> > The user may rightfully expect that clearing a buffer in SRAM
> > to zero and then reading it back should show all zero bytes.
> > But inserting advanced commands in the middle may cause data
> > corruption.
> >
> > In order to resolve this problem, just move the scratch area
> > away from the 0x2000-0x5BFF addresses range. These particular
> > addresses are already known to the users as a safe place for
> > their bare metal expariments in FEL mode. The "sunxi-fel spl"
> > command is a special case though and it is expected to
> > overwrite data in this area too.
> >
> > A possible alternative would be to just backup & restore data
> > in the scratch area. But this has some disadvantages:
> >   1. Extra code in the sunxi-fel tool and extra roundtrips over
> >      USB to do the backup/restore job.
> >   2. If we allow the OpenRISC core to use the 0x2000-0x5C00
> >      range in SRAM A1, then this becomes unsafe and racy
> >      (we can't really backup & restore data without causing
> >      a temporarily glitch for the currently running code on
> >      the OpenRISC core).
> >
> > To sum it up. With this patch we make it so that now the
> > 0x2000-0x5BFF range is freely available for the users of the
> > sunxi-fel tool. The 0x1000-0x1FFF range is off limits (the
> > upper part of it is used by the FEL IRQ handler, the lower
> > part of it is reserved for internal use by the sunxi-fel
> > tool). The 0x0000-0x1FFF addresses range is reserved for

BTW, just noticed, it was a typo and this comment should read
as "0x0000-0x0FFF addresses".

> > passing data from the SPL to the main U-Boot binary (via
> > the SPL header) and is also off limits.
> >
> > Signed-off-by: Siarhei Siamashka <[email protected]>
> > ---
> >  fel.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/fel.c b/fel.c
> > index 40cb82a..a597e11 100644
> > --- a/fel.c
> > +++ b/fel.c
> > @@ -505,51 +505,51 @@ sram_swap_buffers ar100_abusing_sram_swap_buffers[] = 
> > {
> >  soc_sram_info soc_sram_info_table[] = {
> >     {
> >             .soc_id       = 0x1623, /* Allwinner A10 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0xA200, .thunk_size = 0x200,
> >             .swap_buffers = a10_a13_a20_sram_swap_buffers,
> >             .needs_l2en   = 1,
> >     },
> >     {
> >             .soc_id       = 0x1625, /* Allwinner A13 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0xA200, .thunk_size = 0x200,
> >             .swap_buffers = a10_a13_a20_sram_swap_buffers,
> >             .needs_l2en   = 1,
> >     },
> >     {
> >             .soc_id       = 0x1651, /* Allwinner A20 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0xA200, .thunk_size = 0x200,
> >             .swap_buffers = a10_a13_a20_sram_swap_buffers,
> >     },
> >     {
> >             .soc_id       = 0x1650, /* Allwinner A23 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0x46E00, .thunk_size = 0x200,
> >             .swap_buffers = ar100_abusing_sram_swap_buffers,
> >     },
> >     {
> >             .soc_id       = 0x1633, /* Allwinner A31 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0x22E00, .thunk_size = 0x200,
> >             .swap_buffers = a31_sram_swap_buffers,
> >     },
> >     {
> >             .soc_id       = 0x1667, /* Allwinner A33 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0x46E00, .thunk_size = 0x200,
> >             .swap_buffers = ar100_abusing_sram_swap_buffers,
> >     },
> >     {
> >             .soc_id       = 0x1673, /* Allwinner A83T */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .thunk_addr   = 0x46E00, .thunk_size = 0x200,
> >             .swap_buffers = ar100_abusing_sram_swap_buffers,
> >     },
> >     {
> >             .soc_id       = 0x1680, /* Allwinner H3 */
> > -           .scratch_addr = 0x2000,
> > +           .scratch_addr = 0x1000,
> >             .mmu_tt_addr  = 0x8000,
> >             .thunk_addr   = 0xA200, .thunk_size = 0x200,
> >             .swap_buffers = a10_a13_a20_sram_swap_buffers,
> > --
> > 2.4.10  
> 
> This patch does not modify/adjust the .scratch_addr for the 
> generic_sram_info - is that intentional?

That's a good catch, thanks.

However we should just get rid of the generic_sram_info. It does not
work on A64 and A80 because of a different SPL load address. Also the
way it works (defragment the first 32KiB in SRAM) allows to support
SPL sizes only up to ~21KiB. It was an improvement over the ~15KiB
size limitation of the legacy FEL boot handling implementation, but
the SPL in U-Boot keeps getting more and more bloated all the time and
we are already over this limit. In addition, because of the special
NAND boot requirements, the SPL size has to be aligned at 8KiB boundary
instead of the 512 bytes alignment used in the past. This way even if
we have an SPL with 20KiB size, this size has to be rounded up to
24KiB because of the new NAND alignment requirements and exceed the
~21KiB limitation. For additional details about the SPL size and
alignment requirements check

    https://linux-sunxi.org/BROM#U-Boot_SPL_limitations

As far as I know, "sunxi-fel spl" boot has been tested to work on all
known Allwinner SoC variants at least in an experimental form (even
though not all of this code has been merged yet):

    https://linux-sunxi.org/FEL/USBBoot#SoC_support_status

So instead of a fallback to this early generic implementation hack,
IMHO the sunxi-fel tool should just fail with an error message on
unsupported SoC variants.

> Successfully started ("sunxi-fel uboot") U-Boot 2015.10 on A20 with this 
> modification, _after_ applying 1/7 
> (https://github.com/ssvb/sunxi-tools/commit/a99c75f4d05349622b4ebd4c3054a0b6acb9d5af)
>  
> first.
> 
> Tested-by: Bernhard Nortmann <[email protected]>

Thanks.

-- 
Best regards,
Siarhei Siamashka

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to