Hi Glass,
> -----Original Message-----
> From: Simon Glass <s...@chromium.org>
> Sent: Wednesday, August 7, 2024 3:21 AM
> To: Kummari, Prasad <prasad.kumm...@amd.com>
> Cc: u-boot@lists.denx.de; git (AMD-Xilinx) <g...@amd.com>; Simek, Michal
> <michal.si...@amd.com>; Abbarapu, Venkatesh
> <venkatesh.abbar...@amd.com>; g...@xilinx.com;
> ja...@amarulasolutions.com; n-fran...@ti.com; d-g...@ti.com
> Subject: Re: [PATCH] cmd: sf: prevent overwriting the reserved memory
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Prasad,
>
> On Tue, 6 Aug 2024 at 06:08, Prasad Kummari <prasad.kumm...@amd.com> wrote:
> >
> > Added LMB API to prevent SF command from overwriting reserved memory
> > areas. The current SPI code does not use LMB APIs for loading data
> > into memory addresses. To resolve this, LMB APIs were added to check
> > the load address of an SF command and ensure it does not overwrite
> > reserved memory addresses. Similar checks are used in TFTP, serial
> > load, and boot code to prevent overwriting reserved memory.
>
> The SPI flash may be used to load other things, not just an OS. What is your
> use case or problem here?
[Prasad]: We have observed that SF command can overwrite the reserved area
without throwing any errors or warnings.
This issue was noticed when the TF-A area is reserved in the Device Tree at
address 0xf000000. The sf command is
corrupting the reserved area, and U-Boot relocation address too.
EX: TF-A reserved at ddr address 0xf000000
Versal NET> sf read 0x0f000000 0x0 0x100 ----> Overwriting reserved
area.
device 0 offset 0x0, size 0x100
SF: 256 bytes @ 0x0 Read: OK
U-boot relocation address relocaddr = 0x000000007fec2000
Versal NET> sf write 0x0000000077ec2000 0x0 0x100 --> Overwriting
reserved area.
device 0 offset 0x0, size 0x100
SF: 256 bytes @ 0x0 Written: OK
>
> Regards,
> Simon
>
>
> >
> > Signed-off-by: Prasad Kummari <prasad.kumm...@amd.com>
> > ---
> > UT:
> > Tested on Versal NET board
> >
> > Versal NET> sf read 0xf000000 0x0 0x100 device 0 offset 0x0, size
> > 0x100
> > ERROR: trying to overwrite reserved memory...
> >
> > Versal NET> sf read 0x79ea9000 0x 0x100 device 0 offset 0x0, size
> > 0x100
> > ERROR: trying to overwrite reserved memory...
> >
> > Versal NET> sf write 0x79ea9000 0x 0x100 device 0 offset 0x0, size
> > 0x100
> > ERROR: trying to overwrite reserved memory...
> >
> > Versal NET> sf write 0x79eaf000 0x20000 0x100 device 0 offset 0x20000,
> > size 0x100
> > ERROR: trying to overwrite reserved memory...
> >
> > cmd/sf.c | 38 +++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/sf.c b/cmd/sf.c
> > index f43a2e08b3..d2ce1af8a0 100644
> > --- a/cmd/sf.c
> > +++ b/cmd/sf.c
> > @@ -10,6 +10,7 @@
> > #include <div64.h>
> > #include <dm.h>
> > #include <log.h>
> > +#include <lmb.h>
> > #include <malloc.h>
> > #include <mapmem.h>
> > #include <spi.h>
> > @@ -272,6 +273,35 @@ static int spi_flash_update(struct spi_flash *flash,
> u32 offset,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_LMB
> > +static int do_spi_read_lmb_check(ulong start_addr, loff_t len) {
> > + struct lmb lmb;
> > + phys_size_t max_size;
> > + ulong end_addr;
> > +
> > + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> > + lmb_dump_all(&lmb);
> > +
> > + max_size = lmb_get_free_size(&lmb, start_addr);
> > + if (!max_size) {
> > + printf("ERROR: trying to overwrite reserved memory...\n");
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + end_addr = start_addr + max_size;
> > + if (!end_addr)
> > + end_addr = ULONG_MAX;
> > +
> > + if ((start_addr + len) > end_addr) {
> > + printf("ERROR: trying to overwrite reserved memory...\n");
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > static int do_spi_flash_read_write(int argc, char *const argv[]) {
> > unsigned long addr;
> > @@ -315,7 +345,13 @@ static int do_spi_flash_read_write(int argc, char
> *const argv[])
> > ret = spi_flash_update(flash, offset, len, buf);
> > } else if (strncmp(argv[0], "read", 4) == 0 ||
> > strncmp(argv[0], "write", 5) == 0) {
> > - int read;
> > + int read, ret;
> > +
> > +#ifdef CONFIG_LMB
> > + ret = do_spi_read_lmb_check(addr, len);
> > + if (ret)
> > + return ret;
> > +#endif
> >
> > read = strncmp(argv[0], "read", 4) == 0;
> > if (read)
> > --
> > 2.25.1
> >
