On Thu, Jan 02, 2025 at 11:14:51AM +1300, Simon Glass wrote: > Hi Tom, > > On Fri, 20 Dec 2024 at 05:36, Tom Rini <tr...@konsulko.com> wrote: > > > > On Thu, Dec 19, 2024 at 10:37:37AM -0500, Raymond Mao wrote: > > > Hi Simon, > > > > > > On Thu, 19 Dec 2024 at 10:08, Simon Glass <s...@chromium.org> wrote: > > > > > > > Hi, > > > > > > > > On Mon, 16 Dec 2024 at 11:30, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > On Mon, Dec 16, 2024 at 10:37:30AM -0700, Simon Glass wrote: > > > > > > Hi Raymond, > > > > > > > > > > > > On Mon, 16 Dec 2024 at 10:24, Raymond Mao <raymond....@linaro.org> > > > > wrote: > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > On Mon, 16 Dec 2024 at 10:43, Simon Glass <s...@chromium.org> > > > > > > > wrote: > > > > > > >> > > > > > > >> Hi Raymond, > > > > > > >> > > > > > > >> On Mon, 16 Dec 2024 at 08:29, Raymond Mao > > > > > > >> <raymond....@linaro.org> > > > > wrote: > > > > > > >> > > > > > > > >> > Hi Simon, > > > > > > >> > > > > > > > >> > On Mon, 16 Dec 2024 at 10:20, Simon Glass <s...@chromium.org> > > > > wrote: > > > > > > >> >> > > > > > > >> >> Hi Raymond, > > > > > > >> >> > > > > > > >> >> On Mon, 16 Dec 2024 at 07:52, Raymond Mao < > > > > raymond....@linaro.org> wrote: > > > > > > >> >> > > > > > > > >> >> > Hi Simon, > > > > > > >> >> > > > > > > > >> >> > On Sun, 15 Dec 2024 at 19:25, Simon Glass > > > > > > >> >> > <s...@chromium.org> > > > > wrote: > > > > > > >> >> >> > > > > > > >> >> >> Hi Raymond, > > > > > > >> >> >> > > > > > > >> >> >> On Fri, 13 Dec 2024 at 15:56, Raymond Mao < > > > > raymond....@linaro.org> wrote: > > > > > > >> >> >> > > > > > > > >> >> >> > Get tpm event log from bloblist instead of FDT when > > > > bloblist is > > > > > > >> >> >> > enabled and valid from previous boot stage. > > > > > > >> >> >> > > > > > > > >> >> >> > Note: > > > > > > >> >> >> > This patch depends on: > > > > > > >> >> >> > [PATCH 1/2] bloblist: Introduce BLOBLIST_PRIOR_STAGE > > > > > > >> >> >> > options > > > > > > >> >> >> > > > > > https://lore.kernel.org/u-boot/20241212151142.1562825-1-tr...@konsulko.com/ > > > > > > >> >> >> > > > > > > > >> >> >> > Signed-off-by: Raymond Mao <raymond....@linaro.org> > > > > > > >> >> >> > --- > > > > > > >> >> >> > lib/tpm_tcg2.c | 7 +++++++ > > > > > > >> >> >> > 1 file changed, 7 insertions(+) > > > > > > >> >> >> > > > > > > > >> >> >> > diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c > > > > > > >> >> >> > index 7f868cc883..acaf0acb88 100644 > > > > > > >> >> >> > --- a/lib/tpm_tcg2.c > > > > > > >> >> >> > +++ b/lib/tpm_tcg2.c > > > > > > >> >> >> > @@ -19,6 +19,7 @@ > > > > > > >> >> >> > #include <linux/unaligned/generic.h> > > > > > > >> >> >> > #include <linux/unaligned/le_byteshift.h> > > > > > > >> >> >> > #include "tpm-utils.h" > > > > > > >> >> >> > +#include <bloblist.h> > > > > > > >> >> >> > > > > > > > >> >> >> > int tcg2_get_pcr_info(struct udevice *dev, u32 > > > > *supported_pcr, u32 *active_pcr, > > > > > > >> >> >> > u32 *pcr_banks) > > > > > > >> >> >> > @@ -672,6 +673,12 @@ __weak int > > > > tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size) > > > > > > >> >> >> > *addr = NULL; > > > > > > >> >> >> > *size = 0; > > > > > > >> >> >> > > > > > > > >> >> >> > + if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE) && > > > > !bloblist_maybe_init()) { > > > > > > >> >> >> > > > > > > >> >> >> Please drop the second term...the bloblist is inited by > > > > U-Boot already. > > > > > > >> >> >> > > > > > > >> >> > > > > > > > >> >> > As an independent library, from the tpm library's angle, > > > > whether the bloblist init was already called or not is agnostic. > > > > > > >> >> > I think this is the purpose of the "maybe" function. > > > > > > >> >> > > > > > > >> >> No. U-Boot does init at the start. We don't want each > > > > > > >> >> subsystem > > > > > > >> >> 'maybe' initing each other subsystem that it uses. If you > > > > > > >> >> want to > > > > > > >> >> check whether bloblist is available, you can use the GD flag. > > > > > > >> >> > > > > > > >> > > > > > > > >> > OK. I can use the gd instead. > > > > > > >> > > > > > > > >> >> > > > > > > >> >> > > > > > > > >> >> >> > > > > > > >> >> >> Also please drop the first term. If you find it in the > > > > bloblist, just use it. > > > > > > >> >> >> > > > > > > >> >> > > > > > > > >> >> > I think here we should have the same logic as the existing > > > > > > >> >> > one > > > > in fdtdec: > > > > > > >> >> > If bloblist is not enabled or errors observed, do a fallback > > > > to the original process. > > > > > > >> >> > At least I want to keep the same logic in different places > > > > before everything is ready from TF-A to U-boot. > > > > > > >> >> > > > > > > >> >> Yes, your second line is right. But PRIOR_STAGE doesn't exist, > > > > nor > > > > > > >> >> should it. Nor do you need it, since you can just check the > > > > bloblist > > > > > > >> >> for what you want. If you don't find it, fall back. > > > > > > >> >> > > > > > > >> > > > > > > > >> > Since Firmware Handoff is still an experimental feature and not > > > > mandatory yet (at least now), > > > > > > >> > I put everything under PRIOR_STAGE to allow users to disable it > > > > by their own decisions. > > > > > > >> > I saw PRIOR_STAGE in one of Tom's recent patches so I have > > > > > > >> > added > > > > a dependency claim in my commit message. > > > > > > >> > > > > > > >> Yes, but even if Tom does pick his patch, which I object to, by > > > > > > >> the > > > > > > >> time your code is called, there will be a bloblist. > > > > > > >> > > > > > > >> So the end result of your test is to allow the user to ignore any > > > > TPM > > > > > > >> log in the bloblist. If that's what you want (which seems fine to > > > > me), > > > > > > >> add a Kconfig for it. > > > > > > >> > > > > > > > > > > > > > > Per my understanding of PRIOR_STAGE, it means we have a bloblist > > > > from the previous stage and we want to use it (as a Transfer List). > > > > > > > If this is the case, it is not for DT only but can be applied for > > > > eventlog or anything for Firmware Handoff in the future. > > > > > > > That is the reason I use PRIOR_STAGE here, I don't think we need > > > > > > > to > > > > add another kconfig. > > > > > > > > > > > > Who knows what it is supposed to mean...anyway, it should not be > > > > > > used > > > > > > like that. The devicetree is special in that we want to get it from > > > > > > the bloblist very early, if available. Nothing else (so far) in the > > > > > > bloblist has this constraint, certainly not the TPM log. > > > > > > > > > > Raymond is exactly right, and the device tree is not special. > > > > > > > > That is quite wrong IMO. Devicetree is special because we need it > > > > early and it might come from bloblist before the bloblist has been set > > > > up by U-Boot. It also controls the whole operation of U-Boot. There is > > > > nothing else in U-Boot quite like the devicetree. > > > > > > > > > The > > > > > specialness is "do we have a valid bloblist upon entry or not". Which > > > > > means that also no, this series (since it happens late enough) > > > > > shouldn't > > > > > care about if the previous stage had created it or not, merely as you > > > > > said if the tag for the TPM (or other blob) is found. > > > > > > > > I would feel more comfortable having the discussions about what > > > > previous stages need to pass U-Boot a bloblist once we have that > > > > situation. This particular patch seems clear enough, and well > > > > motivated. It just needs to drop the conditions. > > > > > > > I think we still need a condition here indicating that the kconfig of > > > "handoff via bloblist" is enabled. > > > And this "handoff via bloblist" kconfig should mean to handoff everything > > > via bloblist, including DT. > > No, I don't agree with that at all. > > What actually breaks if you look for the TPM log in the bloblist? > Please answer this question.
Yes, the case of bloblist found, TPM log isn't found needs to work. > > There's at least two sets of challenges here. One, being solved by > > vexpress64 right now, is that we didn't have CONFIG_BLOBLIST_PASSAGE as > > an actual option. And in that case, there's no U-Boot before full U-Boot > > and the bloblist exists for us. Two, U-Boot is what is creating the > > bloblist. The contentious parts are *when* it's created and *where* it > > resides prior to full U-Boot seeing it. > > There isn't contention, so far as I am aware. The normal case is that > U-Boot creates and uses the bloblist itself. Pre-U-Boot blobs (like > TF-A, sadly) are not the normal case and should be discouraged in an > open-source project. That doesn't mean we shouldn't support them, but > it is the tail wagging the dog. No. The normal case is that no one is using a bloblist. The rare case is the platforms you developed bloblist on. The new and growing case is other platforms using the standard mechanism you designed to communicate across projects in a not ad-hoc manner. > > In both cases, full U-Boot will copy the bloblist to where it likes. > > > > And this is separate from "bloblist has device tree or we hang()". > > Yes, well that has been the case in mainline for over a year with > several of my boards, so it is a bit late to start worrying about it > now. I have OF_BLOBLIST and it works fine. You mean your boards that are broken in mainline, because no one quite understood how you were trying to use the bloblist? And that are AFAIK end of life platforms? No, they aren't the important use cases for a bloblist. They're apparently not ideal references for demonstrating how to use a bloblist. -- Tom
signature.asc
Description: PGP signature
