Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile: Add ability to build dtb #747

Closed
wants to merge 1 commit into from

Conversation

nmenon
Copy link
Contributor

@nmenon nmenon commented Nov 2, 2016

Current build system has no means to automatically generate dtbs from
dts, instead, stores the dts and correponding dtbs in the fdts/ folder.

However, it makes much sense for us to build the dtbs while building
the ATF binaries by purely describing which dts sources we need to
build for which BL* stage and generate dtbs as required.

For example, with this change, we will now be able to describe the
dtbs we need for the platform in the corresponding platform.mk file:
BL31_SOURCES += fdts/abc.dts

This should be able to generate the abc.dtb appropriately.

Further, this approach allows us to maintain multiple dtbs which may be
required depending on platforms that we need to support, and all of
those can easily be generated without maintaining dtbs in the source.

If dtc is not available in default paths, but is available in an
alternate location, it can be chosen by overriding the DTC variable
such as 'make DTC=~/dtc/dtc ....`

Signed-off-by: Benjamin Fair b-fair@ti.com
Signed-off-by: Nishanth Menon nm@ti.com

@ssg-bot
Copy link

ssg-bot commented Nov 2, 2016

Can one of the admins verify this patch?

Current build system has no means to automatically generate dtbs from
dts, instead, stores the dts and corresponding dtbs in the fdts/ folder.

However, it makes much sense for us to build the dtbs while building
the ATF binaries by purely describing which dts sources we need to
build for which BL* stage and generate dtbs as required.

For example, with this change, we will now be able to describe the
dtbs we need for the platform in the corresponding platform.mk file:
BL31_SOURCES += fdts/abc.dts

This should be able to generate the abc.dtb appropriately.

Further, this approach allows us to maintain multiple dtbs which may be
required depending on platforms that we need to support, and all of
those can easily be generated without maintaining dtbs in the source.

If dtc is not available in default paths, but is available in an
alternate location, it can be chosen by overriding the DTC variable
such as 'make DTC=~/dtc/dtc ....`

Signed-off-by: Benjamin Fair <b-fair@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
@danh-arm
Copy link
Contributor

danh-arm commented Nov 2, 2016

The device trees stored in the fdts folder are the kernel device trees for the FVP platforms. They are only stored here as there is a lack of a sensible upstream location for them.

Your patch seems to be adding dtb build support per BL image. Presumably these dtbs are different to the kernel dtb? How are you using these dtbs? Presumably your platform(s) pull in the libfdt sources into each image? Having this code affect the RAM footprint for all images, especially the runtime ones may be expensive. Adding dtb build support in the generic Makefile without any generic support for using these dtbs looks odd.

@danh-arm
Copy link
Contributor

danh-arm commented Nov 2, 2016

Also, PRs should target the integration branch, not the master branch

@nmenon
Copy link
Contributor Author

nmenon commented Nov 2, 2016

The device trees stored in the fdts folder are the kernel device trees for the FVP platforms. They
are only stored here as there is a lack of a sensible upstream location for them.

Yes, we had guessed as much. fdts/ folder is just fine, just that dtb storage is just plain redundant.

Your patch seems to be adding dtb build support per BL image. Presumably these dtbs are
different to the kernel dtb? How are you using these dtbs?

That is correct. ATF view of the hardware in EL3 is different from what normal world(kernel in EL1 or bootloader in EL1/2) gets to see. The platform dt for ATF exposes communication path details, and descriptions required for ATF to ensure a clean configuration.

Presumably your platform(s) pull in the libfdt sources into each image?

Mostly interesting in BL31 level, but yes.

Having this code affect the RAM footprint for all images, especially the runtime ones may be
expensive. Adding dtb build support in the generic Makefile without any generic support for using
these dtbs looks odd.

The plan was to switch foundation model to generate dtb using the generic change (should be trivial to do so), but ofcourse, that effort is worth it if the approach is sensible.

Also, PRs should target the integration branch, not the master branch

Will fix the pull request for sure. Thanks for the guidance.

@danh-arm
Copy link
Contributor

danh-arm commented Nov 3, 2016

Hi @nmenon

Yes, we had guessed as much. fdts/ folder is just fine, just that dtb storage is just plain redundant.

This is mainly a convenience for users who only want to build TF and use existing binaries for other components in the stack (including the kernel DT). But we could consider removing them if there's not much value.

That is correct. ATF view of the hardware in EL3 is different from what normal world(kernel in EL1 or bootloader in EL1/2) gets to see. The platform dt for ATF exposes communication path details, and descriptions required for ATF to ensure a clean configuration.

This sounds similar to the dynamic configuration support that was discussed here: ARM-software/tf-issues#420. For that we envisaged a single config DT, which may be shared by the various TF binaries, rather than a config per image. What are your future upstreaming plans in this area, if any?

The plan was to switch foundation model to generate dtb using the generic change (should be trivial to do so), but ofcourse, that effort is worth it if the approach is sensible.

But your change is to generate dtbs that the firmware uses (i.e. that are part of the sources), not to generate the kernel dtb. So I don't understand what dtb you would get the foundation model to generate. Or are you planning to upstream support for the foundation model to use a config DT?

@nmenon
Copy link
Contributor Author

nmenon commented Nov 3, 2016

@danh-arm

This is mainly a convenience for users who only want to build TF and use existing binaries for other
components in the stack (including the kernel DT). But we could consider removing them if there's not
much value.

We can easily generate the dtb when building ATF binaries with the current patch.

This sounds similar to the dynamic configuration support that was discussed here: ARM-software/tf-issues#420.

I briefly looked at the same.. @masahir0y 's initial requirement seems slightly different - though dt helps part of it, from experience, stuff that is too early: MPIDR deltas etc, dont fit into dt -> however, we like DT since there is a lot of backend generation tools that can work with DT. an alternate format might even be https://standards.ieee.org/develop/project/2415.html - but, from what I hear, IEEE P2415 is still too much in nascent stages.. DT suffices for 99%+ of usage we can think of.

For that we envisaged a single config DT, which may be shared by the various TF binaries, rather
than a config per image. What are your future upstreaming plans in this area, if any

we are working to send support to libfdt upstream to add "chosen" and few additional parsing support options, based on feedback of the same, we will eventually request the support to bring back here.

But your change is to generate dtbs that the firmware uses (i.e. that are part of the sources), not to
generate the kernel dtb.

That is correct -> the ATF dtbs are independent of kernel (or u-boot) dtbs. it makes perfect sense to move the hardware abstraction out into dtb -> example: same SoC is used on 5 different boards. each of the board uses a different uart port.. why'd we want to rebuild the same ATF for just a dumb uart port base address change, the dtb should describe that adequately.

So I don't understand what dtb you would get the foundation model to generate. Or are you planning to upstream support for the foundation model to use a config DT?

I was mostly thinking of getting rid of:
fdts/fvp-base-gicv2-psci-aarch32.dtb fdts/fvp-base-gicv3-psci-aarch32.dtb fdts/fvp-foundation-gicv2-psci.dtb fdts/fvp-base-gicv2-psci.dtb fdts/fvp-base-gicv3-psci.dtb fdts/fvp-foundation-gicv3-psci.dtb

instead generate them from dts at build. that way (just like kernel or other systems using devicetree), all the git repo maintains is the source and binaries are generated from source on need basis.

@danh-arm in a nutshell, we have been trying to find ATF port accross to as many board variants as possible and need a "hardware description" mechanism that does not involve ATF binary rebuild. IF dt is not the ATF way to do it, what is?

@BenjaminFair
Copy link
Contributor

we are working to send support to libfdt upstream to add "chosen" and few additional parsing support options

Just to clarify, the functionality that will be upstreamed to libfdt is for parsing the address and size parameters from the reg property.

@nmenon
Copy link
Contributor Author

nmenon commented Nov 3, 2016

@BenjaminFair Thanks for clarifying..

@danh-arm
Copy link
Contributor

danh-arm commented Nov 8, 2016

Hi @nmenon

I briefly looked at the same.. @masahir0y 's initial requirement seems slightly different - though dt helps part of it, from experience, stuff that is too early: MPIDR deltas etc, dont fit into dt -> however, we like DT since there is a lot of backend generation tools that can work with DT. an alternate format might even be https://standards.ieee.org/develop/project/2415.html - but, from what I hear, IEEE P2415 is still too much in nascent stages.. DT suffices for 99%+ of usage we can think of.

