Hi Ard, On Thu, 7 Sept 2023 at 10:19, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > 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. Nevertheless, that is the goal. > > 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? Please don't widen the discussion as we are having enough trouble as it is. Let's focus on the memory reservations. > > > > > > > > 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 That looks like Tianocore internals rather than a binding, so far as I can tell. I do need a binding. > > 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) OK I'll try that patch again with compatible strings instead of node names[1] > > > > > > > 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. OK I'll try again. Regards, Simon [1] I was led down the node-name path by this text in the DT spec "Following the generic-names recommended practice, node names should reflect the purpose of the node (ie. “framebuffer” or “dma-pool”). Unit address (@<address>) should be appended to the name if the node is a static allocation."
