Re: [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API

Christoffer Dall
2017-03-22T13:37:24Z

Hi Marc,

On Tue, Mar 21, 2017 at 07:20:30PM +0000, Marc Zyngier wrote:
> As noticed by RMK in this thread[1], the hyp-stub API on 32bit ARM
> could do with some TLC (it cannot perform a soft-restart at HYP, and
> has holes in the hyp-stub support in a number of places). In general,
> it would be desirable for the 32bit behaviour to align on 64bit, if
> only to ease maintenance.
>
> This series implements the following:
> - Add HVC_[GS]ET_VECTORS and HVC_SOFT_RESTART to the 32bit code
> - Add HVC_RESET_VECTORS to both arm and arm64, removing the need for
> __hyp_reset_vectors
> - Implement add the stub entry points in the KVM init code, which
> didn't implement any so far
> - Convert the HYP code to use the init code stubs directly
> - Some general cleanup as a result of these changes (which includes
> killing HVC_GET_VECTORS)
> - Add some API documentation that covers the above
>
> Patches 12 to 14 would be better squashed into 10 and 11, but I've
> kept them separate so that I can take the blame for everything I've
> broken.

This series looks great overall. I'm still going through the details of
the patches, but one high level questions:

What if we formalized the way to call a function in Hyp mode, whatever
its current configuration may be, via a specific HVC number. That would
mean that the documentation say nothing of a hypervisor specific
implementaiton.

Could we then use that to initialize hyp mode for a hypervisor, i.e.
KVM, without having to actually change the vectors? Couldn't we simply
use the the hvc stub to call a function in the physical address space,
and be rid of the concept of hyp-init alltogether?

I'm probably missing something, but let me know your thoughts.

Thanks,
-Christoffer

>
> I've tested this on arm (Cubietruck, Jetson TK1) and arm64 (Seattle),
> both as host and guest. Keerthy has been kind enough to test the 32bit
> code on DRA7-EVM, AM57XX-EVM and KEYSTONE-K2E-EVM.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/473472.html
>
> * From v3:
> - Reworked the way we save/restore lr on arm64, making it EL2's job
> in a consistent way (these are the three initial patches)
> - Collected RBs, TBs and Acks from James, Keerthy and Russell
> - Rebased on 4.11-rc3
>
> * From v2:
> - Kill HVC_GET_VECTORS and the corresponding __hyp_get_vectors
>
> * From v1:
> - Fixed some glaring bugs (reported by Ard and James)
> - Tidy up stub vector export on 32bit (Ard)
> - Nicer VA/PA conversion on 32bit (Ard)
> - Updated cpu_v7_reset documentation
> - Cleaned up HYP reset on PM events
> - Minor stub documentation update
>
> Marc Zyngier (26):
> arm64: hyp-stub: Stop pointlessly clobbering lr
> arm64: KVM: Move lr save/restore to do_el2_call
> arm64: hyp-stub: Don't save lr in the EL1 code
> arm64: hyp-stub: Implement HVC_RESET_VECTORS stub hypercall
> arm64: KVM: Implement HVC_RESET_VECTORS stub hypercall in the init
> code
> arm64: KVM: Implement HVC_GET_VECTORS in the init code
> arm64: KVM: Allow the main HYP code to use the init hyp stub
> implementation
> arm64: KVM: Convert __cpu_reset_hyp_mode to using __hyp_reset_vectors
> arm64: KVM: Implement HVC_SOFT_RESTART in the init code
> ARM: KVM: Convert KVM to use HVC_GET_VECTORS
> ARM: Update cpu_v7_reset documentation
> ARM: hyp-stub: Use r1 for the soft-restart address
> ARM: Expose the VA/IDMAP offset
> ARM: hyp-stub: Implement HVC_RESET_VECTORS stub hypercall
> ARM: KVM: Implement HVC_RESET_VECTORS stub hypercall in the init code
> ARM: KVM: Implement HVC_GET_VECTORS in the init code
> ARM: KVM: Allow the main HYP code to use the init hyp stub
> implementation
> ARM: KVM: Convert __cpu_reset_hyp_mode to using __hyp_reset_vectors
> ARM: KVM: Implement HVC_SOFT_RESTART in the init code
> arm/arm64: KVM: Use __hyp_reset_vectors() directly
> arm/arm64: KVM: Remove kvm_get_idmap_start
> arm/arm64: KVM: Use HVC_RESET_VECTORS to reinit HYP mode
> ARM: decompressor: Remove __hyp_get_vectors usage
> ARM: hyp-stub/KVM: Kill __hyp_get_vectors
> arm64: hyp-stub/KVM: Kill __hyp_get_vectors
> arm/arm64: Add hyp-stub API documentation
>
> Russell King (2):
> ARM: hyp-stub: improve ABI
> ARM: soft-reboot into same mode that we entered the kernel
>
> Documentation/virtual/kvm/arm/hyp-abi.txt | 45 ++++++++++++++++++++++++++++
> arch/arm/boot/compressed/head.S | 5 +++-
> arch/arm/include/asm/kvm_asm.h | 2 --
> arch/arm/include/asm/kvm_host.h | 6 ----
> arch/arm/include/asm/kvm_mmu.h | 1 -
> arch/arm/include/asm/proc-fns.h | 4 +--
> arch/arm/include/asm/virt.h | 12 +++++++-
> arch/arm/kernel/hyp-stub.S | 39 +++++++++++++++++++-----
> arch/arm/kernel/reboot.c | 7 +++--
> arch/arm/kvm/arm.c | 25 ++++++----------
> arch/arm/kvm/hyp/hyp-entry.S | 29 ++++++++++++++----
> arch/arm/kvm/init.S | 49 ++++++++++++++++++++++++++-----
> arch/arm/kvm/interrupts.S | 4 ---
> arch/arm/kvm/mmu.c | 5 ----
> arch/arm/mm/mmu.c | 5 ++++
> arch/arm/mm/proc-v7.S | 15 ++++++----
> arch/arm64/include/asm/kvm_asm.h | 1 -
> arch/arm64/include/asm/kvm_host.h | 7 -----
> arch/arm64/include/asm/kvm_mmu.h | 1 -
> arch/arm64/include/asm/virt.h | 17 +++++++----
> arch/arm64/kernel/hyp-stub.S | 34 +++++++--------------
> arch/arm64/kvm/hyp-init.S | 45 +++++++++++++++++++++-------
> arch/arm64/kvm/hyp.S | 2 +-
> arch/arm64/kvm/hyp/hyp-entry.S | 43 +++++++++++++--------------
> 24 files changed, 266 insertions(+), 137 deletions(-)
> create mode 100644 Documentation/virtual/kvm/arm/hyp-abi.txt
>
> --
> 2.11.0
>