linux-embedded.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bird, Tim" <Tim.Bird@sony.com>
To: Federico Giovanardi <fede.dev@giovanardi.dev>,
	"linux-embedded@vger.kernel.org" <linux-embedded@vger.kernel.org>
Subject: RE: [RFC PATCH 0/1] Add driver for bootstage stash
Date: Sat, 24 May 2025 00:07:07 +0000	[thread overview]
Message-ID: <MW5PR13MB5632A2522BFAF3066DCC57CDFD9BA@MW5PR13MB5632.namprd13.prod.outlook.com> (raw)
In-Reply-To: <473a062e4f939bc58a5c0e636569b826@giovanardi.dev>

> -----Original Message-----
> From: Federico Giovanardi <fede.dev@giovanardi.dev>
> Hello,
> 
> The note about the data format also was my initial thought, by just
> copying a C structure we might have issues as soon one party changes it,
> and they might not be perfectly aligned.
> 
> To avoid inventing yet-another-data-format I've used msgpack in the past
> for that (the format
> https://github.com/msgpack/msgpack/blob/master/spec.md, not the library
> ); because the specs are so simple they can be implemented in a few
> lines, and it's something with a reference. But I don't have a lot of
> experience in upstreaming stuff on the kernel, so I don't know if it
> might cause someone to don't be happy. Anyway, I can contribute the
> implementation if needed.
> 
> Something as simple as an array of fixarray will give extensibility with
> only a few bytes of overhead.
> 
> Which gets encoded as:
> 
> 0xdc # lenght 16 bit << array header
>       # 0xB << 4 | ( array_size & 0xF) << fixarray header ( 3 elements,
> simplest case)
>             # 0xce # time_us
>             # 0xce # start_us
>             # 0xc << 4 | strlen(name) # name
>             /*no flags, no id*/
>       # 0xB << 4 | ( array_size & 0xF) << fixarray header ( 5 elements
> bigger case)
>             # 0xce # time_us
>             # 0xce # start_us
>             # 0xc << 4 | strlen(name) # name
>             # 0xcc # flags
>             # 0xcc # id
>       .. repeat ...
> 
> 
> Since the goal is to use that in many different contexts, defining the
> fields that we need early is important.

My own opinion is that the current bootstash format is too complicated, and that the
Unified Boot Log record format should be simplified to just
name, counter_id, and counter value, with fixed limits on the size of each.

struct UBL_entry {
  char name[7];
  u8 counter_id;
  u64 counter_value;
}

None of this looking up names in a separate table business.

One counter_id could be 0 - indicating that the counter is time in microseconds
(for U-boot bootstage data, where the values are already converted to microseconds).
I would prefer doing that counter conversion at boot-time, when the cycle counter
values can (almost) as easily be normalized to microseconds at report time.

That is, other cycle-counters used in a single Unified Boot Log could be used, and
instrumentation can just store their value, without doing the conversion.  The reporting
tool would need to know the conversion rate for each counter_id.  But it
should be able to calculate that at report time.

It would be good to discuss whether that reporting (and conversion) should be in the kernel
or a separate user-space tool.  The current patch puts reporting into the kernel.
   -- Tim


  parent reply	other threads:[~2025-05-24  0:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-22 22:42 [RFC PATCH 0/1] Add driver for bootstage stash Francesco Valla
2025-05-22 22:42 ` [PATCH 1/1] drivers: misc: add " Francesco Valla
2025-05-23  6:29   ` Krzysztof Kozlowski
2025-05-23 19:34     ` Rob Landley
2025-05-24  6:16       ` Krzysztof Kozlowski
2025-05-23 19:43     ` Francesco Valla
2025-05-24  6:15       ` Krzysztof Kozlowski
2025-05-23 23:43   ` Bird, Tim
2025-05-24  7:18   ` kernel test robot
2025-05-23  7:04 ` [RFC PATCH 0/1] Add " Geert Uytterhoeven
2025-05-23 20:06   ` Francesco Valla
     [not found] ` <PA4PR08MB604681FF6392B25A19926A11ED98A@PA4PR08MB6046.eurprd08.prod.outlook.com>
2025-05-23  7:34   ` Federico Giovanardi
2025-05-23 19:43     ` Rob Landley
2025-05-23 20:11     ` Francesco Valla
2025-05-24  0:07     ` Bird, Tim [this message]
2025-05-24  0:28       ` Bird, Tim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW5PR13MB5632A2522BFAF3066DCC57CDFD9BA@MW5PR13MB5632.namprd13.prod.outlook.com \
    --to=tim.bird@sony.com \
    --cc=fede.dev@giovanardi.dev \
    --cc=linux-embedded@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).