On Fri, 19 Jan 2024 at 21:12, Bjorn Andersson <quic_bjorande@xxxxxxxxxxx> wrote: > > On Thu, Jan 18, 2024 at 06:58:19PM -0500, Eric Chanudet wrote: > > On Thu, Jan 18, 2024 at 09:27:11PM +0530, Ninad Naik wrote: > > > New memory map layout changes (by Qualcomm firmware) have brought > > > in updates to base addresses and/or size for different memory regions > > > like cpcucp_fw, tz-stat, and also introduces new memory regions for > > > resource manager firmware. This change brings in these corresponding > > > memory map updates to the SA8775P SoC device tree. > > > > > > Signed-off-by: Ninad Naik <quic_ninanaik@xxxxxxxxxxx> > > > > With next-20240118, without this patch, running "./memtester 32G"[1] > > crashed the board quickly during the mlock: > > > > [ 42.144892] Internal error: synchronous external abort: 0000000096000010 [#1] PREEMPT SMP > > Sounds like just passing "memtest=1" on the kernel command line (with > CONFIG_MEMTEST=y) would trip this... > > > [ 42.153316] Modules linked in: r8153_ecm cdc_ether usbnet marvell dwmac_qcom_ethqos stmmac_platform r8152 rfkill stmmac crct10dif_ce qcom_spmi_temp_alarm pcs_xpcs nvmem_qcom_spmi_sdam qcom_stats i2c_qcom_geni qcom_pon spi_geni_qcom qcom_wdt socinfo phy_qcom_sgmii_eth nvmem_reboot_mode phy_qcom_qmp_usb gpucc_sa8775p phy_qcom_snps_femto_v2 phy_qcom_qmp_pcie qcom_rng drm fuse backlight ipv6 > > [ 42.188566] CPU: 3 PID: 472 Comm: memtester Not tainted 6.7.0-next-20240118-00001-g10a3c9d045cf #169 > > [ 42.197944] Hardware name: Qualcomm SA8775P Ride (DT) > > [ 42.203138] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > [ 42.210292] pc : clear_page+0x18/0x58 > > [ 42.214063] lr : clear_huge_page+0x84/0x1a0 > > [ 42.218370] sp : ffff800081ef3a30 > > [ 42.221776] x29: ffff800081ef3a30 x28: 0000000000000000 x27: 0000000000210009 > > [ 42.229108] x26: 0000000000000000 x25: fffffc6abc053380 x24: ffff000000000000 > > [ 42.236439] x23: 0000000000000000 x22: 0000000000000000 x21: 0000006a89b87f80 > > [ 42.243770] x20: 00000000000001fe x19: fffffc6a89b80000 x18: ffff800081ef3d18 > > [ 42.251101] x17: 0000000000000068 x16: 0000000000000001 x15: 00000000000001c2 > > [ 42.258431] x14: 0000000000000002 x13: fffffc6a89b90008 x12: 0000000000000001 > > [ 42.265761] x11: 0000000000440dc0 x10: 0000000000000100 x9 : ffffc570ba60c604 > > [ 42.273090] x8 : 0000000000000030 x7 : ffff554053756000 x6 : ffff800081ef39f0 > > [ 42.280420] x5 : 0000000000000130 x4 : ffffc570bd029ae0 x3 : ffff554053756000 > > [ 42.287752] x2 : 0000000000000004 x1 : 0000000000000040 x0 : ffff1aa26e1ff000 > > [ 42.295083] Call trace: > > [ 42.297607] clear_page+0x18/0x58 > > [ 42.301015] do_huge_pmd_anonymous_page+0x254/0x8f8 > > [ 42.306036] __handle_mm_fault+0x728/0x1548 > > [ 42.310338] handle_mm_fault+0x70/0x290 > > [ 42.314281] __get_user_pages+0x144/0x3c0 > > [ 42.318404] populate_vma_page_range+0x7c/0xc8 > > [ 42.322972] __mm_populate+0xc8/0x1d8 > > [ 42.326736] do_mlock+0x194/0x2d0 > > [ 42.330144] __arm64_sys_mlock+0x20/0x38 > > [ 42.334178] invoke_syscall+0x50/0x120 > > [ 42.338034] el0_svc_common.constprop.0+0xc8/0xf0 > > [ 42.342874] do_el0_svc+0x24/0x38 > > [ 42.346284] el0_svc+0x34/0xb8 > > [ 42.349425] el0t_64_sync_handler+0x120/0x130 > > [ 42.353906] el0t_64_sync+0x190/0x198 > > [ 42.357674] Code: 37200121 12000c21 d2800082 9ac12041 (d50b7420) > > [ 42.363932] ---[ end trace 0000000000000000 ]--- > > > > With next-20240118 and this patch, memtester continues through the > > test-suite. > > > > But the commit message says that this is a new memory map, not that it > fixes critical shortcomings in the existing definition. > > If that's the case the commit message needs to be updated so that we can > get this into v6.8-rc and the stable kernel (and do we really need all > those changes for that?). This kind of change sets a very bad precedent. This way old kernels become incompatible with the updated firmware. For me it looks like Linux kernel suddenly being unable to boot after the BIOS upgrade. Generally memory map updates should be disallowed after the board hits the production and the DT is published and merged. There can be other users of DT. BSD systems, U-Boot. We spend sensible efforts in making sure that DT is an ABI: newer kernel remain compatible with older DT files. We expect the same kind of efforts from device manufacturers. I think unless there is a good reason, the memory map update should be reverted on the Qualcomm side as a breaking change. If this kind of update is absolutely necessary, it might be better to define a new set of board files utilising the new memory map, marking existing DT files as legacy. -- With best wishes Dmitry
