KVM: arm64: Implement KVM_TRANSLATE ioctl for arm64
PATCH 1/1 KVM: arm64: Implement KVM_TRANSLATE ioctl for arm64
There is a KVM_TRANSLATE ioctl for x86 to translate a GVA (guest virtual address) to a GPA (guest physical address) in EL1 which is not yet implemented for arm64.
Implement KVM_TRANSLATE on arm64 for both configurations that support and do not support VHE. The VHE path uses the AT instruction directly while the non-VHE implementation wraps the AT call in a hypercall to allow for its execution in EL2. Add selftest that tests the ioctl in both configurations.
--- arch/arm64/include/asm/kvm_asm.h | 2 + arch/arm64/kvm/guest.c | 89 ++++++++++++++- arch/arm64/kvm/hyp/nvhe/Makefile | 3 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++ arch/arm64/kvm/hyp/nvhe/translate.c | 84 ++++++++++++++ tools/testing/selftests/kvm/Makefile.kvm | 1 + tools/testing/selftests/kvm/arm64/translate.c | 107 ++++++++++++++++++ 7 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 arch/arm64/kvm/hyp/nvhe/translate.c create mode 100644 tools/testing/selftests/kvm/arm64/translate.c
arch/arm64/include/asm/kvm_asm.h +2 -0
@@ -87,6 +87,7 @@ enum __kvm_host_smccc_func { __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load, __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put, __KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid, + __KVM_HOST_SMCCC_FUNC___kvm_hyp_translate, };#define DECLARE_KVM_VHE_SYM (sym) extern char sym[]
@@ -289,6 +290,7 @@ asmlinkage void __noreturn hyp_panic_bad_stack(void); asmlinkage void kvm_unexpected_el2_exception(void);struct kvm_cpu_context ;void handle_trap (struct kvm_cpu_context *host_ctxt); +extern u64 __kvm_hyp_translate (struct kvm_vcpu *vcpu,u64 gva); asmlinkage void __noreturn __kvm_host_psci_cpu_entry(bool is_cpu_on);void __noreturn __pkvm_init_finalise (void );void kvm_nvhe_prepare_backtrace (unsigned long fp,unsigned long pc);
arch/arm64/kvm/guest.c +86 -3
102 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/Makefile +2 -1
@@ -24,7 +24,8 @@ CFLAGS_switch.nvhe.o += -Wno-override-init hyp-obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \ hyp-main.o hyp-smp.o psci-relay.o early_alloc.o page_alloc.o \ - cache.o setup.o mm.o mem_protect.o sys_regs.o pkvm.o stacktrace.o ffa.o + cache.o setup.o mm.o mem_protect.o sys_regs.o pkvm.o stacktrace.o ffa.o \ + translate.o hyp-obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \ ../fpsimd.o ../hyp-entry.o ../exception.o ../pgtable.o hyp-obj-$(CONFIG_LIST_HARDENED) += list_debug.o
arch/arm64/kvm/hyp/nvhe/hyp-main.c +10 -0
@@ -573,6 +573,15 @@ static void handle___pkvm_teardown_vm(struct kvm_cpu_context *host_ctxt)cpu_reg (host_ctxt,1 ) =__pkvm_teardown_vm (handle); } +static void handle___kvm_hyp_translate (struct kvm_cpu_context *host_ctxt) +{ + DECLARE_REG(struct kvm_vcpu *, host_vcpu, host_ctxt, 1); +DECLARE_REG (u64, gva, host_ctxt,2 ); + + host_vcpu =kern_hyp_va (host_vcpu); +cpu_reg (host_ctxt,1 ) =__kvm_hyp_translate (host_vcpu, gva); +} +typedef void (*hcall_t )(struct kvm_cpu_context *);#define HANDLE_FUNC (x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##x
@@ -612,6 +621,7 @@ static const hcall_t host_hcall[] = {HANDLE_FUNC (__pkvm_vcpu_load),HANDLE_FUNC (__pkvm_vcpu_put),HANDLE_FUNC (__pkvm_tlb_flush_vmid), + HANDLE_FUNC(__kvm_hyp_translate), };static void handle_host_hcall (struct kvm_cpu_context *host_ctxt)
arch/arm64/kvm/hyp/nvhe/translate.c new file +84 -0
84 lines not on this page · raw message
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -163,6 +163,7 @@ TEST_GEN_PROGS_arm64 += arm64/page_fault_test TEST_GEN_PROGS_arm64 += arm64/psci_test TEST_GEN_PROGS_arm64 += arm64/set_id_regs TEST_GEN_PROGS_arm64 += arm64/smccc_filter +TEST_GEN_PROGS_arm64 += arm64/translate TEST_GEN_PROGS_arm64 += arm64/vcpu_width_config TEST_GEN_PROGS_arm64 += arm64/vgic_init TEST_GEN_PROGS_arm64 += arm64/vgic_irq
tools/testing/selftests/kvm/arm64/translate.c new file +107 -0
107 lines not on this page · raw message
Signature · 1 line
2.43.0
Hi Priscilla,
Quoted 11 lines · Priscilla Lam
There is a
KVM_TRANSLATEioctl for x86 to translate a GVA (guest virtual address) to a GPA (guest physical address) in EL1 which is not yet implemented for arm64.Implement
KVM_TRANSLATEon arm64 for both configurations that support and do not support VHE. The VHE path uses the AT instruction directly while the non-VHE implementation wraps the AT call in a hypercall to allow for its execution in EL2. Add selftest that tests the ioctl in both configurations.Signed-off-by Priscilla Lam <[email protected]>
Quoted 8 lines
--- arch/arm64/include/asm/kvm_asm.h | 2 + arch/arm64/kvm/guest.c | 89 ++++++++++++++- arch/arm64/kvm/hyp/nvhe/Makefile | 3 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++ arch/arm64/kvm/hyp/nvhe/translate.c | 84 ++++++++++++++ tools/testing/selftests/kvm/Makefile.kvm | 1 + tools/testing/selftests/kvm/arm64/translate.c | 107 ++++++++++++++++++
Please do selftests changes in a separate patch.
Quoted 1 line
+static int
kvm_translate_vhe(struct kvm_vcpu *vcpu, struct kvm_translation *tr)
So arch/arm64/kvm/at.c exists for this exact purpose: walking guest page tables. While it was previously constrained to handling NV-enabled VMs, Marc's SEA TTW series opens up the stage-1 walker for general use.
I am not keen on adding yet another page-table walker implementation for the sole purpose of servicing this ioctl.
https://lore.kernel.org/kvmarm/[email protected]
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 947
Quoted 4 lines
arch/arm64/kvm/guest.c +4
-0+{ +unsigned long flags; +uint64_t hcr_old, hcr_new, par; +const uint64_t gva = tr->linear_address;
"linear_address" is a delightful x86-ism. I'd prefer naming that was either architecture-generic -or- an arm64-specific struct that used our architectural terms.
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 957
Quoted 9 lines
arch/arm64/kvm/guest.c +9
-0+preempt_disable (); +local_irq_save (flags); + +/* Ensure we're in the expected VHE regime and enable S2 so PAR returns IPA. */ + hcr_old =read_sysreg (hcr_el2); + hcr_new = hcr_old |HCR_E2H |HCR_VM ; + hcr_new &= ~HCR_TGE ; +write_sysreg (hcr_new, hcr_el2); +isb ();
Thanks to borken hardware, this needs to go through the write_sysreg_hcr() accessor.
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 964
Quoted 7 lines
arch/arm64/kvm/guest.c +7
-0+ +/* Load guest EL1 S1 context into *_EL12 (do not write into _EL1). */ +write_sysreg_s (vcpu_read_sys_reg (vcpu,TTBR0_EL1 ),SYS_TTBR0_EL12 ); +write_sysreg_s (vcpu_read_sys_reg (vcpu,TTBR1_EL1 ),SYS_TTBR1_EL12 ); +write_sysreg_s (vcpu_read_sys_reg (vcpu,TCR_EL1 ),SYS_TCR_EL12 ); +write_sysreg_s (vcpu_read_sys_reg (vcpu,MAIR_EL1 ),SYS_MAIR_EL12 ); +write_sysreg_s (vcpu_read_sys_reg (vcpu,SCTLR_EL1 ),SYS_SCTLR_EL12 );
KVM supports both FEAT_S1PIE and FEAT_S1POE, so this is not a complete MMU context.
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 968
Quoted 3 lines
arch/arm64/kvm/guest.c +3
-0+/* Check address read */ + asmvolatile ("at s1e1r, %0" ::"r" (gva)); +isb ();
The AT instruction can generate an exception, which is why __kvm_at() exists.
And this is where reusing the existing translation infrastructure is really important. The AT instruction *will* fail if the stage-1 translation tables are unmapped at stage-2. The only option at that point is falling back to a software table walker that potentially faults in the missing translation tables.
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 973
Quoted 5 lines
arch/arm64/kvm/guest.c +5
-0+ + par =read_sysreg (par_el1); +if (!(par &1 )) { + tr->valid = true; + tr->physical_address =par_to_ipa (par, gva);
What about permissions besides RW?
arch/arm64/kvm/guest.c int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) · line 1022
Quoted 7 lines
arch/arm64/kvm/guest.c +6
-0+int kvm_arch_vcpu_ioctl_translate (struct kvm_vcpu *vcpu,struct kvm_translation *tr) +{ +if (has_vhe ()) +return kvm_translate_vhe (vcpu, tr); +else +return kvm_translate_nvhe (vcpu, tr); }
Yet another interesting consideration around this entire infrastructure is the guest's view of the translation that the VMM will now use. KVM uses a pseudo-TLB for the guest's VNCR page and maintains it just like a literal TLB.
How would the guest invalidate the translation fetched by the VMM when unmapping/remapping the VA? Doesn't the stage-1 PTW need to set the Access flag as this amounts to a TLB fill?
Understanding what it is you're trying to accomplish would be quite helpful. I'm concerned this trivializes some of the gory details of participating in the guest's virtual memory.
Thanks,
Oliver
Marc Zyngier
maintainerHi Priscilla,
Quoted 12 lines · Priscilla Lam
There is a
KVM_TRANSLATEioctl for x86 to translate a GVA (guest virtual address) to a GPA (guest physical address) in EL1 which is not yet implemented for arm64.Implement
KVM_TRANSLATEon arm64 for both configurations that support and do not support VHE. The VHE path uses the AT instruction directly while the non-VHE implementation wraps the AT call in a hypercall to allow for its execution in EL2. Add selftest that tests the ioctl in both configurations.Signed-off-by Priscilla Lam <[email protected]>
Quoted 11 lines
--- arch/arm64/include/asm/kvm_asm.h | 2 + arch/arm64/kvm/guest.c | 89 ++++++++++++++- arch/arm64/kvm/hyp/nvhe/Makefile | 3 +- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 10 ++ arch/arm64/kvm/hyp/nvhe/translate.c | 84 ++++++++++++++ tools/testing/selftests/kvm/Makefile.kvm | 1 + tools/testing/selftests/kvm/arm64/translate.c | 107 ++++++++++++++++++ 7 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 arch/arm64/kvm/hyp/nvhe/translate.c create mode 100644 tools/testing/selftests/kvm/arm64/translate.c
Oliver already gave a review of some aspects of the code. In short, you're going about walking the page tables the wrong way, both by ignoring some of the complicated architectural features (PIE, POE), trusting the S1 PTs to be vaguely correct, and by assuming that S1 PTs are never swapped out.
But there is more: you are assuming that the only translation regime we care about is EL1&0, which isn't true. This would also need to cater for EL2 and EL2&0.
As Oliver also pointed out, all that infrastructure already exists in the kernel, and is able to do the right thing, with full support of the expected feature set as presented to the guest.
But at the end of the day, what do you need KVM_TRANSLATE for? This interface is an absolute turd that is unable to represent the bare minimum of the architecture (writable by whom? physical address in which translation regime? what about S2 translations?), and is better left in the "utter brain fart" category.
Finally, and assuming that you actually need something of the sort, why do you expect that something in the kernel will be better than a pure userspace implementation?
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Hi Oliver and Marc,
Thanks for the detailed feedback.
Quoted 5 lines
But at the end of the day, what do you need
KVM_TRANSLATEfor? This interface is an absolute turd that is unable to represent the bare minimum of the architecture (writable by whom? physical address in which translation regime? what about S2 translations?), and is better left in the "utter brain fart" category.
Regarding motivation, this patch is intended to give a userspace vmm the ability to handle non-ISV guest faults. The Arm Arm (DDI 0487L.b, section B3.13.6) notes that for load/store pair faults, the syndrome may not provide the specifics of the access that faulted. In those cases, the vmm must manually decode the instruction to emulate it. The introduction of KVM_CAP_ARM_NISV_TO_USER
(https://lore.kernel.org/kvm/[email protected]/)
seems to have anticipated that flow by allowing exits to userspace on trapped NISV instructions. What is still missing is a reliable way for userspace to query VA->IPA translations in order to complete emulation.
Quoted 1 line
Please do selftests changes in a separate patch.
Ack, will split the kernel changes and selftests into 1/2 and 2/2.
Quoted 3 lines
So
arch/arm64/kvm/at.cexists for this exact purpose: walking guest page tables. While it was previously constrained to handling NV-enabled VMs, Marc's SEA TTW series opens up the stage-1 walker for general use.
Thanks for the reference, I wasn't aware of this. I'll drop the bespoke VHE/NVHE paths and use the shared S1 walker in v2.
Quoted 3 lines
"
linear_address" is a delightful x86-ism. I'd prefer naming that was either architecture-generic -or- an arm64-specific struct that used our architectural terms.
I'll switch internal naming to VA/IPA. For uAPI, I'll retain the field for compatibility and translate internally.
Quoted 2 lines
Thanks to borken hardware, this needs to go through the
write_sysreg_hcr()accessor.
Ack, will use write_sysreg_hcr().
Quoted 2 lines
KVM supports both
FEAT_S1PIEandFEAT_S1POE, so this is not a complete MMU context.
Understood. v2 will rely on the shared walker to avoid missing S1PIE/S1POE.
Quoted 8 lines
The AT instruction can generate an exception, which is why
__kvm_at()exists.And this is where reusing the existing translation infrastructure is really important. The AT instruction *will* fail if the stage-1 translation tables are unmapped at stage-2. The only option at that point is falling back to a software table walker that potentially faults in the missing translation tables.
v2 will use __kvm_at() and the fallback software walk.
Quoted 1 line
What about permissions besides RW?
I'll add support for the additional bit (execute and EL0) in v2.
Quoted 11 lines
Yet another interesting consideration around this entire infrastructure is the guest's view of the translation that the VMM will now use. KVM uses a pseudo-TLB for the guest's VNCR page and maintains it just like a literal TLB.
How would the guest invalidate the translation fetched by the VMM when unmapping/remapping the VA? Doesn't the stage-1 PTW need to set the Access flag as this amounts to a TLB fill?
Understanding what it is you're trying to accomplish would be quite helpful. I'm concerned this trivializes some of the gory details of participating in the guest's virtual memory.
My intent is for this ioctl to be side-effect free with no AF updates and guest-visible TLB fills. I’ll send v2 as two patches with the above changes.
Thanks,
Priscilla
Hi Priscilla,
Quoted 9 lines · Priscilla Lam
Hi Oliver and Marc,
Thanks for the detailed feedback.
Quoted 5 lines
But at the end of the day, what do you need
KVM_TRANSLATEfor? This interface is an absolute turd that is unable to represent the bare minimum of the architecture (writable by whom? physical address in which translation regime? what about S2 translations?), and is better left in the "utter brain fart" category.
Quoted 10 lines
Regarding motivation, this patch is intended to give a userspace vmm the ability to handle non-ISV guest faults. The Arm Arm (DDI 0487L.b, section B3.13.6) notes that for load/store pair faults, the syndrome may not provide the specifics of the access that faulted. In those cases, the vmm must manually decode the instruction to emulate it. The introduction of
KVM_CAP_ARM_NISV_TO_USER
(https://lore.kernel.org/kvm/[email protected]/)
seems to have anticipated that flow by allowing exits to userspace on trapped NISV instructions. What is still missing is a reliable way for userspace to queryVA->IPAtranslations in order to complete emulation.
All modern OSs constrain themselves to only ISV enabled MMIO instructions. NISV is really only useful to run non hypervisor enlightened guests. Did you encounter a real world OS that was causing NISV faults? And if so, what was causing these? It's most likely a driver or OS bug.
Alex
Amazon Web Services Development Center Germany GmbH Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
Quoted 3 lines · Alexander Graf
All modern OSs constrain themselves to only ISV enabled MMIO instructions.
Why? Such does not appear to be required by the architecture, AFAICT?
And what is "MMIO" architecturally anyway? Using LDP is actually fine on MMIO, if a PCI BAR is passed through to a guest (and assuming there are no ordering dependencies on the loads). It's the fact that it's missing from stage2 page tables and causes a guest exit which is the problem.
Other than lying to a compiler and *pretending* there are ordering constraints, or explicitly using inline asm, how would a guest OS *prevent* the compiler from using problematic instructions?
Quoted 4 lines
NISV is really only useful to run non hypervisor
enlightened guests. Did you encounter a real world OS that was causing NISV faults? And if so, what was causing these? It's most likely a driver or OS bug.
The Windows CRB TPM driver uses LDP. Even if it *were* forbidden by the architecture, the track record of getting Microsoft to fix bugs *even* when they are egregious violations of both specifications and common sense is... poor.
Quoted 10 lines · David Woodhouse
Quoted 3 lines · Alexander Graf
All modern OSs constrain themselves to only ISV enabled MMIO instructions.
Why? Such does not appear to be required by the architecture, AFAICT?
FYI, we are in the process of clarifying this area of limitation the architecture’s support for virtualization.
There is a note on the expectations for LDP and STP here:
https://developer.arm.com/documentation/102374/0103/Loads-and-stores---load-pair-and-store-pair?lang=en
Thanks,
Christoffer
Marc Zyngier
maintainerQuoted 11 lines · Priscilla Lam
Hi Oliver and Marc,
Thanks for the detailed feedback.
Quoted 5 lines
But at the end of the day, what do you need
KVM_TRANSLATEfor? This interface is an absolute turd that is unable to represent the bare minimum of the architecture (writable by whom? physical address in which translation regime? what about S2 translations?), and is better left in the "utter brain fart" category.
Quoted 10 lines
Regarding motivation, this patch is intended to give a userspace vmm the ability to handle non-ISV guest faults. The Arm Arm (DDI 0487L.b, section B3.13.6) notes that for load/store pair faults, the syndrome may not provide the specifics of the access that faulted. In those cases, the vmm must manually decode the instruction to emulate it. The introduction of
KVM_CAP_ARM_NISV_TO_USER
(https://lore.kernel.org/kvm/[email protected]/)
seems to have anticipated that flow by allowing exits to userspace on trapped NISV instructions. What is still missing is a reliable way for userspace to queryVA->IPAtranslations in order to complete emulation.
A guest doing this is a sure indication that it is completely broken, and will fail on actual HW, because it clearly ignores small insignificant details such as *ordering*.
My other question still remains: why can't you perform this page table walk in userspace? It is actually much safer to do so because you can stop other vcpus while inspecting the PTs, and avoid a vcpu playing tricks behind your back -- something the in-kernel PTW doesn't try to avoid.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Quoted 9 lines · Marc Zyngier
A guest doing this is a sure indication that it is completely broken, and will fail on actual HW, because it clearly ignores small insignificant details such as *ordering*.
My other question still remains: why can't you perform this page table walk in userspace? It is actually much safer to do so because you can stop other vcpus while inspecting the PTs, and avoid a vcpu playing tricks behind your back -- something the in-kernel PTW doesn't try to avoid.
In our case, this comes from the Windows TPM driver using ldp32 to read static adjacent CRB fields where ordering doesn't matter, but I agree that a guest relying on this kind of translation is problematic. We'll drop this KVM_TRANSLATE implementation and pursue a different approach instead.
Thanks,
Priscilla