On Tue, Dec 17, 2024 at 12:45:46PM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 16 Dec 2024 at 21:00, Tom Rini <tr...@konsulko.com> wrote: > > > > On Sun, Dec 15, 2024 at 05:25:24PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 13 Dec 2024 at 10:07, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > On Fri, Dec 13, 2024 at 07:32:24AM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 12 Dec 2024 at 08:11, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > > > Introduce an option to control if we expect that a prior stage has > > > > > > created a bloblist already and thus it is safe to scan the address. > > > > > > We > > > > > > need to have this be configurable because we do not (and cannot) > > > > > > know > > > if > > > > > > the address at CONFIG_BLOBLIST_ADDR is in regular DRAM or some > > > > > > special > > > > > > on-chip memory and so it may or may not be safe to read from this > > > > > > address this early. > > > > > > > > > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > > > > > --- > > > > > > Cc: Simon Glass <s...@chromium.org> > > > > > > --- > > > > > > common/Kconfig | 32 ++++++++++++++++++++++++++++++++ > > > > > > lib/fdtdec.c | 11 +++-------- > > > > > > 2 files changed, 35 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > Since this is the essentially the same as my OF_BLOBLIST patch, which > > > > > was rejected: > > > > > > > > > > NAK > > > > > > > > > > The issue is not whether some 'previous stage' set up a bloblist. For > > > > > example, if VPL_BLOBLIST_PRIOR_STAGE is enabled, that presumably means > > > > > that TPL set it up. But it doesn't mean that TPL put the FDT in there. > > > > > > > > This is wrong. The root problem is saying that the bloblist is in > > > > possibly uninitialized memory. The code is quite happy to NOT find the > > > > device tree in the bloblist and continue searching other paths. > > > > > > But until the bloblist is set up, it doesn't exist. > > > > An almost philosophical statement, yes. How should the generic code know > > if it exists, or does not exist? That is the question. > > U-Boot code 'knows', since if this is the first phase which enables > CONFIG_IS_ENABLED(BLOBLOST), then the bloblist is available > before/after relocation as well as after RAM setup in SPL.
And when U-Boot isn't the first stage? > > > We should not be > > > looking for a bloblist that we know is not there yet. The bloblist is set > > > up by bloblist_init(). You seem to be imputing your own semantics for > > > bloblist. > > > > We don't know if the bloblist exists or not. That's the problem. It > > could already exist. That's the whole reason we're in this particular > > argument. > > See above. Yes, you seem to be missing the case where U-Boot isn't the first stage. > > > In SPL, init happens in board_init_r(), i.e. after RAM is set up. So don't > > > look in the bloblist until it is set up! It doesn't exist. > > > > Unless of course it was setup before. We don't know if it was setup > > before or not until we look. A big part of the problem is that for prior > > to U-Boot bloblist we don't need to use main DRAM, the bloblist is tiny > > and we have some other persistent memory available. Unfortunately, x86 > > does not. > > The only situation where this matters is for the devicetree, which I > why i created OF_BLOBLIST, a way to indicate that the bloblist should > be checked for a devicetree. Yes, you keep looking at this from the wrong direction and seem to have backed yourself in to a corner. Why are you insisting that for the normal use case of memory infos and maybe display parameters we need to carve out a hunk of main memory? A bloblist is perfectly capable of NOT containing things. We can avoid having to care about what phase of things we're in too. > > > In U-Boot proper, we look for it before relocation, so we can relocate and > > > expand it for use with ACPI tables, etc. > > > > I'm not sure that's relevant. This means we've taken what we were passed > > at a fixed address and allocated more space for it and are using it > > somewhere else. > > Yes > > > > > > Take a look at this patch, too: > > > > > > 3d6531803e1 bloblist: Support initing from multiple places > > > > I'm not sure it's relevant. It does show we have a problem of not > > knowing if the bloblist exists, or not. And I don't think we're solving > > that right today (nor does v1 of my patch). > > OK > > > > > > > An alternative here would be to better document the requirements of > > > > BLOBLIST_ADDR, and then just disable it on chromebook_coral until > > > > someone is inclined to move DDR init in to TPL, or someone finds a > > > > non-main-memory address to use for BLOBLIST_ADDR or switches to using > > > > BLOBLIST_ALLOC. > > > > > > Coral's TPL runs in cache-as-ram, with a 30KB limit on code size. It > > > cannot > > > do DDR init in TPL because the DDR blob is large (160KB?) and there is a > > > ton of setup to do first, in any case. When CAR goes away, its memory > > > vanishes, so you need to have copied the bloblist somewhere else (the > > > bloblist holds the MRC data which needs to be written to SPI flash later > > > on, when possible). This all works perfectly and there is really no need > > > to > > > change anything. > > > > Ah, so there's some of the details finally. And oh goodness, we're > > writing to the SPI flash on each boot? That's not good for longevity... > > It writes a sector each time, across an area which can hold quite a > few writes. It only writes when the details change, so it is fine. > This algorithm has worked in the field for years across tens of > millions of devices and is common on x86. To be clear, I wasn't saying that its your design. > > > Again, you seem to be imputing semantics to bloblist which don't exist. > > > > Unfortunately some things were missed along the way, and are still > > missing. > > Well we should have started from OF_BLOBLIST and then dealt with > problems as they came up. In fact, so far there are no problems which > it can't solve. > > As it is, we have Raymond sending a patch to push the prior-stage mess > into TPM, for reasons which make no sense to me. It is just adding > confusion. > > One other thing we would have likely done by now if my patch[1] had > gone in a year ago, is moving away from BLOBLIST_FIXED to using a > register protocol. That is more deterministic. > > Regards, > Simon > > [1] > https://patchwork.ozlabs.org/project/uboot/patch/20230830180524.315916-31-...@chromium.org/ I think once again your obsession with OF_BLOBLIST and device tree is causing you to miss everything else. Today we can take a *bloblist* via register. Linaro wrote that, even. The next problem really is that we can't pass that bloblist along to the next stage because all of the U-Boot side of things assumes fixed address. -- Tom
signature.asc
Description: PGP signature
