Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <YuuBrUmiRYoaSmPw@google.com> <20220804142856.306032-1-jrosenth@chromium.org>
 <CAE-0n51UcauP1ojoskV9_2+CFPoJrs08jGHVPCZdSA_oXX1zUw@mail.gmail.com>
In-Reply-To: <CAE-0n51UcauP1ojoskV9_2+CFPoJrs08jGHVPCZdSA_oXX1zUw@mail.gmail.com>
From:   Julius Werner <jwerner@chromium.org>
Date:   Thu, 4 Aug 2022 16:14:43 -0700
Message-ID: <CAODwPW_mYQ1gcd2Xw5s+7dL2kLcYn-VTxKHUH1iM1V4mn7pT_w@mail.gmail.com>
Subject: Re: [PATCH v7] firmware: google: Implement cbmem in sysfs driver
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     Jack Rosenthal <jrosenth@chromium.org>,
        chrome-platform@lists.linux.dev,
        LKML <linux-kernel@vger.kernel.org>,
        Tzung-Bi Shih <tzungbi@kernel.org>,
        Guenter Roeck <groeck@chromium.org>,
        Julius Werner <jwerner@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> Quoting Jack Rosenthal (2022-08-04 07:28:56)
> > cbmem entries can be read from coreboot table
> > 0x31 (LB_TAG_CBMEM_ENTRY).  This commit exports access to cbmem
> > entries in sysfs under /sys/firmware/coreboot/cbmem-*.
> >
> > Link: https://issuetracker.google.com/239604743
>
> Is what you intend to use from here essentially an nvram? If so, it may
> make more sense to expose just that entry in drivers/nvmem/ as a
> read-only sort of nvmem.

No, it is not NV. It's just a normal memory buffer allocated and
filled by firmware on boot and placed in a region of normal DRAM
that's marked as reserved.

> It isn't clear to me what the structure of this path is. I'd expect to
> see an entry for each 'address', 'size', 'id', 'mem' with a different
> What/Date/Contact/Description. If those attributes are all within a
> directory in sysfs then there could be a top-level description for that
> as well.

CBMEM buffers are used by coreboot for all sorts of things and we
regularly define new ones. We can't maintain a full list of all IDs in
kernel sources because it would be a ton of churn for no reason --
best we could do is to add a link to the ID list in the coreboot repo
(e.g. https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h).
We really only care about one of these right now and many of them will
never be relevant to the kernel, but I still thought that while we're
doing this we might as well provide a generic interface to all of them
because others may become useful in the future (and then you don't
have to update the kernel every time you get a new use case for some
userspace tool wanting to interact with some resident data structure
from coreboot).

> Do you need to be able to write cbmem entries?

Yes, see the use case in the bug (using the vboot workbuffer from
coreboot as a write-through cache for the vboot nvdata on flash).