Re: [POC 3/5] sframe: add new SFrame library

[Peter Zijlstra <peterz@infradead.org>]

On Mon, May 01, 2023 at 06:40:08PM -0400, Steven Rostedt wrote:
> > --- /dev/null
> > +++ b/lib/sframe/sframe.h
> > @@ -0,0 +1,263 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2023, Oracle and/or its affiliates.
> > + */
> > +
> > +#ifndef	SFRAME_H
> > +#define	SFRAME_H
> > +
> > +#include <linux/types.h>
> > +
> > +/* This file contains definitions for the SFrame stack tracing format, which is
> > + * documented at https://sourceware.org/binutils/docs */

More CodingStyle nits, this also isn't a valid multi line comment style
for the kernel.

> > +
> > +#define SFRAME_VERSION_1	1
> > +#define SFRAME_MAGIC		0xdee2
> > +#define SFRAME_VERSION	SFRAME_VERSION_1
> > +
> > +/* Function Descriptor Entries are sorted on PC. */
> > +#define SFRAME_F_FDE_SORTED	0x1
> > +/* Frame-pointer based stack tracing. Defined, but not set. */
> > +#define SFRAME_F_FRAME_POINTER 0x2
> > +
> > +#define SFRAME_CFA_FIXED_FP_INVALID 0
> > +#define SFRAME_CFA_FIXED_RA_INVALID 0
> > +
> > +/* Supported ABIs/Arch. */
> > +#define SFRAME_ABI_AARCH64_ENDIAN_BIG      1 /* AARCH64 big endian. */
> > +#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE   2 /* AARCH64 little endian. */
> > +#define SFRAME_ABI_AMD64_ENDIAN_LITTLE     3 /* AMD64 little endian. */
> > +
> > +/* SFrame FRE types. */
> > +#define SFRAME_FRE_TYPE_ADDR1	0
> > +#define SFRAME_FRE_TYPE_ADDR2	1
> > +#define SFRAME_FRE_TYPE_ADDR4	2
> > +
> > +/*
> > + * SFrame Function Descriptor Entry types.
> > + *
> > + * The SFrame format has two possible representations for functions.  The
> > + * choice of which type to use is made according to the instruction patterns
> > + * in the relevant program stub.
> > + */

This is.

> > +/* Unwinders perform a (PC >= FRE_START_ADDR) to look up a matching FRE. */
> > +#define SFRAME_FDE_TYPE_PCINC   0
> > +/*
> > + * Unwinders perform a (PC & FRE_START_ADDR_AS_MASK >= FRE_START_ADDR_AS_MASK)
> > + * to look up a matching FRE.  Typical usecases are pltN entries, trampolines
> > + * etc.
> > + */
> > +#define SFRAME_FDE_TYPE_PCMASK  1
> > +

> > +static bool sframe_header_sanity_check_p(struct sframe_header *hp)
> > +{
> > +	unsigned char all_flags = SFRAME_F_FDE_SORTED | SFRAME_F_FRAME_POINTER;
> 
> Add a space here.
> 
> > +	/* Check that the preamble is valid. */
> > +	if ((hp->preamble.magic != SFRAME_MAGIC)
> > +	    || (hp->preamble.version != SFRAME_VERSION)
> > +	    || ((hp->preamble.flags | all_flags) != all_flags))
> > +		return false;

In kernel we prefer the operators at the end of a line in case of
linebreak.

> > +
> > +	/* Check that the offsets are valid. */
> > +	if (hp->fdeoff > hp->freoff)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static bool sframe_fre_sanity_check_p(struct sframe_fre *frep)
> > +{
> > +	unsigned int offset_size, offset_cnt;
> > +
> > +	if (frep == NULL)
> > +		return false;
> > +
> > +	offset_size = sframe_fre_get_offset_size(frep->fre_info);
> > +
> > +	if (offset_size != SFRAME_FRE_OFFSET_1B
> > +	    && offset_size != SFRAME_FRE_OFFSET_2B
> > +	    && offset_size != SFRAME_FRE_OFFSET_4B)
> > +		return false;

idem.

> > +
> > +	offset_cnt = sframe_fre_get_offset_count(frep->fre_info);
> > +	if (offset_cnt > MAX_NUM_STACK_OFFSETS)
> > +		return false;
> > +
> > +	return true;
> > +}