I agree the use-cases are a bit different, although the solutions could be shared. From what we've heard, the more immediate priority is to be able to describe the firmware configuration at runtime (e.g. normal world entrypoint information, firmware images to load/authenticate, load addresses etc...). Providing a full hardware description for firmware to use is more ambitious. I think aiming for a single firmware across a family of boards is a reasonable ambition, but aiming for a single firmware everywhere is too much.

For both of these use-cases, I agree that DT suffices. I'm not particularly interested in blazing a trail for new standards in this area.

we are working to send support to libfdt upstream to add "chosen" and few additional parsing support options, based on feedback of the same, we will eventually request the support to bring back here.

That's fine. We intend to regularly pull from libfdt anyway. Just let us know if an when you need anything specific.

That is correct -> the ATF dtbs are independent of kernel (or u-boot) dtbs. it makes perfect sense to move the hardware abstraction out into dtb -> example: same SoC is used on 5 different boards. each of the board uses a different uart port.. why'd we want to rebuild the same ATF for just a dumb uart port base address change, the dtb should describe that adequately.

This is where I'm still confused. If your proposed TF dtbs are independent of the kernel (or u-boot) dtbs, then your makefile changes should not need to build the existing in-source kernel dtbs. Those dtbs are independent to the firmware images and should not be part of their sources.

We could potentially add a new build target for these kernel dtbs in the top-level Makefile but this support would look very different to your current PR. I don't think this would be appropriate anyway, since generic Trusted Firmware should be independent to normal world software. Perhaps the FVP platform makefile could build these instead.

@danh-arm in a nutshell, we have been trying to find ATF port accross to as many board variants as possible and need a "hardware description" mechanism that does not involve ATF binary rebuild. IF dt is not the ATF way to do it, what is?

As I've said, DT is a reasonable way to go, but I don't think this PR is a reasonable way to implement it, and I don't want to add a mechanism for building a firmware DT, unless there is also a generic framework provided to parse and use this firmware DT. Such a thing probably requires more in-depth design discussion (probably not in this PR!).

Regards

Dan.

@nmenon
Copy link
Contributor Author

nmenon commented Nov 8, 2016

On Tue, Nov 8, 2016 at 5:10 AM, danh-arm notifications@github.com wrote:

As I've said, DT is a reasonable way to go, but I don't think this PR is a
reasonable way to implement it, and I don't want to add a mechanism for
building a firmware DT, unless there is also a generic framework provided
to parse and use this firmware DT. Such a thing probably requires more
in-depth design discussion (probably not in this PR!).

I am confused, we do have libfdt included in ATF - so that generic
libraries are present already (and used by QEMU to an extent), there is a
need for few additional functionality that we'd have to add -> something
like what u-boot already has, but we start building things up in pieces
right?

How do you propose we start working on this?

@danh-arm
Copy link
Contributor

danh-arm commented Nov 9, 2016

The way QEMU is using DT is completely different from this use-case; it dynamically updates the kernel DT in non-secure memory to reflect the PSCI topology.

The kind of framework I'm talking about is:

  • A way of packing the firmware DT in the FIP, and eventually the impact on the Chain of Trust.
  • The fundamental data that comprises the initial firmware DT. Some of the information that is currently in the FIP header should perhaps be in this DT instead, which would fundamentally alter the FIP format.
  • Generic support for loading (and eventually authenticating) the firmware DT. We expect this to be in BL2.
  • A way of passing the information in the DT between images. We expect some nodes to be passed around in memory structures, rather than getting each image to reparse the DT.

If this kind of framework is the right direction to go in, then I'd expect a separate Makefile target for the firmware DT, rather than having DT sources within images as your PR implements. If you are interested in collaborating in this direction, we can discuss in more detail offline. If you have something else in mind, then you need to describe it.

I'm all for building things up in pieces, but there needs to be an upfront vision for what we're trying to achieve, and there needs to be a working use-case at each step of the way.

@nmenon
Copy link
Contributor Author

nmenon commented Nov 9, 2016

On Wed, Nov 9, 2016 at 11:44 AM, danh-arm notifications@github.com wrote:

