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: <20220726235115.1184532-1-jrosenth@chromium.org>
In-Reply-To: <20220726235115.1184532-1-jrosenth@chromium.org>
From:   Julius Werner <jwerner@chromium.org>
Date:   Tue, 26 Jul 2022 17:26:41 -0700
Message-ID: <CAODwPW-9UPokepL_VVGuCLaWTDT7hmN9R9Z2yXABrP4g3_N4uQ@mail.gmail.com>
Subject: Re: [PATCH] firmware: google: Implement vboot workbuf in sysfs
To:     Jack Rosenthal <jrosenth@chromium.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        chrome-platform@lists.linux.dev,
        Stephen Boyd <swboyd@chromium.org>,
        Guenter Roeck <groeck@chromium.org>,
        Julius Werner <jwerner@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Sorry, this wasn't quite what I meant. I think we should definitely
not hardcode any details about the vboot data structure layout here.
It's already enough of a pain to keep crossystem in sync with
different data structure versions, we absolutely don't want to have to
keep updating that in the kernel as well.

I assume that the reason you went that route seems to have been mostly
that the lb_cbmem_ref coreboot table entry has no size field, so you
had to infer the size from within the data structure. Thankfully, we
don't need to use lb_cbmem_ref for this, that's somewhat of a legacy
entry type that we're trying to phase out where it's no longer needed
for backwards-compatibility anyway (and in fact I think we should be
okay to remove the vboot entry there nowadays). We also have
lb_cbmem_entry (see
https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/lib/imd_cbmem.c#222)
that exports info about every area in CBMEM, with address, size and
CBMEM ID. The vboot workbuffer is CBMEM ID 0x78007343.

I think we should just add a general driver for lb_cbmem_entry tags
here, that uses the "id" as (part of) the device name and contains
node to read/write the raw bytes of the buffer. Then crossystem can
easily find the right one for vboot, and the infrastructure may also
come in handy for other uses (or debugging) in the future.