From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API Date: Wed, 22 Mar 2017 14:37:24 +0100 Message-ID: <20170322133724.GA10969@cbox> References: <20170321192058.9300-1-marc.zyngier@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Russell King , kvm@vger.kernel.org, Ard Biesheuvel , Catalin Marinas , linux-arm-kernel@lists.infradead.org, Keerthy , kvmarm@lists.cs.columbia.edu To: Marc Zyngier Return-path: Content-Disposition: inline In-Reply-To: <20170321192058.9300-1-marc.zyngier@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: cdall@linaro.org (Christoffer Dall) Date: Wed, 22 Mar 2017 14:37:24 +0100 Subject: [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API In-Reply-To: <20170321192058.9300-1-marc.zyngier@arm.com> References: <20170321192058.9300-1-marc.zyngier@arm.com> Message-ID: <20170322133724.GA10969@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >