Hi Bruno,
That didn't seem to solve the problem. The padding didn't seem to take
effect.
I reverted 0f81a0979 and 0c707d37b.
dev: size erasesize name
mtd0: 00020000 00010000 "factory-uboot"
mtd1: 00020000 00010000 "u-boot"
mtd2: 00ec0000 00010000 "firmware"
mtd3: 001a3a07 00010000 "kernel"
mtd4: 00d1c5f9 00010000 "rootfs"
mtd5: 009f0000 00010000 "rootfs_data"
mtd6: 00020000 00010000 "info"
mtd7: 00050000 00010000 "config"
mtd8: 00010000 00010000 "partition-table"
mtd9: 00010000 00010000 "art"
[ 0.414677] Creating 7 MTD partitions on "spi0.0":
[ 0.419655] 0x000000000000-0x000000020000 : "factory-uboot"
[ 0.426092] 0x000000020000-0x000000040000 : "u-boot"
[ 0.431906] 0x000000040000-0x000000f00000 : "firmware"
[ 0.439772] 2 uimage-fw partitions found on MTD device firmware
[ 0.445891] Creating 2 MTD partitions on "firmware":
[ 0.451065] 0x000000000000-0x0000001a3a07 : "kernel"
[ 0.456840] 0x0000001a3a07-0x000000ec0000 : "rootfs"
[ 0.462643] mtd: device 4 (rootfs) set to be root filesystem
[ 0.469746] 1 squashfs-split partitions found on MTD device rootfs
[ 0.476142] 0x0000004d0000-0x000000ec0000 : "rootfs_data"
[ 0.482441] 0x000000f40000-0x000000f60000 : "info"
[ 0.488033] 0x000000f60000-0x000000fb0000 : "config"
[ 0.493856] 0x000000fc0000-0x000000fd0000 : "partition-table"
[ 0.500494] 0x000000ff0000-0x000001000000 : "art"
diff --git a/target/linux/ath79/image/common-tp-link.mk
b/target/linux/ath79/image/common-tp-link.mk
index 8aa6a5a2be..89e73a85f3 100644
--- a/target/linux/ath79/image/common-tp-link.mk
+++ b/target/linux/ath79/image/common-tp-link.mk
@@ -63,7 +63,7 @@ endef
define Device/tplink-safeloader
$(Device/tplink)
- KERNEL := kernel-bin | append-dtb | lzma | tplink-v1-header -O
+ KERNEL := kernel-bin | append-dtb | lzma | pad-to $$$$(BLOCKSIZE) |
tplink-v1-header -O
IMAGE/sysupgrade.bin := append-rootfs | tplink-safeloader sysupgrade | \
append-metadata | check-size $$$$(IMAGE_SIZE)
IMAGE/factory.bin := append-rootfs | tplink-safeloader factory
Can you verify that I did this right?
Steve
On Tue, 2020-01-21 at 22:10 +0100, Bruno Pena wrote:
> Hi everyone,
>
> I was finally able to replicate the issue you are observing.
>
> The problem comes from the fact that the kernel partition on the TP-
> Link images is not padded to the write blocksize - which can be
> observed on the dmesg from Steve:
> [ 0.450987] 0x000000000000-0x0000001a39ea : "kernel"
> [ 0.456772] 0x0000001a39ea-0x000000ec0000 : "rootfs"
> The blocksize can be seen observed on the /proc/mtd and for that
> device is 0x10000:
> mtd3: 001a38de 00010000 "kernel"
> mtd4: 00d1c722 00010000 "rootfs"
> This triggers the following code on drivers/mtd/mtdpart.c that
> removes the write flag from the rootfs partition:
> tmp = part_absolute_offset(parent) + slave->offset;
> remainder = do_div(tmp, wr_alignment);
> if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
> /* Doesn't start on a boundary of major erase size */
> slave->mtd.flags |= MTD_ERASE_PARTIAL;
> if (((u32)slave->mtd.size) > parent->erasesize)
> slave->mtd.flags &= ~MTD_WRITEABLE;
> else
> slave->mtd.erasesize = slave->mtd.size;
> }
>
> tmp = part_absolute_offset(parent) + slave->offset + slave-
> >mtd.size;
> remainder = do_div(tmp, wr_alignment);
> if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
> slave->mtd.flags |= MTD_ERASE_PARTIAL;
>
> if ((u32)slave->mtd.size > parent->erasesize)
> slave->mtd.flags &= ~MTD_WRITEABLE;
> else
> slave->mtd.erasesize = slave->mtd.size;
> }
> Now we have a rootfs partition that is set to read-only, and with the
> kernel patch that forces sub-partitions to match the access mode of
> the parent, when the split runs it will force the rootfs_data
> partition to match the parent access mode (read-only).
>
> My suggestion is to change the target/linux/ath79/image/common-tp-
> link.mk so it pads the kernel partition to the blocksize (untested
> suggestion below):
> define Device/tplink-safeloader
> $(Device/tplink)
> KERNEL := kernel-bin | append-dtb | lzma | pad-to $$$$(BLOCKSIZE) |
> tplink-v1-header -O
> [...]
> Would any of you be willing to spend some time testing this change on
> your TP-Link?
>
> Thank you and best regards,
> Bruno Pena
>
> On Tue, Jan 21, 2020 at 8:15 PM Bruno Pena <brunomp...@gmail.com>
> wrote:
> > Hi Petr,
> >
> > Thank you for reverting the patches.
> >
> > I'm trying to replicate the issue but so far I haven't had any
> > luck, and just by looking at the code I'm not seeing where/what is
> > could be breaking.
> >
> > Regarding the upstream patch, that one is just an enabler (read: it
> > only extends the "mtd_add_partition" function but it does not add
> > any code to force the access mode on sub-partitions).
> > The reason for this is because this enforcement is done on the mtd
> > parser code that is OpenWrt specific and implemented via kernel
> > patches (not present on upstream).
> >
> > Best regards,
> > Bruno Pena
> >
> > On Tue, Jan 21, 2020 at 7:57 PM Petr Štetiar <yn...@true.cz> wrote:
> > > Bruno Pena <brunomp...@gmail.com> [2020-01-21 14:53:54]:
> > >
> > > Hi,
> > >
> > > > Based on the last comment from Steve (fstools patch was not
> > > reverted), I'm
> > > > not sure if that's the root cause.
> > >
> > > you need to find out, but that Daniel's remark seems legit to me,
> > > your patch
> > > likely changed the mtd device open order/flags.
> > >
> > > > The kernel patch (which when reverted makes rootfs_data
> > > writable again)
> > > > only enforces the parent mtd access mode on the sub-partitions.
> > >
> > > FYI I currently dont have time to fix that regression myself and
> > > since its
> > > likely affecting a lot of users, so I've reverted those changes.
> > > I think, that
> > > this change is useful, so I'm still willing to merge it once
> > > fixed, no
> > > worries. Having some upstream feedback beforehand would be a
> > > plus.
> > >
> > > BTW it would be fair to inform upstream about this possible issue
> > > as well, so
> > > the patch wont get merged in its current state, unless its double
> > > checked (or
> > > just write them, that you're planning v2, so the patch is removed
> > > from their
> > > Patchwork).
> > >
> > > -- ynezz
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
