Re: [U-Boot] [PATCH v3 10/13] fdtdec: test: Add carveout tests

2019-04-12 Thread sjg
On Fri, Mar 22, 2019 at 03:53:07PM +0800, Simon Glass wrote:
> Hi Thierry,
>
> On Fri, 22 Mar 2019 at 02:10, Thierry Reding  wrote:
> >
> > From: Thierry Reding 
> >
> > Implement carveout tests for 32-bit and 64-bit builds.
> >
> > Signed-off-by: Thierry Reding 
> > ---
> > Changes in v2:
> > - new patch
> >
> >  lib/fdtdec_test.c | 152 ++
> >  1 file changed, 152 insertions(+)
>
> Reviewed-by: Simon Glass 
>
> Just an idea - as an alternative you could use the built-in device
> tree as your base rather than creating a new one. But perhaps that
> would only be safe on sandbox?

Yeah, running that test on a live system would mess up the reserved
memory regions. If U-Boot does something based on the reserved-memory
node, or passes it on to the kernel, that's going to cause issues.

The test also uses rather arbitrary values for the carveout, which may
not point at system memory at all.

Thierry

Applied to u-boot-dm, thanks!
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v3 10/13] fdtdec: test: Add carveout tests

2019-03-22 Thread Thierry Reding
On Fri, Mar 22, 2019 at 03:53:07PM +0800, Simon Glass wrote:
> Hi Thierry,
> 
> On Fri, 22 Mar 2019 at 02:10, Thierry Reding  wrote:
> >
> > From: Thierry Reding 
> >
> > Implement carveout tests for 32-bit and 64-bit builds.
> >
> > Signed-off-by: Thierry Reding 
> > ---
> > Changes in v2:
> > - new patch
> >
> >  lib/fdtdec_test.c | 152 ++
> >  1 file changed, 152 insertions(+)
> 
> Reviewed-by: Simon Glass 
> 
> Just an idea - as an alternative you could use the built-in device
> tree as your base rather than creating a new one. But perhaps that
> would only be safe on sandbox?

Yeah, running that test on a live system would mess up the reserved
memory regions. If U-Boot does something based on the reserved-memory
node, or passes it on to the kernel, that's going to cause issues.

The test also uses rather arbitrary values for the carveout, which may
not point at system memory at all.

Thierry


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v3 10/13] fdtdec: test: Add carveout tests

2019-03-22 Thread Simon Glass
Hi Thierry,

On Fri, 22 Mar 2019 at 02:10, Thierry Reding  wrote:
>
> From: Thierry Reding 
>
> Implement carveout tests for 32-bit and 64-bit builds.
>
> Signed-off-by: Thierry Reding 
> ---
> Changes in v2:
> - new patch
>
>  lib/fdtdec_test.c | 152 ++
>  1 file changed, 152 insertions(+)

Reviewed-by: Simon Glass 

Just an idea - as an alternative you could use the built-in device
tree as your base rather than creating a new one. But perhaps that
would only be safe on sandbox?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v3 10/13] fdtdec: test: Add carveout tests

2019-03-21 Thread Thierry Reding
From: Thierry Reding 

Implement carveout tests for 32-bit and 64-bit builds.

Signed-off-by: Thierry Reding 
---
Changes in v2:
- new patch

 lib/fdtdec_test.c | 152 ++
 1 file changed, 152 insertions(+)

diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
index 928950918413..f6defe16c5a6 100644
--- a/lib/fdtdec_test.c
+++ b/lib/fdtdec_test.c
@@ -141,6 +141,156 @@ static int run_test(const char *aliases, const char 
*nodes, const char *expect)
return 0;
 }
 
+static int make_fdt_carveout_device(void *fdt, uint32_t na, uint32_t ns)
+{
+   const char *basename = "/display";
+   struct fdt_memory carveout = {
+#ifdef CONFIG_PHYS_64BIT
+   .start = 0x18000,
+   .end = 0x18fff,
+#else
+   .start = 0x8000,
+   .end = 0x8fff,
+#endif
+   };
+   fdt32_t cells[4], *ptr = cells;
+   uint32_t upper, lower;
+   char name[32];
+   int offset;
+
+   /* store one or two address cells */
+   lower = fdt_addr_unpack(carveout.start, );
+
+   if (na > 1 && upper > 0)
+   snprintf(name, sizeof(name), "%s@%x,%x", basename, upper,
+lower);
+   else
+   snprintf(name, sizeof(name), "%s@%x", basename, lower);
+
+   if (na > 1)
+   *ptr++ = cpu_to_fdt32(upper);
+
+   *ptr++ = cpu_to_fdt32(lower);
+
+   /* store one or two size cells */
+   lower = fdt_size_unpack(carveout.end - carveout.start + 1, );
+
+   if (ns > 1)
+   *ptr++ = cpu_to_fdt32(upper);
+
+   *ptr++ = cpu_to_fdt32(lower);
+
+   offset = CHECK(fdt_add_subnode(fdt, 0, name + 1));
+   CHECK(fdt_setprop(fdt, offset, "reg", cells, (na + ns) * 
sizeof(*cells)));
+
+   return fdtdec_set_carveout(fdt, name, "memory-region", 0,
+  "framebuffer", );
+}
+
+static int check_fdt_carveout(void *fdt, uint32_t address_cells,
+ uint32_t size_cells)
+{
+#ifdef CONFIG_PHYS_64BIT
+   const char *name = "/display@1,8000";
+   const struct fdt_memory expected = {
+   .start = 0x18000,
+   .end = 0x18fff,
+   };
+#else
+   const char *name = "/display@8000";
+   const struct fdt_memory expected = {
+   .start = 0x8000,
+   .end = 0x8fff,
+   };
+#endif
+   struct fdt_memory carveout;
+
+   printf("carveout: %pap-%pap na=%u ns=%u: ", ,
+  , address_cells, size_cells);
+
+   CHECK(fdtdec_get_carveout(fdt, name, "memory-region", 0, ));
+
+   if ((carveout.start != expected.start) ||
+   (carveout.end != expected.end)) {
+   printf("carveout: %pap-%pap, expected %pap-%pap\n",
+  , ,
+  , );
+   return 1;
+   }
+
+   printf("pass\n");
+   return 0;
+}
+
+static int make_fdt_carveout(void *fdt, int size, uint32_t address_cells,
+uint32_t size_cells)
+{
+   fdt32_t na = cpu_to_fdt32(address_cells);
+   fdt32_t ns = cpu_to_fdt32(size_cells);
+#if defined(DEBUG) && defined(CONFIG_SANDBOX)
+   char filename[512];
+   int fd;
+#endif
+   int err;
+
+   CHECK(fdt_create(fdt, size));
+   CHECK(fdt_finish_reservemap(fdt));
+   CHECK(fdt_begin_node(fdt, ""));
+   CHECK(fdt_property(fdt, "#address-cells", , sizeof(na)));
+   CHECK(fdt_property(fdt, "#size-cells", , sizeof(ns)));
+   CHECK(fdt_end_node(fdt));
+   CHECK(fdt_finish(fdt));
+   CHECK(fdt_pack(fdt));
+
+   CHECK(fdt_open_into(fdt, fdt, FDT_SIZE));
+
+   err = make_fdt_carveout_device(fdt, address_cells, size_cells);
+
+#if defined(DEBUG) && defined(CONFIG_SANDBOX)
+   snprintf(filename, sizeof(filename), "/tmp/fdtdec-carveout-%u-%u.dtb",
+address_cells, size_cells);
+
+   fd = os_open(filename, OS_O_CREAT | OS_O_WRONLY);
+   if (fd < 0) {
+   printf("could not open .dtb file to write\n");
+   goto out;
+   }
+
+   os_write(fd, fdt, size);
+   os_close(fd);
+
+out:
+#endif
+   return err;
+}
+
+static int check_carveout(void)
+{
+   void *fdt;
+
+   fdt = malloc(FDT_SIZE);
+   if (!fdt) {
+   printf("%s: out of memory\n", __func__);
+   return 1;
+   }
+
+#ifndef CONFIG_PHYS_64BIT
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 1), 0);
+   CHECKOK(check_fdt_carveout(fdt, 1, 1));
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 2), 0);
+   CHECKOK(check_fdt_carveout(fdt, 1, 2));
+#else
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 1), -FDT_ERR_BADVALUE);
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 2), -FDT_ERR_BADVALUE);
+#endif
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 2, 1), 0);
+   CHECKOK(check_fdt_carveout(fdt, 2, 1));
+   CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 2, 2),