Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18/04/2018 15:26, Udit Kumar wrote:
Hi Grant
> I am not also in favor to do DTB update like this patch , but DTB
update is needed feature !!

Oh! Yes. I 100% agree. I am not arguing against DTB updates..

Ideally DTB defined once should be done forever but unfortunately we are not in ideal world.

Also agree. This isn't about whether or not DTB updates are required. Of course they are! However, there needs to be a clear division of responsibilities. There are basically two scenarios we need to support:

1) Firmware provides DTB
In this case the firmware has a copy of the DTB, which I may modify at boot time and then passes to the kernel. That DTB might be replaced with a different version, or it might have overlays applied, but regardless the firmware is entirely responsible for the DTB. We can have tooling in firmware to make it easy to manage the DTB, including loading a newer DTB, but that step must be distinct from the kernel's OS loader stub.

2) Kernel provides/overrides DTB
The kernel always has the option of loading it's own DTB, either because firmware doesn't provide one, or it needs a newer DTB. This is similar to CONFIG_ACPI_CUSTOM_DSDT in ACPI land. However, if the kernel is overriding the DTB, then firmware has no part of it, and it should not be allowed to modify the DTB at ExitBootServices() time.

Similarly, Grub might decide to provide the override DTB instead of the kernel, but that is effectively the same scenario. If the OS boot process overrides the platform description, then the firmware no longer has any part of it.

The second case is by far the most common scenario today, but it is unfriendly to distributions, which is why it is important to change the behaviour. Here is how I think it should be designed:

- Firmware should have a built-in DTB that will boot a mainline kernel
- Firmware should provide mechanism for installing updated DTB
  - Reflashing entire firmware just for DTB updates is discouraged
  - DTB update could be either temporary or persistant
    - If persistent, DTB should be stored in firmware flash region
      - ie. not in system partitition.
  - Firmware can require DTB update to be signed
  - Built-in DTB remains as failsafe that can always recovered to
  - Firmware remains 'responsible' for DTB
    - Firmware allowed to modify updated DTB
  - Firmware update mechanism logically separate from OS boot
    - Though it could be scripted by Grub
  - Firmware 'vendor' responsible for official updates to DTB
- Question: should there be a standard capsule update mechanism for DTB updates?
- By default, OS distributions should use firmare provided DTB
  - Require user intervention DTB override is needed

However, if the DTB update mechanism is standard, and if we have a common database of DTBs, it would be possible for the distribution to automatically manage DTB updates.

More inline please

Regards
Udit

-----Original Message-----
From: Grant Likely [mailto:grant.likely@xxxxxxx]
Sent: Wednesday, April 18, 2018 5:03 PM
To: Udit Kumar <udit.kumar@xxxxxxx>; 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>; nd@xxxxxxx; Rod Dorris <rod.dorris@xxxxxxx>
Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration
table

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

Let's start using ACPI then 😊 , With DTB we are going to hit similar problems

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.
DTB are defining hardware in bit and byte basics,  this is difficult to have full responsibility
of DTB by kernel e.g. platform supports psci or not this is known to firmware as firmware
created needed infrastructure for the same.
There are many other examples, which will say DTB shouldn't be owned by kernel.
But having under firmware,  we need to see how to update DTB.

Allowing firmware to modify a kernel-supplied DTB can result in side effects that
cannot be overridden by the dtb= kernel parameter.

I guess, no one here prefer to update dtb in this way.
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.

Ideally we shouldn’t practically we need to
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.

Agreed,  we need to define, that with given OS update some notification
if DTB updates are needed or not
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.

Sure, hope in EBBR we can find some standard way



* 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.

Say platform is shipped with kernel 4.4 and associated DTBs.
Now user wants to upgrade to kernel 4.9 , for sure platform should boot at same level of functions as of 4.4.
In new kernel, added DT bindings are agreed with Linux community, But old DTB enhanced feature cannot be used,
Then what is point to update to new OS

e.g
iommu-map is added for few devices (https://github.com/qoriq-open-source/linux/blob/linux-4.9/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi)


     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.

Ok

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux