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

From: [email-masked] (Christoffer Dall)
Date: Wed, 22 Mar 2017 14:37:24 +0100

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
>