Hi Takahiro,
On 27/05/15 06:21, AKASHI Takahiro wrote:
> Marc,
>
> Thank you for your reviews:
>
> On 05/26/2015 06:26 PM, Marc Zyngier wrote:
>> Hi Takahiro,
>>
>> On 08/05/15 02:18, AKASHI Takahiro wrote:
>>> Cpu must be put back into its initial state, at least, in the
>>> following cases in order to shutdown the system and/or re-initialize cpus
>>> later on:
>>> 1) kexec/kdump
>>> 2) cpu hotplug (offline)
>>> 3) removing kvm as a module
>>>
>>> To address those issues in later patches, this patch adds a tear-down
>>> function, kvm_cpu_reset(), that disables D-cache & MMU and restore a vector
>>> table to the initial stub at EL2.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
>>> ---
>>> arch/arm/kvm/arm.c | 15 +++++++++++++++
>>> arch/arm/kvm/mmu.c | 5 +++++
>>> arch/arm64/include/asm/kvm_asm.h | 1 +
>>> arch/arm64/include/asm/kvm_host.h | 11 +++++++++++
>>> arch/arm64/include/asm/kvm_mmu.h | 7 +++++++
>>> arch/arm64/include/asm/virt.h | 11 +++++++++++
>>> arch/arm64/kvm/hyp-init.S | 32 ++++++++++++++++++++++++++++++++
>>> arch/arm64/kvm/hyp.S | 16 +++++++++++++---
>>> 8 files changed, 95 insertions(+), 3 deletions(-)
>>>
[...]
>>> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
>>> index c319116..2614cfc 100644
>>> --- a/arch/arm64/kvm/hyp-init.S
>>> +++ b/arch/arm64/kvm/hyp-init.S
>>> @@ -115,6 +115,38 @@ target: /* We're now in the trampoline code, switch
>>> page tables */
>>> eret
>>> ENDPROC(__kvm_hyp_init)
>>>
>>> + /*
>>> + * x0: HYP boot pgd
>>> + * x1: HYP phys_idmap_start
>>> + * x2: HYP stub vectors
>>> + */
>>> +ENTRY(__kvm_hyp_reset)
>>> + /* We're in trampoline code in VA, switch back to boot page tables */
>>> + msr ttbr0_el2, x0
>>> + isb
>>> +
>>> + /* Invalidate the old TLBs */
>>> + tlbi alle2
>>> + dsb sy
>>> +
>>> + /* Branch into PA space */
>>> + adr x0, 1f
>>> + bfi x1, x0, #0, #PAGE_SHIFT
>>> + br x1
>>> +
>>> + /* We're now in idmap, disable MMU */
>>> +1: mrs x0, sctlr_el2
>>> + ldr x1, =SCTLR_EL2_FLAGS
>>> + bic x0, x0, x1 // Clear SCTL_M and etc
>>> + msr sctlr_el2, x0
>>> + isb
>>> +
>>> + /* Install stub vectors */
>>> + msr vbar_el2, x2
>>
>> Instead of using a parameter, can't you just do
>>
>> adr x2, __hyp_stub_vectors
>> msr vbar_el2, x2
>>
>> ? I can't imagine a case where we don't want this behaviour, and this
>> would slightly simplify the calling convention.
>
> Yeah, but it will also enforces kvm to be statically linked to the kernel.
> (not module-capable) Is that OK?
I don't think this enforces anything to be statically linked. You don't
have to refer to the symbol from C code for module linking to work. You
may have to use a combination of ADRP + ADD :lo12: to make sure you can
reach the symbol (ADR is limited to +/- 1MB).
[...]
>>> -1: /* Default to HVC_CALL_HYP. */
>>> + /* jump into trampoline code */
>>> +1: cmp x18, #HVC_RESET
>>> + b.ne 2f
>>> + br x3 // no return
>>
>> Same here. If we're always jumping to the trampoline code, why do we
>> have to pass it as a parameter?
>
> We need here a physical address of reset function. It seems easy to call
> kvm_virt_to_trampoline() in C code. But, yes, we can also do it in asm.
I think that would be a lot clearer as it will be self-contained.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
