kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path
@ 2021-03-16 15:24 Nikos Nikoleris
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr Nikos Nikoleris
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Nikos Nikoleris @ 2021-03-16 15:24 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, drjones, alexandru.elisei, andre.przywara

This set of patches fixes the way we parse the stdout-path which is
used to set up the console. Prior to this, the code ignored the fact
that stdout-path is made of the path to the uart node as well as
parameters and as a result it would fail to find the relevant DT
node. In addition to minor fixes in the device tree code, this series
pulls a new version of libfdt from upstream.

Thanks,

Nikos

Nikos Nikoleris (4):
  lib/string: add strnlen and strrchr
  libfdt: Pull v1.6.0
  Makefile: Avoid double definition of libfdt_clean
  devicetree: Parse correctly the stdout-path

 lib/libfdt/README            |   3 +-
 Makefile                     |  12 +-
 lib/libfdt/Makefile.libfdt   |  10 +-
 lib/libfdt/version.lds       |  24 +-
 lib/libfdt/fdt.h             |  53 +--
 lib/libfdt/libfdt.h          | 766 +++++++++++++++++++++++++-----
 lib/libfdt/libfdt_env.h      | 109 ++---
 lib/libfdt/libfdt_internal.h | 206 +++++---
 lib/string.h                 |   5 +-
 lib/devicetree.c             |  15 +-
 lib/libfdt/fdt.c             | 200 +++++---
 lib/libfdt/fdt_addresses.c   | 101 ++++
 lib/libfdt/fdt_check.c       |  74 +++
 lib/libfdt/fdt_empty_tree.c  |  48 +-
 lib/libfdt/fdt_overlay.c     | 881 +++++++++++++++++++++++++++++++++++
 lib/libfdt/fdt_ro.c          | 512 +++++++++++++++-----
 lib/libfdt/fdt_rw.c          | 231 +++++----
 lib/libfdt/fdt_strerror.c    |  53 +--
 lib/libfdt/fdt_sw.c          | 297 ++++++++----
 lib/libfdt/fdt_wip.c         |  90 ++--
 lib/string.c                 |  30 +-
 21 files changed, 2890 insertions(+), 830 deletions(-)
 create mode 100644 lib/libfdt/fdt_addresses.c
 create mode 100644 lib/libfdt/fdt_check.c
 create mode 100644 lib/libfdt/fdt_overlay.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr
  2021-03-16 15:24 [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Nikos Nikoleris
@ 2021-03-16 15:24 ` Nikos Nikoleris
  2021-03-16 16:04   ` Andrew Jones
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 3/4] Makefile: Avoid double definition of libfdt_clean Nikos Nikoleris
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Nikos Nikoleris @ 2021-03-16 15:24 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, drjones, alexandru.elisei, andre.przywara

This change adds two functions from <string.h> in preparation for an
update in the libfdt.

Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
---
 lib/string.h |  4 +++-
 lib/string.c | 23 +++++++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/lib/string.h b/lib/string.h
index 493d51b..8da687e 100644
--- a/lib/string.h
+++ b/lib/string.h
@@ -7,12 +7,14 @@
 #ifndef __STRING_H
 #define __STRING_H
 
-extern unsigned long strlen(const char *buf);
+extern size_t strlen(const char *buf);
+extern size_t strnlen(const char *buf, size_t maxlen);
 extern char *strcat(char *dest, const char *src);
 extern char *strcpy(char *dest, const char *src);
 extern int strcmp(const char *a, const char *b);
 extern int strncmp(const char *a, const char *b, size_t n);
 extern char *strchr(const char *s, int c);
+extern char *strrchr(const char *s, int c);
 extern char *strstr(const char *haystack, const char *needle);
 extern void *memset(void *s, int c, size_t n);
 extern void *memcpy(void *dest, const void *src, size_t n);
diff --git a/lib/string.c b/lib/string.c
index 75257f5..9cd626f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -7,15 +7,24 @@
 
 #include "libcflat.h"
 
-unsigned long strlen(const char *buf)
+size_t strlen(const char *buf)
 {
-    unsigned long len = 0;
+    size_t len = 0;
 
     while (*buf++)
 	++len;
     return len;
 }
 
+size_t strnlen(const char *buf, size_t maxlen)
+{
+    const char *sc;
+
+    for (sc = buf; maxlen-- && *sc != '\0'; ++sc)
+        /* nothing */;
+    return sc - buf;
+}
+
 char *strcat(char *dest, const char *src)
 {
     char *p = dest;
@@ -55,6 +64,16 @@ char *strchr(const char *s, int c)
     return (char *)s;
 }
 
