Re: [PATCH RFC 1/2] dt-bindings: mtd: ubi-volume: add 'volume-is-critical' property

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

 



在 2024/9/28 22:38, Daniel Golle 写道:
On Sat, Sep 28, 2024 at 03:45:49PM +0200, Krzysztof Kozlowski wrote:
On 28/09/2024 15:09, Daniel Golle wrote:
On Sat, Sep 28, 2024 at 03:02:47PM +0200, Krzysztof Kozlowski wrote:
On 28/09/2024 14:47, Daniel Golle wrote:
Add the 'volume-is-critical' boolean property which marks a UBI volume
as critical for the device to boot. If set it prevents the user from
all kinds of write access to the volume as well as from renaming it or
detaching the UBI device it is located on.

Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
---
  .../devicetree/bindings/mtd/partitions/ubi-volume.yaml   | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml b/Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml
index 19736b26056b..2bd751bb7f9e 100644
--- a/Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml
@@ -29,6 +29,15 @@ properties:
      description:
        This container may reference an NVMEM layout parser.
+ volume-is-critical:
+    description: This parameter, if present, indicates that the UBI volume
+      contains early-boot firmware images or data which should not be clobbered.
+      If set, it prevents the user from renaming the volume, writing to it or
+      making any changes affecting it, as well as detaching the UBI device it is
+      located on, so direct access to the underlying MTD device is prevented as
+      well.
+    type: boolean

UBI volumes are mapping to partitions 1-to-1, right? So rather I would
propose to use partition.yaml - we already have read-only there with
very similar description.

No, that's not the case.

An MTD partition can be used as UBI device. A UBI device (and hence MTD
partition) can host *several* UBI volumes.

Marking the MTD partition as 'read-only' won't work, as UBI needs
read-write access to perform bad block relocation, scrubbing, ...

OK, so not partition but read-only volume.

+1



Also, typically not all UBI volumes on a UBI device are
read-only/critical but only a subset of them.

But you are right that the description is inspired by the description
of the 'read-only' property in partition.yaml ;)

I initially thought to also name the property 'read-only', just like
for MTD partitions. However, as the desired effect goes beyond
preventing write access to the volume itself, I thought it'd be
better to use a new name.

Yeah, maybe... critical indeed covers multiple cases but is also
subjective. For some bootloader is critical, for other bootloader still
might be fully A/B updateable thus could be modifiable. For others, they
want to use fw_setenv from user-space so not critical at all.

The case I want to cover here is the bootloader itself being stored
inside a UBI volume. MediaTek's fork of ARM TrustedFirmware-A bl2 comes
with support for UBI and loads BL3 (which is TF-A BL31 and U-Boot, and
maybe OP-TEE as well) from a static UBI volume. Removing, renaming or
altering that volume results in the device not being able to boot any
more and requiring a complicated intervention (at attaching debugging
UART and using low-level recovery tool) in order to recover.

Who removes/renames the 'critical' volume? I suggest to fix it in the upper layer(not in kernel). After looking through the patch 2, it seems a hack solution.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux