linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Julius Werner <jwerner@chromium.org>, patrick.rudolph@9elements.com
Cc: 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: Thu, 10 Oct 2019 07:09:34 -0700	[thread overview]
Message-ID: <5d9f3b9f.1c69fb81.62109.325d@mx.google.com> (raw)
In-Reply-To: <6cfca8c34ccd51f12b4418e9a74d8961e32077ed.camel@9elements.com>

-Filipe bounce

Quoting patrick.rudolph@9elements.com (2019-10-10 02:46:53)
> On Wed, 2019-10-09 at 14:19 -0700, Julius Werner wrote:
> 
> > > But I also wonder why this is being exposed by the kernel at all?
> > 
> 
> It's difficult for userspace tools to find out how to access the flash
> and then to find the FMAP, which resides somewhere in it on 4KiB boundary.
> The "boot media params" usually give the offset to the FMAP from beginning
> of the flash, but tell nothing about how to access it.

Great! That's what I wanted to know. If it's difficult to find then it
must be easier to use the coreboot tables to find it and it improves
overall speed for the firmware update.

> 
> > I share Stephen's concern that I'm not sure this belongs in the
> > kernel
> > at all. There are existing ways for userspace to access this
> > information like the cbmem utility does... if you want it accessible
> > from fwupd, it could chain-call into cbmem or we could factor that
> > functionality out into a library. If you want to get away from using
> > /dev/mem for this, we could maybe add a driver that exports CBMEM or
> > coreboot table areas via sysfs... but then I think that should be a
> > generic driver which makes them all accessible in one go, rather than
> > having to add yet another driver whenever someone needs to parse
> > another coreboot table blob for some reason. We could design an
> > interface like /sys/firmware/coreboot/table/<tag> where every entry
> > in
> > the table gets exported as a binary file.
> 
> I don't even consider using binaries that operate on /dev/mem.
> In my opinion CBMEM is something coreboot internal, the OS or userspace
> shouldn't even known about.

Yes. To be clear I'm not suggesting we make this a binary file design,
although patch 1 is exporting a binary file. I was just wondering why we
couldn't search the flash itself.

> 
> > I think a specific sysfs driver only makes sense for things that are
> > human readable and that you'd actually expect a human to want to go
> > read directly, like the log. Maybe exporting FMAP entries one by one
> > like Stephen suggests could be such a case, but I doubt that there's
> > a
> > common enough need for that since there are plenty of existing ways
> > to
> > show FMAP entries from userspace (and if there was a generic
> > interface
> > like /sys/firmware/coreboot/table/37 to access it, we could just add
> > a
> > new flag to the dump_fmap utility to read it from there).
> 
> 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.
> 

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?


  reply	other threads:[~2019-10-10 14:09 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 [this message]
2019-10-10 18:37           ` Julius Werner
2019-10-16 20:12             ` Stephen Boyd
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=5d9f3b9f.1c69fb81.62109.325d@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;
as well as URLs for NNTP newsgroup(s).