If you have something else in mind, then you need to describe it.

My system does not use BL2. all it uses is BL31. the equivalent of BL2 is
done by another solution. from the cortex processor perspective, DTB for
ATF is already loaded, and I dont need to use FIP model.

That said, I do agree that the model you are talking about does make sense.

@danh-arm
Copy link
Contributor

My system does not use BL2. all it uses is BL31. the equivalent of BL2 is done by another solution. from the cortex processor perspective, DTB for ATF is already loaded, and I dont need to use FIP model.

OK, in your case BL31 will need to parse the DT and the other framework stuff I mentioned is not very relevant to you. However, building the firmware DT as a separate target should cater for both our cases. Perhaps the first step to upstreaming something could be to add:

  • An optional firmware DT makefile target
  • An optional build flag for compiling libfdt into BL31
  • A very simple use-case in one of the reference platforms (e.g. FVP), which provides a basic DT that is parsed in BL31 platform code and varies behaviour according to the DT contents. This could be as simple printing something on the console according to the value of a flag in the DT. A more useful use-case could be to optionally select the RESET_TO_BL31 functionality at runtime.
  • Some documentation for how to build and execute the use-case.

@danh-arm
Copy link
Contributor

This PR has stalled. If this is still relevant, please raise a new PR with the requested changes, or continue the discussion on the issue tracker.

@danh-arm danh-arm closed this Jan 11, 2017
@nmenon
Copy link
Contributor Author

nmenon commented Jan 11, 2017

agreed. we need to revamp the solution.

nmenon added a commit to nmenon/arm-trusted-firmware that referenced this pull request Sep 20, 2017
This is a revamp of the original approach in:
ARM-software#747

Current build system has no means to automatically generate dtbs from
dts, instead, stores the dtbs in the fdts/ folder. While this makes
perfect sense for many reference platforms, this becomes a minor
breakage in development flow for newer platforms.

However, this can be solved by providing a rule for the dtbs while
building the ATF binaries by purely describing which dts sources we
need.

For example, with this change, we will now be able to describe the
dtbs we need for the platform in the corresponding platform.mk file:
FDT_SOURCES += fdts/abc.dts

This should be able to generate the abc.dtb appropriately.

Since device trees are specification of hardware, we don't tie the rule
to any specific BL, instead a generic rule is introduced.

Further, this approach allows us to generate appropriate dtbs which may be
need to be regenerated when a common dtsi gets updated, by just
restricting changes to the dtsi alone, instead of synchronizing all the
dtbs as well.

If dtc is not available in default paths, but is available in an
alternate location, it can be chosen by overriding the DTC variable
such as 'make DTC=~/dtc/dtc ....`

NOTE: dtbs are built only with the explicit make dtbs command. The rule
is only available if the platform defines a FDT_SOURCES variable.

Signed-off-by: Benjamin Fair <b-fair@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
kph pushed a commit to platinasystems/arm-trusted-firmware that referenced this pull request Feb 27, 2020
This is a revamp of the original approach in:
ARM-software/arm-trusted-firmware#747

Current build system has no means to automatically generate dtbs from
dts, instead, stores the dtbs in the fdts/ folder. While this makes
perfect sense for many reference platforms, this becomes a minor
breakage in development flow for newer platforms.

However, this can be solved by providing a rule for the dtbs while
building the ATF binaries by purely describing which dts sources we
need.

For example, with this change, we will now be able to describe the
dtbs we need for the platform in the corresponding platform.mk file:
FDT_SOURCES += fdts/abc.dts

This should be able to generate the abc.dtb appropriately.

Since device trees are specification of hardware, we don't tie the rule
to any specific BL, instead a generic rule is introduced.

Further, this approach allows us to generate appropriate dtbs which may be
need to be regenerated when a common dtsi gets updated, by just
restricting changes to the dtsi alone, instead of synchronizing all the
dtbs as well.

If dtc is not available in default paths, but is available in an
alternate location, it can be chosen by overriding the DTC variable
such as 'make DTC=~/dtc/dtc ....`

NOTE: dtbs are built only with the explicit make dtbs command. The rule
is only available if the platform defines a FDT_SOURCES variable.

Signed-off-by: Benjamin Fair <b-fair@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants