[PATCH linux-next] arm64: kexec: Support the case of VA_BITS=39 in trans_pgd_idmap_page()

[PATCH linux-next] arm64: kexec: Support the case of VA_BITS=39 in trans_pgd_idmap_page()

[cgel.zte]

From: sihao <si.hao@zte.com.cn>

When the values of CONFIG_ARM64_VA_BITS and CONFIG_ARM64_PA_BITS are not
equal, the following panic occurs when kexec is executed.

This happens because trans_pgd_idmap_page() does not support VA_BITS=39.
So the patch supports the case of VA_BITS=39.

->kexec -e
[   36.168728] 000: kexec_core: Starting new kernel
[   36.168735] 000: Disabling non-boot CPUs ...
[   36.168739] 000: Bye!
[   36.189457] 000: Unable to handle kernel paging request at virtual
address 000000000fb3c000
[   36.189460] 000: Mem abort info:
[   36.189462] 000:   ESR = 0x86000006
[   36.189463] 000:   EC = 0x21: IABT (current EL), IL = 32 bits
[   36.189466] 000:   SET = 0, FnV = 0
[   36.189468] 000:   EA = 0, S1PTW = 0
[   36.189470] 000: user pgtable: 4k pages, 39-bit VAs,
pgdp=000000000de68000
[   36.189475] 000: [000000000fb3c000] pgd=000000000fb24003,
pud=000000000fb24003, pmd=0000000000000000
[   36.189482] 000: Internal error: Oops: 86000006 [#1] PREEMPT SMP
[   36.189488] 000: Modules linked in:
[   36.189493] 000: CPU: 0 PID: 825 Comm: kexec Not tainted XXX
[   36.189498] 000: Hardware name: XXX
[   36.189500] 000: pstate: 600003c5 (nZCv DAIF -PAN -UAO)
[   36.189503] 000: pc : 0xfb3c000
[   36.189510] 000: lr : machine_kexec+0x2a4/0x300
[   36.189520] 000: sp : ffffffc0111fbc90
[   36.189523] 000: x29: ffffffc0111fbc90 x28: ffffff800fb66440
[   36.189527] 000: x27: 0000000000000000 x26: 0000000000000000
[   36.189531] 000: x25: 0000000056000000 x24: 0000000000000015
[   36.189534] 000: x23: 000000000de64000 x22: 000000000fb3c000
[   36.189538] 000: x21: ffffffc010699298 x20: ffffffc010752e90
[   36.189542] 000: x19: ffffff800acc0400 x18: 0000000000000001
[   36.189545] 000: x17: 0000000000000000 x16: 0000000000000000
[   36.189549] 000: x15: ffffff800fb66838 x14: ffffffffffffffff
[   36.189552] 000: x13: ffffffc0108f0b48 x12: ffffffc0111fbba0
[   36.189556] 000: x11: ffffffc01088d450 x10: ffffffc01088d468
[   36.189559] 000: x9 : 0000000000000001 x8 : 0000000000024d30
[   36.189563] 000: x7 : ffffffc0108f0738 x6 : ffffffc0108ebe18
[   36.189566] 000: x5 : 0000000000024d30 x4 : 0000000000004920
[   36.189570] 000: x3 : 0000000000000006 x2 : b9bb936edd96cd00
[   36.189573] 000: x1 : 0000000000000000 x0 : ffffff800acc0400
[   36.189576] 000: Call trace:
[   36.189578] 000:  0xfb3c000
[   36.189580] 000:  kernel_kexec+0x98/0xe8
[   36.189586] 000:  __do_sys_reboot+0x1d0/0x208
[   36.189593] 000:  __arm64_sys_reboot+0x2c/0x38
[   36.189597] 000:  el0_svc_common.constprop.0+0x74/0x168
[   36.189603] 000:  el0_svc_handler+0x80/0xa0
[   36.189607] 000:  el0_svc+0x8/0x204
[   36.189616] 000: Code: bad PC value
[   36.189621] 000: ---[ end trace f930c021f9bf10a9 ]---
[   36.189625] 000: Kernel panic - not syncing: Fatal exception

Reported-by: sihao <si.hao@zte.com.cn>
Signed-off-by: sihao <si.hao@zte.com.cn>
---
 arch/arm64/mm/trans_pgd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
index d7da8ca40d2e..3d88185adcf5 100644
--- a/arch/arm64/mm/trans_pgd.c
+++ b/arch/arm64/mm/trans_pgd.c
@@ -232,7 +232,7 @@ int trans_pgd_idmap_page(struct trans_pgd_info *info, phys_addr_t *trans_ttbr0,
 {
 	phys_addr_t dst_addr = virt_to_phys(page);
 	unsigned long pfn = __phys_to_pfn(dst_addr);
-	int max_msb = (dst_addr & GENMASK(52, 48)) ? 51 : 47;
+	int max_msb = (dst_addr & GENMASK(52, VA_BITS)) ? 51 : (VA_BITS - 1);
 	int bits_mapped = PAGE_SHIFT - 4;
 	unsigned long level_mask, prev_level_entry, *levels[4];
 	int this_level, index, level_lsb, level_msb;
-- 
2.25.1

Re: [PATCH linux-next] arm64: kexec: Support the case of VA_BITS=39 in trans_pgd_idmap_page()

[Catalin Marinas]

On Fri, Jan 21, 2022 at 06:52:16AM +0000, cgel.zte@gmail.com wrote:
> From: sihao <si.hao@zte.com.cn>
> 
> When the values of CONFIG_ARM64_VA_BITS and CONFIG_ARM64_PA_BITS are not
> equal, the following panic occurs when kexec is executed.
> 
> This happens because trans_pgd_idmap_page() does not support VA_BITS=39.
> So the patch supports the case of VA_BITS=39.
[...]
> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> index d7da8ca40d2e..3d88185adcf5 100644
> --- a/arch/arm64/mm/trans_pgd.c
> +++ b/arch/arm64/mm/trans_pgd.c
> @@ -232,7 +232,7 @@ int trans_pgd_idmap_page(struct trans_pgd_info *info, phys_addr_t *trans_ttbr0,
>  {
>  	phys_addr_t dst_addr = virt_to_phys(page);
>  	unsigned long pfn = __phys_to_pfn(dst_addr);
> -	int max_msb = (dst_addr & GENMASK(52, 48)) ? 51 : 47;

This should have been GENMASK(51, 48), though it doesn't make any
difference and may work better with the change below:

> +	int max_msb = (dst_addr & GENMASK(52, VA_BITS)) ? 51 : (VA_BITS - 1);

So when VA_BITS == 52, the mask is 0 and we set max_msb to 51.

I wonder, could we use fls64() instead here?

-- 
Catalin

Re: [PATCH linux-next] arm64: kexec: Support the case of VA_BITS=39 in trans_pgd_idmap_page()

[Will Deacon]

On Fri, Jan 21, 2022 at 06:52:16AM +0000, cgel.zte@gmail.com wrote:
> From: sihao <si.hao@zte.com.cn>
> 
> When the values of CONFIG_ARM64_VA_BITS and CONFIG_ARM64_PA_BITS are not
> equal, the following panic occurs when kexec is executed.
> 
> This happens because trans_pgd_idmap_page() does not support VA_BITS=39.
> So the patch supports the case of VA_BITS=39.

--->8

> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> index d7da8ca40d2e..3d88185adcf5 100644
> --- a/arch/arm64/mm/trans_pgd.c
> +++ b/arch/arm64/mm/trans_pgd.c
> @@ -232,7 +232,7 @@ int trans_pgd_idmap_page(struct trans_pgd_info *info, phys_addr_t *trans_ttbr0,
>  {
>  	phys_addr_t dst_addr = virt_to_phys(page);
>  	unsigned long pfn = __phys_to_pfn(dst_addr);
> -	int max_msb = (dst_addr & GENMASK(52, 48)) ? 51 : 47;
> +	int max_msb = (dst_addr & GENMASK(52, VA_BITS)) ? 51 : (VA_BITS - 1);
>  	int bits_mapped = PAGE_SHIFT - 4;
>  	unsigned long level_mask, prev_level_entry, *levels[4];
>  	int this_level, index, level_lsb, level_msb;

Do you plan to respin this based on Catalin's comments?

Will