| Message ID | 1402689965-19397-1-git-send-email-jwerner@chromium.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Jun 13, 2014 at 3:06 PM, Julius Werner <jwerner@chromium.org> wrote: > This patch adds documentation describing a device tree binding for the > coreboot firmware project (www.coreboot.org). It is meant to be > dynamically added during boot and contains address definitions for the > coreboot table (a list of variable-sized descriptors providing > information about various compile- and run-time generated firmware > parameters) and the CBMEM area (the structure containing most run-time > resident memory regions set up by coreboot). > > These definitions allow kernel drivers to easily access data contained > in and pointed to by these regions (such as coreboot's in-memory log). > (An example implementation can be seen at http://crosreview.com/203371, > which will be submitted at a later point.) This is just to export a fixed log to userspace (like a DMI table) or the kernel will actually use the data in some way? Based on the link, it looks like the former to me. > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > .../devicetree/bindings/firmware/coreboot.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt > > diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt > new file mode 100644 > index 0000000..89d7bf3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/firmware/coreboot.txt > @@ -0,0 +1,28 @@ > +COREBOOT firmware information > + > +The device tree node to communicate the location of coreboot's memory-resident > +bookkeeping structures to the kernel. Since coreboot itself cannot boot a > +device-tree-based kernel (yet), this node needs to be inserted by a > +second-stage bootloader (a coreboot "payload"). > + > +Required properties: > + - compatible: Should be "coreboot" > + - reg: Address and length of the following two memory regions, in order: > + 1.) The coreboot table. This is a list of variable-sized descriptors > + that contain various compile- and run-time generated firmware > + parameters. It is identified by the magic string "LBIO" in its first > + four bytes. See coreboot's src/include/boot/coreboot_tables.h for > + details. > + 2.) The CBMEM area. This is a downward-growing memory region used by > + coreboot to dynamically allocate data structures that remain resident. > + It may or may not include the coreboot table as one of its members. It > + is identified by a root node descriptor with the magic number > + 0xc0389479 that resides in the topmost 8 bytes of the area. See > + coreboot's src/lib/dynamic_cbmem.c for details. Don't you need need to keep the kernel from allocating this memory by using one of the reserved memory mechanisms? The recently added one should be able to specific what the memory is reserved for IIRC. > + > +Example: > + firmware { /firmware is already used IIRC. What if you have other firmware such as Trustzone? > + compatible = "coreboot"; > + reg = <0xfdfea000 0x264>, > + <0xfdfea000 0x16000>; > + }; > -- > 1.8.3.2 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> This is just to export a fixed log to userspace (like a DMI table) or > the kernel will actually use the data in some way? Based on the link, > it looks like the former to me. I could imagine both. The link is an in-kernel driver that exposes a log through a sysfs node (in a way that has already been established on x86 systems, which find the location through EBDA or ACPI entries instead). We are also using a user-space tool that reads the address from /proc/device-tree and accesses it through /dev/mem. The areas can contain many interesting entries (like the location of an early framebuffer set up by the firmware), so I could also imagine use cases where the kernel makes use of it directly. > Don't you need need to keep the kernel from allocating this memory by > using one of the reserved memory mechanisms? The recently added one > should be able to specific what the memory is reserved for IIRC. Our bootloader is carving the location out of the /memory node and adding it to the device tree reserve map. As far as I know, that only contains a list of raw start and size entries. At any rate, I think it's useful (and in line with other bindings) to add a more explicit node like this (if only to make it easier accessible through /proc/device-tree). > /firmware is already used IIRC. What if you have other firmware such > as Trustzone? I'm not quite sure how Trusted Foundations works and whether it would even make sense to use it in parallel to coreboot, but it seems to be using the /firmware/trusted-foundations subnode so that should be fine. "firmware" seems to be used by other firmware implementations (like "samsung,secure-firmware") which are similar in nature to and mutually exclusive with coreboot, so I thought the node makes sense. (The kernel should use the compatible string to find it anyway, so a future name clash would not be world-ending.) -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 13, 2014 at 4:58 PM, Julius Werner <jwerner@chromium.org> wrote: >> This is just to export a fixed log to userspace (like a DMI table) or >> the kernel will actually use the data in some way? Based on the link, >> it looks like the former to me. > > I could imagine both. The link is an in-kernel driver that exposes a > log through a sysfs node (in a way that has already been established > on x86 systems, which find the location through EBDA or ACPI entries > instead). We are also using a user-space tool that reads the address > from /proc/device-tree and accesses it through /dev/mem. The areas can > contain many interesting entries (like the location of an early > framebuffer set up by the firmware), so I could also imagine use cases > where the kernel makes use of it directly. I can be argued that the boot interface is DT and any configuration data should be put there in a common way. We don't really need yet another boot mechanism as we already have: UEFI + FDT UEFI + ACPI "standard" bootloaders (e.g. u-boot, grub, barebox, etc.) + FDT Allowing every bootloader to define its own boot interfaces would only result in a mess for both code and testing. I don't want to get into a debate about this now as it is not too relevant to this patch, but just want to highlight the resistance you will face going down this path. >> Don't you need need to keep the kernel from allocating this memory by >> using one of the reserved memory mechanisms? The recently added one >> should be able to specific what the memory is reserved for IIRC. > > Our bootloader is carving the location out of the /memory node and > adding it to the device tree reserve map. As far as I know, that only > contains a list of raw start and size entries. At any rate, I think > it's useful (and in line with other bindings) to add a more explicit > node like this (if only to make it easier accessible through > /proc/device-tree). Understand there are 3 different memory reservation bindings. The original "/memreserve/" method is indeed limited. What I think you should use is the binding documented in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt. So you could do something like this: reserved-memory { #address-cells = <1>; #size-cells = <1>; ranges; /* global autoconfigured region for contiguous allocations */ linux,cma { compatible = "shared-dma-pool"; reusable; size = <0x4000000>; alignment = <0x2000>; linux,cma-default; }; coreboot_reserved: coreboot@fdfea000 { compatible = "coreboot"; reg = <0xfdfea000 0x264>, <0xfdfea000 0x16000>; }; >> /firmware is already used IIRC. What if you have other firmware such >> as Trustzone? > > I'm not quite sure how Trusted Foundations works and whether it would > even make sense to use it in parallel to coreboot, but it seems to be > using the /firmware/trusted-foundations subnode so that should be > fine. "firmware" seems to be used by other firmware implementations > (like "samsung,secure-firmware") which are similar in nature to and > mutually exclusive with coreboot, so I thought the node makes sense. > (The kernel should use the compatible string to find it anyway, so a > future name clash would not be world-ending.) They are not mutually exclusive. What runs in secure world or not is entirely independent of non-secure boot. You may not care about it, but other platforms could. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2014-06-13 14:58 GMT-07:00 Julius Werner <jwerner@chromium.org>: >> This is just to export a fixed log to userspace (like a DMI table) or >> the kernel will actually use the data in some way? Based on the link, >> it looks like the former to me. > > I could imagine both. The link is an in-kernel driver that exposes a > log through a sysfs node (in a way that has already been established > on x86 systems, which find the location through EBDA or ACPI entries > instead). We are also using a user-space tool that reads the address > from /proc/device-tree and accesses it through /dev/mem. The areas can > contain many interesting entries (like the location of an early > framebuffer set up by the firmware), so I could also imagine use cases > where the kernel makes use of it directly. Hmm. It'd be much better if the early framebuffer was communicated using the already existing methods (i.e. simplefb device tree node). That way we don't have to add custom code to grab it out of the coreboot memory structure. Adding a platform driver for coreboot to do it later in boot isn't so hard (and registering platform devices based on the contents), but we probably need to be more generic if it is to be used in actual early boot, which is the main use case for it. >> Don't you need need to keep the kernel from allocating this memory by >> using one of the reserved memory mechanisms? The recently added one >> should be able to specific what the memory is reserved for IIRC. > > Our bootloader is carving the location out of the /memory node and > adding it to the device tree reserve map. As far as I know, that only > contains a list of raw start and size entries. At any rate, I think > it's useful (and in line with other bindings) to add a more explicit > node like this (if only to make it easier accessible through > /proc/device-tree). > >> /firmware is already used IIRC. What if you have other firmware such >> as Trustzone? > > I'm not quite sure how Trusted Foundations works and whether it would > even make sense to use it in parallel to coreboot, but it seems to be > using the /firmware/trusted-foundations subnode so that should be > fine. "firmware" seems to be used by other firmware implementations > (like "samsung,secure-firmware") which are similar in nature to and > mutually exclusive with coreboot, so I thought the node makes sense. > (The kernel should use the compatible string to find it anyway, so a > future name clash would not be world-ending.) Right, coreboot really should go under /firmware/coreboot -- we already use /firmware/chromeos on chromebooks to specify chromeos-specific firmware properties, this follows that convention. The samsung,secure-firmware should probably also be moved to a subnode. The binding shouldn't require a specific location no matter what. -Olof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 16, 2014 at 6:30 AM, Rob Herring <robherring2@gmail.com> wrote: > On Fri, Jun 13, 2014 at 4:58 PM, Julius Werner <jwerner@chromium.org> wrote: >>> This is just to export a fixed log to userspace (like a DMI table) or >>> the kernel will actually use the data in some way? Based on the link, >>> it looks like the former to me. >> >> I could imagine both. The link is an in-kernel driver that exposes a >> log through a sysfs node (in a way that has already been established >> on x86 systems, which find the location through EBDA or ACPI entries >> instead). We are also using a user-space tool that reads the address >> from /proc/device-tree and accesses it through /dev/mem. The areas can >> contain many interesting entries (like the location of an early >> framebuffer set up by the firmware), so I could also imagine use cases >> where the kernel makes use of it directly. > > I can be argued that the boot interface is DT and any configuration > data should be put there in a common way. We don't really need yet > another boot mechanism as we already have: > > UEFI + FDT > UEFI + ACPI > "standard" bootloaders (e.g. u-boot, grub, barebox, etc.) + FDT > > Allowing every bootloader to define its own boot interfaces would only > result in a mess for both code and testing. I don't want to get into a > debate about this now as it is not too relevant to this patch, but > just want to highlight the resistance you will face going down this > path. > >>> Don't you need need to keep the kernel from allocating this memory by >>> using one of the reserved memory mechanisms? The recently added one >>> should be able to specific what the memory is reserved for IIRC. >> >> Our bootloader is carving the location out of the /memory node and >> adding it to the device tree reserve map. As far as I know, that only >> contains a list of raw start and size entries. At any rate, I think >> it's useful (and in line with other bindings) to add a more explicit >> node like this (if only to make it easier accessible through >> /proc/device-tree). > > Understand there are 3 different memory reservation bindings. The > original "/memreserve/" method is indeed limited. What I think you > should use is the binding documented in > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt. > So you could do something like this: > > reserved-memory { > #address-cells = <1>; > #size-cells = <1>; > ranges; > > /* global autoconfigured region for contiguous allocations */ > linux,cma { > compatible = "shared-dma-pool"; > reusable; > size = <0x4000000>; > alignment = <0x2000>; > linux,cma-default; > }; > > coreboot_reserved: coreboot@fdfea000 { > compatible = "coreboot"; > reg = <0xfdfea000 0x264>, > <0xfdfea000 0x16000>; > }; > > Okay, I see. But do you really think this is the best way to specify that interface? Bindings for other firmware also seems to prefer some form of /firmware, so I think putting it there or under there is more consistent. Especially if we later find a need to add more properties to the coreboot node (maybe a version number, feature availability, or things like that), a reserved-memory node would feel like the wrong place for it to me. >>> /firmware is already used IIRC. What if you have other firmware such >>> as Trustzone? >> >> I'm not quite sure how Trusted Foundations works and whether it would >> even make sense to use it in parallel to coreboot, but it seems to be >> using the /firmware/trusted-foundations subnode so that should be >> fine. "firmware" seems to be used by other firmware implementations >> (like "samsung,secure-firmware") which are similar in nature to and >> mutually exclusive with coreboot, so I thought the node makes sense. >> (The kernel should use the compatible string to find it anyway, so a >> future name clash would not be world-ending.) > > They are not mutually exclusive. What runs in secure world or not is > entirely independent of non-secure boot. You may not care about it, > but other platforms could. On Mon, Jun 16, 2014 at 9:39 AM, Olof Johansson <olofj@chromium.org> wrote: > 2014-06-13 14:58 GMT-07:00 Julius Werner <jwerner@chromium.org>: >>> This is just to export a fixed log to userspace (like a DMI table) or >>> the kernel will actually use the data in some way? Based on the link, >>> it looks like the former to me. >> >> I could imagine both. The link is an in-kernel driver that exposes a >> log through a sysfs node (in a way that has already been established >> on x86 systems, which find the location through EBDA or ACPI entries >> instead). We are also using a user-space tool that reads the address >> from /proc/device-tree and accesses it through /dev/mem. The areas can >> contain many interesting entries (like the location of an early >> framebuffer set up by the firmware), so I could also imagine use cases >> where the kernel makes use of it directly. > > Hmm. It'd be much better if the early framebuffer was communicated > using the already existing methods (i.e. simplefb device tree node). > That way we don't have to add custom code to grab it out of the > coreboot memory structure. > > Adding a platform driver for coreboot to do it later in boot isn't so > hard (and registering platform devices based on the contents), but we > probably need to be more generic if it is to be used in actual early > boot, which is the main use case for it. > The framebuffer was just a further example I could think of, I don't have any plans to add kernel code using it for the moment. I agree that it's better to define generic interfaces and have the firmware support those... but these take some time to flesh out, and I think there may always be cases of things that are too new or too specific to a particular firmware to have a generic interface (e.g. debug tools that could dump left-over firmware state to analyze a problem). I think having direct access to data structures like these is still useful, even if you also have support for generic interfaces. >>> Don't you need need to keep the kernel from allocating this memory by >>> using one of the reserved memory mechanisms? The recently added one >>> should be able to specific what the memory is reserved for IIRC. >> >> Our bootloader is carving the location out of the /memory node and >> adding it to the device tree reserve map. As far as I know, that only >> contains a list of raw start and size entries. At any rate, I think >> it's useful (and in line with other bindings) to add a more explicit >> node like this (if only to make it easier accessible through >> /proc/device-tree). >> >>> /firmware is already used IIRC. What if you have other firmware such >>> as Trustzone? >> >> I'm not quite sure how Trusted Foundations works and whether it would >> even make sense to use it in parallel to coreboot, but it seems to be >> using the /firmware/trusted-foundations subnode so that should be >> fine. "firmware" seems to be used by other firmware implementations >> (like "samsung,secure-firmware") which are similar in nature to and >> mutually exclusive with coreboot, so I thought the node makes sense. >> (The kernel should use the compatible string to find it anyway, so a >> future name clash would not be world-ending.) > > Right, coreboot really should go under /firmware/coreboot -- we > already use /firmware/chromeos on chromebooks to specify > chromeos-specific firmware properties, this follows that convention. > > The samsung,secure-firmware should probably also be moved to a > subnode. The binding shouldn't require a specific location no matter > what. Okay, fair enough. I'll upload a new patch that suggests /firmware/coreboot instead. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/16/2014 07:30 AM, Rob Herring wrote: > On Fri, Jun 13, 2014 at 4:58 PM, Julius Werner <jwerner@chromium.org> wrote: ... >> Rob Herring wrote: >>> Don't you need need to keep the kernel from allocating this memory by >>> using one of the reserved memory mechanisms? The recently added one >>> should be able to specific what the memory is reserved for IIRC. >> >> Our bootloader is carving the location out of the /memory node and >> adding it to the device tree reserve map. As far as I know, that only >> contains a list of raw start and size entries. At any rate, I think >> it's useful (and in line with other bindings) to add a more explicit >> node like this (if only to make it easier accessible through >> /proc/device-tree). > > Understand there are 3 different memory reservation bindings. The > original "/memreserve/" method is indeed limited. What I think you > should use is the binding documented in > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt. > So you could do something like this: > > reserved-memory { > #address-cells = <1>; > #size-cells = <1>; > ranges; > > /* global autoconfigured region for contiguous allocations */ > linux,cma { > compatible = "shared-dma-pool"; > reusable; > size = <0x4000000>; > alignment = <0x2000>; > linux,cma-default; > }; > > coreboot_reserved: coreboot@fdfea000 { > compatible = "coreboot"; > reg = <0xfdfea000 0x264>, > <0xfdfea000 0x16000>; > }; > I thought that the /reserved-memory node was more so that the (preferred?) location of firmware images or data buffers used by HW accelerators could be communicated to the kernel. This feels like pure data. The coreboot binding seems to be more about defining an interface to a particular firmware (this feels like semantics), which as a side-effect needs to communicate the location of certain data. If /reserved-memory is used to communicate the address of the memory regions, I think we still need a /firmware/coreboot node to indicate the semantics of the reserved memory region, and point at the phandle of the region. As such, it seems simpler just to put the addresses in the coreboot node's reg property. The only exception I see to that argument is if putting the region in /reserved-memory automatically carves that region out of the memory the kernel will allocate from. That would simplify the bootloader, since it wouldn't have to fiddle with the /memory node and do the carveout itself. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 16, 2014 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 06/16/2014 07:30 AM, Rob Herring wrote: >> On Fri, Jun 13, 2014 at 4:58 PM, Julius Werner <jwerner@chromium.org> wrote: > ... >>> Rob Herring wrote: >>>> Don't you need need to keep the kernel from allocating this memory by >>>> using one of the reserved memory mechanisms? The recently added one >>>> should be able to specific what the memory is reserved for IIRC. >>> >>> Our bootloader is carving the location out of the /memory node and >>> adding it to the device tree reserve map. As far as I know, that only >>> contains a list of raw start and size entries. At any rate, I think >>> it's useful (and in line with other bindings) to add a more explicit >>> node like this (if only to make it easier accessible through >>> /proc/device-tree). >> >> Understand there are 3 different memory reservation bindings. The >> original "/memreserve/" method is indeed limited. What I think you >> should use is the binding documented in >> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt. >> So you could do something like this: >> >> reserved-memory { >> #address-cells = <1>; >> #size-cells = <1>; >> ranges; >> >> /* global autoconfigured region for contiguous allocations */ >> linux,cma { >> compatible = "shared-dma-pool"; >> reusable; >> size = <0x4000000>; >> alignment = <0x2000>; >> linux,cma-default; >> }; >> >> coreboot_reserved: coreboot@fdfea000 { >> compatible = "coreboot"; >> reg = <0xfdfea000 0x264>, >> <0xfdfea000 0x16000>; >> }; >> > > I thought that the /reserved-memory node was more so that the > (preferred?) location of firmware images or data buffers used by HW > accelerators could be communicated to the kernel. This feels like pure data. That was the initial use, but I think this case fits well. PPC may have some other uses already. I haven't looked. > The coreboot binding seems to be more about defining an interface to a > particular firmware (this feels like semantics), which as a side-effect > needs to communicate the location of certain data. The DT is only defining it's presence and location. Knowledge of what the data is is (optionally) in the kernel. > If /reserved-memory is used to communicate the address of the memory > regions, I think we still need a /firmware/coreboot node to indicate the > semantics of the reserved memory region, and point at the phandle of the > region. As such, it seems simpler just to put the addresses in the > coreboot node's reg property. The only exception I see to that argument > is if putting the region in /reserved-memory automatically carves that > region out of the memory the kernel will allocate from. That would > simplify the bootloader, since it wouldn't have to fiddle with the > /memory node and do the carveout itself. The semantics are defined by a compatible property which the /reserved-memory binding has. Knowledge of what is in that range is in the kernel, not the DT. And yes, either /memreserve/ or /reserved-memory is needed to prevent kernel allocations. I'm not tied to /reserved-memory, but I just want a consistent way to describe random blocks of static data in memory. DMI tables are another similar example. We've already got some scattered usage of /firmware. There's also /psci for PSCI. UEFI is using /chosen for its tables (probably for good reason since the tables are needed early and need to be simple to parse). Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt new file mode 100644 index 0000000..89d7bf3 --- /dev/null +++ b/Documentation/devicetree/bindings/firmware/coreboot.txt @@ -0,0 +1,28 @@ +COREBOOT firmware information + +The device tree node to communicate the location of coreboot's memory-resident +bookkeeping structures to the kernel. Since coreboot itself cannot boot a +device-tree-based kernel (yet), this node needs to be inserted by a +second-stage bootloader (a coreboot "payload"). + +Required properties: + - compatible: Should be "coreboot" + - reg: Address and length of the following two memory regions, in order: + 1.) The coreboot table. This is a list of variable-sized descriptors + that contain various compile- and run-time generated firmware + parameters. It is identified by the magic string "LBIO" in its first + four bytes. See coreboot's src/include/boot/coreboot_tables.h for + details. + 2.) The CBMEM area. This is a downward-growing memory region used by + coreboot to dynamically allocate data structures that remain resident. + It may or may not include the coreboot table as one of its members. It + is identified by a root node descriptor with the magic number + 0xc0389479 that resides in the topmost 8 bytes of the area. See + coreboot's src/lib/dynamic_cbmem.c for details. + +Example: + firmware { + compatible = "coreboot"; + reg = <0xfdfea000 0x264>, + <0xfdfea000 0x16000>; + };
This patch adds documentation describing a device tree binding for the coreboot firmware project (www.coreboot.org). It is meant to be dynamically added during boot and contains address definitions for the coreboot table (a list of variable-sized descriptors providing information about various compile- and run-time generated firmware parameters) and the CBMEM area (the structure containing most run-time resident memory regions set up by coreboot). These definitions allow kernel drivers to easily access data contained in and pointed to by these regions (such as coreboot's in-memory log). (An example implementation can be seen at http://crosreview.com/203371, which will be submitted at a later point.) Signed-off-by: Julius Werner <jwerner@chromium.org> --- .../devicetree/bindings/firmware/coreboot.txt | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt