[PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Ninad Naik]

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@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 103 +++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index a7eaca33d326..20b16fb5f537 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -356,13 +356,18 @@ uefi_log: uefi-log@908b0000 {
 			no-map;
 		};
 
+		ddr_training_checksum: ddr_training_checksum@908c0000 {
+			reg = <0x0 0x908c0000 0x0 0x1000>;
+			no-map;
+		};
+
 		reserved_mem: reserved@908f0000 {
-			reg = <0x0 0x908f0000 0x0 0xf000>;
+			reg = <0x0 0x908f0000 0x0 0xe000>;
 			no-map;
 		};
 
-		secdata_apss_mem: secdata-apss@908ff000 {
-			reg = <0x0 0x908ff000 0x0 0x1000>;
+		secdata_apss_mem: secdata-apss@908fe000 {
+			reg = <0x0 0x908fe000 0x0 0x2000>;
 			no-map;
 		};
 
@@ -373,8 +378,43 @@ smem_mem: smem@90900000 {
 			hwlocks = <&tcsr_mutex 3>;
 		};
 
-		cpucp_fw_mem: cpucp-fw@90b00000 {
-			reg = <0x0 0x90b00000 0x0 0x100000>;
+		tz_sail_mailbox_mem: tz-sail-mailbox@90c00000 {
+			reg = <0x0 0x90c00000 0x0 0x100000>;
+			no-map;
+		};
+
+		sail_mailbox_mem: sail-ss@90d00000 {
+			reg = <0x0 0x90d00000 0x0 0x100000>;
+			no-map;
+		};
+
+		sail_ota_mem: sail-ss@90e00000 {
+			reg = <0x0 0x90e00000 0x0 0x300000>;
+			no-map;
+		};
+
+		aoss_backup_mem: aoss-backup@91b00000 {
+			reg = <0x0 0x91b00000 0x0 0x40000>;
+			no-map;
+		};
+
+		cpucp_backup_mem: cpucp-backup@91b40000 {
+			reg = <0x0 0x91b40000 0x0 0x40000>;
+			no-map;
+		};
+
+		tz_config_backup_mem: tz-config-backup@91b80000 {
+			reg = <0x0 0x91b80000 0x0 0x10000>;
+			no-map;
+		};
+
+		ddr_training_data_mem: ddr-training-data@91b90000 {
+			reg = <0x0 0x91b90000 0x0 0x10000>;
+			no-map;
+		};
+
+		cdt_data_backup_mem: cdt-data-backup@91ba0000 {
+			reg = <0x0 0x91ba0000 0x0 0x1000>;
 			no-map;
 		};
 
@@ -433,14 +473,44 @@ pil_video_mem: pil-video@9fc00000 {
 			no-map;
 		};
 
+		audio_mdf_mem: audio_mdf_region@ae000000 {
+			reg = <0x0 0xae000000 0x0 0x1000000>;
+			no-map;
+		};
+
+		firmware_mem: firmware-region@b0000000 {
+			reg = <0x0 0xb0000000 0x0 0x800000>;
+			no-map;
+		};
+
 		hyptz_reserved_mem: hyptz-reserved@beb00000 {
 			reg = <0x0 0xbeb00000 0x0 0x11500000>;
 			no-map;
 		};
 
-		tz_stat_mem: tz-stat@d0000000 {
-			reg = <0x0 0xd0000000 0x0 0x100000>;
+		scmi_mem: scmi_region@d0000000 {
+			no-map;
+			reg = <0x0 0xd0000000 0x0 0x40000>;
+		};
+
+		firmware_logs_mem: firmware-logs@d0040000 {
+			no-map;
+			reg = <0x0 0xd0040000 0x0 0x10000>;
+		};
+
+		firmware_audio_mem: firmware-audio@d0050000 {
+			no-map;
+			reg = <0x0 0xd0050000 0x0 0x4000>;
+		};
+
+		firmware_reserved_mem: firmware-reserved@d0054000 {
 			no-map;
+			reg = <0x0 0xd0054000 0x0 0x9c000>;
+		};
+
+		firmware_quantum_test_mem: firmware-quantum-test@d00f0000 {
+			no-map;
+			reg = <0x0 0xd00f0000 0x0 0x10000>;
 		};
 
 		tags_mem: tags@d0100000 {
@@ -453,8 +523,23 @@ qtee_mem: qtee@d1300000 {
 			no-map;
 		};
 
-		trusted_apps_mem: trusted-apps@d1800000 {
-			reg = <0x0 0xd1800000 0x0 0x3900000>;
+		deepsleep_backup_mem: deepsleep_backup@d1800000 {
+			reg = <0x0 0xd1800000 0x0 0x100000>;
+			no-map;
+		};
+
+		trusted_apps_mem: trusted-apps@d1900000 {
+			reg = <0x0 0xd1900000 0x0 0x3800000>;
+			no-map;
+		};
+
+		tz_stat_mem: tz-stat@db100000 {
+			reg = <0x0 0xdb100000 0x0 0x100000>;
+			no-map;
+		};
+
+		cpucp_fw_mem: cpucp-fw@db200000 {
+			reg = <0x0 0xdb200000 0x0 0x100000>;
 			no-map;
 		};
 	};
-- 
2.42.0

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Brian Masney]

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@quicinc.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>

Krzysztof: It'd be nice if you could submit this patch for inclusion
to the stable trees since the system can crash without the updated
memory regions.

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Krzysztof Kozlowski]

On 18/01/2024 21:28, Brian Masney 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@quicinc.com>
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> 
> Krzysztof: It'd be nice if you could submit this patch for inclusion
> to the stable trees since the system can crash without the updated
> memory regions.

???

It would be nice if you could submit what you want to submit...

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Eric Chanudet]

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@quicinc.com>

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

Tested-by: Eric Chanudet <echanude@redhat.com> # sa8775p-ride

[1] https://github.com/jnavila/memtester

> ---
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 103 +++++++++++++++++++++++---
>  1 file changed, 94 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index a7eaca33d326..20b16fb5f537 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -356,13 +356,18 @@ uefi_log: uefi-log@908b0000 {
>  			no-map;
>  		};
>  
> +		ddr_training_checksum: ddr_training_checksum@908c0000 {
> +			reg = <0x0 0x908c0000 0x0 0x1000>;
> +			no-map;
> +		};
> +
>  		reserved_mem: reserved@908f0000 {
> -			reg = <0x0 0x908f0000 0x0 0xf000>;
> +			reg = <0x0 0x908f0000 0x0 0xe000>;
>  			no-map;
>  		};
>  
> -		secdata_apss_mem: secdata-apss@908ff000 {
> -			reg = <0x0 0x908ff000 0x0 0x1000>;
> +		secdata_apss_mem: secdata-apss@908fe000 {
> +			reg = <0x0 0x908fe000 0x0 0x2000>;
>  			no-map;
>  		};
>  
> @@ -373,8 +378,43 @@ smem_mem: smem@90900000 {
>  			hwlocks = <&tcsr_mutex 3>;
>  		};
>  
> -		cpucp_fw_mem: cpucp-fw@90b00000 {
> -			reg = <0x0 0x90b00000 0x0 0x100000>;
> +		tz_sail_mailbox_mem: tz-sail-mailbox@90c00000 {
> +			reg = <0x0 0x90c00000 0x0 0x100000>;
> +			no-map;
> +		};
> +
> +		sail_mailbox_mem: sail-ss@90d00000 {
> +			reg = <0x0 0x90d00000 0x0 0x100000>;
> +			no-map;
> +		};
> +
> +		sail_ota_mem: sail-ss@90e00000 {
> +			reg = <0x0 0x90e00000 0x0 0x300000>;
> +			no-map;
> +		};
> +
> +		aoss_backup_mem: aoss-backup@91b00000 {
> +			reg = <0x0 0x91b00000 0x0 0x40000>;
> +			no-map;
> +		};
> +
> +		cpucp_backup_mem: cpucp-backup@91b40000 {
> +			reg = <0x0 0x91b40000 0x0 0x40000>;
> +			no-map;
> +		};
> +
> +		tz_config_backup_mem: tz-config-backup@91b80000 {
> +			reg = <0x0 0x91b80000 0x0 0x10000>;
> +			no-map;
> +		};
> +
> +		ddr_training_data_mem: ddr-training-data@91b90000 {
> +			reg = <0x0 0x91b90000 0x0 0x10000>;
> +			no-map;
> +		};
> +
> +		cdt_data_backup_mem: cdt-data-backup@91ba0000 {
> +			reg = <0x0 0x91ba0000 0x0 0x1000>;
>  			no-map;
>  		};
>  
> @@ -433,14 +473,44 @@ pil_video_mem: pil-video@9fc00000 {
>  			no-map;
>  		};
>  
> +		audio_mdf_mem: audio_mdf_region@ae000000 {
> +			reg = <0x0 0xae000000 0x0 0x1000000>;
> +			no-map;
> +		};
> +
> +		firmware_mem: firmware-region@b0000000 {
> +			reg = <0x0 0xb0000000 0x0 0x800000>;
> +			no-map;
> +		};
> +
>  		hyptz_reserved_mem: hyptz-reserved@beb00000 {
>  			reg = <0x0 0xbeb00000 0x0 0x11500000>;
>  			no-map;
>  		};
>  
> -		tz_stat_mem: tz-stat@d0000000 {
> -			reg = <0x0 0xd0000000 0x0 0x100000>;
> +		scmi_mem: scmi_region@d0000000 {
> +			no-map;
> +			reg = <0x0 0xd0000000 0x0 0x40000>;
> +		};
> +
> +		firmware_logs_mem: firmware-logs@d0040000 {
> +			no-map;
> +			reg = <0x0 0xd0040000 0x0 0x10000>;
> +		};
> +
> +		firmware_audio_mem: firmware-audio@d0050000 {
> +			no-map;
> +			reg = <0x0 0xd0050000 0x0 0x4000>;
> +		};
> +
> +		firmware_reserved_mem: firmware-reserved@d0054000 {
>  			no-map;
> +			reg = <0x0 0xd0054000 0x0 0x9c000>;
> +		};
> +
> +		firmware_quantum_test_mem: firmware-quantum-test@d00f0000 {
> +			no-map;
> +			reg = <0x0 0xd00f0000 0x0 0x10000>;
>  		};
>  
>  		tags_mem: tags@d0100000 {
> @@ -453,8 +523,23 @@ qtee_mem: qtee@d1300000 {
>  			no-map;
>  		};
>  
> -		trusted_apps_mem: trusted-apps@d1800000 {
> -			reg = <0x0 0xd1800000 0x0 0x3900000>;
> +		deepsleep_backup_mem: deepsleep_backup@d1800000 {
> +			reg = <0x0 0xd1800000 0x0 0x100000>;
> +			no-map;
> +		};
> +
> +		trusted_apps_mem: trusted-apps@d1900000 {
> +			reg = <0x0 0xd1900000 0x0 0x3800000>;
> +			no-map;
> +		};
> +
> +		tz_stat_mem: tz-stat@db100000 {
> +			reg = <0x0 0xdb100000 0x0 0x100000>;
> +			no-map;
> +		};
> +
> +		cpucp_fw_mem: cpucp-fw@db200000 {
> +			reg = <0x0 0xdb200000 0x0 0x100000>;
>  			no-map;
>  		};
>  	};
> -- 
> 2.42.0
> 
> 

-- 
Eric Chanudet

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Bjorn Andersson]

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@quicinc.com>
> 
> 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?).

Regards,
Bjorn

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Dmitry Baryshkov]

On Fri, 19 Jan 2024 at 21:12, Bjorn Andersson <quic_bjorande@quicinc.com> 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@quicinc.com>
> >
> > 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

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Brian Masney]

Hi Dmitry,

On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
> 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.

This is on a development board that's not in production yet, so
personally I think this change is fine. It's in all of our best
interests to have SoC vendors push their code upstream early, even if
it means that later on we need to make memory map changes like this.

Once this is in production, then I agree with you that changes like
this should be avoided if possible.

Brian

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Bjorn Andersson]

On Mon, Jan 22, 2024 at 08:45:51AM -0500, Brian Masney wrote:
> Hi Dmitry,
> 
> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
> > 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.
> 
> This is on a development board that's not in production yet, so
> personally I think this change is fine. It's in all of our best
> interests to have SoC vendors push their code upstream early, even if
> it means that later on we need to make memory map changes like this.
> 

The problem I have with the patch is that I don't know which precedence
it sets, because the commit message indicates that we have a new
firmware version, while Eric's report lacks this information.

As long as everyone with access to the hardware agrees that breaking
backwards compatibility is the right thing to do, I'm not against it.

But then again, if the support is under active development, why would
anyone run a stable@ kernel on this thing?
Or are you asking for it to be included in v6.8-rc, so that you guys
have a "stable" tree to do further development (with this patch) on?

> Once this is in production, then I agree with you that changes like
> this should be avoided if possible.
> 

Agreed

Regards,
Bjorn

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Brian Masney]

On Mon, Jan 22, 2024 at 3:02 PM Bjorn Andersson
<quic_bjorande@quicinc.com> wrote:
> The problem I have with the patch is that I don't know which precedence
> it sets, because the commit message indicates that we have a new
> firmware version, while Eric's report lacks this information.
>
> As long as everyone with access to the hardware agrees that breaking
> backwards compatibility is the right thing to do, I'm not against it.
>
> But then again, if the support is under active development, why would
> anyone run a stable@ kernel on this thing?

Good point about the stable@ tag. This can go in the normal route then.

Brian

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Trilok Soni]

On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
> On Mon, Jan 22, 2024 at 08:45:51AM -0500, Brian Masney wrote:
>> Hi Dmitry,
>>
>> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
>>> 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.
>>
>> This is on a development board that's not in production yet, so
>> personally I think this change is fine. It's in all of our best
>> interests to have SoC vendors push their code upstream early, even if
>> it means that later on we need to make memory map changes like this.
>>
> 
> The problem I have with the patch is that I don't know which precedence
> it sets, because the commit message indicates that we have a new
> firmware version, while Eric's report lacks this information.
> 
> As long as everyone with access to the hardware agrees that breaking
> backwards compatibility is the right thing to do, I'm not against it.
> 
> But then again, if the support is under active development, why would
> anyone run a stable@ kernel on this thing?
> Or are you asking for it to be included in v6.8-rc, so that you guys
> have a "stable" tree to do further development (with this patch) on?

I agree with what Bjorn is mentioning here. Why we are freezing the kernel version
here/commit of it here. Memory map can change during the active development
and this target is under active development. 

New board file approach doesn't work - since how do you select the new 
board file? Both old and new board file will still point to the same
platform type and version.

We also saw recently that IOT SOCs which are similar to in some
sense Mobile SOCs are having the different map. The same almost
same SOCs used in the different product segments like Chrome
and Mobile and IOT can have different memory map as well. The good
part there was that they had different soc-id and it will be easier
to differentiate them. 

As Brian M mentioned earlier, we want soc vendors to submit the support
for their SOCs and platforms on top it as early as possible and it means
such memory map changes will continue. Even memory map changes
continue even few months after the commercial s/w release in certain cases
due to critical bugs were found in some usecases which warrants the changes. 


-- 
---Trilok Soni

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Dmitry Baryshkov]

On Tue, 23 Jan 2024 at 04:58, Trilok Soni <quic_tsoni@quicinc.com> wrote:
>
> On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
> > On Mon, Jan 22, 2024 at 08:45:51AM -0500, Brian Masney wrote:
> >> Hi Dmitry,
> >>
> >> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
> >>> 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.
> >>
> >> This is on a development board that's not in production yet, so
> >> personally I think this change is fine. It's in all of our best
> >> interests to have SoC vendors push their code upstream early, even if
> >> it means that later on we need to make memory map changes like this.
> >>
> >
> > The problem I have with the patch is that I don't know which precedence
> > it sets, because the commit message indicates that we have a new
> > firmware version, while Eric's report lacks this information.
> >
> > As long as everyone with access to the hardware agrees that breaking
> > backwards compatibility is the right thing to do, I'm not against it.
> >
> > But then again, if the support is under active development, why would
> > anyone run a stable@ kernel on this thing?
> > Or are you asking for it to be included in v6.8-rc, so that you guys
> > have a "stable" tree to do further development (with this patch) on?
>
> I agree with what Bjorn is mentioning here. Why we are freezing the kernel version
> here/commit of it here. Memory map can change during the active development
> and this target is under active development.
>
> New board file approach doesn't work - since how do you select the new
> board file? Both old and new board file will still point to the same
> platform type and version.

The developer knows which firmware version is used. So the user can
select the correct DT file manually. There is no need to pack all
files together.
Also it might be nice to bump the platform version when performing
such drastic changes.

>
> We also saw recently that IOT SOCs which are similar to in some
> sense Mobile SOCs are having the different map. The same almost
> same SOCs used in the different product segments like Chrome
> and Mobile and IOT can have different memory map as well. The good
> part there was that they had different soc-id and it will be easier
> to differentiate them.

Having device-specific memory maps is also fine.

>
> As Brian M mentioned earlier, we want soc vendors to submit the support
> for their SOCs and platforms on top it as early as possible and it means
> such memory map changes will continue. Even memory map changes
> continue even few months after the commercial s/w release in certain cases
> due to critical bugs were found in some usecases which warrants the changes.

So, can one handle such changes? Are we going to publish a list of
kernels to be used with the corresponding firmware images? Then what
if the developer wants to update just the kernel? Just to get this or
that non-platform-related feature. Or vice versa, what if the user is
stuck with an older kernel because some driver gets broken in the main
branch (which unfortunately happens sometimes)  Or what if the memory
map patch gets backported via the AUTOSEL process?
Unlike the Qualcomm binary distributions, the firmware and the kernel
version are no longer connected.

That's why I keep on saying that memory map is an ABI. If it gets
changed, it is a completely new, incompatible platform.

-- 
With best wishes
Dmitry

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Bjorn Andersson]

On Tue, Jan 23, 2024 at 08:23:37AM +0200, Dmitry Baryshkov wrote:
> On Tue, 23 Jan 2024 at 04:58, Trilok Soni <quic_tsoni@quicinc.com> wrote:
> > On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
[..]
> > As Brian M mentioned earlier, we want soc vendors to submit the support
> > for their SOCs and platforms on top it as early as possible and it means
> > such memory map changes will continue. Even memory map changes
> > continue even few months after the commercial s/w release in certain cases
> > due to critical bugs were found in some usecases which warrants the changes.
> 
> So, can one handle such changes? Are we going to publish a list of
> kernels to be used with the corresponding firmware images? Then what
> if the developer wants to update just the kernel? Just to get this or
> that non-platform-related feature. Or vice versa, what if the user is
> stuck with an older kernel because some driver gets broken in the main
> branch (which unfortunately happens sometimes)  Or what if the memory
> map patch gets backported via the AUTOSEL process?
> Unlike the Qualcomm binary distributions, the firmware and the kernel
> version are no longer connected.
> 
> That's why I keep on saying that memory map is an ABI. If it gets
> changed, it is a completely new, incompatible platform.

This is only a problem because we think the DeviceTree is a part of the
kernel. If we actually tied the DeviceTree to the firmware - as it was
intended - different firmware versions could come with different memory
map.

The one exception would be any remoteproc/pil firmware that is not
relocatable, as these are distributed together with the OS (in some
form) and not the boot/security/etc firmware.

Regards,
Bjorn

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Dmitry Baryshkov]

On Tue, 23 Jan 2024 at 18:02, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On Tue, Jan 23, 2024 at 08:23:37AM +0200, Dmitry Baryshkov wrote:
> > On Tue, 23 Jan 2024 at 04:58, Trilok Soni <quic_tsoni@quicinc.com> wrote:
> > > On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
> [..]
> > > As Brian M mentioned earlier, we want soc vendors to submit the support
> > > for their SOCs and platforms on top it as early as possible and it means
> > > such memory map changes will continue. Even memory map changes
> > > continue even few months after the commercial s/w release in certain cases
> > > due to critical bugs were found in some usecases which warrants the changes.
> >
> > So, can one handle such changes? Are we going to publish a list of
> > kernels to be used with the corresponding firmware images? Then what
> > if the developer wants to update just the kernel? Just to get this or
> > that non-platform-related feature. Or vice versa, what if the user is
> > stuck with an older kernel because some driver gets broken in the main
> > branch (which unfortunately happens sometimes)  Or what if the memory
> > map patch gets backported via the AUTOSEL process?
> > Unlike the Qualcomm binary distributions, the firmware and the kernel
> > version are no longer connected.
> >
> > That's why I keep on saying that memory map is an ABI. If it gets
> > changed, it is a completely new, incompatible platform.
>
> This is only a problem because we think the DeviceTree is a part of the
> kernel. If we actually tied the DeviceTree to the firmware - as it was
> intended - different firmware versions could come with different memory
> map.

Yes, up to some point. Because then DT gets incorporated into U-Boot...

> The one exception would be any remoteproc/pil firmware that is not
> relocatable, as these are distributed together with the OS (in some
> form) and not the boot/security/etc firmware.

-- 
With best wishes
Dmitry

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Trilok Soni]

On 1/22/2024 10:23 PM, Dmitry Baryshkov wrote:
> On Tue, 23 Jan 2024 at 04:58, Trilok Soni <quic_tsoni@quicinc.com> wrote:
>>
>> On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
>>> On Mon, Jan 22, 2024 at 08:45:51AM -0500, Brian Masney wrote:
>>>> Hi Dmitry,
>>>>
>>>> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
>>>>> 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.
>>>>
>>>> This is on a development board that's not in production yet, so
>>>> personally I think this change is fine. It's in all of our best
>>>> interests to have SoC vendors push their code upstream early, even if
>>>> it means that later on we need to make memory map changes like this.
>>>>
>>>
>>> The problem I have with the patch is that I don't know which precedence
>>> it sets, because the commit message indicates that we have a new
>>> firmware version, while Eric's report lacks this information.
>>>
>>> As long as everyone with access to the hardware agrees that breaking
>>> backwards compatibility is the right thing to do, I'm not against it.
>>>
>>> But then again, if the support is under active development, why would
>>> anyone run a stable@ kernel on this thing?
>>> Or are you asking for it to be included in v6.8-rc, so that you guys
>>> have a "stable" tree to do further development (with this patch) on?
>>
>> I agree with what Bjorn is mentioning here. Why we are freezing the kernel version
>> here/commit of it here. Memory map can change during the active development
>> and this target is under active development.
>>
>> New board file approach doesn't work - since how do you select the new
>> board file? Both old and new board file will still point to the same
>> platform type and version.
> 
> The developer knows which firmware version is used. So the user can
> select the correct DT file manually. There is no need to pack all
> files together.
> Also it might be nice to bump the platform version when performing
> such drastic changes.

Well - we can do that and pick the DT we want, but please understand
that as a community we don't allow such memory map changes it will
be going to stop SOC vendors from contributing early for the ARM platforms,
which is a regression in my view. 

This will also make SOC vendors to move the memory maps out of the
DT to some bootloader code which we won't be able to access due
to the location on their websites and versions. We may not want to
be in such situation as well. 

> 
>>
>> We also saw recently that IOT SOCs which are similar to in some
>> sense Mobile SOCs are having the different map. The same almost
>> same SOCs used in the different product segments like Chrome
>> and Mobile and IOT can have different memory map as well. The good
>> part there was that they had different soc-id and it will be easier
>> to differentiate them.
> 
> Having device-specific memory maps is also fine.
> 
>>
>> As Brian M mentioned earlier, we want soc vendors to submit the support
>> for their SOCs and platforms on top it as early as possible and it means
>> such memory map changes will continue. Even memory map changes
>> continue even few months after the commercial s/w release in certain cases
>> due to critical bugs were found in some usecases which warrants the changes.
> 
> So, can one handle such changes? Are we going to publish a list of
> kernels to be used with the corresponding firmware images? Then what
> if the developer wants to update just the kernel? Just to get this or
> that non-platform-related feature. Or vice versa, what if the user is
> stuck with an older kernel because some driver gets broken in the main
> branch (which unfortunately happens sometimes)  Or what if the memory
> map patch gets backported via the AUTOSEL process?
> Unlike the Qualcomm binary distributions, the firmware and the kernel
> version are no longer connected.
> 
> That's why I keep on saying that memory map is an ABI. If it gets
> changed, it is a completely new, incompatible platform.

I don't think that in-development platform should be treated it such. Like
other ABIs we can mark the board as experimental before they get frozen.

I see ABIs in the kernel which starts w/ the EXPERIMENTAL before they
get stable state, so why not these platforms? I don't want to discourage
SOC vendor developers from submitting their platforms. 

Let's just mark these boards as EXPERIMENTAL in the Kconfig or anywhere
else we decide. 

-- 
---Trilok Soni

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Konrad Dybcio]

On 1/25/24 07:05, Trilok Soni wrote:
> On 1/22/2024 10:23 PM, Dmitry Baryshkov wrote:
>> On Tue, 23 Jan 2024 at 04:58, Trilok Soni <quic_tsoni@quicinc.com> wrote:
>>>
>>> On 1/22/2024 12:02 PM, Bjorn Andersson wrote:
>>>> On Mon, Jan 22, 2024 at 08:45:51AM -0500, Brian Masney wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
>>>>>> 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.
>>>>>
>>>>> This is on a development board that's not in production yet, so
>>>>> personally I think this change is fine. It's in all of our best
>>>>> interests to have SoC vendors push their code upstream early, even if
>>>>> it means that later on we need to make memory map changes like this.
>>>>>
>>>>
>>>> The problem I have with the patch is that I don't know which precedence
>>>> it sets, because the commit message indicates that we have a new
>>>> firmware version, while Eric's report lacks this information.
>>>>
>>>> As long as everyone with access to the hardware agrees that breaking
>>>> backwards compatibility is the right thing to do, I'm not against it.
>>>>
>>>> But then again, if the support is under active development, why would
>>>> anyone run a stable@ kernel on this thing?
>>>> Or are you asking for it to be included in v6.8-rc, so that you guys
>>>> have a "stable" tree to do further development (with this patch) on?
>>>
>>> I agree with what Bjorn is mentioning here. Why we are freezing the kernel version
>>> here/commit of it here. Memory map can change during the active development
>>> and this target is under active development.
>>>
>>> New board file approach doesn't work - since how do you select the new
>>> board file? Both old and new board file will still point to the same
>>> platform type and version.
>>
>> The developer knows which firmware version is used. So the user can
>> select the correct DT file manually. There is no need to pack all
>> files together.
>> Also it might be nice to bump the platform version when performing
>> such drastic changes.
> 
> Well - we can do that and pick the DT we want, but please understand
> that as a community we don't allow such memory map changes it will
> be going to stop SOC vendors from contributing early for the ARM platforms,
> which is a regression in my view.
> 
> This will also make SOC vendors to move the memory maps out of the
> DT to some bootloader code which we won't be able to access due
> to the location on their websites and versions. We may not want to
> be in such situation as well.
> 
>>
>>>
>>> We also saw recently that IOT SOCs which are similar to in some
>>> sense Mobile SOCs are having the different map. The same almost
>>> same SOCs used in the different product segments like Chrome
>>> and Mobile and IOT can have different memory map as well. The good
>>> part there was that they had different soc-id and it will be easier
>>> to differentiate them.
>>
>> Having device-specific memory maps is also fine.
>>
>>>
>>> As Brian M mentioned earlier, we want soc vendors to submit the support
>>> for their SOCs and platforms on top it as early as possible and it means
>>> such memory map changes will continue. Even memory map changes
>>> continue even few months after the commercial s/w release in certain cases
>>> due to critical bugs were found in some usecases which warrants the changes.
>>
>> So, can one handle such changes? Are we going to publish a list of
>> kernels to be used with the corresponding firmware images? Then what
>> if the developer wants to update just the kernel? Just to get this or
>> that non-platform-related feature. Or vice versa, what if the user is
>> stuck with an older kernel because some driver gets broken in the main
>> branch (which unfortunately happens sometimes)  Or what if the memory
>> map patch gets backported via the AUTOSEL process?
>> Unlike the Qualcomm binary distributions, the firmware and the kernel
>> version are no longer connected.
>>
>> That's why I keep on saying that memory map is an ABI. If it gets
>> changed, it is a completely new, incompatible platform.
> 
> I don't think that in-development platform should be treated it such. Like
> other ABIs we can mark the board as experimental before they get frozen.
> 
> I see ABIs in the kernel which starts w/ the EXPERIMENTAL before they
> get stable state, so why not these platforms? I don't want to discourage
> SOC vendor developers from submitting their platforms.

Trilok, Dmitry,

I believe the best solution for in-dev platforms would be to introduce
a concept of "staging" dt-bindings and devicetrees, so that both
developers and users will expect that things may break, as they are still
being figured out. That way, we can have the cake and eat it too.

Konrad

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Dmitry Baryshkov]

On Mon, 22 Jan 2024 at 15:45, Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Dmitry,
>
> On Fri, Jan 19, 2024 at 10:35:43PM +0200, Dmitry Baryshkov wrote:
> > 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.
>
> This is on a development board that's not in production yet, so
> personally I think this change is fine. It's in all of our best
> interests to have SoC vendors push their code upstream early, even if
> it means that later on we need to make memory map changes like this.

Then this should be clearly a part of the commit message. And I anyway
would suggest having two board files, even if it's just for a few
releases. Otherwise you have a tight lock between kernel and
bootloader versions. In case of any regression it becomes next to
impossible to debug if it is caused by the kernel or by the firmware
itself.

> Once this is in production, then I agree with you that changes like
> this should be avoided if possible.

Please strike through the 'if possible' part. It must be avoided at all costs.

-- 
With best wishes
Dmitry

Re: Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Eric Chanudet]

On Fri, Jan 19, 2024 at 11:11:44AM -0800, Bjorn Andersson 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@quicinc.com>
> > 
> > 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...

Actually, this does a better job, not having to deal with the
oom-killer, and picks up the following missing region with this patch:

[    0.000000] early_memtest: # of tests: 1
[    0.000000]   0x0000000090880000 - 0x00000000908b0000 pattern 0000000000000000
[    0.000000]   0x00000000908c1000 - 0x00000000908f0000 pattern 0000000000000000
[    0.000000]   0x0000000090b00000 - 0x0000000090c00000 pattern 0000000000000000
[    0.000000] Internal error: synchronous external abort: 0000000096000010 [#1] PREEMPT SMP

> > > @@ -373,8 +378,43 @@ smem_mem: smem@90900000 {
> > >  			hwlocks = <&tcsr_mutex 3>;
> > >  		};
> > >  
> > > -		cpucp_fw_mem: cpucp-fw@90b00000 {
> > > -			reg = <0x0 0x90b00000 0x0 0x100000>;
> > > +		tz_sail_mailbox_mem: tz-sail-mailbox@90c00000 {
> > > +			reg = <0x0 0x90c00000 0x0 0x100000>;
> > > +			no-map;
> > > +		};

Re-introducing 0x90b00000-0x90bfffff as no-map passes the kernel
memtest=1 and boots with the firmware I have.
My previous test was incomplete. Please disregard the Tested-by.

-- 
Eric Chanudet

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Krzysztof Kozlowski]

On 18/01/2024 16:57, 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@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 103 +++++++++++++++++++++++---
>  1 file changed, 94 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index a7eaca33d326..20b16fb5f537 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -356,13 +356,18 @@ uefi_log: uefi-log@908b0000 {
>  			no-map;
>  		};
>  
> +		ddr_training_checksum: ddr_training_checksum@908c0000 {

Underscores are not allowed. Please rewrite downstream patches to match
upstream code.

Also, useful is to run basic tests/tools on your patches, like W=1
builds, before posting.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sa8775p: Add new memory map updates to SA8775P

[Ninad Naik]

On 1/23/2024 1:56 PM, Krzysztof Kozlowski wrote:
> On 18/01/2024 16:57, 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@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sa8775p.dtsi | 103 +++++++++++++++++++++++---
>>   1 file changed, 94 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> index a7eaca33d326..20b16fb5f537 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> @@ -356,13 +356,18 @@ uefi_log: uefi-log@908b0000 {
>>   			no-map;
>>   		};
>>   
>> +		ddr_training_checksum: ddr_training_checksum@908c0000 {
> 
> Underscores are not allowed. Please rewrite downstream patches to match
> upstream code.
> 
> Also, useful is to run basic tests/tools on your patches, like W=1
> builds, before posting.
Ack. I will make the corresponding fixes.

Regards,
Ninad