[Skiboot] [RFC PATCH] Introduce OPAL_GET_SYMBOL / OPAL_LOOKUP_SYMBOL
Oliver O'Halloran
oohall at gmail.com
Tue Dec 10 11:30:31 AEDT 2019
On Mon, Dec 9, 2019 at 5:43 PM Nicholas Piggin <npiggin at gmail.com> wrote:
>
> These calls can be used by Linux to annotate BUG addresses with symbols,
> look up symbol addresses in xmon, etc.
>
> This is preferable over having Linux parse the OPAL symbol map itself,
> because it could possibly be used to support other code regions, e.g.,
> the wake-up code in the HOMER (where CPUs have been seen to get stuck).
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> core/opal.c | 2 +
> core/utils.c | 92 +++++++++++++++++++++++--
> doc/opal-api/opal-get-symbol-181.rst | 42 +++++++++++
> doc/opal-api/opal-lookup-symbol-182.rst | 35 ++++++++++
> include/opal-api.h | 4 +-
> 5 files changed, 168 insertions(+), 7 deletions(-)
> create mode 100644 doc/opal-api/opal-get-symbol-181.rst
> create mode 100644 doc/opal-api/opal-lookup-symbol-182.rst
>
> diff --git a/core/opal.c b/core/opal.c
> index d1e5bac86..dfeb497d6 100644
> --- a/core/opal.c
> +++ b/core/opal.c
> @@ -142,6 +142,8 @@ int64_t opal_entry_check(struct stack_frame *eframe)
> case OPAL_CEC_REBOOT:
> case OPAL_CEC_REBOOT2:
> case OPAL_SIGNAL_SYSTEM_RESET:
> + case OPAL_GET_SYMBOL:
> + case OPAL_LOOKUP_SYMBOL:
I hate the names since I'd expect getting a symbol and looking up a
symbol to be different names for the same thing. How about something
like: OPAL_SYMBOL_TO_ADDR and OPAL_ADDR_TO_SYMBOL?
> break;
> default:
> printf("CPU ATTEMPT TO RE-ENTER FIRMWARE! PIR=%04lx cpu @%p -> pir=%04x token=%llu\n",
> diff --git a/core/utils.c b/core/utils.c
> index 8fd63fcb7..5f0d5130b 100644
> --- a/core/utils.c
> +++ b/core/utils.c
> @@ -48,40 +48,120 @@ char __attrconst tohex(uint8_t nibble)
> return __tohex[nibble];
> }
>
> -static unsigned long get_symbol(unsigned long addr, char **sym, char **sym_end)
> +static unsigned long get_symbol(unsigned long addr, char **sym, char **sym_end, unsigned long *size)
> {
> unsigned long prev = 0, next;
> char *psym = NULL, *p = __sym_map_start;
>
> *sym = *sym_end = NULL;
> - while(p < __sym_map_end) {
> + while (p < __sym_map_end) {
> next = strtoul(p, &p, 16) | SKIBOOT_BASE;
> if (next > addr && prev <= addr) {
> - p = psym + 3;;
> + p = psym + 3;
> if (p >= __sym_map_end)
> return 0;
> *sym = p;
> - while(p < __sym_map_end && *p != 10)
> + while (p < __sym_map_end && *p != '\n')
> p++;
> *sym_end = p;
> + *size = next - prev;
> return prev;
> }
> prev = next;
> psym = p;
> - while(p < __sym_map_end && *p != 10)
> + while (p < __sym_map_end && *p != '\n')
> p++;
> p++;
> }
> return 0;
> }
>
> +static unsigned long lookup_symbol(const char *name, unsigned long *size)
> +{
> + size_t len = strlen(name);
> + unsigned long addr = 0;
> + char *sym;
> + char *p = __sym_map_start;
> +
> + while (p < __sym_map_end) {
> + addr = strtoul(p, &p, 16) | SKIBOOT_BASE;
> + p += 3;
> + if (p >= __sym_map_end)
> + return 0;
> +
> + if (*(p + len) == '\n' && !strncmp(name, p, len)) {
> + char *sym_end;
> +
> + if (get_symbol(addr, &sym, &sym_end, size) == 0) {
> + assert(!strcmp(name, "_end"));
Looking up _end is a little weird, but I don't see why it needs to be an assert.
> + *size = 0;
> + }
> +
> + /*
> + * May be more than one symbol at this address but
> + * symbol length calculation should still work in
> + * that case.
> + */
> +
> + return addr;
> + }
> +
> + while(p < __sym_map_end && *p != '\n')
> + p++;
> + p++;
> + }
> + return 0;
> +}
Calling get_symbol() which re-scans the whole map is a little gross.
Granted the alternative is more string parsing in C so it's fine I
guess.
> +static int64_t opal_get_symbol(uint64_t addr, __be64 *symaddr, __be64 *symsize, char *namebuf, uint64_t buflen)
> +{
> + unsigned long saddr;
> + unsigned long ssize;
> + char *sym, *sym_end;
> + size_t l;
> +
> + saddr = get_symbol(addr, &sym, &sym_end, &ssize);
> + if (!saddr)
> + return OPAL_RESOURCE;
Validating the pointers are non-null is probably a good idea.
> +
> + if (buflen > sym_end - sym)
> + l = sym_end - sym;
> + else
> + l = buflen - 1;
> + memcpy(namebuf, sym, l);
> + namebuf[l] = '\0';
> +
> + *symaddr = cpu_to_be64(saddr);
> + *symsize = cpu_to_be64(ssize);
> +
> + return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_GET_SYMBOL, opal_get_symbol, 5);
> +
> +static int64_t opal_lookup_symbol(const char *name, __be64 *symaddr, __be64 *symsize)
> +{
> + unsigned long saddr;
> + unsigned long ssize;
> +
same here
> + saddr = lookup_symbol(name, &ssize);
> + if (!saddr)
> + return OPAL_RESOURCE;
> +
> + *symaddr = cpu_to_be64(saddr);
> + *symsize = cpu_to_be64(ssize);
> +
> + return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_LOOKUP_SYMBOL, opal_lookup_symbol, 3);
> +
> size_t snprintf_symbol(char *buf, size_t len, uint64_t addr)
> {
> unsigned long saddr;
> + unsigned long ssize;
> char *sym, *sym_end;
> size_t l;
>
> - saddr = get_symbol(addr, &sym, &sym_end);
> + saddr = get_symbol(addr, &sym, &sym_end, &ssize);
> if (!saddr)
> return 0;
>
> diff --git a/doc/opal-api/opal-get-symbol-181.rst b/doc/opal-api/opal-get-symbol-181.rst
> new file mode 100644
> index 000000000..a57de6c1c
> --- /dev/null
> +++ b/doc/opal-api/opal-get-symbol-181.rst
> @@ -0,0 +1,42 @@
> +.. _OPAL_GET_SYMBOL:
> +
> +OPAL_GET_SYMBOL
> +================
> +
> +.. code-block:: c
> +
> + #define OPAL_GET_SYMBOL 181
> +
> + static int64_t opal_get_symbol(uint64_t addr, __be64 *symaddr, __be64 *symsize, char *namebuf, uint64_t buflen);
> +
> +This OPAL call looks up a firmware code address for symbol information.
> +
> +Arguments
> +---------
> +
> + ``addr``
> + Contains address to be looked up.
> +
> + ``symaddr``
> + Returns the start address of the symbol for the object which
> + contains addr or immediately precedes addr.
> +
> + ``symsize``
> + Returns the size of the object, or the number of bytes until the
> + next symbol.
> +
> + ``namebuf``
> + Contains a buffer for the symbol name to be copied into, as a NUL
> + terminated string.
> +
> + ``buflen``
> + Contains the length of the bufer that may be used.
s/bufer/buffer/
> +
> +
> +Returns
> +-------
> +
> +:ref:`OPAL_SUCCESS`
> + Found a symbol.
> +:ref:`OPAL_RESOURCE`
> + Did not find a symbol.
OPAL_EMPTY might be more appropriate.
More information about the Skiboot
mailing list