On Thu, 7 Sept 2023 at 17:57, Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > Hi Ard, > > On Thu, 7 Sept 2023 at 09:07, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > On Thu, 7 Sept 2023 at 16:50, Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > > > > > Hi Ard, > > > > > > On Thu, 7 Sept 2023 at 08:12, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > > > On Thu, 7 Sept 2023 at 15:56, Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > > > > > > > > > Hi Ard, > > > > > > > > > > On Thu, 7 Sept 2023 at 07:31, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > ... > > > > > > > > > > > > I'm happy to help flesh this out, but you still have not provided us > > > > > > with an actual use case, so I can only draw from my own experience > > > > > > putting together firmware for virtual and physical ARM machines. > > > > > > > > > > I did explain that this is needed when Tianocore is on both sides of > > > > > the interface, since Platform Init places some things in memory and > > > > > the Payload needs to preserve them there, and/or know where they are. > > > > > > > > > > I think the problem might be that you don't agree with that, but it > > > > > seems to be a fact, so I am not sure how I can alter it. > > > > > > > > > > Please can you clearly explain which part of the use case you are missing. > > > > > > > > > > > > > 'Tianocore on both sides of the interface' means that Tianocore runs > > > > as the platform init code, and uses a bespoke DT based protocol to > > > > launch another instance of Tianocore as the payload, right? > > > > > > Not another instance, no. Just the other half of Tianocore. The first > > > half does platform init and the second half does the loading of the > > > OS. > > > > > > > That doesn't make any sense to me. > > > > > > > > > > Tianocore/EDK2 already implements methods to reinvoke itself if needed > > > > (e.g., during a firmware update), and does so by launching a new DXE > > > > core. So the boot sequence looks like > > > > > > > > SEC -> PEI -> DXE -> BDS -> app that invokes UpdateCapsule() -> DXE -> > > > > firmware update > > > > > > > > So please elaborate on how this Tianocore on both sides of the > > > > interface is put together when it uses this DT based handover. We > > > > really need a better understanding of this in order to design a DT > > > > binding that meets its needs. > > > > > > Are you familiar with building Tianocore as a coreboot payload, for > > > example? That shows Tianocore running as just the Payload, with > > > coreboot doing the platform init. So the use case I am talking about > > > is similar to that. > > > > > > > Yes I am familiar with that, and it is a completely different thing. > > Right, but that is my use case. > OK. You alluded to Tianocore <-> Tianocore being your use case, which is why I kept asking for clarification, as using a DT with this binding seems unusual at the very least. So coreboot does the platform init, and then hands over to Tianocore. I take it we are not talking about x86 here, so there are no Intel FSP blobs that may have dependencies on Tianocore/EDK2 pieces, right? So there are no UEFI semantics in the memory descriptions that coreboot provides to Tianocore. So coreboot provides information to TIanocore about - the platform topology (DT as usual) - DRAM memory banks - memory reservations - secure firmware services perhaps? - anything else? > > > > As i explained before, there is already prior art for this in > > Tianocore, i.e., launching a Tianocore build based on a DT description > > of the platform, including /memory and /reserved-memory nodes. > > By prior art do you mean code, or an existing binding? In either case, > can you please point me to it? Is this a generic binding used on x86 > as well, or just for ARM? > > My goal here is to augment the binding. > No I mean code. There is https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Drivers/FdtClientDxe which encapsulates the DT received from the previous boot stage, and exposes it as a DXE protocol. There are other drivers that depend on this protocol, e.g., to discover additional memory nodes, virtio-mmio drivers and PCI host bridges. https://github.com/tianocore/edk2/tree/master/OvmfPkg/Fdt The bindings used are the ones documented in the Linux kernel tree - no ad-hoc bindings are being used as far as I know. But the point I was making before re prior art was really about using existing bindings rather than inventing new ones. Since we are now talking about augmenting /reserved-memory, I think we're already on the same page in this regard (with the caveat that the EDK2 code does not actually honour /reserved-memory at this point, but this is because none of the platforms it is being used on today uses that node) > > > > I argued that Tianocore never consumes memory reservations with UEFI > > semantics, given that it supplants whatever UEFI functionality the > > previous stage may have provided. But it shouldn't step on the code > > and data regions used by the previous stage if it is still running in > > the background (e.g., OS at EL1 and PSCI at EL2 on ARM) > > > > So this brings me back to the things I proposed in my previous reply: > > - memory reservations should be described in detail so the consumer > > knows what to do with it > > Yes I can add more detail, if that is all that is needed. But we seem > to still not be aligned on the goal. > > I do think we're converging, actually - it is just taking me some time to get a clear mental picture of how this will be used. > > - memory reservations should have attributes that describe how the > > memory may be used if not for the described purpose > > > > I still don't see a reason for things like runtime-code and > > runtime-data etc based on the above. If stage N describes the memory > > it occupies itself as system memory, it should reserve it as well if > > it needs to be preserved after stage N+1 has taken over, so perhaps it > > should be described as a discardable memory reservation but I don't > > think it necessarily needs a type in that case. > > Well if you can find another way to do this in the DT, that is fine. > It will all be under /reserved-memory, as far as I understand. We just need to get to the right level of abstraction.
