On Fri, Mar 24, 2017 at 03:01:23PM +0000, Marc Zyngier wrote:
> On 24/03/17 14:34, Christoffer Dall wrote:
> > On Tue, Mar 21, 2017 at 07:20:49PM +0000, Marc Zyngier wrote:
> >> We now have a full hyp-stub implementation in the KVM init code,
> >> but the main KVM code only supports HVC_GET_VECTORS, which is not
> >> enough.
> >>
> >> Instead of reinventing the wheel, let's reuse the init implementation
> >> by branching to the idmap page when called with a hyp-stub hypercall.
> >>
> >> Tested-by: Keerthy <j-keerthy@xxxxxx>
> >> Acked-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> >> ---
> >> arch/arm/kvm/hyp/hyp-entry.S | 29 ++++++++++++++++++++++++-----
> >> 1 file changed, 24 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> >> index 1f8db7d21fc5..a35baa81fd23 100644
> >> --- a/arch/arm/kvm/hyp/hyp-entry.S
> >> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> >> @@ -126,11 +126,30 @@ hyp_hvc:
> >> */
> >> pop {r0, r1, r2}
> >>
> >> - /* Check for __hyp_get_vectors */
> >> - cmp r0, #HVC_GET_VECTORS
> >> - mrceq p15, 4, r0, c12, c0, 0 @ get HVBAR
> >> - beq 1f
> >> + /*
> >> + * Check if we have a kernel function, which is guaranteed to be
> >> + * bigger than the maximum hyp stub hypercall
> >> + */
> >> + cmp r0, #HVC_STUB_HCALL_NR
> >> + bhs 1f
> >>
> >> + /*
> >> + * Not a kernel function, treat it as a stub hypercall.
> >> + * Compute the physical address for __kvm_handle_stub_hvc
> >> + * (as the code lives in the idmaped page) and branch there.
> >> + * We hijack ip (r12) as a tmp register.
> >> + */
> >
> > How can we just clobber r12 and be sure we don't corrupt the caller?
>
> r12 (aka ip) is allowed to be clobbered by the linker (used by inserted
> code veneers, for example). Given that this is a standalone object, we
> can safely assume that r12 has been saved if it was used by the caller.
>
> Here is what the PCS says:
>
> "Register r12 (IP) may be used by a linker as a scratch register between
> a routine and any subroutine it calls (for details, see
> §5.3.1.1, Use of IP by the linker). It can also be used within a routine
> to hold intermediate values between subroutine calls."
>
So isn't this similar to my comment on the arm64 code, which relies on
this being called via a function call, as opposed to directly issuring
an HVC via inline assembly?
If so, documenting this limitation/restriction/feature would be nice.
Thanks,
-Christoffer
