Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 6 Dec 2018 18:37:20 +0800
From:   Chao Fan <fanc.fnst@cn.fujitsu.com>
To:     Borislav Petkov <bp@alien8.de>
CC:     Masayoshi Mizuma <msys.mizuma@gmail.com>,
        <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <hpa@zytor.com>,
        <keescook@chromium.org>, <bhe@redhat.com>,
        <indou.takao@jp.fujitsu.com>, <caoj.fnst@cn.fujitsu.com>
Subject: Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in
 cmdline from KEXEC
Message-ID: <20181206103720.GA2046@localhost.localdomain>
References: <20181129081631.11139-1-fanc.fnst@cn.fujitsu.com>
 <20181129081631.11139-2-fanc.fnst@cn.fujitsu.com>
 <20181129211003.saxfvyboxpsn5mdr@gabell>
 <20181130024346.GB30941@localhost.localdomain>
 <20181130173515.6zx5wvs52s5aqnge@gabell>
 <20181201060539.GD30941@localhost.localdomain>
 <20181204184220.GC16705@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20181204184220.GC16705@zn.tnic>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Boris, Baoquan and Masa,

When copying kstrtoull() and functions needed to arch/x86/boot/string.c,
I got an error in LD period:

  LD      arch/x86/boot/setup.elf
ld: arch/x86/boot/string.o: in function `div_u64_rem':
/home/cfan/code/tip/./include/linux/math64.h:28: undefined reference to `__udivdi3'
make[1]: *** [arch/x86/boot/Makefile:105: arch/x86/boot/setup.elf] Error 1
make: *** [arch/x86/Makefile:289: bzImage] Error 2

I google it and the key problem is div_u64(ULLONG_MAX - val, base))
when dividend is u64 and divisor is u32, the dividend must be a variable,
since the result will be stored in the first variable.
So here when I changed it to ((ULLONG_MAX - val) / base), I got the
same error.
But what puzzled me is, why the cdoe in lib/kstrtox.c works well.
And in old version PATCHSET, I ever copy the code to
arch/x86/boot/compressed/misc.c, it also worked well.

So I wonder if there are some options I miss or some problems.
The patch is below.

Thanks,
Chao Fan


diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index c4428a176973..62ffe0437c8e 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -13,6 +13,9 @@
  */

 #include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
 #include <asm/asm.h>
 #include "ctype.h"
 #include "string.h"
@@ -187,3 +190,112 @@ char *strchr(const char *s, int c)
                        return NULL;
        return (char *)s;
 }
+
+static inline char _tolower(const char c)
+{
+       return c | 0x20;
+}
+
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
+{
+       if (*base == 0) {
+               if (s[0] == '0') {
+                       if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
+                               *base = 16;
+                       else
+                               *base = 8;
+               } else
+                       *base = 10;
+       }
+       if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
+               s += 2;
+       return s;
+}
+
+/*
+ * Convert non-negative integer string representation in explicitly given radix
+ * to an integer.
+ * Return number of characters consumed maybe or-ed with overflow bit.
+ * If overflow occurs, result integer (incorrect) is still returned.
+ *
+ * Don't you dare use this function.
+ */
+unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *p)
+{
+       unsigned long long res;
+       unsigned int rv;
+
+       res = 0;
+       rv = 0;
+       while (1) {
+               unsigned int c = *s;
+               unsigned int lc = c | 0x20; /* don't tolower() this line */
+               unsigned int val;
+
+               if ('0' <= c && c <= '9')
+                       val = c - '0';
+               else if ('a' <= lc && lc <= 'f')
+                       val = lc - 'a' + 10;
+               else
+                       break;
+
+               if (val >= base)
+                       break;
+               /*
+                * Check for overflow only if we are within range of
+                * it in the max base we support (16)
+                */
+               if (unlikely(res & (~0ull << 60))) {
+                       if (res > div_u64(ULLONG_MAX - val, base))
+                               rv |= KSTRTOX_OVERFLOW;
+               }
+               res = res * base + val;
+               rv++;
+               s++;
+       }
+       *p = res;
+       return rv;
+}
+
+static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+       unsigned long long _res;
+       unsigned int rv;
+
+       s = _parse_integer_fixup_radix(s, &base);
+       rv = _parse_integer(s, base, &_res);
+       if (rv & KSTRTOX_OVERFLOW)
+               return -ERANGE;
+       if (rv == 0)
+               return -EINVAL;
+       s += rv;
+       if (*s == '\n')
+               s++;
+       if (*s)
+               return -EINVAL;
+       *res = _res;
+       return 0;
+}
+
+/**
+ * kstrtoull - convert a string to an unsigned long long
+ * @s: The start of the string. The string must be null-terminated, and may also
+ *  include a single newline before its terminating null. The first character
+ *  may also be a plus sign, but not a minus sign.
+ * @base: The number base to use. The maximum supported base is 16. If base is
+ *  given as 0, then the base of the string is automatically detected with the
+ *  conventional semantics - If it begins with 0x the number will be parsed as a
+ *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
+ *  parsed as an octal number. Otherwise it will be parsed as a decimal.
+ * @res: Where to write the result of the conversion on success.
+ *
+ * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
+ * Used as a replacement for the obsolete simple_strtoull. Return code must
+ * be checked.
+ */
+int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+       if (s[0] == '+')
+               s++;
+       return _kstrtoull(s, base, res);
+}
diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
index 3d78e27077f4..9619c8990d95 100644
--- a/arch/x86/boot/string.h
+++ b/arch/x86/boot/string.h
@@ -29,4 +29,6 @@ extern unsigned int atou(const char *s);
 extern unsigned long long simple_strtoull(const char *cp, char **endp,
                                          unsigned int base);

+#define KSTRTOX_OVERFLOW       (1U << 31)
+
 #endif /* BOOT_STRING_H */