Re: [PATCH 03/15] arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe
Hi David, On Thu, 07 May 2020 15:36:17 +0100, David Brazdil wrote: > > Hi Marc, > > > > > What breaks without this constraint? Is it a fix that should go in > > early? Otherwise looks good. > > This only becomes an issue when __hyp_call_panic_nvhe() and > __hyp_call_panic_vhe() are moved to separate files, so I don't think > it's necessary to go in early. > > Currently the string variable (declared static) is seen by the C > compiler as used by __hyp_call_panic_vhe(). But when split, the > variable in the nVHE source file becomes unused, is dropped by the > compiler and the inline assembly's reference is unresolved. We could > then alias __hyp_text___hyp_panic_string back to the VHE copy, but > this is the right way of addressing it. Thanks for the detailed explanation. I think some of it should make it in the commit message, pointing what breaks and when. M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/15] arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe
Hi Marc, > > What breaks without this constraint? Is it a fix that should go in > early? Otherwise looks good. This only becomes an issue when __hyp_call_panic_nvhe() and __hyp_call_panic_vhe() are moved to separate files, so I don't think it's necessary to go in early. Currently the string variable (declared static) is seen by the C compiler as used by __hyp_call_panic_vhe(). But when split, the variable in the nVHE source file becomes unused, is dropped by the compiler and the inline assembly's reference is unresolved. We could then alias __hyp_text___hyp_panic_string back to the VHE copy, but this is the right way of addressing it. Thanks for the review, David ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/15] arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe
On Thu, 30 Apr 2020 15:48:19 +0100,
David Brazdil wrote:
>
> __hyp_call_panic_nvhe contains inline assembly which did not declare
> its dependency on the __hyp_panic_string symbol.
>
> Signed-off-by: David Brazdil
> ---
> arch/arm64/kvm/hyp/switch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 8a1e81a400e0..7a7c08029d81 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -836,7 +836,7 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr,
> u64 elr, u64 par,
>* making sure it is a kernel address and not a PC-relative
>* reference.
>*/
> - asm volatile("ldr %0, =__hyp_panic_string" : "=r" (str_va));
> + asm volatile("ldr %0, =%1" : "=r" (str_va) : "S" (__hyp_panic_string));
>
> __hyp_do_panic(str_va,
> spsr, elr,
> --
> 2.26.1
>
>
What breaks without this constraint? Is it a fix that should go in
early? Otherwise looks good.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
