public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Julius Werner <jwerner@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>,
	Patrick Rudolph <patrick.rudolph@9elements.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ben Zhang <benzh@chromium.org>,
	Duncan Laurie <dlaurie@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Samuel Holland <samuel@sholland.org>
Subject: Re: [PATCH 2/2] firmware: coreboot: Export active CBFS partition
Date: Wed, 16 Oct 2019 13:12:35 -0700	[thread overview]
Message-ID: <5da779b4.1c69fb81.2d904.a23f@mx.google.com> (raw)
In-Reply-To: <CAODwPW-x1fwGSrvLNWCU4GAfGbD0zqo2HLm+33D8eUtxbnFLCg@mail.gmail.com>

Quoting Julius Werner (2019-10-10 11:37:58)
> > > I'll expose the coreboot tables using a sysfs driver, which then can be
> > > used by coreboot tools instead of accessing /dev/mem. As it holds the
> > > FMAP and "boot media params" that's all I need for now.
> > >
> > > The downside is that the userspace tools need to be keep in sync with
> > > the binary interface the coreboot tables are providing.
> 
> Well, in the other version the kernel needs to be kept in sync
> instead. No matter where you do the parsing, something needs to be
> kept in sync. I think userspace would be easier, especially if we
> would host a small userspace library in the coreboot repository that
> other tools could just link.
> 
> > I'd rather we export this information in sysfs via some coreboot_fmap
> > class and then make the "boot media params" another property of one of
> > the fmap devices. Then userspace can search through all the fmap devices
> > and find the "boot media params" one. Is anything else needed?
> 
> Okay, this is the fundamental question we need to answer first... do
> you really think it's better to add a separate interface for each of
> these, Stephen? The coreboot table[1] currently contains 49 entries
> with all sorts of random firmware information, and most of these will
> never be interesting to the kernel. Do we want to establish a pattern
> where we add a new sysfs interface for each of them whenever someone
> has a use case for reading it from userspace? I think this might be a
> good point to implement a generic interface to read any coreboot table
> entry from userspace instead, and say that if the kernel doesn't need
> the information in a specific entry itself, it shouldn't need to know
> how to parse it.

I don't know why we need to draw a line in the sand and say that if the
kernel doesn't need to know about it then it shouldn't parse it. I want
there to be a consistent userspace ABI that doesn't just move things
straight from memory to userspace in some binary format. I'd rather we
have an ABI that decodes and exposes information about the coreboot
tables through existing frameworks/subsystems where possible and makes
up new ones otherwise.

One concern I have is endianness of the binary data. Is it big endian or
little endian or CPU native endian? The kernel can boot into big or
little endian on ARM platforms and userspace can be different vs. the
bootloader too. Userspace shouldn't need to know this detail, the kernel
should know and do the conversions and expose it somehow. That's why I'm
suggesting in this case we describe fmap as a sysfs class. I don't see
how we could export that information otherwise, besides in a binary blob
that falls into traps like this.

Right now we make devices for all the coreboot table entries, which is
pretty weird considering that some table entries are things like
LB_TAG_LINKER. That isn't a device. It's some information about how
coreboot was linked. We should probably blacklist tags so we don't make
devices and capture these ones in the bus code and expose them in
/sys/firware/coreboot/ somehow. We should also add device randomness
from the serial numbers, etc. that coreboot has stashed away in the
tables.

> 
> [1] https://review.coreboot.org/cgit/coreboot.git/tree/src/commonlib/include/commonlib/coreboot_tables.h

  reply	other threads:[~2019-10-16 20:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-08 11:53 [PATCH 1/2] firmware: coreboot: Export the binary FMAP patrick.rudolph
2019-10-08 11:53 ` [PATCH 2/2] firmware: coreboot: Export active CBFS partition patrick.rudolph
2019-10-08 22:47   ` Stephen Boyd
2019-10-09 21:19     ` Julius Werner
2019-10-10  9:46       ` patrick.rudolph
2019-10-10 14:09         ` Stephen Boyd
2019-10-10 18:37           ` Julius Werner
2019-10-16 20:12             ` Stephen Boyd [this message]
2019-10-19  2:14               ` Julius Werner
2019-10-11  0:03       ` Samuel Holland
2019-10-08 12:03 ` [PATCH 1/2] firmware: coreboot: Export the binary FMAP Greg Kroah-Hartman
2019-10-08 22:47 ` Stephen Boyd

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=5da779b4.1c69fb81.2d904.a23f@mx.google.com \
    --to=swboyd@chromium.org \
    --cc=benzh@chromium.org \
    --cc=dlaurie@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jwerner@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patrick.rudolph@9elements.com \
    --cc=samuel@sholland.org \
    --cc=tglx@linutronix.de \
    /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