On 18/04/2018 09:44, Udit Kumar wrote:
-----Original Message-----
From: Grant Likely [mailto:grant.likely@xxxxxxx]
Sent: Monday, March 26, 2018 4:10 PM
To: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>; Pankaj Bansal
<pankaj.bansal@xxxxxxx>; mark.rutland@xxxxxxx; leif.lindholm@xxxxxxxxxx
Cc: linux-efi@xxxxxxxxxxxxxxx; Varun Sethi <V.Sethi@xxxxxxx>; Wasim Khan
<wasim.khan@xxxxxxx>; Udit Kumar <udit.kumar@xxxxxxx>; nd@xxxxxxx
Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration
table
On 25/03/2018 16:29, Ard Biesheuvel wrote:
On 25 Mar 2018, at 14:56, Pankaj Bansal <pankaj.bansal@xxxxxxx> wrote:
Bootloader may need to fixup the device tree before OS can use it.
e.g. a UEFI/DXE driver that has initialized a controller can add
controller's clock frequency in controller node. This way OS need not
to call get/set clock for that controller.
Therefore, install fdt used by OS in configuration tables and
associate it with device tree guid.
The firmware should be providing the dtb in the first place. On a uefi system,
you should not use the dtbs shipped with the kernel, or use the dtb= command
line parameter.
This may sound overly strict, but it is the only maintainable way: the
dt bindings are the contract between firmware and os, and the os
shipped dtbs essentially implement the platform side of the contract,
pushing the firmware-os boundary down to a layer that is entirely
unstandardized. Exposing the bundled dtb to the firmware is fragile,
since the firmware has no guarantees whatsoever about its contents.
(dt bindings are [supposed to be] backward compatible, but the device
trees themselves are not)
So please find another way to solve this, and move away from dtb=
Completely agree here. I do agree that it is sometimes important to override the
firmware provided data. We've got precidence for doing this with ACPI also.
However, this approach is something different entirely.
Sorry to come back late on this topic,
With ACPI, we have clear abstraction on code location and ownership.
Normally device tree are copied from kernel and build into UEFI , This is different situation here
where code location is different than ownership.
This is unrelated but u-boot ITB boot, everything is assumed external to boot-loader
Kernel and device tree are tied together.
That's the model we're trying to break away from. Having the DTB tied to
the kernel is a bad model for supporting distributions. I want to make
sure the model supports both:
1) DTB responsibilty of firmware - in which case the kernel either uses
it or ignores it; but the kernel cannot provide a new dtb that the
firmware manipulates
2) DTB responsibility of the kernel - in which case the firmware won't
modify the DTB at all.
Allowing firmware to modify a kernel-supplied DTB can result in side
effects that cannot be overridden by the dtb= kernel parameter.
This patch tries to take a kernel-provided .dtb, register it *back with
firmware* so that firmware can modify it. That is a completely different
contract, and not one that should be encouraged.
If firmware provides a DTB, then the acceptable options are either accept that
DTB as is, or completely replace it*, with a strong preference for trusting the
firmware provided DTB. If the upstream kernel breaks with the firmware-
provided DTB, then that is a kernel bug and should be fixed.**
Well, kernel must boot even with old device tree But say some driver is adding new feature
then with old device tree, this feature may not be used.
Therefore device-tree update and needed, and platform specific information needs to be
fixed before handing over to OS
I don't disagree with needing to update the DTB, but I disagree with the
approach.
If a DTB update is needed, then the right model is to update the DTB in
firmwre *before* going to the DTB boot stage. We should create a
standard interface for doing this, but it should not be part of the the
kernel UEFI stub, and it should not magically change the DTB based on
the kernel's dtb= parameters line.
The firmware DTB update could be either temporary or persistant, but it
needs to be explicitly a separate step from the kernel boot. Exactly
what this looks like can be discussed in the EBBR call.
* I'm not talking about overlays here. That's an extension scenario
which builds on what firmware provides instead of replaces it.
** Two notes on this;
1) it is okay to require a firmware dtb update to enable new
functionality, but it is not okay for the kernel to break
things that work.
For first part I agree and second is not understood my me ' but it is not okay for the kernel to break things that work'
Using kernel provided device tree and fixing by UEFI, why you think kernel will break the things.
I only mean that once a platform ships with a DTB embedded in firmware
(presuming that DTB will boot a mainline kernel), future releases of
that kernel need to continue supporting that platform DTB with the same
level of functionality. We've not been good at that in the past, but if
a kernel update breaks booting, that is a kernel bug and it needs to be
fixed.
2) We really should have a standard way in Tianocore to update
the built in DTB without reflashing the firmware image.
We need to find a better way for DTB updating , for sure above method should be avoided.
Can capsule update be used here ?
Possibly. I'd like to make sure the U-Boot UEFI implementation can use
capsule updates.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
