2020-10-08 17:24:30

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH v3] arm/arm64: xen: Fix to convert percpu address to gfn correctly

Use per_cpu_ptr_to_phys() instead of virt_to_phys() and __pa()
for per-cpu address conversion.

In xen_starting_cpu(), per-cpu xen_vcpu_info address is converted
to gfn by virt_to_gfn() macro. However, since the virt_to_gfn(v)
assumes the given virtual address is in linear mapped kernel memory
area, it can not convert the per-cpu memory if it is allocated on
vmalloc area.

This depends on from where the xen_vcpu_info percpu object is
allocated. If CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y, the
first chunk of percpu is on the kernel linear mapped area.
If the first chunk is enough large, xen_guest_init() can allocate
the xen_vcpu_info on the first chunk. However, if it is not large
enough, or other subsystem init functions called earlier than
xen_guest_init() runs out the first chunk, the xen_vcpu_info is
allocated on the 2nd chunk of percpu, and it can be on vmalloc
memory (depends on CONFIG_NEED_PER_CPU_KM=n).

Without this fix and, unfortnunately, xen_vcpu_info is allocated
on the vmalloc area, the Dom0 kernel will fail to boot with
following errors.

[ 0.466172] Xen: initializing cpu0
[ 0.469601] ------------[ cut here ]------------
[ 0.474295] WARNING: CPU: 0 PID: 1 at arch/arm64/xen/../../arm/xen/enlighten.c:153 xen_starting_cpu+0x160/0x180
[ 0.484435] Modules linked in:
[ 0.487565] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4+ #4
[ 0.493895] Hardware name: Socionext Developer Box (DT)
[ 0.499194] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
[ 0.504836] pc : xen_starting_cpu+0x160/0x180
[ 0.509263] lr : xen_starting_cpu+0xb0/0x180
[ 0.513599] sp : ffff8000116cbb60
[ 0.516984] x29: ffff8000116cbb60 x28: ffff80000abec000
[ 0.522366] x27: 0000000000000000 x26: 0000000000000000
[ 0.527754] x25: ffff80001156c000 x24: fffffdffbfcdb600
[ 0.533129] x23: 0000000000000000 x22: 0000000000000000
[ 0.538511] x21: ffff8000113a99c8 x20: ffff800010fe4f68
[ 0.543892] x19: ffff8000113a9988 x18: 0000000000000010
[ 0.549274] x17: 0000000094fe0f81 x16: 00000000deadbeef
[ 0.554655] x15: ffffffffffffffff x14: 0720072007200720
[ 0.560037] x13: 0720072007200720 x12: 0720072007200720
[ 0.565418] x11: 0720072007200720 x10: 0720072007200720
[ 0.570801] x9 : ffff8000100fbdc0 x8 : ffff800010715208
[ 0.576182] x7 : 0000000000000054 x6 : ffff00001b790f00
[ 0.581564] x5 : ffff800010bbf880 x4 : 0000000000000000
[ 0.586945] x3 : 0000000000000000 x2 : ffff80000abec000
[ 0.592327] x1 : 000000000000002f x0 : 0000800000000000
[ 0.597716] Call trace:
[ 0.600232] xen_starting_cpu+0x160/0x180
[ 0.604309] cpuhp_invoke_callback+0xac/0x640
[ 0.608736] cpuhp_issue_call+0xf4/0x150
[ 0.612728] __cpuhp_setup_state_cpuslocked+0x128/0x2c8
[ 0.618030] __cpuhp_setup_state+0x84/0xf8
[ 0.622192] xen_guest_init+0x324/0x364
[ 0.626097] do_one_initcall+0x54/0x250
[ 0.630003] kernel_init_freeable+0x12c/0x2c8
[ 0.634428] kernel_init+0x1c/0x128
[ 0.637988] ret_from_fork+0x10/0x18
[ 0.641635] ---[ end trace d95b5309a33f8b27 ]---
[ 0.646337] ------------[ cut here ]------------
[ 0.651005] kernel BUG at arch/arm64/xen/../../arm/xen/enlighten.c:158!
[ 0.657697] Internal error: Oops - BUG: 0 [#1] SMP
[ 0.662548] Modules linked in:
[ 0.665676] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc4+ #4
[ 0.673398] Hardware name: Socionext Developer Box (DT)
[ 0.678695] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
[ 0.684338] pc : xen_starting_cpu+0x178/0x180
[ 0.688765] lr : xen_starting_cpu+0x144/0x180
[ 0.693188] sp : ffff8000116cbb60
[ 0.696573] x29: ffff8000116cbb60 x28: ffff80000abec000
[ 0.701955] x27: 0000000000000000 x26: 0000000000000000
[ 0.707344] x25: ffff80001156c000 x24: fffffdffbfcdb600
[ 0.712718] x23: 0000000000000000 x22: 0000000000000000
[ 0.718107] x21: ffff8000113a99c8 x20: ffff800010fe4f68
[ 0.723481] x19: ffff8000113a9988 x18: 0000000000000010
[ 0.728863] x17: 0000000094fe0f81 x16: 00000000deadbeef
[ 0.734245] x15: ffffffffffffffff x14: 0720072007200720
[ 0.739626] x13: 0720072007200720 x12: 0720072007200720
[ 0.745008] x11: 0720072007200720 x10: 0720072007200720
[ 0.750390] x9 : ffff8000100fbdc0 x8 : ffff800010715208
[ 0.755771] x7 : 0000000000000054 x6 : ffff00001b790f00
[ 0.761153] x5 : ffff800010bbf880 x4 : 0000000000000000
[ 0.766534] x3 : 0000000000000000 x2 : 00000000deadbeef
[ 0.771916] x1 : 00000000deadbeef x0 : ffffffffffffffea
[ 0.777304] Call trace:
[ 0.779819] xen_starting_cpu+0x178/0x180
[ 0.783898] cpuhp_invoke_callback+0xac/0x640
[ 0.788325] cpuhp_issue_call+0xf4/0x150
[ 0.792317] __cpuhp_setup_state_cpuslocked+0x128/0x2c8
[ 0.797619] __cpuhp_setup_state+0x84/0xf8
[ 0.801779] xen_guest_init+0x324/0x364
[ 0.805683] do_one_initcall+0x54/0x250
[ 0.809590] kernel_init_freeable+0x12c/0x2c8
[ 0.814016] kernel_init+0x1c/0x128
[ 0.817583] ret_from_fork+0x10/0x18
[ 0.821226] Code: d0006980 f9427c00 cb000300 17ffffea (d4210000)
[ 0.827415] ---[ end trace d95b5309a33f8b28 ]---
[ 0.832076] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 0.839815] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

Theoletically, this issue has been introduced by commit 9a9ab3cc00dc
("xen/arm: SMP support") because it uses __pa() on percpu address.

Signed-off-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Stefano Stabellini <[email protected]>
---
Changes in v3:
Update patch description to explain the mechanism of the problem.
---
arch/arm/xen/enlighten.c | 2 +-
include/xen/arm/page.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index e93145d72c26..a6ab3689b2f4 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -150,7 +150,7 @@ static int xen_starting_cpu(unsigned int cpu)
pr_info("Xen: initializing cpu%d\n", cpu);
vcpup = per_cpu_ptr(xen_vcpu_info, cpu);

- info.mfn = virt_to_gfn(vcpup);
+ info.mfn = percpu_to_gfn(vcpup);
info.offset = xen_offset_in_page(vcpup);

err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu),
diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
index 39df751d0dc4..ac1b65470563 100644
--- a/include/xen/arm/page.h
+++ b/include/xen/arm/page.h
@@ -83,6 +83,9 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
})
#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << XEN_PAGE_SHIFT))

+#define percpu_to_gfn(v) \
+ (pfn_to_gfn(per_cpu_ptr_to_phys(v) >> XEN_PAGE_SHIFT))
+
/* Only used in PV code. But ARM guests are always HVM. */
static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
{


2020-10-08 20:25:14

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v3] arm/arm64: xen: Fix to convert percpu address to gfn correctly

Thanks Masami, the patch is already in our queue for going upstream.

