Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <5d9f3b9f.1c69fb81.62109.325d@mx.google.com>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <6cfca8c34ccd51f12b4418e9a74d8961e32077ed.camel@9elements.com>
References: <20191008115342.28483-1-patrick.rudolph@9elements.com> <20191008115342.28483-2-patrick.rudolph@9elements.com> <5d9d120b.1c69fb81.b6201.1477@mx.google.com> <CAODwPW-mfySMQUejCwT+G45BtOysq_JCRQa8GwoYTkjY_yRwgA@mail.gmail.com> <6cfca8c34ccd51f12b4418e9a74d8961e32077ed.camel@9elements.com>
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
User-Agent: alot/0.8.1
Date:   Thu, 10 Oct 2019 07:09:34 -0700
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

-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:
>=20
> > > But I also wonder why this is being exposed by the kernel at all?
> >=20
>=20
> 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.

>=20
> > 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.
>=20
> 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.

>=20
> > 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).
>=20
> 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.
>=20
> The downside is that the userspace tools need to be keep in sync with
> the binary interface the coreboot tables are providing.
>=20

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?