Hi Tom, On Tue, 17 Dec 2024 at 17:18, Tom Rini <[email protected]> wrote: > > On Tue, Dec 17, 2024 at 04:42:26PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 17 Dec 2024 at 13:15, Tom Rini <[email protected]> wrote: > > > > > > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > > > > > > 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 <[email protected]> > > > > > > > > > --- > > > > > > > > > Cc: Simon Glass <[email protected]> > > > > > > > > > --- > > > > > > > > > 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? > > > > The only case where that matters is for the devicetree, so we have > > OF_BLOBLIST to control whether U-Boot looks for it. Eventually we can > > drop BLOBLIST_FIXED and use registers, perhaps even on x86, which will > > make things easier. > > The only reason it ever matters is if you're picking memory that's not > always accessible. Which is a bad choice and should be avoided whenever > possible and practical. Which it is here. > > > > > > > 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. > > > > I really can't see how you came to that conclusion. Just enable > > OF_BLOBLIST - in fact we can enable it for almost all boards today and > > it won't harm anything. > > I mean, there's very few places using BLOBLIST today. But we can't > enable OF_BLOBLIST for any of them unless we say that the BLOBLIST > exists in always accessible memory. And that wouldn't be most of them. > > > > > > > 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. > > > > Devicetree is the only 'strange' thing here because we need to look at > > it early. > > Device tree is also "strange" in that it's the only thing we read (not > write) today prior to U-Boot too from bloblist. > > > For Raymond's patch, it shouldn't need any conditions. > > Yes, and I replied to him that way 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. > > > > No, it isn't my design, but I would be happy if it were. You were > > suggesting it had a longevity problem, so I wanted to explain it. > > > > > > > > > > > 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/[email protected]/ > > > > > > 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. > > > > Perhaps if we had applied OF_BLOBLIST a year ago, so my special 3 > > ancient chromebooks that nobody cares about or uses apart from me, > > worked, we could have got bloblist to where it should be today. You > > are missing the big picture, but stopping me from pursuing it. > > > > I am disappointed that it is only at this stage that you have shown > > any interest in getting my special 3 ancient chromebooks running. > > And I am disappointed that it's only at this stage there's an > understanding of what the actual problem is with your special 3 ancient > chromebooks. Everything last year was read by me as you arguing that you > need a way to control passing the device tree because that's where the > device tree is: > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/#3174895 > And if not every at least most of your other replies in that thread are > about how device tree is passed in. This is why I assumed that your > boards were working after having a way to enforce that the device tree > comes from bloblist.
That thread is crazy. I should have given up at v1. I wish you hadn't assumed that and I am still a bit shocked that you did, particularly after 4-5 revisions. > > So if there's some "big picture" about bloblist being delayed, it is > because once again your vision isn't communicated to everyone else > you're trying to work with and that is the big problem. Yes, agreed, that is exactly the problem. I hope my tree will solve that problem and everyone will be happier. Regards, Simon