On Fri, 9 Oct 2020, Masami Hiramatsu wrote:
> Use per_cpu_ptr_to_phys() instead of virt_to_phys() and __pa()
> for per-cpu address conversion.
>
> In xen_starting_cpu(), per-cpu xen_vcpu_info address is converted
> to gfn by virt_to_gfn() macro. However, since the virt_to_gfn(v)
> assumes the given virtual address is in linear mapped kernel memory
> area, it can not convert the per-cpu memory if it is allocated on
> vmalloc area.
>
> This depends on from where the xen_vcpu_info percpu object is
> allocated. If CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y, the
> first chunk of percpu is on the kernel linear mapped area.
> If the first chunk is enough large, xen_guest_init() can allocate
> the xen_vcpu_info on the first chunk. However, if it is not large
> enough, or other subsystem init functions called earlier than
> xen_guest_init() runs out the first chunk, the xen_vcpu_info is
> allocated on the 2nd chunk of percpu, and it can be on vmalloc
> memory (depends on CONFIG_NEED_PER_CPU_KM=n).
>
> Without this fix and, unfortnunately, xen_vcpu_info is allocated
> on the vmalloc area, the Dom0 kernel will fail to boot with
> following errors.
>
> [ 0.466172] Xen: initializing cpu0
> [ 0.469601] ------------[ cut here ]------------
> [ 0.474295] WARNING: CPU: 0 PID: 1 at arch/arm64/xen/../../arm/xen/enlighten.c:153 xen_starting_cpu+0x160/0x180
> [ 0.484435] Modules linked in:
> [ 0.487565] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4+ #4
> [ 0.493895] Hardware name: Socionext Developer Box (DT)
> [ 0.499194] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
> [ 0.504836] pc : xen_starting_cpu+0x160/0x180
> [ 0.509263] lr : xen_starting_cpu+0xb0/0x180
> [ 0.513599] sp : ffff8000116cbb60
> [ 0.516984] x29: ffff8000116cbb60 x28: ffff80000abec000
> [ 0.522366] x27: 0000000000000000 x26: 0000000000000000
> [ 0.527754] x25: ffff80001156c000 x24: fffffdffbfcdb600
> [ 0.533129] x23: 0000000000000000 x22: 0000000000000000
> [ 0.538511] x21: ffff8000113a99c8 x20: ffff800010fe4f68
> [ 0.543892] x19: ffff8000113a9988 x18: 0000000000000010
> [ 0.549274] x17: 0000000094fe0f81 x16: 00000000deadbeef
> [ 0.554655] x15: ffffffffffffffff x14: 0720072007200720
> [ 0.560037] x13: 0720072007200720 x12: 0720072007200720
> [ 0.565418] x11: 0720072007200720 x10: 0720072007200720
> [ 0.570801] x9 : ffff8000100fbdc0 x8 : ffff800010715208
> [ 0.576182] x7 : 0000000000000054 x6 : ffff00001b790f00
> [ 0.581564] x5 : ffff800010bbf880 x4 : 0000000000000000
> [ 0.586945] x3 : 0000000000000000 x2 : ffff80000abec000
> [ 0.592327] x1 : 000000000000002f x0 : 0000800000000000
> [ 0.597716] Call trace:
> [ 0.600232] xen_starting_cpu+0x160/0x180
> [ 0.604309] cpuhp_invoke_callback+0xac/0x640
> [ 0.608736] cpuhp_issue_call+0xf4/0x150
> [ 0.612728] __cpuhp_setup_state_cpuslocked+0x128/0x2c8
> [ 0.618030] __cpuhp_setup_state+0x84/0xf8
> [ 0.622192] xen_guest_init+0x324/0x364
> [ 0.626097] do_one_initcall+0x54/0x250
> [ 0.630003] kernel_init_freeable+0x12c/0x2c8
> [ 0.634428] kernel_init+0x1c/0x128
> [ 0.637988] ret_from_fork+0x10/0x18
> [ 0.641635] ---[ end trace d95b5309a33f8b27 ]---
> [ 0.646337] ------------[ cut here ]------------
> [ 0.651005] kernel BUG at arch/arm64/xen/../../arm/xen/enlighten.c:158!
> [ 0.657697] Internal error: Oops - BUG: 0 [#1] SMP
> [ 0.662548] Modules linked in:
> [ 0.665676] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc4+ #4
> [ 0.673398] Hardware name: Socionext Developer Box (DT)
> [ 0.678695] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
> [ 0.684338] pc : xen_starting_cpu+0x178/0x180
> [ 0.688765] lr : xen_starting_cpu+0x144/0x180
> [ 0.693188] sp : ffff8000116cbb60
> [ 0.696573] x29: ffff8000116cbb60 x28: ffff80000abec000
> [ 0.701955] x27: 0000000000000000 x26: 0000000000000000
> [ 0.707344] x25: ffff80001156c000 x24: fffffdffbfcdb600
> [ 0.712718] x23: 0000000000000000 x22: 0000000000000000
> [ 0.718107] x21: ffff8000113a99c8 x20: ffff800010fe4f68
> [ 0.723481] x19: ffff8000113a9988 x18: 0000000000000010
> [ 0.728863] x17: 0000000094fe0f81 x16: 00000000deadbeef
> [ 0.734245] x15: ffffffffffffffff x14: 0720072007200720
> [ 0.739626] x13: 0720072007200720 x12: 0720072007200720
> [ 0.745008] x11: 0720072007200720 x10: 0720072007200720
> [ 0.750390] x9 : ffff8000100fbdc0 x8 : ffff800010715208
> [ 0.755771] x7 : 0000000000000054 x6 : ffff00001b790f00
> [ 0.761153] x5 : ffff800010bbf880 x4 : 0000000000000000
> [ 0.766534] x3 : 0000000000000000 x2 : 00000000deadbeef
> [ 0.771916] x1 : 00000000deadbeef x0 : ffffffffffffffea
> [ 0.777304] Call trace:
> [ 0.779819] xen_starting_cpu+0x178/0x180
> [ 0.783898] cpuhp_invoke_callback+0xac/0x640
> [ 0.788325] cpuhp_issue_call+0xf4/0x150
> [ 0.792317] __cpuhp_setup_state_cpuslocked+0x128/0x2c8
> [ 0.797619] __cpuhp_setup_state+0x84/0xf8
> [ 0.801779] xen_guest_init+0x324/0x364
> [ 0.805683] do_one_initcall+0x54/0x250
> [ 0.809590] kernel_init_freeable+0x12c/0x2c8
> [ 0.814016] kernel_init+0x1c/0x128
> [ 0.817583] ret_from_fork+0x10/0x18
> [ 0.821226] Code: d0006980 f9427c00 cb000300 17ffffea (d4210000)
> [ 0.827415] ---[ end trace d95b5309a33f8b28 ]---
> [ 0.832076] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [ 0.839815] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>
> Theoletically, this issue has been introduced by commit 9a9ab3cc00dc
> ("xen/arm: SMP support") because it uses __pa() on percpu address.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reviewed-by: Stefano Stabellini <[email protected]>
> ---
> Changes in v3:
> Update patch description to explain the mechanism of the problem.
> ---
> arch/arm/xen/enlighten.c | 2 +-
> include/xen/arm/page.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index e93145d72c26..a6ab3689b2f4 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -150,7 +150,7 @@ static int xen_starting_cpu(unsigned int cpu)
> pr_info("Xen: initializing cpu%d\n", cpu);
> vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
> - info.mfn = virt_to_gfn(vcpup);
> + info.mfn = percpu_to_gfn(vcpup);
> info.offset = xen_offset_in_page(vcpup);
>
> err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu),
> diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
> index 39df751d0dc4..ac1b65470563 100644
> --- a/include/xen/arm/page.h
> +++ b/include/xen/arm/page.h
> @@ -83,6 +83,9 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
> })
> #define gfn_to_virt(m) (__va(gfn_to_pfn(m) << XEN_PAGE_SHIFT))
>
> +#define percpu_to_gfn(v) \
> + (pfn_to_gfn(per_cpu_ptr_to_phys(v) >> XEN_PAGE_SHIFT))
> +
> /* Only used in PV code. But ARM guests are always HVM. */
> static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
> {
>