+char *strrchr(const char *s, int c)
+{
+    const char *last = NULL;
+    do {
+        if (*s != (char)c)
+            last = s;
+    } while (*s++);
+    return (char *)last;
+}
+
 char *strstr(const char *s1, const char *s2)
 {
     size_t l1, l2;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [kvm-unit-tests PATCH 3/4] Makefile: Avoid double definition of libfdt_clean
  2021-03-16 15:24 [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Nikos Nikoleris
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr Nikos Nikoleris
@ 2021-03-16 15:24 ` Nikos Nikoleris
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 4/4] devicetree: Parse correctly the stdout-path Nikos Nikoleris
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Nikos Nikoleris @ 2021-03-16 15:24 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, drjones, alexandru.elisei, andre.przywara

libfdt_clean is now defined in the libfdt Makefile and as a result, we
don't need to redefine it in the top Makefile. In addition we define
some variables for the remaining libfdt_clean rule.

Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
---
 Makefile | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index e0828fe..37aa3c6 100644
--- a/Makefile
+++ b/Makefile
@@ -40,8 +40,6 @@ cflatobjs := \
 LIBFDT_objdir = lib/libfdt
 LIBFDT_srcdir = $(SRCDIR)/lib/libfdt
 LIBFDT_archive = $(LIBFDT_objdir)/libfdt.a
-LIBFDT_include = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_INCLUDES))
-LIBFDT_version = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_VERSION))
 
 OBJDIRS += $(LIBFDT_objdir)
 
@@ -90,6 +88,10 @@ $(LIBFDT_archive): CFLAGS += -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/li
 $(LIBFDT_archive): $(addprefix $(LIBFDT_objdir)/,$(LIBFDT_OBJS))
 	$(AR) rcs $@ $^
 
+libfdt_clean: VECHO = echo " "
+libfdt_clean: STD_CLEANFILES = *.o *.d
+libfdt_clean: LIBFDT_dir = $(LIBFDT_objdir)
+libfdt_clean: SHAREDLIB_EXT = so
 
 # Build directory target
 .PHONY: directories
@@ -113,12 +115,8 @@ install: standalone
 clean: arch_clean
 	$(RM) lib/.*.d $(libcflat) $(cflatobjs)
 
-libfdt_clean:
-	$(RM) $(LIBFDT_archive) \
-	$(addprefix $(LIBFDT_objdir)/,$(LIBFDT_OBJS)) \
-	$(LIBFDT_objdir)/.*.d
-
 distclean: clean libfdt_clean
+	$(RM) $(LIBFDT_archive)
 	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
 	$(RM) -r tests logs logs.old
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [kvm-unit-tests PATCH 4/4] devicetree: Parse correctly the stdout-path
  2021-03-16 15:24 [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Nikos Nikoleris
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr Nikos Nikoleris
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 3/4] Makefile: Avoid double definition of libfdt_clean Nikos Nikoleris
@ 2021-03-16 15:24 ` Nikos Nikoleris
       [not found] ` <20210316152405.50363-3-nikos.nikoleris@arm.com>
  2021-03-16 17:03 ` [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Andrew Jones
  4 siblings, 0 replies; 9+ messages in thread
From: Nikos Nikoleris @ 2021-03-16 15:24 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, drjones, alexandru.elisei, andre.przywara

The stdout-path property in the device tree is a string of the form

"UART_NODE:OPTS"

Where UART_NODE is a compatible serial port and OPTS specify
parameters such as the baud. Fix the way we parse the node and, at
least for now, ignore options as we don't act on them.

Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
---
 lib/string.h     |  1 +
 lib/devicetree.c | 15 +++++++++------
 lib/string.c     |  7 +++++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/lib/string.h b/lib/string.h
index 8da687e..e1febfe 100644
--- a/lib/string.h
+++ b/lib/string.h
@@ -15,6 +15,7 @@ extern int strcmp(const char *a, const char *b);
 extern int strncmp(const char *a, const char *b, size_t n);
 extern char *strchr(const char *s, int c);
 extern char *strrchr(const char *s, int c);
+extern char *strchrnul(const char *s, int c);
 extern char *strstr(const char *haystack, const char *needle);
 extern void *memset(void *s, int c, size_t n);
 extern void *memcpy(void *dest, const void *src, size_t n);
diff --git a/lib/devicetree.c b/lib/devicetree.c
index 1020324..409d18b 100644
--- a/lib/devicetree.c
+++ b/lib/devicetree.c
@@ -265,21 +265,24 @@ int dt_get_bootargs(const char **bootargs)
 
 int dt_get_default_console_node(void)
 {
-	const struct fdt_property *prop;
+	const char *p, *q;
 	int node, len;
 
 	node = fdt_path_offset(fdt, "/chosen");
 	if (node < 0)
 		return node;
 
-	prop = fdt_get_property(fdt, node, "stdout-path", &len);
-	if (!prop) {
-		prop = fdt_get_property(fdt, node, "linux,stdout-path", &len);
-		if (!prop)
+	p = fdt_getprop(fdt, node, "stdout-path", &len);
+	if (!p) {
+		p = fdt_getprop(fdt, node, "linux,stdout-path", &len);
+		if (!p)
 			return len;
 	}
 
-	return fdt_path_offset(fdt, prop->data);
+	q = strchrnul(p, ':');
+	len = q - p;
+
+	return fdt_path_offset_namelen(fdt, p, len);
 }
 
 int dt_get_initrd(const char **initrd, u32 *size)
diff --git a/lib/string.c b/lib/string.c
index 9cd626f..c5853dc 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -74,6 +74,13 @@ char *strrchr(const char *s, int c)
     return (char *)last;
 }
 
+char *strchrnul(const char *s, int c)
+{
+    while (*s && *s != (char)c)
+        s++;
+    return (char *)s;
+}
+
 char *strstr(const char *s1, const char *s2)
 {
     size_t l1, l2;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr
  2021-03-16 15:24 ` [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr Nikos Nikoleris
@ 2021-03-16 16:04   ` Andrew Jones
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jones @ 2021-03-16 16:04 UTC (permalink / raw)
  To: Nikos Nikoleris; +Cc: kvm, pbonzini, alexandru.elisei, andre.przywara

On Tue, Mar 16, 2021 at 03:24:02PM +0000, Nikos Nikoleris wrote:
> This change adds two functions from <string.h> in preparation for an
> update in the libfdt.
> 
> Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
>  lib/string.h |  4 +++-
>  lib/string.c | 23 +++++++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/string.h b/lib/string.h
> index 493d51b..8da687e 100644
> --- a/lib/string.h
> +++ b/lib/string.h
> @@ -7,12 +7,14 @@
>  #ifndef __STRING_H
>  #define __STRING_H
>  
> -extern unsigned long strlen(const char *buf);
> +extern size_t strlen(const char *buf);
> +extern size_t strnlen(const char *buf, size_t maxlen);
>  extern char *strcat(char *dest, const char *src);
>  extern char *strcpy(char *dest, const char *src);
>  extern int strcmp(const char *a, const char *b);
>  extern int strncmp(const char *a, const char *b, size_t n);
>  extern char *strchr(const char *s, int c);
> +extern char *strrchr(const char *s, int c);
>  extern char *strstr(const char *haystack, const char *needle);
>  extern void *memset(void *s, int c, size_t n);
>  extern void *memcpy(void *dest, const void *src, size_t n);
> diff --git a/lib/string.c b/lib/string.c
> index 75257f5..9cd626f 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -7,15 +7,24 @@
>  
>  #include "libcflat.h"
>  
> -unsigned long strlen(const char *buf)
> +size_t strlen(const char *buf)
>  {
> -    unsigned long len = 0;
> +    size_t len = 0;
>  
>      while (*buf++)
>  	++len;
>      return len;
>  }
>  
> +size_t strnlen(const char *buf, size_t maxlen)
> +{
> +    const char *sc;
> +
> +    for (sc = buf; maxlen-- && *sc != '\0'; ++sc)
> +        /* nothing */;
> +    return sc - buf;
> +}
> +
>  char *strcat(char *dest, const char *src)
>  {
>      char *p = dest;
> @@ -55,6 +64,16 @@ char *strchr(const char *s, int c)
>      return (char *)s;
>  }
>  
> +char *strrchr(const char *s, int c)
> +{
> +    const char *last = NULL;
> +    do {
> +        if (*s != (char)c)

This should be ==

> +            last = s;
> +    } while (*s++);
> +    return (char *)last;
> +}
> +
>  char *strstr(const char *s1, const char *s2)
>  {
>      size_t l1, l2;
> -- 
> 2.25.1
>

Thanks,
drew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [kvm-unit-tests PATCH 2/4] libfdt: Pull v1.6.0
       [not found] ` <20210316152405.50363-3-nikos.nikoleris@arm.com>
@ 2021-03-16 16:12   ` Andrew Jones
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jones @ 2021-03-16 16:12 UTC (permalink / raw)
  To: Nikos Nikoleris; +Cc: kvm, pbonzini, alexandru.elisei, andre.przywara

On Tue, Mar 16, 2021 at 03:24:03PM +0000, Nikos Nikoleris wrote:
> This change updates the libfdt source files to v1.6.0 from
> git://git.kernel.org/pub/scm/utils/dtc/dtc.git
> 
> Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
>  lib/libfdt/README            |   3 +-
>  lib/libfdt/Makefile.libfdt   |  10 +-
>  lib/libfdt/version.lds       |  24 +-
>  lib/libfdt/fdt.h             |  53 +--
>  lib/libfdt/libfdt.h          | 766 +++++++++++++++++++++++++-----
>  lib/libfdt/libfdt_env.h      | 109 ++---
>  lib/libfdt/libfdt_internal.h | 206 +++++---
>  lib/libfdt/fdt.c             | 200 +++++---
>  lib/libfdt/fdt_addresses.c   | 101 ++++
>  lib/libfdt/fdt_check.c       |  74 +++
>  lib/libfdt/fdt_empty_tree.c  |  48 +-
>  lib/libfdt/fdt_overlay.c     | 881 +++++++++++++++++++++++++++++++++++
>  lib/libfdt/fdt_ro.c          | 512 +++++++++++++++-----
>  lib/libfdt/fdt_rw.c          | 231 +++++----
>  lib/libfdt/fdt_strerror.c    |  53 +--
>  lib/libfdt/fdt_sw.c          | 297 ++++++++----
>  lib/libfdt/fdt_wip.c         |  90 ++--
>  17 files changed, 2844 insertions(+), 814 deletions(-)
>  create mode 100644 lib/libfdt/fdt_addresses.c
>  create mode 100644 lib/libfdt/fdt_check.c
>  create mode 100644 lib/libfdt/fdt_overlay.c
> 
> diff --git a/lib/libfdt/README b/lib/libfdt/README
> index 24ad4fe..aed3454 100644
> --- a/lib/libfdt/README
> +++ b/lib/libfdt/README
> @@ -1,4 +1,5 @@
>  
>  The code in this directory is originally imported from the libfdt
                                 ^^  says originally, so we should
replace the whole paragraph with something like

The code in this directory has been imported from the libfdt directory
of git://git.kernel.org/pub/scm/utils/dtc/dtc.git - version 1.6.0.

> -directory of git://git.jdl.com/software/dtc.git - version 1.4.0.
> +directory of git://git.kernel.org/pub/scm/utils/dtc/dtc.git -
> +version 1.60
>

Thanks,
drew  


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path
  2021-03-16 15:24 [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Nikos Nikoleris
                   ` (3 preceding siblings ...)
       [not found] ` <20210316152405.50363-3-nikos.nikoleris@arm.com>
@ 2021-03-16 17:03 ` Andrew Jones
  2021-03-16 18:54   ` Nikos Nikoleris
  4 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-03-16 17:03 UTC (permalink / raw)
  To: Nikos Nikoleris; +Cc: kvm, pbonzini, alexandru.elisei, andre.przywara

On Tue, Mar 16, 2021 at 03:24:01PM +0000, Nikos Nikoleris wrote:
> This set of patches fixes the way we parse the stdout-path which is
> used to set up the console. Prior to this, the code ignored the fact
> that stdout-path is made of the path to the uart node as well as
> parameters and as a result it would fail to find the relevant DT
> node. In addition to minor fixes in the device tree code, this series
> pulls a new version of libfdt from upstream.
> 
> Thanks,
> 
> Nikos
> 
> Nikos Nikoleris (4):
>   lib/string: add strnlen and strrchr
>   libfdt: Pull v1.6.0
>   Makefile: Avoid double definition of libfdt_clean
>   devicetree: Parse correctly the stdout-path
> 
>  lib/libfdt/README            |   3 +-
>  Makefile                     |  12 +-
>  lib/libfdt/Makefile.libfdt   |  10 +-
>  lib/libfdt/version.lds       |  24 +-
>  lib/libfdt/fdt.h             |  53 +--
>  lib/libfdt/libfdt.h          | 766 +++++++++++++++++++++++++-----
>  lib/libfdt/libfdt_env.h      | 109 ++---
>  lib/libfdt/libfdt_internal.h | 206 +++++---
>  lib/string.h                 |   5 +-
>  lib/devicetree.c             |  15 +-
>  lib/libfdt/fdt.c             | 200 +++++---
>  lib/libfdt/fdt_addresses.c   | 101 ++++
>  lib/libfdt/fdt_check.c       |  74 +++
>  lib/libfdt/fdt_empty_tree.c  |  48 +-
>  lib/libfdt/fdt_overlay.c     | 881 +++++++++++++++++++++++++++++++++++
>  lib/libfdt/fdt_ro.c          | 512 +++++++++++++++-----
>  lib/libfdt/fdt_rw.c          | 231 +++++----
>  lib/libfdt/fdt_strerror.c    |  53 +--
>  lib/libfdt/fdt_sw.c          | 297 ++++++++----
>  lib/libfdt/fdt_wip.c         |  90 ++--
>  lib/string.c                 |  30 +-
>  21 files changed, 2890 insertions(+), 830 deletions(-)
>  create mode 100644 lib/libfdt/fdt_addresses.c
>  create mode 100644 lib/libfdt/fdt_check.c
>  create mode 100644 lib/libfdt/fdt_overlay.c
> 
> -- 
> 2.25.1
>

Just tried to give this a test run, but I couldn't compile it on my x86
Fedora machine with my cross compiler:

  gcc-aarch64-linux-gnu-9.2.1-3.fc32.1.x86_64 

Every file that includes libfdt_env.h gives me a message like this

In file included from lib/libfdt/fdt_overlay.c:7:
lib/libfdt/libfdt_env.h:13:10: fatal error: stdlib.h: No such file or directory
   13 | #include <stdlib.h>
      |          ^~~~~~~~~~
compilation terminated

So I commented out the #include line to see why it was there. We need
strtoul(). I quick hacked an incomplete one (below) and was able to
compile and run tests. However I see that 'make clean' is leaving behind
several libfdt files

$ git clean -ndx
Would remove lib/libfdt/.fdt.d
Would remove lib/libfdt/.fdt_addresses.d
Would remove lib/libfdt/.fdt_check.d
Would remove lib/libfdt/.fdt_empty_tree.d
Would remove lib/libfdt/.fdt_overlay.d
Would remove lib/libfdt/.fdt_ro.d
Would remove lib/libfdt/.fdt_rw.d
Would remove lib/libfdt/.fdt_strerror.d
Would remove lib/libfdt/.fdt_sw.d
Would remove lib/libfdt/.fdt_wip.d

Thanks,
drew

diff --git a/lib/stdlib.h b/lib/stdlib.h
new file mode 100644
index 000000000000..23a3f318d526
--- /dev/null
+++ b/lib/stdlib.h
@@ -0,0 +1,4 @@
+#ifndef _STDLIB_H_
+#define _STDLIB_H_
+unsigned long int strtoul(const char *nptr, char **endptr, int base);
+#endif
diff --git a/lib/string.c b/lib/string.c
index 9258625c3d15..2336559cd5a1 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -6,6 +6,7 @@
  */
 
 #include "libcflat.h"
+#include "stdlib.h"
 
 size_t strlen(const char *buf)
 {
@@ -161,7 +162,7 @@ void *memchr(const void *s, int c, size_t n)
     return NULL;
 }
 
-long atol(const char *ptr)
+static long __atol(const char *ptr, char **endptr)
 {
     long acc = 0;
     const char *s = ptr;
@@ -189,9 +190,23 @@ long atol(const char *ptr)
     if (neg)
         acc = -acc;
 
+    if (endptr)
+        *endptr = (char *)s;
+
     return acc;
 }
 
+long atol(const char *ptr)
+{
+       return __atol(ptr, NULL);
+}
+
+unsigned long int strtoul(const char *nptr, char **endptr, int base)
+{
+       assert(base == 10);
+       return __atol(nptr, endptr);
+}
+
 extern char **environ;
 
 char *getenv(const char *name)


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path
  2021-03-16 17:03 ` [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Andrew Jones
@ 2021-03-16 18:54   ` Nikos Nikoleris
  2021-03-16 19:01     ` Andrew Jones
  0 siblings, 1 reply; 9+ messages in thread
From: Nikos Nikoleris @ 2021-03-16 18:54 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, pbonzini, alexandru.elisei, andre.przywara


Hey Drew,

On 16/03/2021 17:03, Andrew Jones wrote:
> On Tue, Mar 16, 2021 at 03:24:01PM +0000, Nikos Nikoleris wrote:
>> This set of patches fixes the way we parse the stdout-path which is
>> used to set up the console. Prior to this, the code ignored the fact
>> that stdout-path is made of the path to the uart node as well as
>> parameters and as a result it would fail to find the relevant DT
>> node. In addition to minor fixes in the device tree code, this series
>> pulls a new version of libfdt from upstream.
>>
>> Thanks,
>>
>> Nikos
>>
>> Nikos Nikoleris (4):
>>    lib/string: add strnlen and strrchr
>>    libfdt: Pull v1.6.0
>>    Makefile: Avoid double definition of libfdt_clean
>>    devicetree: Parse correctly the stdout-path
>>
>>   lib/libfdt/README            |   3 +-
>>   Makefile                     |  12 +-
>>   lib/libfdt/Makefile.libfdt   |  10 +-
>>   lib/libfdt/version.lds       |  24 +-
>>   lib/libfdt/fdt.h             |  53 +--
>>   lib/libfdt/libfdt.h          | 766 +++++++++++++++++++++++++-----
>>   lib/libfdt/libfdt_env.h      | 109 ++---
>>   lib/libfdt/libfdt_internal.h | 206 +++++---
>>   lib/string.h                 |   5 +-
>>   lib/devicetree.c             |  15 +-
>>   lib/libfdt/fdt.c             | 200 +++++---
>>   lib/libfdt/fdt_addresses.c   | 101 ++++
>>   lib/libfdt/fdt_check.c       |  74 +++
>>   lib/libfdt/fdt_empty_tree.c  |  48 +-
>>   lib/libfdt/fdt_overlay.c     | 881 +++++++++++++++++++++++++++++++++++
>>   lib/libfdt/fdt_ro.c          | 512 +++++++++++++++-----
>>   lib/libfdt/fdt_rw.c          | 231 +++++----
>>   lib/libfdt/fdt_strerror.c    |  53 +--
>>   lib/libfdt/fdt_sw.c          | 297 ++++++++----
>>   lib/libfdt/fdt_wip.c         |  90 ++--
>>   lib/string.c                 |  30 +-
>>   21 files changed, 2890 insertions(+), 830 deletions(-)
>>   create mode 100644 lib/libfdt/fdt_addresses.c
>>   create mode 100644 lib/libfdt/fdt_check.c
>>   create mode 100644 lib/libfdt/fdt_overlay.c
>>
>> --
>> 2.25.1
>>
>
> Just tried to give this a test run, but I couldn't compile it on my x86
> Fedora machine with my cross compiler:
>
>    gcc-aarch64-linux-gnu-9.2.1-3.fc32.1.x86_64
>
> Every file that includes libfdt_env.h gives me a message like this
>
> In file included from lib/libfdt/fdt_overlay.c:7:
> lib/libfdt/libfdt_env.h:13:10: fatal error: stdlib.h: No such file or directory
>     13 | #include <stdlib.h>
>        |          ^~~~~~~~~~
> compilation terminated
>
> So I commented out the #include line to see why it was there. We need
> strtoul(). I quick hacked an incomplete one (below) and was able to
> compile and run tests. However I see that 'make clean' is leaving behind
> several libfdt files
>
Thanks for testing and for the fixes!

For some reason this isn't causing problems in my setup. gcc is
eliminating unused symbols and it doesn't need to link with strtoul(). I
see how this is a problem though and I will include your fix in the next
version.

> $ git clean -ndx
> Would remove lib/libfdt/.fdt.d
> Would remove lib/libfdt/.fdt_addresses.d
> Would remove lib/libfdt/.fdt_check.d
> Would remove lib/libfdt/.fdt_empty_tree.d
> Would remove lib/libfdt/.fdt_overlay.d
> Would remove lib/libfdt/.fdt_ro.d
> Would remove lib/libfdt/.fdt_rw.d
> Would remove lib/libfdt/.fdt_strerror.d
> Would remove lib/libfdt/.fdt_sw.d
> Would remove lib/libfdt/.fdt_wip.d
>

Sorry for that, I will fix in the Makefile.

Thanks,

Nikos

> Thanks,
> drew
>
> diff --git a/lib/stdlib.h b/lib/stdlib.h
> new file mode 100644
> index 000000000000..23a3f318d526
> --- /dev/null
> +++ b/lib/stdlib.h
> @@ -0,0 +1,4 @@
> +#ifndef _STDLIB_H_
> +#define _STDLIB_H_
> +unsigned long int strtoul(const char *nptr, char **endptr, int base);
> +#endif
> diff --git a/lib/string.c b/lib/string.c
> index 9258625c3d15..2336559cd5a1 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -6,6 +6,7 @@
>    */
>
>   #include "libcflat.h"
> +#include "stdlib.h"
>
>   size_t strlen(const char *buf)
>   {
> @@ -161,7 +162,7 @@ void *memchr(const void *s, int c, size_t n)
>       return NULL;
>   }
>
> -long atol(const char *ptr)
> +static long __atol(const char *ptr, char **endptr)
>   {
>       long acc = 0;
>       const char *s = ptr;
> @@ -189,9 +190,23 @@ long atol(const char *ptr)
>       if (neg)
>           acc = -acc;
>
> +    if (endptr)
> +        *endptr = (char *)s;
> +
>       return acc;
>   }
>
> +long atol(const char *ptr)
> +{
> +       return __atol(ptr, NULL);
> +}
> +
> +unsigned long int strtoul(const char *nptr, char **endptr, int base)
> +{
> +       assert(base == 10);
> +       return __atol(nptr, endptr);
> +}
> +
>   extern char **environ;
>
>   char *getenv(const char *name)
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path
  2021-03-16 18:54   ` Nikos Nikoleris
@ 2021-03-16 19:01     ` Andrew Jones
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jones @ 2021-03-16 19:01 UTC (permalink / raw)
  To: Nikos Nikoleris; +Cc: kvm, pbonzini, alexandru.elisei, andre.przywara

On Tue, Mar 16, 2021 at 06:54:21PM +0000, Nikos Nikoleris wrote:
> 
> Hey Drew,
> 
> On 16/03/2021 17:03, Andrew Jones wrote:
> > On Tue, Mar 16, 2021 at 03:24:01PM +0000, Nikos Nikoleris wrote:
> > > This set of patches fixes the way we parse the stdout-path which is
> > > used to set up the console. Prior to this, the code ignored the fact
> > > that stdout-path is made of the path to the uart node as well as
> > > parameters and as a result it would fail to find the relevant DT
> > > node. In addition to minor fixes in the device tree code, this series
> > > pulls a new version of libfdt from upstream.
> > > 
> > > Thanks,
> > > 
> > > Nikos
> > > 
> > > Nikos Nikoleris (4):
> > >    lib/string: add strnlen and strrchr
> > >    libfdt: Pull v1.6.0
> > >    Makefile: Avoid double definition of libfdt_clean
> > >    devicetree: Parse correctly the stdout-path
> > > 
> > >   lib/libfdt/README            |   3 +-
> > >   Makefile                     |  12 +-
> > >   lib/libfdt/Makefile.libfdt   |  10 +-
> > >   lib/libfdt/version.lds       |  24 +-
> > >   lib/libfdt/fdt.h             |  53 +--
> > >   lib/libfdt/libfdt.h          | 766 +++++++++++++++++++++++++-----
> > >   lib/libfdt/libfdt_env.h      | 109 ++---
> > >   lib/libfdt/libfdt_internal.h | 206 +++++---
> > >   lib/string.h                 |   5 +-
> > >   lib/devicetree.c             |  15 +-
> > >   lib/libfdt/fdt.c             | 200 +++++---
> > >   lib/libfdt/fdt_addresses.c   | 101 ++++
> > >   lib/libfdt/fdt_check.c       |  74 +++
> > >   lib/libfdt/fdt_empty_tree.c  |  48 +-
> > >   lib/libfdt/fdt_overlay.c     | 881 +++++++++++++++++++++++++++++++++++
> > >   lib/libfdt/fdt_ro.c          | 512 +++++++++++++++-----
> > >   lib/libfdt/fdt_rw.c          | 231 +++++----
> > >   lib/libfdt/fdt_strerror.c    |  53 +--
> > >   lib/libfdt/fdt_sw.c          | 297 ++++++++----
> > >   lib/libfdt/fdt_wip.c         |  90 ++--
> > >   lib/string.c                 |  30 +-
> > >   21 files changed, 2890 insertions(+), 830 deletions(-)
> > >   create mode 100644 lib/libfdt/fdt_addresses.c
> > >   create mode 100644 lib/libfdt/fdt_check.c
> > >   create mode 100644 lib/libfdt/fdt_overlay.c
> > > 
> > > --
> > > 2.25.1
> > > 
> > 
> > Just tried to give this a test run, but I couldn't compile it on my x86
> > Fedora machine with my cross compiler:
> > 
> >    gcc-aarch64-linux-gnu-9.2.1-3.fc32.1.x86_64
> > 
> > Every file that includes libfdt_env.h gives me a message like this
> > 
> > In file included from lib/libfdt/fdt_overlay.c:7:
> > lib/libfdt/libfdt_env.h:13:10: fatal error: stdlib.h: No such file or directory
> >     13 | #include <stdlib.h>
> >        |          ^~~~~~~~~~
> > compilation terminated
> > 
> > So I commented out the #include line to see why it was there. We need
> > strtoul(). I quick hacked an incomplete one (below) and was able to
> > compile and run tests. However I see that 'make clean' is leaving behind
> > several libfdt files
> > 
> Thanks for testing and for the fixes!
> 
> For some reason this isn't causing problems in my setup. gcc is
> eliminating unused symbols and it doesn't need to link with strtoul(). I
> see how this is a problem though and I will include your fix in the next
> version.

Please write a proper strtoul(). Mine was just a minimal hack so I could
progress and see if there was anything else to comment on.

Thanks,
drew

> 
> > $ git clean -ndx
> > Would remove lib/libfdt/.fdt.d
> > Would remove lib/libfdt/.fdt_addresses.d
> > Would remove lib/libfdt/.fdt_check.d
> > Would remove lib/libfdt/.fdt_empty_tree.d
> > Would remove lib/libfdt/.fdt_overlay.d
> > Would remove lib/libfdt/.fdt_ro.d
> > Would remove lib/libfdt/.fdt_rw.d
> > Would remove lib/libfdt/.fdt_strerror.d
> > Would remove lib/libfdt/.fdt_sw.d
> > Would remove lib/libfdt/.fdt_wip.d
> > 
> 
> Sorry for that, I will fix in the Makefile.
> 
> Thanks,
> 
> Nikos
> 
> > Thanks,
> > drew
> > 
> > diff --git a/lib/stdlib.h b/lib/stdlib.h
> > new file mode 100644
> > index 000000000000..23a3f318d526
> > --- /dev/null
> > +++ b/lib/stdlib.h
> > @@ -0,0 +1,4 @@
> > +#ifndef _STDLIB_H_
> > +#define _STDLIB_H_
> > +unsigned long int strtoul(const char *nptr, char **endptr, int base);
> > +#endif
> > diff --git a/lib/string.c b/lib/string.c
> > index 9258625c3d15..2336559cd5a1 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -6,6 +6,7 @@
> >    */
> > 
> >   #include "libcflat.h"
> > +#include "stdlib.h"
> > 
> >   size_t strlen(const char *buf)
> >   {
> > @@ -161,7 +162,7 @@ void *memchr(const void *s, int c, size_t n)
> >       return NULL;
> >   }
> > 
> > -long atol(const char *ptr)
> > +static long __atol(const char *ptr, char **endptr)
> >   {
> >       long acc = 0;
> >       const char *s = ptr;
> > @@ -189,9 +190,23 @@ long atol(const char *ptr)
> >       if (neg)
> >           acc = -acc;
> > 
> > +    if (endptr)
> > +        *endptr = (char *)s;
> > +
> >       return acc;
> >   }
> > 
> > +long atol(const char *ptr)
> > +{
> > +       return __atol(ptr, NULL);
> > +}
> > +
> > +unsigned long int strtoul(const char *nptr, char **endptr, int base)
> > +{
> > +       assert(base == 10);
> > +       return __atol(nptr, endptr);
> > +}
> > +
> >   extern char **environ;
> > 
> >   char *getenv(const char *name)
> > 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-16 19:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 15:24 [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Nikos Nikoleris
2021-03-16 15:24 ` [kvm-unit-tests PATCH 1/4] lib/string: add strnlen and strrchr Nikos Nikoleris
2021-03-16 16:04   ` Andrew Jones
2021-03-16 15:24 ` [kvm-unit-tests PATCH 3/4] Makefile: Avoid double definition of libfdt_clean Nikos Nikoleris
2021-03-16 15:24 ` [kvm-unit-tests PATCH 4/4] devicetree: Parse correctly the stdout-path Nikos Nikoleris
     [not found] ` <20210316152405.50363-3-nikos.nikoleris@arm.com>
2021-03-16 16:12   ` [kvm-unit-tests PATCH 2/4] libfdt: Pull v1.6.0 Andrew Jones
2021-03-16 17:03 ` [kvm-unit-tests PATCH 0/4] Fix the devicetree parser for stdout-path Andrew Jones
2021-03-16 18:54   ` Nikos Nikoleris
2021-03-16 19:01     ` Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).