Re: [PATCH] KVM: x86/mmu: consider the hva in mmu_notifer retry
+Cc the other architectures, I'm guessing this would be a helpful optimization for all archs. Quite a few comments, but they're all little more than nits. Nice! On Mon, Jan 25, 2021, David Stevens wrote: > From: David Stevens > > Use the range passed to mmu_notifer's invalidate_range_start to prevent s/mmu_notifer/mmu_notifier. And maybe avoid calling out invalidate_range_start() by name? It took me a few reads to understand it's referring to the function, i.e. the start of the invalidation, not the start of the range. > spurious page fault retries due to changes in unrelated host virtual > addresses. This needs to elaborate on the exact scenario this is handling, as is it sounds like KVM is tracking the history of invalidations or something. Understanding this patch requires a priori knowledge of mmu_notifier_count. Something like: Track the range being invalidated by mmu_notifier and skip page fault retries if the fault address is not affected by the in-progress invalidation. Disable the optimization if multiple invalidations are in-progress to keep things simple, as tracking multiple ranges has diminishing returns. > This has the secondary effect of greatly reducing the likelihood of extreme Out of curiosity, is this really the _secondary_ effect? I would expect this change to primarily benefit scenarios where the invalidation has gotten waylaid for whatever reason. > latency when handing a page fault due to another thread having been preempted > while modifying host virtual addresses. > > Signed-off-by: David Stevens > --- ... > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 6d16481aa29d..79166288ed03 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -3658,8 +3658,8 @@ static bool kvm_arch_setup_async_pf(struct kvm_vcpu > *vcpu, gpa_t cr2_or_gpa, > } > > static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn, > - gpa_t cr2_or_gpa, kvm_pfn_t *pfn, bool write, > - bool *writable) > + gpa_t cr2_or_gpa, kvm_pfn_t *pfn, hva_t *hva, > + bool write, bool *writable) Side topic, I'm all for creating a 'struct kvm_page_fault' or whatever to hold all these variables. The helper functions stacks are getting unwieldy. Definitely doesn't need to be addressed here, this just reminded of how ugly these stacks are. > { > struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn); > bool async; > @@ -3672,7 +3672,8 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool > prefault, gfn_t gfn, > } > > async = false; > - *pfn = __gfn_to_pfn_memslot(slot, gfn, false, &async, write, writable); > + *pfn = __gfn_to_pfn_memslot(slot, gfn, false, &async, > + write, writable, hva); > if (!async) > return false; /* *pfn has correct page already */ > > @@ -3686,7 +3687,8 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool > prefault, gfn_t gfn, > return true; > } > > - *pfn = __gfn_to_pfn_memslot(slot, gfn, false, NULL, write, writable); > + *pfn = __gfn_to_pfn_memslot(slot, gfn, false, NULL, > + write, writable, hva); > return false; > } > > @@ -3699,6 +3701,7 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, > gpa_t gpa, u32 error_code, > gfn_t gfn = gpa >> PAGE_SHIFT; > unsigned long mmu_seq; > kvm_pfn_t pfn; > + hva_t hva; > int r; > > if (page_fault_handle_page_track(vcpu, error_code, gfn)) > @@ -3717,7 +3720,8 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, > gpa_t gpa, u32 error_code, > mmu_seq = vcpu->kvm->mmu_notifier_seq; > smp_rmb(); > > - if (try_async_pf(vcpu, prefault, gfn, gpa, &pfn, write, &map_writable)) > + if (try_async_pf(vcpu, prefault, gfn, gpa, &pfn, &hva, > + write, &map_writable)) > return RET_PF_RETRY; > > if (handle_abnormal_pfn(vcpu, is_tdp ? 0 : gpa, gfn, pfn, ACC_ALL, &r)) > @@ -3725,7 +3729,7 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, > gpa_t gpa, u32 error_code, > > r = RET_PF_RETRY; > spin_lock(&vcpu->kvm->mmu_lock); > - if (mmu_notifier_retry(vcpu->kvm, mmu_seq)) > + if (mmu_notifier_retry_hva(vcpu->kvm, mmu_seq, hva)) > goto out_unlock; > r = make_mmu_pages_available(vcpu); > if (r) > diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h > index 50e268eb8e1a..3171784139a4 100644 > --- a/arch/x86/kvm/mmu/paging_tmpl.h > +++ b/arch/x86/kvm/mmu/paging_tmpl.h > @@ -790,6 +790,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gpa_t > addr, u32 error_code, > struct guest_walker walker; > int r; > kvm_pfn_t pfn; > + hva_t hva; > unsigned long mmu_seq; > bool map_writable, is_self_change_mapping; >
Re: (subset) [PATCH v6 0/5] ARM: arm64: Add SMCCC TRNG entropy service
On Wed, 6 Jan 2021 10:34:48 +, Andre Przywara wrote: > a fix to v5, now *really* fixing the wrong priority of SMCCC vs. RNDR > in arch_get_random_seed_long_early(). Apologies for messing this up > in v5 and thanks to broonie for being on the watch! > > Will, Catalin: it would be much appreciated if you could consider taking > patch 1/5. This contains the common definitions, and is a prerequisite > for every other patch, although they are somewhat independent and likely > will need to go through different subsystems. > > [...] Applied to kvm-arm64/rng-5.12, thanks! [5/5] KVM: arm64: implement the TRNG hypervisor call commit: a8e190cdae1bf8e9e490776b8179babc1962bb25 Cheers, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 1/2] KVM: arm64: Simplify __kvm_hyp_init HVC detection
The arguments for __do_hyp_init are now passed with a pointer to a struct which means there are scratch registers available for use. Thanks to this, we no longer need to use clever, but hard to read, tricks that avoid the need for scratch registers when checking for the __kvm_hyp_init HVC. Tested-by: David Brazdil Signed-off-by: Andrew Scull --- arch/arm64/kvm/hyp/nvhe/hyp-init.S | 15 --- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S index 31b060a44045..b3915ccb23b0 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S @@ -55,17 +55,10 @@ __do_hyp_init: cmp x0, #HVC_STUB_HCALL_NR b.lo__kvm_handle_stub_hvc - // We only actively check bits [24:31], and everything - // else has to be zero, which we check at build time. -#if (KVM_HOST_SMCCC_FUNC(__kvm_hyp_init) & 0x00FF) -#error Unexpected __KVM_HOST_SMCCC_FUNC___kvm_hyp_init value -#endif - - ror x0, x0, #24 - eor x0, x0, #((KVM_HOST_SMCCC_FUNC(__kvm_hyp_init) >> 24) & 0xF) - ror x0, x0, #4 - eor x0, x0, #((KVM_HOST_SMCCC_FUNC(__kvm_hyp_init) >> 28) & 0xF) - cbz x0, 1f + mov x3, #KVM_HOST_SMCCC_FUNC(__kvm_hyp_init) + cmp x0, x3 + b.eq1f + mov x0, #SMCCC_RET_NOT_SUPPORTED eret -- 2.30.0.280.ga3ce27912f-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 2/2] KVM: arm64: Don't clobber x4 in __do_hyp_init
arm_smccc_1_1_hvc() only adds write contraints for x0-3 in the inline assembly for the HVC instruction so make sure those are the only registers that change when __do_hyp_init is called. Tested-by: David Brazdil Signed-off-by: Andrew Scull --- arch/arm64/kvm/hyp/nvhe/hyp-init.S | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S index b3915ccb23b0..3df583ad12af 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S @@ -47,6 +47,8 @@ __invalid: b . /* +* Only uses x0..x3 so as to not clobber callee-saved SMCCC registers. +* * x0: SMCCC function ID * x1: struct kvm_nvhe_init_params PA */ @@ -63,9 +65,9 @@ __do_hyp_init: eret 1: mov x0, x1 - mov x4, lr - bl ___kvm_hyp_init - mov lr, x4 + mov x3, lr + bl ___kvm_hyp_init // Clobbers x0..x2 + mov lr, x3 /* Hello, World! */ mov x0, #SMCCC_RET_SUCCESS @@ -75,8 +77,8 @@ SYM_CODE_END(__kvm_hyp_init) /* * Initialize the hypervisor in EL2. * - * Only uses x0..x3 so as to not clobber callee-saved SMCCC registers - * and leave x4 for the caller. + * Only uses x0..x2 so as to not clobber callee-saved SMCCC registers + * and leave x3 for the caller. * * x0: struct kvm_nvhe_init_params PA */ @@ -105,9 +107,9 @@ alternative_else_nop_endif /* * Set the PS bits in TCR_EL2. */ - ldr x1, [x0, #NVHE_INIT_TCR_EL2] - tcr_compute_pa_size x1, #TCR_EL2_PS_SHIFT, x2, x3 - msr tcr_el2, x1 + ldr x0, [x0, #NVHE_INIT_TCR_EL2] + tcr_compute_pa_size x0, #TCR_EL2_PS_SHIFT, x1, x2 + msr tcr_el2, x0 isb @@ -186,7 +188,7 @@ SYM_CODE_START_LOCAL(__kvm_hyp_init_cpu) /* Enable MMU, set vectors and stack. */ mov x0, x28 - bl ___kvm_hyp_init // Clobbers x0..x3 + bl ___kvm_hyp_init // Clobbers x0..x2 /* Leave idmap. */ mov x0, x29 -- 2.30.0.280.ga3ce27912f-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 0/2] __do_hyp_init fix and tweak
These apply on 5.11-rc4. The second is a fix for a clobbered register, the first is more cosmetic. Thanks, David, for helping with build and boot tests after my logistical issues. Andrew Scull (2): KVM: arm64: Simplify __kvm_hyp_init HVC detection KVM: arm64: Don't clobber x4 in __do_hyp_init arch/arm64/kvm/hyp/nvhe/hyp-init.S | 35 +- 1 file changed, 15 insertions(+), 20 deletions(-) -- 2.30.0.280.ga3ce27912f-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH] KVM: arm64: Avoid unconditional PMU register access
Hi Andre, On 2021-01-18 17:30, Andre Przywara wrote: The ARM PMU is an optional CPU feature, so we should consult the CPUID registers before accessing any PMU related registers. However the KVM code accesses some PMU registers (PMUSERENR_EL0 and PMSEL_EL0) unconditionally, when activating the traps. This wasn't a problem so far, because every(?) silicon implements the PMU, with KVM guests being the lone exception, and those never ran KVM host code. As this is about to change with nested virt, we need to guard PMU register accesses with a proper CPU feature check. Add a new CPU capability, which marks whether we have at least the basic PMUv3 registers available. Use that in the KVM VHE code to check before accessing the PMU registers. Signed-off-by: Andre Przywara --- Hi, not sure a new CPU capability isn't a bit over the top here, and we should use a simple static key instead? Yes, I think this is a bit excessive, specially as we already have a predicate for the HW having a PMU (or the PMU being able on the host, which amounts to the same thing). I'm definitely not keen on adding more another one that has slightly different semantics. How about this instead, completely untested? Thanks, M. diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h index 23f1a557bd9f..5aa9ed1e9ec6 100644 --- a/arch/arm64/kernel/image-vars.h +++ b/arch/arm64/kernel/image-vars.h @@ -101,6 +101,9 @@ KVM_NVHE_ALIAS(__stop___kvm_ex_table); /* Array containing bases of nVHE per-CPU memory regions. */ KVM_NVHE_ALIAS(kvm_arm_hyp_percpu_base); +/* PMU available static key */ +KVM_NVHE_ALIAS(kvm_arm_pmu_available); + #endif /* CONFIG_KVM */ #endif /* __ARM64_KERNEL_IMAGE_VARS_H */ diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 54f4860cd87c..6c1f51f25eb3 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -90,15 +90,18 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) * counter, which could make a PMXEVCNTR_EL0 access UNDEF at * EL1 instead of being trapped to EL2. */ - write_sysreg(0, pmselr_el0); - write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0); + if (kvm_arm_support_pmu_v3()) { + write_sysreg(0, pmselr_el0); + write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0); + } write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); } static inline void __deactivate_traps_common(void) { write_sysreg(0, hstr_el2); - write_sysreg(0, pmuserenr_el0); + if (kvm_arm_support_pmu_v3()) + write_sysreg(0, pmuserenr_el0); } static inline void ___activate_traps(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c index d45b8b9a4415..198fa4266b2d 100644 --- a/arch/arm64/kvm/perf.c +++ b/arch/arm64/kvm/perf.c @@ -11,6 +11,8 @@ #include +DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available); + static int kvm_is_in_guest(void) { return kvm_get_running_vcpu() != NULL; @@ -48,6 +50,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = { int kvm_perf_init(void) { + /* +* Check if HW_PERF_EVENTS are supported by checking the number of +* hardware performance counters. This could ensure the presence of +* a physical PMU and CONFIG_PERF_EVENT is selected. +*/ + if (perf_num_counters() > 0) + static_branch_enable(&kvm_arm_pmu_available); + return perf_register_guest_info_callbacks(&kvm_guest_cbs); } diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 247422ac78a9..8d5ff7f3d416 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -817,16 +817,6 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1) return val & mask; } -bool kvm_arm_support_pmu_v3(void) -{ - /* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ - return (perf_num_counters() > 0); -} - int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu) { if (!kvm_vcpu_has_pmu(vcpu)) diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h index 8dcb3e1477bc..45631af820cd 100644 --- a/include/kvm/arm_pmu.h +++ b/include/kvm/arm_pmu.h @@ -13,6 +13,13 @@ #define ARMV8_PMU_CYCLE_IDX(ARMV8_PMU_MAX_COUNTERS - 1) #define ARMV8_PMU_MAX_COUNTER_PAIRS((ARMV8_PMU_MAX_COUNTERS + 1) >> 1) +DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available); + +static __always_inline bool kvm_arm_support_pmu_v3(void) +{ + return static_branch_likely(&kvm_arm_pmu_available); +} + #ifdef CONFIG_HW_PERF_EVENTS struct kvm_pmc { @@ -47,7 +54,6 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val); void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 v
Re: [PATCH v5 14/21] arm64: Honor VHE being disabled from the command-line
On Mon, Jan 25, 2021 at 10:50:12AM +, Marc Zyngier wrote: > Finally we can check whether VHE is disabled on the command line, > and not enable it if that's the user's wish. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Acked-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 13/21] arm64: Allow ID_AA64MMFR1_EL1.VH to be overridden from the command line
On Mon, Jan 25, 2021 at 10:50:11AM +, Marc Zyngier wrote: > As we want to be able to disable VHE at runtime, let's match > "id_aa64mmfr1.vh=" from the command line as an override. > This doesn't have much effect yet as our boot code doesn't look > at the cpufeature, but only at the HW registers. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Reviewed-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 12/21] arm64: cpufeature: Add an early command-line cpufeature override facility
On Mon, Jan 25, 2021 at 10:50:10AM +, Marc Zyngier wrote: > In order to be able to override CPU features at boot time, > let's add a command line parser that matches options of the > form "cpureg.feature=value", and store the corresponding > value into the override val/mask pair. > > No features are currently defined, so no expected change in > functionality. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Reviewed-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 08/12] arm/arm64: gic: Split check_acked() into two functions
Hi Andre, On 12/18/20 3:52 PM, André Przywara wrote: > On 17/12/2020 14:13, Alexandru Elisei wrote: >> check_acked() has several peculiarities: is the only function among the >> check_* functions which calls report() directly, it does two things >> (waits for interrupts and checks for misfired interrupts) and it also >> mixes printf, report_info and report calls. >> >> check_acked() also reports a pass and returns as soon all the target CPUs >> have received interrupts, However, a CPU not having received an interrupt >> *now* does not guarantee not receiving an erroneous interrupt if we wait >> long enough. >> >> Rework the function by splitting it into two separate functions, each with >> a single responsibility: wait_for_interrupts(), which waits for the >> expected interrupts to fire, and check_acked() which checks that interrupts >> have been received as expected. >> >> wait_for_interrupts() also waits an extra 100 milliseconds after the >> expected interrupts have been received in an effort to make sure we don't >> miss misfiring interrupts. >> >> Splitting check_acked() into two functions will also allow us to >> customize the behavior of each function in the future more easily >> without using an unnecessarily long list of arguments for check_acked(). > Yes, splitting this up looks much better, in general this is a nice > cleanup, thank you! > > Some comments below: > >> CC: Andre Przywara >> Signed-off-by: Alexandru Elisei >> --- >> arm/gic.c | 73 +++ >> 1 file changed, 47 insertions(+), 26 deletions(-) >> >> diff --git a/arm/gic.c b/arm/gic.c >> index ec733719c776..a9ef1a5def56 100644 >> --- a/arm/gic.c >> +++ b/arm/gic.c >> @@ -62,41 +62,42 @@ static void stats_reset(void) >> } >> } >> >> -static void check_acked(const char *testname, cpumask_t *mask) >> +static void wait_for_interrupts(cpumask_t *mask) >> { >> -int missing = 0, extra = 0, unexpected = 0; >> int nr_pass, cpu, i; >> -bool bad = false; >> >> /* Wait up to 5s for all interrupts to be delivered */ >> -for (i = 0; i < 50; ++i) { >> +for (i = 0; i < 50; i++) { >> mdelay(100); >> nr_pass = 0; >> for_each_present_cpu(cpu) { >> +/* >> + * A CPU having received more than one interrupts will >> + * show up in check_acked(), and no matter how long we >> + * wait it cannot un-receive it. Consider at least one >> + * interrupt as a pass. >> + */ >> nr_pass += cpumask_test_cpu(cpu, mask) ? >> -acked[cpu] == 1 : acked[cpu] == 0; >> -smp_rmb(); /* pairs with smp_wmb in ipi_handler */ >> - >> -if (bad_sender[cpu] != -1) { >> -printf("cpu%d received IPI from wrong sender >> %d\n", >> -cpu, bad_sender[cpu]); >> -bad = true; >> -} >> - >> -if (bad_irq[cpu] != -1) { >> -printf("cpu%d received wrong irq %d\n", >> -cpu, bad_irq[cpu]); >> -bad = true; >> -} >> +acked[cpu] >= 1 : acked[cpu] == 0; > > I wonder if this logic was already flawed to begin with: For interrupts > we expect to fire, we wait for up to 5 seconds (really that long?), but > for interrupts we expect *not* to fire we are OK if they don't show up > in the first 100 ms. That does not sound consistent. There are two ways that I see to fix this: - Have the caller wait for however long it sees fit, and *after* that waiting period call wait_for_interrupts(). - Pass a flag to wait_for_interrupts() to specify that the behaviour should be to wait for the entire duration instead of until the expected interrupts have been received. Neither sounds appealing to me for inclusion in this patch set, since I want to concentrate on reworking check_acked() while keeping much of the current behaviour intact. > > I am wondering if we should *not* have the initial 100ms wait at all, > since most interrupts will fire immediately (especially in KVM). And > then have *one* extra wait for, say 100ms, to cover latecomers and > spurious interrupts. I don't think it really matters where the 100 millisecond delay is in the loop. If we call wait_for_interrupts() to actually check that interrupts have fired (as opposed to checking that they haven't been asserted), then at most we save 100ms when they are asserted before the start of the loop. I don't think the GIC spec guarantees that interrupts written to the LR registers will be presented to the CPU after the guest resumes, so it is conceivable that there might be a delay, thus ending up in waiting the extra 100ms even if the delay is at the end of the
Re: [kvm-unit-tests PATCH v2 10/12] arm64: gic: its-trigger: Don't trigger the LPI while it is pending
Hi Andre, On 12/18/20 6:15 PM, André Przywara wrote: > On 17/12/2020 14:13, Alexandru Elisei wrote: >> The its-trigger test checks that LPI 8195 is not delivered to the CPU while >> it is disabled at the ITS level. After that it is re-enabled and the test >> checks that the interrupt is properly asserted. After it's re-enabled and >> before the stats are examined, the test triggers the interrupt again, which >> can lead to the same interrupt being delivered twice: once after the >> configuration invalidation and before the INT command, and once after the >> INT command. >> >> Get rid of the INT command after the interrupt is re-enabled to prevent the > This is confusing to read, since you don't remove anything in the patch. > Can you reword this? Something like "Before explicitly triggering the > interrupt, check that the unmasking worked, ..." That's a good point, I'll reword it. > >> LPI from being asserted twice and add a separate check to test that the INT >> command still works for the now re-enabled LPI 8195. >> >> CC: Auger Eric >> Suggested-by: Zenghui Yu >> Signed-off-by: Alexandru Elisei > Otherwise this looks fine, but I think there is another flaw: There is > no requirement that an INV(ALL) is *needed* to update the status, it > could also update anytime (think: "cache invalidate"). > > The KVM ITS emulation *only* bothers to read the memory on an INV(ALL) > command, so that matches the test. But that's not how unit-tests should > work ;-) > > But that's a separate issue, just mentioning this to not forget about it. That's a good point, I must admit that I didn't check how caching is defined by the architecture. I would prefer creating a patch independent of this series to change what test_its_trigger() checks for, to get input from Eric just for that particular change. Thanks, Alex > > For this patch, with the message fixed: > > Reviewed-by: Andre Przywara > > Cheers, > Andre > >> --- >> arm/gic.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arm/gic.c b/arm/gic.c >> index fb91861900b7..aa3aa1763984 100644 >> --- a/arm/gic.c >> +++ b/arm/gic.c >> @@ -805,6 +805,9 @@ static void test_its_trigger(void) >> >> /* Now call the invall and check the LPI hits */ >> its_send_invall(col3); >> +lpi_stats_expect(3, 8195); >> +check_lpi_stats("dev2/eventid=20 pending LPI is received"); >> + >> lpi_stats_expect(3, 8195); >> its_send_int(dev2, 20); >> check_lpi_stats("dev2/eventid=20 now triggers an LPI"); >> ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 0/3] Some optimization for stage-2 translation
On Thu, 14 Jan 2021 20:13:47 +0800, Yanan Wang wrote: > This patch series(v3) make some optimization for stage-2 translation. > > About patch-1: > Procedures of hyp stage-1 map and guest stage-2 map are quite different, > but they are now tied closely by function kvm_set_valid_leaf_pte(). > So adjust the relative code for ease of code maintenance in the future. > > [...] Applied to kvm-arm64/concurrent-translation-fault, thanks! [1/3] KVM: arm64: Adjust partial code of hyp stage-1 map and guest stage-2 map commit: 8ed80051c8c31d1587722fdb3af16677eba9d693 [2/3] KVM: arm64: Filter out the case of only changing permissions from stage-2 map path commit: 694d071f8d85d504055540a27f0dbe9dbf44584e [3/3] KVM: arm64: Mark the page dirty only if the fault is handled successfully commit: 509552e65ae8287178a5cdea2d734dcd2d6380ab Cheers, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: (subset) [PATCH 0/2] __do_hyp_init fix and tweak
On Mon, 25 Jan 2021 14:54:13 +, Andrew Scull wrote: > These apply on 5.11-rc4. The second is a fix for a clobbered register, > the first is more cosmetic. > > Thanks, David, for helping with build and boot tests after my logistical > issues. > > Andrew Scull (2): > KVM: arm64: Simplify __kvm_hyp_init HVC detection > KVM: arm64: Don't clobber x4 in __do_hyp_init > > [...] Applied to kvm-arm64/misc-5.12, thanks! [1/2] KVM: arm64: Simplify __kvm_hyp_init HVC detection commit: 87b26801f02ca9d7a110eb598dae8cd5d3bcace2 Cheers, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: (subset) [PATCH 0/2] __do_hyp_init fix and tweak
On Mon, 25 Jan 2021 14:54:13 +, Andrew Scull wrote: > These apply on 5.11-rc4. The second is a fix for a clobbered register, > the first is more cosmetic. > > Thanks, David, for helping with build and boot tests after my logistical > issues. > > Andrew Scull (2): > KVM: arm64: Simplify __kvm_hyp_init HVC detection > KVM: arm64: Don't clobber x4 in __do_hyp_init > > [...] Applied to kvmarm-master/fixes, thanks! [2/2] KVM: arm64: Don't clobber x4 in __do_hyp_init commit: e500b805c39daff2670494fff94909d7e3d094d9 Cheers, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 11/12] lib: arm64: gic-v3-its: Add wmb() barrier before INT command
Hi Andre, On 12/18/20 6:36 PM, André Przywara wrote: > On 17/12/2020 14:13, Alexandru Elisei wrote: >> The ITS tests use the INT command like an SGI. The its_send_int() function >> kicks a CPU and then the test checks that the interrupt was observed as >> expected in check_lpi_stats(). This is done by using lpi_stats.observed and >> lpi_stats.expected, where the target CPU only writes to lpi_stats.observed, >> and the source CPU reads it and compares the values with >> lpi_stats.expected. >> >> The fact that the target CPU doesn't read data written by the source CPU >> means that we don't need to do inter-processor memory synchronization >> for that between the two at the moment. >> >> The acked array is used by its-pending-migration test, but the reset value >> for acked (zero) is the same as the initialization value for static >> variables, so memory synchronization is again not needed. >> >> However, that is all about to change when we modify all ITS tests to use >> the same functions as the IPI tests. Add a write memory barrier to >> its_send_int(), similar to the gicv3_ipi_send_mask(), which has similar >> semantics. > I agree to the requirement for having the barrier, but am not sure this > is the right place. Wouldn't it be better to have the barrier in the > callers? > > Besides: This command is written to the command queue (in normal > memory), then we notify the ITS via an MMIO writeq. And this one has a > "wmb" barrier already (though for other reasons). Had another look, and you're totally right: its_post_commands() already has a wmb() in writeq(), thank you for pointing it out. This makes the wmb() which I've added pointless, I'll drop the patch since it doesn't add useful. Thanks, Alex > > Cheers, > Andre > > >> Suggested-by: Auger Eric >> Signed-off-by: Alexandru Elisei >> --- >> lib/arm64/gic-v3-its-cmd.c | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/lib/arm64/gic-v3-its-cmd.c b/lib/arm64/gic-v3-its-cmd.c >> index 34574f71d171..32703147ee85 100644 >> --- a/lib/arm64/gic-v3-its-cmd.c >> +++ b/lib/arm64/gic-v3-its-cmd.c >> @@ -385,6 +385,12 @@ void __its_send_int(struct its_device *dev, u32 >> event_id, bool verbose) >> { >> struct its_cmd_desc desc; >> >> +/* >> + * The INT command is used by tests as an IPI. Ensure stores to Normal >> + * memory are visible to other CPUs before sending the LPI. >> + */ >> +wmb(); >> + >> desc.its_int_cmd.dev = dev; >> desc.its_int_cmd.event_id = event_id; >> desc.verbose = verbose; >> ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, 25 Jan 2021 at 15:28, Marc Zyngier wrote: > > On 2021-01-25 14:19, Ard Biesheuvel wrote: > > On Mon, 25 Jan 2021 at 14:54, Marc Zyngier wrote: > >> > >> On 2021-01-25 12:54, Ard Biesheuvel wrote: > > [...] > > >> > This struct now takes up > >> > - ~100 bytes for the characters themselves (which btw are not emitted > >> > into __initdata or __initconst) > >> > - 6x8 bytes for the char pointers > >> > - 6x24 bytes for the RELA relocations that annotate these pointers as > >> > quantities that need to be relocated at boot (on a kernel built with > >> > KASLR) > >> > > >> > I know it's only a drop in the ocean, but in this case, where the > >> > struct is statically declared and defined only once, and in the same > >> > place, we could easily turn this into > >> > > >> > static const struct { > >> >char alias[24]; > >> >char param[20]; > >> > }; > >> > > >> > and get rid of all the overhead. The only slightly annoying thing is > >> > that the array sizes need to be kept in sync with the largest instance > >> > appearing in the array, but this is easy when the struct type is > >> > declared in the same place where its only instance is defined. > >> > >> Fair enough. I personally find the result butt-ugly, but I agree > >> that it certainly saves some memory. Does the following work for > >> you? I can even give symbolic names to the various constants (how > >> generous of me! ;-). > >> > > > > To be honest, I was anticipating more of a discussion, but this looks > > reasonable to me. > > It looked like a reasonable ask: all the strings are completely useless > once the kernel has booted, and I'm the first to moan that I can't boot > an arm64 kernel with less than 60MB of RAM (OK, it's a pretty bloated > kernel...). > > > Does 'charfeature[80];' really need 80 bytes though? > > It really needs 75 bytes, because of this: > > { "arm64.nopauth", > "id_aa64isar1.gpi=0 id_aa64isar1.gpa=0 " > "id_aa64isar1.api=0 id_aa64isar1.apa=0" }, > > 80 is a round enough number. > Fair enough. This will inflate the struct substantially, but at least it's all __initconst data now, and it's all NUL bytes so it compresses much better than the pointers and RELA entries. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On 2021-01-25 14:19, Ard Biesheuvel wrote: On Mon, 25 Jan 2021 at 14:54, Marc Zyngier wrote: On 2021-01-25 12:54, Ard Biesheuvel wrote: [...] > This struct now takes up > - ~100 bytes for the characters themselves (which btw are not emitted > into __initdata or __initconst) > - 6x8 bytes for the char pointers > - 6x24 bytes for the RELA relocations that annotate these pointers as > quantities that need to be relocated at boot (on a kernel built with > KASLR) > > I know it's only a drop in the ocean, but in this case, where the > struct is statically declared and defined only once, and in the same > place, we could easily turn this into > > static const struct { >char alias[24]; >char param[20]; > }; > > and get rid of all the overhead. The only slightly annoying thing is > that the array sizes need to be kept in sync with the largest instance > appearing in the array, but this is easy when the struct type is > declared in the same place where its only instance is defined. Fair enough. I personally find the result butt-ugly, but I agree that it certainly saves some memory. Does the following work for you? I can even give symbolic names to the various constants (how generous of me! ;-). To be honest, I was anticipating more of a discussion, but this looks reasonable to me. It looked like a reasonable ask: all the strings are completely useless once the kernel has booted, and I'm the first to moan that I can't boot an arm64 kernel with less than 60MB of RAM (OK, it's a pretty bloated kernel...). Does 'charfeature[80];' really need 80 bytes though? It really needs 75 bytes, because of this: { "arm64.nopauth", "id_aa64isar1.gpi=0 id_aa64isar1.gpa=0 " "id_aa64isar1.api=0 id_aa64isar1.apa=0"}, 80 is a round enough number. Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, 25 Jan 2021 at 14:54, Marc Zyngier wrote: > > On 2021-01-25 12:54, Ard Biesheuvel wrote: > > On Mon, 25 Jan 2021 at 11:53, Marc Zyngier wrote: > >> > >> Given that the early cpufeature infrastructure has borrowed quite > >> a lot of code from the kaslr implementation, let's reimplement > >> the matching of the "nokaslr" option with it. > >> > >> Signed-off-by: Marc Zyngier > >> Acked-by: Catalin Marinas > >> Acked-by: David Brazdil > >> --- > >> arch/arm64/kernel/idreg-override.c | 15 + > >> arch/arm64/kernel/kaslr.c | 36 > >> ++ > >> 2 files changed, 17 insertions(+), 34 deletions(-) > >> > >> diff --git a/arch/arm64/kernel/idreg-override.c > >> b/arch/arm64/kernel/idreg-override.c > >> index cbb8eaa48742..3ccf51b84ba4 100644 > >> --- a/arch/arm64/kernel/idreg-override.c > >> +++ b/arch/arm64/kernel/idreg-override.c > >> @@ -31,8 +31,22 @@ static const struct ftr_set_desc mmfr1 __initdata = > >> { > >> }, > >> }; > >> > >> +extern struct arm64_ftr_override kaslr_feature_override; > >> + > >> +static const struct ftr_set_desc kaslr __initdata = { > > > > This should be __initconst not __initdata (below too) > > > >> + .name = "kaslr", > >> +#ifdef CONFIG_RANDOMIZE_BASE > >> + .override = &kaslr_feature_override, > >> +#endif > >> + .fields = { > >> + { "disabled", 0 }, > >> + {} > >> + }, > >> +}; > >> + > >> static const struct ftr_set_desc * const regs[] __initdata = { > >> &mmfr1, > >> + &kaslr, > >> }; > >> > >> static const struct { > >> @@ -41,6 +55,7 @@ static const struct { > >> } aliases[] __initdata = { > >> { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, > >> { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, > >> + { "nokaslr","kaslr.disabled=1" }, > >> }; > >> > > > > This struct now takes up > > - ~100 bytes for the characters themselves (which btw are not emitted > > into __initdata or __initconst) > > - 6x8 bytes for the char pointers > > - 6x24 bytes for the RELA relocations that annotate these pointers as > > quantities that need to be relocated at boot (on a kernel built with > > KASLR) > > > > I know it's only a drop in the ocean, but in this case, where the > > struct is statically declared and defined only once, and in the same > > place, we could easily turn this into > > > > static const struct { > >char alias[24]; > >char param[20]; > > }; > > > > and get rid of all the overhead. The only slightly annoying thing is > > that the array sizes need to be kept in sync with the largest instance > > appearing in the array, but this is easy when the struct type is > > declared in the same place where its only instance is defined. > > Fair enough. I personally find the result butt-ugly, but I agree > that it certainly saves some memory. Does the following work for > you? I can even give symbolic names to the various constants (how > generous of me! ;-). > To be honest, I was anticipating more of a discussion, but this looks reasonable to me. Does 'charfeature[80];' really need 80 bytes though? > diff --git a/arch/arm64/kernel/idreg-override.c > b/arch/arm64/kernel/idreg-override.c > index d1310438d95c..9e7043bdc808 100644 > --- a/arch/arm64/kernel/idreg-override.c > +++ b/arch/arm64/kernel/idreg-override.c > @@ -14,15 +14,15 @@ > #include > > struct ftr_set_desc { > - const char *name; > + charname[20]; > struct arm64_ftr_override *override; > struct { > - const char *name; > + charname[20]; > u8 shift; > } fields[]; > }; > > -static const struct ftr_set_desc mmfr1 __initdata = { > +static const struct ftr_set_desc mmfr1 __initconst = { > .name = "id_aa64mmfr1", > .override = &id_aa64mmfr1_override, > .fields = { > @@ -31,7 +31,7 @@ static const struct ftr_set_desc mmfr1 __initdata = { > }, > }; > > -static const struct ftr_set_desc pfr1 __initdata = { > +static const struct ftr_set_desc pfr1 __initconst = { > .name = "id_aa64pfr1", > .override = &id_aa64pfr1_override, > .fields = { > @@ -40,7 +40,7 @@ static const struct ftr_set_desc pfr1 __initdata = { > }, > }; > > -static const struct ftr_set_desc isar1 __initdata = { > +static const struct ftr_set_desc isar1 __initconst = { > .name = "id_aa64isar1", > .override = &id_aa64isar1_override, > .fields = { > @@ -54,7 +54,7 @@ static const struct ftr_set_desc isar1 __initdata = { > > extern struct arm64_ftr_override kaslr_feature_override; > > -static const struct ftr_set_desc kaslr __initdata = { > +static const struct ftr_se
[PATCH 0/2] Performance improvement about cache flush
Hi, This two patches are posted to introduce a new method that can distinguish cases of allocating memcache more precisely, and to elide some unnecessary cache flush. For patch-1: With a guest translation fault, we don't really need the memcache pages when only installing a new entry to the existing page table or replacing the table entry with a block entry. And with a guest permission fault, we also don't need the memcache pages for a write_fault in dirty-logging time if VMs are not configured with huge mappings. So a new method is introduced to distinguish cases of allocating memcache more precisely. For patch-2: If migration of a VM with hugepages is canceled midway, KVM will adjust the stage-2 table mappings back to block mappings. With multiple vCPUs accessing guest pages within the same 1G range, there could be numbers of translation faults to handle, and KVM will uniformly flush data cache for 1G range before handling the faults. As it will cost a long time to flush the data cache for 1G range of memory(130ms on Kunpeng 920 servers, for example), the consequent cache flush for each translation fault will finally lead to vCPU stuck for seconds or even a soft lockup. I have met both the stuck and soft lockup on Kunpeng servers with FWB not supported. When KVM need to recover the table mappings back to block mappings, as we only replace the existing page tables with a block entry and the cacheability has not been changed, the cache maintenance opreations can be skipped. Yanan Wang (2): KVM: arm64: Distinguish cases of allocating memcache more precisely KVM: arm64: Skip the cache flush when coalescing tables into a block arch/arm64/kvm/mmu.c | 37 + 1 file changed, 21 insertions(+), 16 deletions(-) -- 2.19.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 1/2] KVM: arm64: Distinguish cases of allocating memcache more precisely
With a guest translation fault, we don't really need the memcache pages when only installing a new entry to the existing page table or replacing the table entry with a block entry. And with a guest permission fault, we also don't need the memcache pages for a write_fault in dirty-logging time if VMs are not configured with huge mappings. The cases where allocations from memcache are required can be much more precisely distinguished by comparing fault_granule and vma_pagesize. Signed-off-by: Yanan Wang --- arch/arm64/kvm/mmu.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 7d2257cc5438..8e8549ea1d70 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -820,19 +820,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, gfn = fault_ipa >> PAGE_SHIFT; mmap_read_unlock(current->mm); - /* -* Permission faults just need to update the existing leaf entry, -* and so normally don't require allocations from the memcache. The -* only exception to this is when dirty logging is enabled at runtime -* and a write fault needs to collapse a block entry into a table. -*/ - if (fault_status != FSC_PERM || (logging_active && write_fault)) { - ret = kvm_mmu_topup_memory_cache(memcache, -kvm_mmu_cache_min_pages(kvm)); - if (ret) - return ret; - } - mmu_seq = vcpu->kvm->mmu_notifier_seq; /* * Ensure the read of mmu_notifier_seq happens before we call @@ -898,6 +885,18 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, else if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC)) prot |= KVM_PGTABLE_PROT_X; + /* +* Allocations from the memcache are required only when granule of the +* lookup level where a guest fault happened exceeds the vma_pagesize, +* which means new page tables will be created in the fault handlers. +*/ + if (fault_granule > vma_pagesize) { + ret = kvm_mmu_topup_memory_cache(memcache, +kvm_mmu_cache_min_pages(kvm)); + if (ret) + return ret; + } + /* * Under the premise of getting a FSC_PERM fault, we just need to relax * permissions only if vma_pagesize equals fault_granule. Otherwise, -- 2.19.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 2/2] KVM: arm64: Skip the cache flush when coalescing tables into a block
After dirty-logging is stopped for a VM configured with huge mappings, KVM will recover the table mappings back to block mappings. As we only replace the existing page tables with a block entry and the cacheability has not been changed, the cache maintenance opreations can be skipped. Signed-off-by: Yanan Wang --- arch/arm64/kvm/mmu.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 8e8549ea1d70..37b427dcbc4f 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -744,7 +744,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, { int ret = 0; bool write_fault, writable, force_pte = false; - bool exec_fault; + bool exec_fault, adjust_hugepage; bool device = false; unsigned long mmu_seq; struct kvm *kvm = vcpu->kvm; @@ -872,12 +872,18 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, mark_page_dirty(kvm, gfn); } - if (fault_status != FSC_PERM && !device) + /* +* There is no necessity to perform cache maintenance operations if we +* will only replace the existing table mappings with a block mapping. +*/ + adjust_hugepage = fault_granule < vma_pagesize ? true : false; + if (fault_status != FSC_PERM && !device && !adjust_hugepage) clean_dcache_guest_page(pfn, vma_pagesize); if (exec_fault) { prot |= KVM_PGTABLE_PROT_X; - invalidate_icache_guest_page(pfn, vma_pagesize); + if (!adjust_hugepage) + invalidate_icache_guest_page(pfn, vma_pagesize); } if (device) -- 2.19.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 13/21] arm64: Allow ID_AA64MMFR1_EL1.VH to be overridden from the command line
On 2021-01-25 13:15, Suzuki K Poulose wrote: On 1/25/21 10:50 AM, Marc Zyngier wrote: As we want to be able to disable VHE at runtime, let's match "id_aa64mmfr1.vh=" from the command line as an override. This doesn't have much effect yet as our boot code doesn't look at the cpufeature, but only at the HW registers. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/cpufeature.h | 2 ++ arch/arm64/kernel/cpufeature.c | 5 - arch/arm64/kernel/idreg-override.c | 11 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 4179cfc8ed57..b0ed37da4067 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -818,6 +818,8 @@ static inline unsigned int get_vmid_bits(u64 mmfr1) return 8; } +extern struct arm64_ftr_override id_aa64mmfr1_override; + u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 4b84a1e1dc51..c1d6712c4249 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -557,6 +557,8 @@ static const struct arm64_ftr_bits ftr_raz[] = { #define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, &no_override) +struct arm64_ftr_override id_aa64mmfr1_override; Does this need to be ro_after_init ? Could do, together with the other two override targeting system registers. Otherwise, looks good to me: Acked-by: Suzuki K Poulose Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On 2021-01-25 12:54, Ard Biesheuvel wrote: On Mon, 25 Jan 2021 at 11:53, Marc Zyngier wrote: Given that the early cpufeature infrastructure has borrowed quite a lot of code from the kaslr implementation, let's reimplement the matching of the "nokaslr" option with it. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/kernel/idreg-override.c | 15 + arch/arm64/kernel/kaslr.c | 36 ++ 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index cbb8eaa48742..3ccf51b84ba4 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -31,8 +31,22 @@ static const struct ftr_set_desc mmfr1 __initdata = { }, }; +extern struct arm64_ftr_override kaslr_feature_override; + +static const struct ftr_set_desc kaslr __initdata = { This should be __initconst not __initdata (below too) + .name = "kaslr", +#ifdef CONFIG_RANDOMIZE_BASE + .override = &kaslr_feature_override, +#endif + .fields = { + { "disabled", 0 }, + {} + }, +}; + static const struct ftr_set_desc * const regs[] __initdata = { &mmfr1, + &kaslr, }; static const struct { @@ -41,6 +55,7 @@ static const struct { } aliases[] __initdata = { { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, + { "nokaslr","kaslr.disabled=1" }, }; This struct now takes up - ~100 bytes for the characters themselves (which btw are not emitted into __initdata or __initconst) - 6x8 bytes for the char pointers - 6x24 bytes for the RELA relocations that annotate these pointers as quantities that need to be relocated at boot (on a kernel built with KASLR) I know it's only a drop in the ocean, but in this case, where the struct is statically declared and defined only once, and in the same place, we could easily turn this into static const struct { char alias[24]; char param[20]; }; and get rid of all the overhead. The only slightly annoying thing is that the array sizes need to be kept in sync with the largest instance appearing in the array, but this is easy when the struct type is declared in the same place where its only instance is defined. Fair enough. I personally find the result butt-ugly, but I agree that it certainly saves some memory. Does the following work for you? I can even give symbolic names to the various constants (how generous of me! ;-). Thanks, M. diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index d1310438d95c..9e7043bdc808 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -14,15 +14,15 @@ #include struct ftr_set_desc { - const char *name; + charname[20]; struct arm64_ftr_override *override; struct { - const char *name; + charname[20]; u8 shift; } fields[]; }; -static const struct ftr_set_desc mmfr1 __initdata = { +static const struct ftr_set_desc mmfr1 __initconst = { .name = "id_aa64mmfr1", .override = &id_aa64mmfr1_override, .fields = { @@ -31,7 +31,7 @@ static const struct ftr_set_desc mmfr1 __initdata = { }, }; -static const struct ftr_set_desc pfr1 __initdata = { +static const struct ftr_set_desc pfr1 __initconst = { .name = "id_aa64pfr1", .override = &id_aa64pfr1_override, .fields = { @@ -40,7 +40,7 @@ static const struct ftr_set_desc pfr1 __initdata = { }, }; -static const struct ftr_set_desc isar1 __initdata = { +static const struct ftr_set_desc isar1 __initconst = { .name = "id_aa64isar1", .override = &id_aa64isar1_override, .fields = { @@ -54,7 +54,7 @@ static const struct ftr_set_desc isar1 __initdata = { extern struct arm64_ftr_override kaslr_feature_override; -static const struct ftr_set_desc kaslr __initdata = { +static const struct ftr_set_desc kaslr __initconst = { .name = "kaslr", #ifdef CONFIG_RANDOMIZE_BASE .override = &kaslr_feature_override, @@ -65,7 +65,7 @@ static const struct ftr_set_desc kaslr __initdata = { }, }; -static const struct ftr_set_desc * const regs[] __initdata = { +static const struct ftr_set_desc * const regs[] __initconst = { &mmfr1, &pfr1, &isar1, @@ -73,9 +73,9 @@ static const struct ftr_set_desc * const regs[] __initdata = { }; static const struct { - const char *alias; - const char *feature; -} aliases[] __initdata = {
Re: [PATCH v5 13/21] arm64: Allow ID_AA64MMFR1_EL1.VH to be overridden from the command line
On 1/25/21 10:50 AM, Marc Zyngier wrote: As we want to be able to disable VHE at runtime, let's match "id_aa64mmfr1.vh=" from the command line as an override. This doesn't have much effect yet as our boot code doesn't look at the cpufeature, but only at the HW registers. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/cpufeature.h | 2 ++ arch/arm64/kernel/cpufeature.c | 5 - arch/arm64/kernel/idreg-override.c | 11 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 4179cfc8ed57..b0ed37da4067 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -818,6 +818,8 @@ static inline unsigned int get_vmid_bits(u64 mmfr1) return 8; } +extern struct arm64_ftr_override id_aa64mmfr1_override; + u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 4b84a1e1dc51..c1d6712c4249 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -557,6 +557,8 @@ static const struct arm64_ftr_bits ftr_raz[] = { #define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, &no_override) +struct arm64_ftr_override id_aa64mmfr1_override; Does this need to be ro_after_init ? Otherwise, looks good to me: Acked-by: Suzuki K Poulose ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, 25 Jan 2021 at 11:53, Marc Zyngier wrote: > > Given that the early cpufeature infrastructure has borrowed quite > a lot of code from the kaslr implementation, let's reimplement > the matching of the "nokaslr" option with it. > > Signed-off-by: Marc Zyngier > Acked-by: Catalin Marinas > Acked-by: David Brazdil > --- > arch/arm64/kernel/idreg-override.c | 15 + > arch/arm64/kernel/kaslr.c | 36 ++ > 2 files changed, 17 insertions(+), 34 deletions(-) > > diff --git a/arch/arm64/kernel/idreg-override.c > b/arch/arm64/kernel/idreg-override.c > index cbb8eaa48742..3ccf51b84ba4 100644 > --- a/arch/arm64/kernel/idreg-override.c > +++ b/arch/arm64/kernel/idreg-override.c > @@ -31,8 +31,22 @@ static const struct ftr_set_desc mmfr1 __initdata = { > }, > }; > > +extern struct arm64_ftr_override kaslr_feature_override; > + > +static const struct ftr_set_desc kaslr __initdata = { This should be __initconst not __initdata (below too) > + .name = "kaslr", > +#ifdef CONFIG_RANDOMIZE_BASE > + .override = &kaslr_feature_override, > +#endif > + .fields = { > + { "disabled", 0 }, > + {} > + }, > +}; > + > static const struct ftr_set_desc * const regs[] __initdata = { > &mmfr1, > + &kaslr, > }; > > static const struct { > @@ -41,6 +55,7 @@ static const struct { > } aliases[] __initdata = { > { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, > { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, > + { "nokaslr","kaslr.disabled=1" }, > }; > This struct now takes up - ~100 bytes for the characters themselves (which btw are not emitted into __initdata or __initconst) - 6x8 bytes for the char pointers - 6x24 bytes for the RELA relocations that annotate these pointers as quantities that need to be relocated at boot (on a kernel built with KASLR) I know it's only a drop in the ocean, but in this case, where the struct is statically declared and defined only once, and in the same place, we could easily turn this into static const struct { char alias[24]; char param[20]; }; and get rid of all the overhead. The only slightly annoying thing is that the array sizes need to be kept in sync with the largest instance appearing in the array, but this is easy when the struct type is declared in the same place where its only instance is defined. > static char *cmdline_contains_option(const char *cmdline, const char *option) > diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c > index 5fc86e7d01a1..27f8939deb1b 100644 > --- a/arch/arm64/kernel/kaslr.c > +++ b/arch/arm64/kernel/kaslr.c > @@ -51,39 +51,7 @@ static __init u64 get_kaslr_seed(void *fdt) > return ret; > } > > -static __init bool cmdline_contains_nokaslr(const u8 *cmdline) > -{ > - const u8 *str; > - > - str = strstr(cmdline, "nokaslr"); > - return str == cmdline || (str > cmdline && *(str - 1) == ' '); > -} > - > -static __init bool is_kaslr_disabled_cmdline(void *fdt) > -{ > - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { > - int node; > - const u8 *prop; > - > - node = fdt_path_offset(fdt, "/chosen"); > - if (node < 0) > - goto out; > - > - prop = fdt_getprop(fdt, node, "bootargs", NULL); > - if (!prop) > - goto out; > - > - if (cmdline_contains_nokaslr(prop)) > - return true; > - > - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > - goto out; > - > - return false; > - } > -out: > - return cmdline_contains_nokaslr(CONFIG_CMDLINE); > -} > +struct arm64_ftr_override kaslr_feature_override __initdata; > > /* > * This routine will be executed with the kernel mapped at its default > virtual > @@ -126,7 +94,7 @@ u64 __init kaslr_early_init(void) > * Check if 'nokaslr' appears on the command line, and > * return 0 if that is the case. > */ > - if (is_kaslr_disabled_cmdline(fdt)) { > + if (kaslr_feature_override.val & kaslr_feature_override.mask & 0xf) { > kaslr_status = KASLR_DISABLED_CMDLINE; > return 0; > } > -- > 2.29.2 > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 7/7] KVM: arm64: Use symbolic names for the PMU versions
Instead of using a bunch of magic numbers, use the existing definitions that have been added since 8673e02e58410 ("arm64: perf: Add support for ARMv8.5-PMU 64-bit counters") Signed-off-by: Marc Zyngier --- arch/arm64/kvm/pmu-emul.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 72cd704a8368..cb16ca2eee92 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -23,11 +23,11 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu *vcpu, struct kvm_pmc *pmc); static u32 kvm_pmu_event_mask(struct kvm *kvm) { switch (kvm->arch.pmuver) { - case 1: /* ARMv8.0 */ + case ID_AA64DFR0_PMUVER_8_0: return GENMASK(9, 0); - case 4: /* ARMv8.1 */ - case 5: /* ARMv8.4 */ - case 6: /* ARMv8.5 */ + case ID_AA64DFR0_PMUVER_8_1: + case ID_AA64DFR0_PMUVER_8_4: + case ID_AA64DFR0_PMUVER_8_5: return GENMASK(15, 0); default:/* Shouldn't be here, just for sanity */ WARN_ONCE(1, "Unknown PMU version %d\n", kvm->arch.pmuver); -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 4/7] KVM: arm64: Refactor filtering of ID registers
Our current ID register filtering is starting to be a mess of if() statements, and isn't going to get any saner. Let's turn it into a switch(), which has a chance of being more readable, and introduce a FEATURE() macro that allows easy generation of feature masks. No functionnal change intended. Reviewed-by: Eric Auger Signed-off-by: Marc Zyngier --- arch/arm64/kvm/sys_regs.c | 51 +-- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 2bea0494b81d..dda16d60197b 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -9,6 +9,7 @@ * Christoffer Dall */ +#include #include #include #include @@ -1016,6 +1017,8 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, return true; } +#define FEATURE(x) (GENMASK_ULL(x##_SHIFT + 3, x##_SHIFT)) + /* Read a sanitised cpufeature ID register by sys_reg_desc */ static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r, bool raz) @@ -1024,36 +1027,38 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, (u32)r->CRn, (u32)r->CRm, (u32)r->Op2); u64 val = raz ? 0 : read_sanitised_ftr_reg(id); - if (id == SYS_ID_AA64PFR0_EL1) { + switch (id) { + case SYS_ID_AA64PFR0_EL1: if (!vcpu_has_sve(vcpu)) - val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); - val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT); - val &= ~(0xfUL << ID_AA64PFR0_CSV2_SHIFT); - val |= ((u64)vcpu->kvm->arch.pfr0_csv2 << ID_AA64PFR0_CSV2_SHIFT); - val &= ~(0xfUL << ID_AA64PFR0_CSV3_SHIFT); - val |= ((u64)vcpu->kvm->arch.pfr0_csv3 << ID_AA64PFR0_CSV3_SHIFT); - } else if (id == SYS_ID_AA64PFR1_EL1) { - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); - } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) { - val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) | -(0xfUL << ID_AA64ISAR1_API_SHIFT) | -(0xfUL << ID_AA64ISAR1_GPA_SHIFT) | -(0xfUL << ID_AA64ISAR1_GPI_SHIFT)); - } else if (id == SYS_ID_AA64DFR0_EL1) { - u64 cap = 0; - + val &= ~FEATURE(ID_AA64PFR0_SVE); + val &= ~FEATURE(ID_AA64PFR0_AMU); + val &= ~FEATURE(ID_AA64PFR0_CSV2); + val |= FIELD_PREP(FEATURE(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2); + val &= ~FEATURE(ID_AA64PFR0_CSV3); + val |= FIELD_PREP(FEATURE(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3); + break; + case SYS_ID_AA64PFR1_EL1: + val &= ~FEATURE(ID_AA64PFR1_MTE); + break; + case SYS_ID_AA64ISAR1_EL1: + if (!vcpu_has_ptrauth(vcpu)) + val &= ~(FEATURE(ID_AA64ISAR1_APA) | +FEATURE(ID_AA64ISAR1_API) | +FEATURE(ID_AA64ISAR1_GPA) | +FEATURE(ID_AA64ISAR1_GPI)); + break; + case SYS_ID_AA64DFR0_EL1: /* Limit guests to PMUv3 for ARMv8.1 */ - if (kvm_vcpu_has_pmu(vcpu)) - cap = ID_AA64DFR0_PMUVER_8_1; - val = cpuid_feature_cap_perfmon_field(val, - ID_AA64DFR0_PMUVER_SHIFT, - cap); - } else if (id == SYS_ID_DFR0_EL1) { + ID_AA64DFR0_PMUVER_SHIFT, + kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_1 : 0); + break; + case SYS_ID_DFR0_EL1: /* Limit guests to PMUv3 for ARMv8.1 */ val = cpuid_feature_cap_perfmon_field(val, ID_DFR0_PERFMON_SHIFT, kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_1 : 0); + break; } return val; -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 6/7] KVM: arm64: Upgrade PMU support to ARMv8.4
Upgrading the PMU code from ARMv8.1 to ARMv8.4 turns out to be pretty easy. All that is required is support for PMMIR_EL1, which is read-only, and for which returning 0 is a valid option as long as we don't advertise STALL_SLOT as an implemented event. Let's just do that and adjust what we return to the guest. Signed-off-by: Marc Zyngier --- arch/arm64/include/asm/sysreg.h | 3 +++ arch/arm64/kvm/pmu-emul.c | 6 ++ arch/arm64/kvm/sys_regs.c | 11 +++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 8b5e7e5c3cc8..2fb3f386588c 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -846,7 +846,10 @@ #define ID_DFR0_PERFMON_SHIFT 24 +#define ID_DFR0_PERFMON_8_00x3 #define ID_DFR0_PERFMON_8_10x4 +#define ID_DFR0_PERFMON_8_40x5 +#define ID_DFR0_PERFMON_8_50x6 #define ID_ISAR4_SWP_FRAC_SHIFT28 #define ID_ISAR4_PSR_M_SHIFT 24 diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 398f6df1bbe4..72cd704a8368 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -795,6 +795,12 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1) base = 0; } else { val = read_sysreg(pmceid1_el0); + /* +* Don't advertise STALL_SLOT, as PMMIR_EL0 is handled +* as RAZ +*/ + if (vcpu->kvm->arch.pmuver >= ID_AA64DFR0_PMUVER_8_4) + val &= ~BIT_ULL(ARMV8_PMUV3_PERFCTR_STALL_SLOT - 32); base = 32; } diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 8f79ec1fffa7..5da536ab738d 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1051,16 +1051,16 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, /* Limit debug to ARMv8.0 */ val &= ~FEATURE(ID_AA64DFR0_DEBUGVER); val |= FIELD_PREP(FEATURE(ID_AA64DFR0_DEBUGVER), 6); - /* Limit guests to PMUv3 for ARMv8.1 */ + /* Limit guests to PMUv3 for ARMv8.4 */ val = cpuid_feature_cap_perfmon_field(val, ID_AA64DFR0_PMUVER_SHIFT, - kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_1 : 0); + kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_4 : 0); break; case SYS_ID_DFR0_EL1: - /* Limit guests to PMUv3 for ARMv8.1 */ + /* Limit guests to PMUv3 for ARMv8.4 */ val = cpuid_feature_cap_perfmon_field(val, ID_DFR0_PERFMON_SHIFT, - kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_1 : 0); + kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_4 : 0); break; } @@ -1496,6 +1496,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_PMINTENSET_EL1), access_pminten, reset_unknown, PMINTENSET_EL1 }, { SYS_DESC(SYS_PMINTENCLR_EL1), access_pminten, reset_unknown, PMINTENSET_EL1 }, + { SYS_DESC(SYS_PMMIR_EL1), trap_raz_wi }, { SYS_DESC(SYS_MAIR_EL1), access_vm_reg, reset_unknown, MAIR_EL1 }, { SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 }, @@ -1918,6 +1919,8 @@ static const struct sys_reg_desc cp15_regs[] = { { Op1( 0), CRn( 9), CRm(14), Op2( 3), access_pmovs }, { AA32(HI), Op1( 0), CRn( 9), CRm(14), Op2( 4), access_pmceid }, { AA32(HI), Op1( 0), CRn( 9), CRm(14), Op2( 5), access_pmceid }, + /* PMMIR */ + { Op1( 0), CRn( 9), CRm(14), Op2( 6), trap_raz_wi }, /* PRRR/MAIR0 */ { AA32(LO), Op1( 0), CRn(10), CRm( 2), Op2( 0), access_vm_reg, NULL, MAIR_EL1 }, -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 5/7] KVM: arm64: Limit the debug architecture to ARMv8.0
Let's not pretend we support anything but ARMv8.0 as far as the debug architecture is concerned. Reviewed-by: Eric Auger Signed-off-by: Marc Zyngier --- arch/arm64/kvm/sys_regs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index dda16d60197b..8f79ec1fffa7 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1048,6 +1048,9 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, FEATURE(ID_AA64ISAR1_GPI)); break; case SYS_ID_AA64DFR0_EL1: + /* Limit debug to ARMv8.0 */ + val &= ~FEATURE(ID_AA64DFR0_DEBUGVER); + val |= FIELD_PREP(FEATURE(ID_AA64DFR0_DEBUGVER), 6); /* Limit guests to PMUv3 for ARMv8.1 */ val = cpuid_feature_cap_perfmon_field(val, ID_AA64DFR0_PMUVER_SHIFT, -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 1/7] KVM: arm64: Fix missing RES1 in emulation of DBGBIDR
The AArch32 CP14 DBGDIDR has bit 15 set to RES1, which our current emulation doesn't set. Just add the missing bit. Reported-by: Peter Maydell Reviewed-by: Eric Auger Signed-off-by: Marc Zyngier --- arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 3313dedfa505..0c0832472c4a 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1711,7 +1711,7 @@ static bool trap_dbgidr(struct kvm_vcpu *vcpu, p->regval = dfr >> ID_AA64DFR0_WRPS_SHIFT) & 0xf) << 28) | (((dfr >> ID_AA64DFR0_BRPS_SHIFT) & 0xf) << 24) | (((dfr >> ID_AA64DFR0_CTX_CMPS_SHIFT) & 0xf) << 20) -| (6 << 16) | (el3 << 14) | (el3 << 12)); +| (6 << 16) | (1 << 15) | (el3 << 14) | (el3 << 12)); return true; } } -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 3/7] KVM: arm64: Add handling of AArch32 PCMEID{2, 3} PMUv3 registers
Despite advertising support for AArch32 PMUv3p1, we fail to handle the PMCEID{2,3} registers, which conveniently alias with the top bits of PMCEID{0,1}_EL1. Implement these registers with the usual AA32(HI/LO) aliasing mechanism. Reviewed-by: Eric Auger Signed-off-by: Marc Zyngier --- arch/arm64/kvm/sys_regs.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index ce08d28ab15c..2bea0494b81d 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -685,14 +685,18 @@ static bool access_pmselr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, static bool access_pmceid(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r) { - u64 pmceid; + u64 pmceid, mask, shift; BUG_ON(p->is_write); if (pmu_access_el0_disabled(vcpu)) return false; + get_access_mask(r, &mask, &shift); + pmceid = kvm_pmu_get_pmceid(vcpu, (p->Op2 & 1)); + pmceid &= mask; + pmceid >>= shift; p->regval = pmceid; @@ -1895,8 +1899,8 @@ static const struct sys_reg_desc cp15_regs[] = { { Op1( 0), CRn( 9), CRm(12), Op2( 3), access_pmovs }, { Op1( 0), CRn( 9), CRm(12), Op2( 4), access_pmswinc }, { Op1( 0), CRn( 9), CRm(12), Op2( 5), access_pmselr }, - { Op1( 0), CRn( 9), CRm(12), Op2( 6), access_pmceid }, - { Op1( 0), CRn( 9), CRm(12), Op2( 7), access_pmceid }, + { AA32(LO), Op1( 0), CRn( 9), CRm(12), Op2( 6), access_pmceid }, + { AA32(LO), Op1( 0), CRn( 9), CRm(12), Op2( 7), access_pmceid }, { Op1( 0), CRn( 9), CRm(13), Op2( 0), access_pmu_evcntr }, { Op1( 0), CRn( 9), CRm(13), Op2( 1), access_pmu_evtyper }, { Op1( 0), CRn( 9), CRm(13), Op2( 2), access_pmu_evcntr }, @@ -1904,6 +1908,8 @@ static const struct sys_reg_desc cp15_regs[] = { { Op1( 0), CRn( 9), CRm(14), Op2( 1), access_pminten }, { Op1( 0), CRn( 9), CRm(14), Op2( 2), access_pminten }, { Op1( 0), CRn( 9), CRm(14), Op2( 3), access_pmovs }, + { AA32(HI), Op1( 0), CRn( 9), CRm(14), Op2( 4), access_pmceid }, + { AA32(HI), Op1( 0), CRn( 9), CRm(14), Op2( 5), access_pmceid }, /* PRRR/MAIR0 */ { AA32(LO), Op1( 0), CRn(10), CRm( 2), Op2( 0), access_vm_reg, NULL, MAIR_EL1 }, -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 2/7] KVM: arm64: Fix AArch32 PMUv3 capping
We shouldn't expose *any* PMU capability when no PMU has been configured for this VM. Reviewed-by: Eric Auger Signed-off-by: Marc Zyngier --- arch/arm64/kvm/sys_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 0c0832472c4a..ce08d28ab15c 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1048,8 +1048,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, } else if (id == SYS_ID_DFR0_EL1) { /* Limit guests to PMUv3 for ARMv8.1 */ val = cpuid_feature_cap_perfmon_field(val, - ID_DFR0_PERFMON_SHIFT, - ID_DFR0_PERFMON_8_1); + ID_DFR0_PERFMON_SHIFT, + kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_1 : 0); } return val; -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 0/7] KVM: arm64: More PMU/debug ID register fixes
This is a respin of a series I posted in the 5.7 time frame, and completely forgot about it until I noticed again that a few things where not what I remembered... Given how long it has been, I'm pretending this is all new work. Anyway, this covers a few gaps in our ID register handling for PMU and Debug, plus the upgrade of the PMU support to 8.4 when possible. I don't plan to take this into 5.11, but this is a likely candidate for 5.12. Note that this conflicts with fixes that are on their way to 5.11. I'll probably end-up dragging the fixes into 5.12 to solve it. * From v1 [1]: - Upgrade AArch32 to PMUv8.4 if possible - Don't advertise STALL_SLOT when advertising PMU 8.4 - Add an extra patch replacing raw values for the PMU version with the already existing symbolaic values [1] https://lore.kernel.org/r/20210114105633.2558739-1-...@kernel.org Marc Zyngier (7): KVM: arm64: Fix missing RES1 in emulation of DBGBIDR KVM: arm64: Fix AArch32 PMUv3 capping KVM: arm64: Add handling of AArch32 PCMEID{2,3} PMUv3 registers KVM: arm64: Refactor filtering of ID registers KVM: arm64: Limit the debug architecture to ARMv8.0 KVM: arm64: Upgrade PMU support to ARMv8.4 KVM: arm64: Use symbolic names for the PMU versions arch/arm64/include/asm/sysreg.h | 3 ++ arch/arm64/kvm/pmu-emul.c | 14 -- arch/arm64/kvm/sys_regs.c | 79 - 3 files changed, 61 insertions(+), 35 deletions(-) -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 10/21] arm64: cpufeature: Use IDreg override in __read_sysreg_by_encoding()
On Mon, Jan 25, 2021 at 10:50:08AM +, Marc Zyngier wrote: > __read_sysreg_by_encoding() is used by a bunch of cpufeature helpers, > which should take the feature override into account. Let's do that. > > For a good measure (and because we are likely to need to further > down the line), make this helper available to the rest of the > non-modular kernel. > > Code that needs to know the *real* features of a CPU can still > use read_sysreg_s(), and find the bare, ugly truth. > > Signed-off-by: Marc Zyngier > Reviewed-by: Suzuki K Poulose > Acked-by: David Brazdil Reviewed-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 09/21] arm64: cpufeature: Add global feature override facility
On Mon, Jan 25, 2021 at 10:50:07AM +, Marc Zyngier wrote: > Add a facility to globally override a feature, no matter what > the HW says. Yes, this sounds dangerous, but we do respect the > "safe" value for a given feature. This doesn't mean the user > doesn't need to know what they are doing. > > Nothing uses this yet, so we are pretty safe. For now. > > Signed-off-by: Marc Zyngier > Reviewed-by: Suzuki K Poulose > Acked-by: David Brazdil That's more readable with a single pointer. Reviewed-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 07/21] arm64: Simplify init_el2_state to be non-VHE only
On Mon, Jan 25, 2021 at 10:50:05AM +, Marc Zyngier wrote: > As init_el2_state is now nVHE only, let's simplify it and drop > the VHE setup. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Acked-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 06/21] arm64: Move VHE-specific SPE setup to mutate_to_vhe()
On Mon, Jan 25, 2021 at 10:50:04AM +, Marc Zyngier wrote: > There isn't much that a VHE kernel needs on top of whatever has > been done for nVHE, so let's move the little we need to the > VHE stub (the SPE setup), and drop the init_el2_state macro. > > No expected functional change. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Acked-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 05/21] arm64: Initialise as nVHE before switching to VHE
On Mon, Jan 25, 2021 at 10:50:03AM +, Marc Zyngier wrote: > As we are aiming to be able to control whether we enable VHE or > not, let's always drop down to EL1 first, and only then upgrade > to VHE if at all possible. > > This means that if the kernel is booted at EL2, we always start > with a nVHE init, drop to EL1 to initialise the the kernel, and > only then upgrade the kernel EL to EL2 if possible (the process > is obviously shortened for secondary CPUs). > > The resume path is handled similarly to a secondary CPU boot. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Acked-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 04/21] arm64: Provide an 'upgrade to VHE' stub hypercall
On Mon, Jan 25, 2021 at 10:50:02AM +, Marc Zyngier wrote: > As we are about to change the way a VHE system boots, let's > provide the core helper, in the form of a stub hypercall that > enables VHE and replicates the full EL1 context at EL2, thanks > to EL1 and VHE-EL2 being extremely similar. > > On exception return, the kernel carries on at EL2. Fancy! > > Nothing calls this new hypercall yet, so no functional change. > > Signed-off-by: Marc Zyngier > Acked-by: David Brazdil Acked-by: Catalin Marinas ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH] kvm: arm64: Try stage2 block mapping for host device MMIO
I forget to give the link of the bugfix I mentioned below :-). [1] https://lkml.org/lkml/2020/5/1/1294 On 2021/1/25 19:25, Keqian Zhu wrote: > Hi Marc, > > On 2021/1/22 17:45, Marc Zyngier wrote: >> On 2021-01-22 08:36, Keqian Zhu wrote: >>> The MMIO region of a device maybe huge (GB level), try to use block >>> mapping in stage2 to speedup both map and unmap. >>> >>> Especially for unmap, it performs TLBI right after each invalidation >>> of PTE. If all mapping is of PAGE_SIZE, it takes much time to handle >>> GB level range. >> >> This is only on VM teardown, right? Or do you unmap the device more ofet? >> Can you please quantify the speedup and the conditions this occurs in? > > Yes, and there are some other paths (includes what your patch series handles) > will do the unmap action: > > 1、guest reboot without S2FWB: stage2_unmap_vm()which only unmaps guest > regular RAM. > 2、userspace deletes memslot: kvm_arch_flush_shadow_memslot(). > 3、rollback of device MMIO mapping: kvm_arch_prepare_memory_region(). > 4、rollback of dirty log tracking: If we enable hugepage for guest RAM, after > dirty log is stopped, >the newly created block mappings will > unmap all page mappings. > 5、mmu notifier: kvm_unmap_hva_range(). AFAICS, we will use this path when VM > teardown or guest resets pass-through devices. > The bugfix[1] gives the reason for > unmapping MMIO region when guest resets pass-through devices. > > unmap related to MMIO region, as this patch solves: > point 1 is not applied. > point 2 occurs when userspace unplug pass-through devices. > point 3 can occurs, but rarely. > point 4 is not applied. > point 5 occurs when VM teardown or guest resets pass-through devices. > > And I had a look at your patch series, it can solve: > For VM teardown, elide CMO and perform VMALL instead of individually (But > current kernel do not go through this path when VM teardown). > For rollback of dirty log tracking, elide CMO. > For kvm_unmap_hva_range, if event is MMU_NOTIFY_UNMAP. elide CMO. > > (But I doubt the CMOs in unmap. As we perform CMOs in user_mem_abort when > install new stage2 mapping for VM, > maybe the CMO in unmap is unnecessary under all conditions :-) ?) > > So it shows that we are solving different parts of unmap, so they are not > conflicting. At least this patch can > still speedup map of device MMIO region, and speedup unmap of device MMIO > region even if we do not need to perform > CMO and TLBI ;-). > > speedup: unmap 8GB MMIO on FPGA. > >beforeafter opt > cost30+ minutes949ms > > Thanks, > Keqian > >> >> I have the feeling that we are just circling around another problem, >> which is that we could rely on a VM-wide TLBI when tearing down the >> guest. I worked on something like that[1] a long while ago, and parked >> it for some reason. Maybe it is worth reviving. >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/elide-cmo-tlbi >> >>> >>> Signed-off-by: Keqian Zhu >>> --- >>> arch/arm64/include/asm/kvm_pgtable.h | 11 +++ >>> arch/arm64/kvm/hyp/pgtable.c | 15 +++ >>> arch/arm64/kvm/mmu.c | 12 >>> 3 files changed, 34 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h >>> b/arch/arm64/include/asm/kvm_pgtable.h >>> index 52ab38db04c7..2266ac45f10c 100644 >>> --- a/arch/arm64/include/asm/kvm_pgtable.h >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h >>> @@ -82,6 +82,17 @@ struct kvm_pgtable_walker { >>> const enum kvm_pgtable_walk_flagsflags; >>> }; >>> >>> +/** >>> + * kvm_supported_pgsize() - Get the max supported page size of a mapping. >>> + * @pgt:Initialised page-table structure. >>> + * @addr:Virtual address at which to place the mapping. >>> + * @end:End virtual address of the mapping. >>> + * @phys:Physical address of the memory to map. >>> + * >>> + * The smallest return value is PAGE_SIZE. >>> + */ >>> +u64 kvm_supported_pgsize(struct kvm_pgtable *pgt, u64 addr, u64 end, u64 >>> phys); >>> + >>> /** >>> * kvm_pgtable_hyp_init() - Initialise a hypervisor stage-1 page-table. >>> * @pgt:Uninitialised page-table structure to initialise. >>> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c >>> index bdf8e55ed308..ab11609b9b13 100644 >>> --- a/arch/arm64/kvm/hyp/pgtable.c >>> +++ b/arch/arm64/kvm/hyp/pgtable.c >>> @@ -81,6 +81,21 @@ static bool kvm_block_mapping_supported(u64 addr, >>> u64 end, u64 phys, u32 level) >>> return IS_ALIGNED(addr, granule) && IS_ALIGNED(phys, granule); >>> } >>> >>> +u64 kvm_supported_pgsize(struct kvm_pgtable *pgt, u64 addr, u64 end, u64 >>> phys) >>> +{ >>> +u32 lvl; >>> +u64 pgsize = PAGE_SIZE; >>> + >>> +for (lvl = pgt->start_level; lvl < KVM_PGTABLE_MAX_LEVELS; lvl++) { >>> +if (kvm_block_mapping_su
Re: [RFC PATCH] kvm: arm64: Try stage2 block mapping for host device MMIO
Hi Marc, On 2021/1/22 17:45, Marc Zyngier wrote: > On 2021-01-22 08:36, Keqian Zhu wrote: >> The MMIO region of a device maybe huge (GB level), try to use block >> mapping in stage2 to speedup both map and unmap. >> >> Especially for unmap, it performs TLBI right after each invalidation >> of PTE. If all mapping is of PAGE_SIZE, it takes much time to handle >> GB level range. > > This is only on VM teardown, right? Or do you unmap the device more ofet? > Can you please quantify the speedup and the conditions this occurs in? Yes, and there are some other paths (includes what your patch series handles) will do the unmap action: 1、guest reboot without S2FWB: stage2_unmap_vm()which only unmaps guest regular RAM. 2、userspace deletes memslot: kvm_arch_flush_shadow_memslot(). 3、rollback of device MMIO mapping: kvm_arch_prepare_memory_region(). 4、rollback of dirty log tracking: If we enable hugepage for guest RAM, after dirty log is stopped, the newly created block mappings will unmap all page mappings. 5、mmu notifier: kvm_unmap_hva_range(). AFAICS, we will use this path when VM teardown or guest resets pass-through devices. The bugfix[1] gives the reason for unmapping MMIO region when guest resets pass-through devices. unmap related to MMIO region, as this patch solves: point 1 is not applied. point 2 occurs when userspace unplug pass-through devices. point 3 can occurs, but rarely. point 4 is not applied. point 5 occurs when VM teardown or guest resets pass-through devices. And I had a look at your patch series, it can solve: For VM teardown, elide CMO and perform VMALL instead of individually (But current kernel do not go through this path when VM teardown). For rollback of dirty log tracking, elide CMO. For kvm_unmap_hva_range, if event is MMU_NOTIFY_UNMAP. elide CMO. (But I doubt the CMOs in unmap. As we perform CMOs in user_mem_abort when install new stage2 mapping for VM, maybe the CMO in unmap is unnecessary under all conditions :-) ?) So it shows that we are solving different parts of unmap, so they are not conflicting. At least this patch can still speedup map of device MMIO region, and speedup unmap of device MMIO region even if we do not need to perform CMO and TLBI ;-). speedup: unmap 8GB MMIO on FPGA. beforeafter opt cost30+ minutes949ms Thanks, Keqian > > I have the feeling that we are just circling around another problem, > which is that we could rely on a VM-wide TLBI when tearing down the > guest. I worked on something like that[1] a long while ago, and parked > it for some reason. Maybe it is worth reviving. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/elide-cmo-tlbi > >> >> Signed-off-by: Keqian Zhu >> --- >> arch/arm64/include/asm/kvm_pgtable.h | 11 +++ >> arch/arm64/kvm/hyp/pgtable.c | 15 +++ >> arch/arm64/kvm/mmu.c | 12 >> 3 files changed, 34 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_pgtable.h >> b/arch/arm64/include/asm/kvm_pgtable.h >> index 52ab38db04c7..2266ac45f10c 100644 >> --- a/arch/arm64/include/asm/kvm_pgtable.h >> +++ b/arch/arm64/include/asm/kvm_pgtable.h >> @@ -82,6 +82,17 @@ struct kvm_pgtable_walker { >> const enum kvm_pgtable_walk_flagsflags; >> }; >> >> +/** >> + * kvm_supported_pgsize() - Get the max supported page size of a mapping. >> + * @pgt:Initialised page-table structure. >> + * @addr:Virtual address at which to place the mapping. >> + * @end:End virtual address of the mapping. >> + * @phys:Physical address of the memory to map. >> + * >> + * The smallest return value is PAGE_SIZE. >> + */ >> +u64 kvm_supported_pgsize(struct kvm_pgtable *pgt, u64 addr, u64 end, u64 >> phys); >> + >> /** >> * kvm_pgtable_hyp_init() - Initialise a hypervisor stage-1 page-table. >> * @pgt:Uninitialised page-table structure to initialise. >> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c >> index bdf8e55ed308..ab11609b9b13 100644 >> --- a/arch/arm64/kvm/hyp/pgtable.c >> +++ b/arch/arm64/kvm/hyp/pgtable.c >> @@ -81,6 +81,21 @@ static bool kvm_block_mapping_supported(u64 addr, >> u64 end, u64 phys, u32 level) >> return IS_ALIGNED(addr, granule) && IS_ALIGNED(phys, granule); >> } >> >> +u64 kvm_supported_pgsize(struct kvm_pgtable *pgt, u64 addr, u64 end, u64 >> phys) >> +{ >> +u32 lvl; >> +u64 pgsize = PAGE_SIZE; >> + >> +for (lvl = pgt->start_level; lvl < KVM_PGTABLE_MAX_LEVELS; lvl++) { >> +if (kvm_block_mapping_supported(addr, end, phys, lvl)) { >> +pgsize = kvm_granule_size(lvl); >> +break; >> +} >> +} >> + >> +return pgsize; >> +} >> + >> static u32 kvm_pgtable_idx(struct kvm_pgtable_walk_data *data, u32 level) >> { >> u64 shift = kvm_granule_shift(level); >> diff --gi
[PATCH v5 16/21] arm64: Make kvm-arm.mode={nvhe, protected} an alias of id_aa64mmfr1.vh=0
Admitedly, passing id_aa64mmfr1.vh=0 on the command-line isn't that easy to understand, and it is likely that users would much prefer write "kvm-arm.mode=nvhe", or "...=protected". So here you go. This has the added advantage that we can now always honor the "kvm-arm.mode=protected" option, even when booting on a VHE system. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/kernel/idreg-override.c | 2 ++ arch/arm64/kvm/arm.c| 3 +++ 3 files changed, 8 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 9e3cdb271d06..2786fd39a047 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2257,6 +2257,9 @@ kvm-arm.mode= [KVM,ARM] Select one of KVM/arm64's modes of operation. + nvhe: Standard nVHE-based mode, without support for + protected guests. + protected: nVHE-based mode with support for guests whose state is kept private from the host. Not valid if the kernel is running in EL2. diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 4fad3fc4d104..cbb8eaa48742 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -39,6 +39,8 @@ static const struct { const char *alias; const char *feature; } aliases[] __initdata = { + { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, + { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, }; static char *cmdline_contains_option(const char *cmdline, const char *option) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 04c44853b103..597565a65ca2 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1966,6 +1966,9 @@ static int __init early_kvm_mode_cfg(char *arg) return 0; } + if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) + return 0; + return -EINVAL; } early_param("kvm-arm.mode", early_kvm_mode_cfg); -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 21/21] arm64: cpufeatures: Allow disabling of Pointer Auth from the command-line
In order to be able to disable Pointer Authentication at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64ISAR1_EL1.{GPI,GPA,API,APA} fields. This is further mapped on the arm64.nopauth command-line alias. Signed-off-by: Marc Zyngier Reviewed-by: Catalin Marinas Acked-by: David Brazdil --- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/cpufeature.h | 1 + arch/arm64/kernel/cpufeature.c | 4 +++- arch/arm64/kernel/idreg-override.c | 16 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 7599fd0f1ad7..f9cb28a39bd0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -376,6 +376,9 @@ arm64.nobti [ARM64] Unconditionally disable Branch Target Identification support + arm64.nopauth [ARM64] Unconditionally disable Pointer Authentication + support + ataflop=[HW,M68k] atarimouse= [HW,MOUSE] Atari Mouse diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 4e2f2de9d0d7..ec6311903ad4 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -820,6 +820,7 @@ static inline unsigned int get_vmid_bits(u64 mmfr1) extern struct arm64_ftr_override id_aa64mmfr1_override; extern struct arm64_ftr_override id_aa64pfr1_override; +extern struct arm64_ftr_override id_aa64isar1_override; u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index bb99ddb212b5..954a2b7e5d45 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -559,6 +559,7 @@ static const struct arm64_ftr_bits ftr_raz[] = { struct arm64_ftr_override id_aa64mmfr1_override; struct arm64_ftr_override id_aa64pfr1_override; +struct arm64_ftr_override id_aa64isar1_override; static const struct __ftr_reg_entry { u32 sys_id; @@ -604,7 +605,8 @@ static const struct __ftr_reg_entry { /* Op1 = 0, CRn = 0, CRm = 6 */ ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0), - ARM64_FTR_REG(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1), + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1, + &id_aa64isar1_override), /* Op1 = 0, CRn = 0, CRm = 7 */ ARM64_FTR_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0), diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 71349b644246..d1310438d95c 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -40,6 +40,18 @@ static const struct ftr_set_desc pfr1 __initdata = { }, }; +static const struct ftr_set_desc isar1 __initdata = { + .name = "id_aa64isar1", + .override = &id_aa64isar1_override, + .fields = { + { "gpi", ID_AA64ISAR1_GPI_SHIFT }, + { "gpa", ID_AA64ISAR1_GPA_SHIFT }, + { "api", ID_AA64ISAR1_API_SHIFT }, + { "apa", ID_AA64ISAR1_APA_SHIFT }, + {} + }, +}; + extern struct arm64_ftr_override kaslr_feature_override; static const struct ftr_set_desc kaslr __initdata = { @@ -56,6 +68,7 @@ static const struct ftr_set_desc kaslr __initdata = { static const struct ftr_set_desc * const regs[] __initdata = { &mmfr1, &pfr1, + &isar1, &kaslr, }; @@ -66,6 +79,9 @@ static const struct { { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nobti","id_aa64pfr1.bt=0" }, + { "arm64.nopauth", + "id_aa64isar1.gpi=0 id_aa64isar1.gpa=0 " + "id_aa64isar1.api=0 id_aa64isar1.apa=0" }, { "nokaslr","kaslr.disabled=1" }, }; -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 17/21] KVM: arm64: Document HVC_VHE_RESTART stub hypercall
For completeness, let's document the HVC_VHE_RESTART stub. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- Documentation/virt/kvm/arm/hyp-abi.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/virt/kvm/arm/hyp-abi.rst b/Documentation/virt/kvm/arm/hyp-abi.rst index 83cadd8186fa..4d43fbc25195 100644 --- a/Documentation/virt/kvm/arm/hyp-abi.rst +++ b/Documentation/virt/kvm/arm/hyp-abi.rst @@ -58,6 +58,15 @@ these functions (see arch/arm{,64}/include/asm/virt.h): into place (arm64 only), and jump to the restart address while at HYP/EL2. This hypercall is not expected to return to its caller. +* :: + +x0 = HVC_VHE_RESTART (arm64 only) + + Attempt to upgrade the kernel's exception level from EL1 to EL2 by enabling + the VHE mode. This is conditioned by the CPU supporting VHE, the EL2 MMU + being off, and VHE not being disabled by any other means (command line + option, for example). + Any other value of r0/x0 triggers a hypervisor-specific handling, which is not documented here. -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 11/21] arm64: Extract early FDT mapping from kaslr_early_init()
As we want to parse more options very early in the kernel lifetime, let's always map the FDT early. This is achieved by moving that code out of kaslr_early_init(). No functionnal change expected. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/include/asm/setup.h | 11 +++ arch/arm64/kernel/head.S | 3 ++- arch/arm64/kernel/kaslr.c | 7 +++ arch/arm64/kernel/setup.c | 15 +++ 4 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/setup.h diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h new file mode 100644 index ..d3320618ed14 --- /dev/null +++ b/arch/arm64/include/asm/setup.h @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef __ARM64_ASM_SETUP_H +#define __ARM64_ASM_SETUP_H + +#include + +void *get_early_fdt_ptr(void); +void early_fdt_map(u64 dt_phys); + +#endif diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index b425d2587cdb..d74e5f84042e 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -433,6 +433,8 @@ SYM_FUNC_START_LOCAL(__primary_switched) bl __pi_memset dsb ishst // Make zero page visible to PTW + mov x0, x21 // pass FDT address in x0 + bl early_fdt_map // Try mapping the FDT early bl switch_to_vhe #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) bl kasan_early_init @@ -440,7 +442,6 @@ SYM_FUNC_START_LOCAL(__primary_switched) #ifdef CONFIG_RANDOMIZE_BASE tst x23, ~(MIN_KIMG_ALIGN - 1) // already running randomized? b.ne0f - mov x0, x21 // pass FDT address in x0 bl kaslr_early_init// parse FDT for KASLR options cbz x0, 0f // KASLR disabled? just proceed orr x23, x23, x0// record KASLR offset diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c index 1c74c45b9494..5fc86e7d01a1 100644 --- a/arch/arm64/kernel/kaslr.c +++ b/arch/arm64/kernel/kaslr.c @@ -19,6 +19,7 @@ #include #include #include +#include enum kaslr_status { KASLR_ENABLED, @@ -92,12 +93,11 @@ static __init bool is_kaslr_disabled_cmdline(void *fdt) * containing function pointers) to be reinitialized, and zero-initialized * .bss variables will be reset to 0. */ -u64 __init kaslr_early_init(u64 dt_phys) +u64 __init kaslr_early_init(void) { void *fdt; u64 seed, offset, mask, module_range; unsigned long raw; - int size; /* * Set a reasonable default for module_alloc_base in case @@ -111,8 +111,7 @@ u64 __init kaslr_early_init(u64 dt_phys) * and proceed with KASLR disabled. We will make another * attempt at mapping the FDT in setup_machine() */ - early_fixmap_init(); - fdt = fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL); + fdt = get_early_fdt_ptr(); if (!fdt) { kaslr_status = KASLR_DISABLED_FDT_REMAP; return 0; diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index c18aacde8bb0..61845c0821d9 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -168,6 +168,21 @@ static void __init smp_build_mpidr_hash(void) pr_warn("Large number of MPIDR hash buckets detected\n"); } +static void *early_fdt_ptr __initdata; + +void __init *get_early_fdt_ptr(void) +{ + return early_fdt_ptr; +} + +asmlinkage void __init early_fdt_map(u64 dt_phys) +{ + int fdt_size; + + early_fixmap_init(); + early_fdt_ptr = fixmap_remap_fdt(dt_phys, &fdt_size, PAGE_KERNEL); +} + static void __init setup_machine_fdt(phys_addr_t dt_phys) { int size; -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 10/21] arm64: cpufeature: Use IDreg override in __read_sysreg_by_encoding()
__read_sysreg_by_encoding() is used by a bunch of cpufeature helpers, which should take the feature override into account. Let's do that. For a good measure (and because we are likely to need to further down the line), make this helper available to the rest of the non-modular kernel. Code that needs to know the *real* features of a CPU can still use read_sysreg_s(), and find the bare, ugly truth. Signed-off-by: Marc Zyngier Reviewed-by: Suzuki K Poulose Acked-by: David Brazdil --- arch/arm64/include/asm/cpufeature.h | 1 + arch/arm64/kernel/cpufeature.c | 15 +-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index fe469389068e..4179cfc8ed57 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -606,6 +606,7 @@ void __init setup_cpu_features(void); void check_local_cpu_capabilities(void); u64 read_sanitised_ftr_reg(u32 id); +u64 __read_sysreg_by_encoding(u32 sys_id); static inline bool cpu_supports_mixed_endian_el0(void) { diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index cb58c7c991ef..4b84a1e1dc51 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1145,14 +1145,17 @@ u64 read_sanitised_ftr_reg(u32 id) EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg); #define read_sysreg_case(r)\ - case r: return read_sysreg_s(r) + case r: val = read_sysreg_s(r); break; /* * __read_sysreg_by_encoding() - Used by a STARTING cpu before cpuinfo is populated. * Read the system register on the current CPU */ -static u64 __read_sysreg_by_encoding(u32 sys_id) +u64 __read_sysreg_by_encoding(u32 sys_id) { + struct arm64_ftr_reg *regp; + u64 val; + switch (sys_id) { read_sysreg_case(SYS_ID_PFR0_EL1); read_sysreg_case(SYS_ID_PFR1_EL1); @@ -1195,6 +1198,14 @@ static u64 __read_sysreg_by_encoding(u32 sys_id) BUG(); return 0; } + + regp = get_arm64_ftr_reg(sys_id); + if (regp) { + val &= ~regp->override->mask; + val |= (regp->override->val & regp->override->mask); + } + + return val; } #include -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 20/21] arm64: Defer enabling pointer authentication on boot core
From: Srinivas Ramana Defer enabling pointer authentication on boot core until after its required to be enabled by cpufeature framework. This will help in controlling the feature dynamically with a boot parameter. Signed-off-by: Ajay Patil Signed-off-by: Prasad Sodagudi Signed-off-by: Srinivas Ramana Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/1610152163-16554-2-git-send-email-sram...@codeaurora.org Reviewed-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/include/asm/pointer_auth.h | 10 ++ arch/arm64/include/asm/stackprotector.h | 1 + arch/arm64/kernel/head.S| 4 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h index c6b4f0603024..b112a11e9302 100644 --- a/arch/arm64/include/asm/pointer_auth.h +++ b/arch/arm64/include/asm/pointer_auth.h @@ -76,6 +76,15 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) return ptrauth_clear_pac(ptr); } +static __always_inline void ptrauth_enable(void) +{ + if (!system_supports_address_auth()) + return; + sysreg_clear_set(sctlr_el1, 0, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | + SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)); + isb(); +} + #define ptrauth_thread_init_user(tsk) \ ptrauth_keys_init_user(&(tsk)->thread.keys_user) #define ptrauth_thread_init_kernel(tsk) \ @@ -84,6 +93,7 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) ptrauth_keys_switch_kernel(&(tsk)->thread.keys_kernel) #else /* CONFIG_ARM64_PTR_AUTH */ +#define ptrauth_enable() #define ptrauth_prctl_reset_keys(tsk, arg) (-EINVAL) #define ptrauth_strip_insn_pac(lr) (lr) #define ptrauth_thread_init_user(tsk) diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h index 7263e0bac680..33f1bb453150 100644 --- a/arch/arm64/include/asm/stackprotector.h +++ b/arch/arm64/include/asm/stackprotector.h @@ -41,6 +41,7 @@ static __always_inline void boot_init_stack_canary(void) #endif ptrauth_thread_init_kernel(current); ptrauth_thread_switch_kernel(current); + ptrauth_enable(); } #endif /* _ASM_STACKPROTECTOR_H */ diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 3243e3ae9bd8..2e116ef255e1 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -404,10 +404,6 @@ SYM_FUNC_START_LOCAL(__primary_switched) adr_l x5, init_task msr sp_el0, x5 // Save thread_info -#ifdef CONFIG_ARM64_PTR_AUTH - __ptrauth_keys_init_cpu x5, x6, x7, x8 -#endif - adr_l x8, vectors // load VBAR_EL1 with virtual msr vbar_el1, x8// vector table address isb -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 19/21] arm64: cpufeatures: Allow disabling of BTI from the command-line
In order to be able to disable BTI at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64PFR1_EL1.BTI field. This is further mapped on the arm64.nobti command-line alias. Signed-off-by: Marc Zyngier Reviewed-by: Catalin Marinas Acked-by: David Brazdil --- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/cpufeature.h | 1 + arch/arm64/kernel/cpufeature.c | 4 +++- arch/arm64/kernel/idreg-override.c | 11 +++ arch/arm64/mm/mmu.c | 2 +- 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 2786fd39a047..7599fd0f1ad7 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -373,6 +373,9 @@ arcrimi=[HW,NET] ARCnet - "RIM I" (entirely mem-mapped) cards Format: ,, + arm64.nobti [ARM64] Unconditionally disable Branch Target + Identification support + ataflop=[HW,M68k] atarimouse= [HW,MOUSE] Atari Mouse diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index b0ed37da4067..4e2f2de9d0d7 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -819,6 +819,7 @@ static inline unsigned int get_vmid_bits(u64 mmfr1) } extern struct arm64_ftr_override id_aa64mmfr1_override; +extern struct arm64_ftr_override id_aa64pfr1_override; u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index c1d6712c4249..bb99ddb212b5 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -558,6 +558,7 @@ static const struct arm64_ftr_bits ftr_raz[] = { #define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, &no_override) struct arm64_ftr_override id_aa64mmfr1_override; +struct arm64_ftr_override id_aa64pfr1_override; static const struct __ftr_reg_entry { u32 sys_id; @@ -593,7 +594,8 @@ static const struct __ftr_reg_entry { /* Op1 = 0, CRn = 0, CRm = 4 */ ARM64_FTR_REG(SYS_ID_AA64PFR0_EL1, ftr_id_aa64pfr0), - ARM64_FTR_REG(SYS_ID_AA64PFR1_EL1, ftr_id_aa64pfr1), + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64PFR1_EL1, ftr_id_aa64pfr1, + &id_aa64pfr1_override), ARM64_FTR_REG(SYS_ID_AA64ZFR0_EL1, ftr_id_aa64zfr0), /* Op1 = 0, CRn = 0, CRm = 5 */ diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 3ccf51b84ba4..71349b644246 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -31,6 +31,15 @@ static const struct ftr_set_desc mmfr1 __initdata = { }, }; +static const struct ftr_set_desc pfr1 __initdata = { + .name = "id_aa64pfr1", + .override = &id_aa64pfr1_override, + .fields = { + { "bt", ID_AA64PFR1_BT_SHIFT }, + {} + }, +}; + extern struct arm64_ftr_override kaslr_feature_override; static const struct ftr_set_desc kaslr __initdata = { @@ -46,6 +55,7 @@ static const struct ftr_set_desc kaslr __initdata = { static const struct ftr_set_desc * const regs[] __initdata = { &mmfr1, + &pfr1, &kaslr, }; @@ -55,6 +65,7 @@ static const struct { } aliases[] __initdata = { { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, + { "arm64.nobti","id_aa64pfr1.bt=0" }, { "nokaslr","kaslr.disabled=1" }, }; diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index ae0c3d023824..617e704c980b 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -628,7 +628,7 @@ static bool arm64_early_this_cpu_has_bti(void) if (!IS_ENABLED(CONFIG_ARM64_BTI_KERNEL)) return false; - pfr1 = read_sysreg_s(SYS_ID_AA64PFR1_EL1); + pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1); return cpuid_feature_extract_unsigned_field(pfr1, ID_AA64PFR1_BT_SHIFT); } -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
Given that the early cpufeature infrastructure has borrowed quite a lot of code from the kaslr implementation, let's reimplement the matching of the "nokaslr" option with it. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/kernel/idreg-override.c | 15 + arch/arm64/kernel/kaslr.c | 36 ++ 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index cbb8eaa48742..3ccf51b84ba4 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -31,8 +31,22 @@ static const struct ftr_set_desc mmfr1 __initdata = { }, }; +extern struct arm64_ftr_override kaslr_feature_override; + +static const struct ftr_set_desc kaslr __initdata = { + .name = "kaslr", +#ifdef CONFIG_RANDOMIZE_BASE + .override = &kaslr_feature_override, +#endif + .fields = { + { "disabled", 0 }, + {} + }, +}; + static const struct ftr_set_desc * const regs[] __initdata = { &mmfr1, + &kaslr, }; static const struct { @@ -41,6 +55,7 @@ static const struct { } aliases[] __initdata = { { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, + { "nokaslr","kaslr.disabled=1" }, }; static char *cmdline_contains_option(const char *cmdline, const char *option) diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c index 5fc86e7d01a1..27f8939deb1b 100644 --- a/arch/arm64/kernel/kaslr.c +++ b/arch/arm64/kernel/kaslr.c @@ -51,39 +51,7 @@ static __init u64 get_kaslr_seed(void *fdt) return ret; } -static __init bool cmdline_contains_nokaslr(const u8 *cmdline) -{ - const u8 *str; - - str = strstr(cmdline, "nokaslr"); - return str == cmdline || (str > cmdline && *(str - 1) == ' '); -} - -static __init bool is_kaslr_disabled_cmdline(void *fdt) -{ - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { - int node; - const u8 *prop; - - node = fdt_path_offset(fdt, "/chosen"); - if (node < 0) - goto out; - - prop = fdt_getprop(fdt, node, "bootargs", NULL); - if (!prop) - goto out; - - if (cmdline_contains_nokaslr(prop)) - return true; - - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) - goto out; - - return false; - } -out: - return cmdline_contains_nokaslr(CONFIG_CMDLINE); -} +struct arm64_ftr_override kaslr_feature_override __initdata; /* * This routine will be executed with the kernel mapped at its default virtual @@ -126,7 +94,7 @@ u64 __init kaslr_early_init(void) * Check if 'nokaslr' appears on the command line, and * return 0 if that is the case. */ - if (is_kaslr_disabled_cmdline(fdt)) { + if (kaslr_feature_override.val & kaslr_feature_override.mask & 0xf) { kaslr_status = KASLR_DISABLED_CMDLINE; return 0; } -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 14/21] arm64: Honor VHE being disabled from the command-line
Finally we can check whether VHE is disabled on the command line, and not enable it if that's the user's wish. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/kernel/asm-offsets.c | 3 +++ arch/arm64/kernel/hyp-stub.S| 11 +++ 2 files changed, 14 insertions(+) diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index f42fd9e33981..1add0f21bffe 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -99,6 +99,9 @@ int main(void) DEFINE(CPU_BOOT_STACK, offsetof(struct secondary_data, stack)); DEFINE(CPU_BOOT_TASK,offsetof(struct secondary_data, task)); BLANK(); + DEFINE(FTR_OVR_VAL_OFFSET, offsetof(struct arm64_ftr_override, val)); + DEFINE(FTR_OVR_MASK_OFFSET, offsetof(struct arm64_ftr_override, mask)); + BLANK(); #ifdef CONFIG_KVM DEFINE(VCPU_CONTEXT, offsetof(struct kvm_vcpu, arch.ctxt)); DEFINE(VCPU_FAULT_DISR, offsetof(struct kvm_vcpu, arch.fault.disr_el1)); diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index 6b5c73cf9d52..aa7e8d592295 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -87,6 +87,17 @@ SYM_CODE_START_LOCAL(mutate_to_vhe) ubfxx1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4 cbz x1, 1f + // Check whether VHE is disabled from the command line + adr_l x1, id_aa64mmfr1_override + ldr x2, [x1, FTR_OVR_VAL_OFFSET] + ldr x1, [x1, FTR_OVR_MASK_OFFSET] + ubfxx2, x2, #ID_AA64MMFR1_VHE_SHIFT, #4 + ubfxx1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4 + cmp x1, xzr + and x2, x2, x1 + csinv x2, x2, xzr, ne + cbz x2, 1f + // Engage the VHE magic! mov_q x0, HCR_HOST_VHE_FLAGS msr hcr_el2, x0 -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 13/21] arm64: Allow ID_AA64MMFR1_EL1.VH to be overridden from the command line
As we want to be able to disable VHE at runtime, let's match "id_aa64mmfr1.vh=" from the command line as an override. This doesn't have much effect yet as our boot code doesn't look at the cpufeature, but only at the HW registers. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/cpufeature.h | 2 ++ arch/arm64/kernel/cpufeature.c | 5 - arch/arm64/kernel/idreg-override.c | 11 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 4179cfc8ed57..b0ed37da4067 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -818,6 +818,8 @@ static inline unsigned int get_vmid_bits(u64 mmfr1) return 8; } +extern struct arm64_ftr_override id_aa64mmfr1_override; + u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 4b84a1e1dc51..c1d6712c4249 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -557,6 +557,8 @@ static const struct arm64_ftr_bits ftr_raz[] = { #define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, &no_override) +struct arm64_ftr_override id_aa64mmfr1_override; + static const struct __ftr_reg_entry { u32 sys_id; struct arm64_ftr_reg*reg; @@ -604,7 +606,8 @@ static const struct __ftr_reg_entry { /* Op1 = 0, CRn = 0, CRm = 7 */ ARM64_FTR_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0), - ARM64_FTR_REG(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1), + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1, + &id_aa64mmfr1_override), ARM64_FTR_REG(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2), /* Op1 = 0, CRn = 1, CRm = 2 */ diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 2d184d6674fd..489aa274e3ec 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -10,6 +10,7 @@ #include #include +#include #include struct ftr_set_desc { @@ -21,7 +22,17 @@ struct ftr_set_desc { } fields[]; }; +static const struct ftr_set_desc mmfr1 __initdata = { + .name = "id_aa64mmfr1", + .override = &id_aa64mmfr1_override, + .fields = { + { "vh", ID_AA64MMFR1_VHE_SHIFT }, + {} + }, +}; + static const struct ftr_set_desc * const regs[] __initdata = { + &mmfr1, }; static char *cmdline_contains_option(const char *cmdline, const char *option) -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 12/21] arm64: cpufeature: Add an early command-line cpufeature override facility
In order to be able to override CPU features at boot time, let's add a command line parser that matches options of the form "cpureg.feature=value", and store the corresponding value into the override val/mask pair. No features are currently defined, so no expected change in functionality. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/head.S | 1 + arch/arm64/kernel/idreg-override.c | 130 + 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/kernel/idreg-override.c diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 86364ab6f13f..2262f0392857 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -17,7 +17,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ return_address.o cpuinfo.o cpu_errata.o \ cpufeature.o alternative.o cacheinfo.o \ smp.o smp_spin_table.o topology.o smccc-call.o \ - syscall.o proton-pack.o + syscall.o proton-pack.o idreg-override.o targets+= efi-entry.o diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index d74e5f84042e..3243e3ae9bd8 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -435,6 +435,7 @@ SYM_FUNC_START_LOCAL(__primary_switched) mov x0, x21 // pass FDT address in x0 bl early_fdt_map // Try mapping the FDT early + bl init_feature_override bl switch_to_vhe #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) bl kasan_early_init diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c new file mode 100644 index ..2d184d6674fd --- /dev/null +++ b/arch/arm64/kernel/idreg-override.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Early cpufeature override framework + * + * Copyright (C) 2020 Google LLC + * Author: Marc Zyngier + */ + +#include +#include + +#include +#include + +struct ftr_set_desc { + const char *name; + struct arm64_ftr_override *override; + struct { + const char *name; + u8 shift; + } fields[]; +}; + +static const struct ftr_set_desc * const regs[] __initdata = { +}; + +static char *cmdline_contains_option(const char *cmdline, const char *option) +{ + char *str = strstr(cmdline, option); + + if ((str == cmdline || (str > cmdline && *(str - 1) == ' '))) + return str; + + return NULL; +} + +static int __init find_field(const char *cmdline, +const struct ftr_set_desc *reg, int f, u64 *v) +{ + char buf[256], *str; + size_t len; + + snprintf(buf, ARRAY_SIZE(buf), "%s.%s=", reg->name, reg->fields[f].name); + + str = cmdline_contains_option(cmdline, buf); + if (!str) + return -1; + + str += strlen(buf); + len = strcspn(str, " "); + len = min(len, ARRAY_SIZE(buf) - 1); + strncpy(buf, str, len); + buf[len] = 0; + + return kstrtou64(buf, 0, v); +} + +static void __init match_options(const char *cmdline) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(regs); i++) { + int f; + + if (!regs[i]->override) + continue; + + for (f = 0; regs[i]->fields[f].name; f++) { + u64 v; + + if (find_field(cmdline, regs[i], f, &v)) + continue; + + regs[i]->override->val |= (v & 0xf) << regs[i]->fields[f].shift; + regs[i]->override->mask |= 0xfUL << regs[i]->fields[f].shift; + } + } +} + +static __init void parse_cmdline(void) +{ + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { + const u8 *prop; + void *fdt; + int node; + + fdt = get_early_fdt_ptr(); + if (!fdt) + goto out; + + node = fdt_path_offset(fdt, "/chosen"); + if (node < 0) + goto out; + + prop = fdt_getprop(fdt, node, "bootargs", NULL); + if (!prop) + goto out; + + match_options(prop); + + if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND)) + return; + } + +out: + match_options(CONFIG_CMDLINE); +} + +/* Keep checkers quiet */ +void init_feature_override(void); + +asmlinkage void __init init_feature_override(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(regs); i++
[PATCH v5 15/21] arm64: Add an aliasing facility for the idreg override
In order to map the override of idregs to options that a user can easily understand, let's introduce yet another option array, which maps an option to the corresponding idreg options. Signed-off-by: Marc Zyngier Reviewed-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/kernel/idreg-override.c | 17 + 1 file changed, 17 insertions(+) diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 489aa274e3ec..4fad3fc4d104 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -35,6 +35,12 @@ static const struct ftr_set_desc * const regs[] __initdata = { &mmfr1, }; +static const struct { + const char *alias; + const char *feature; +} aliases[] __initdata = { +}; + static char *cmdline_contains_option(const char *cmdline, const char *option) { char *str = strstr(cmdline, option); @@ -88,6 +94,15 @@ static void __init match_options(const char *cmdline) } } +static __init void match_aliases(const char *cmdline) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(aliases); i++) + if (cmdline_contains_option(cmdline, aliases[i].alias)) + match_options(aliases[i].feature); +} + static __init void parse_cmdline(void) { if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { @@ -108,6 +123,7 @@ static __init void parse_cmdline(void) goto out; match_options(prop); + match_aliases(prop); if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND)) return; @@ -115,6 +131,7 @@ static __init void parse_cmdline(void) out: match_options(CONFIG_CMDLINE); + match_aliases(CONFIG_CMDLINE); } /* Keep checkers quiet */ -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 08/21] arm64: Move SCTLR_EL1 initialisation to EL-agnostic code
We can now move the initial SCTLR_EL1 setup to be used for both EL1 and EL2 setup. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/kernel/head.S | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 36212c05df42..b425d2587cdb 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -479,13 +479,14 @@ EXPORT_SYMBOL(kimage_vaddr) * booted in EL1 or EL2 respectively. */ SYM_FUNC_START(init_kernel_el) + mov_q x0, INIT_SCTLR_EL1_MMU_OFF + msr sctlr_el1, x0 + mrs x0, CurrentEL cmp x0, #CurrentEL_EL2 b.eqinit_el2 SYM_INNER_LABEL(init_el1, SYM_L_LOCAL) - mov_q x0, INIT_SCTLR_EL1_MMU_OFF - msr sctlr_el1, x0 isb mov_q x0, INIT_PSTATE_EL1 msr spsr_el1, x0 @@ -494,9 +495,6 @@ SYM_INNER_LABEL(init_el1, SYM_L_LOCAL) eret SYM_INNER_LABEL(init_el2, SYM_L_LOCAL) - mov_q x0, INIT_SCTLR_EL1_MMU_OFF - msr sctlr_el1, x0 - mov_q x0, HCR_HOST_NVHE_FLAGS msr hcr_el2, x0 isb -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 09/21] arm64: cpufeature: Add global feature override facility
Add a facility to globally override a feature, no matter what the HW says. Yes, this sounds dangerous, but we do respect the "safe" value for a given feature. This doesn't mean the user doesn't need to know what they are doing. Nothing uses this yet, so we are pretty safe. For now. Signed-off-by: Marc Zyngier Reviewed-by: Suzuki K Poulose Acked-by: David Brazdil --- arch/arm64/include/asm/cpufeature.h | 6 + arch/arm64/kernel/cpufeature.c | 42 - 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 9a555809b89c..fe469389068e 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -63,6 +63,11 @@ struct arm64_ftr_bits { s64 safe_val; /* safe value for FTR_EXACT features */ }; +struct arm64_ftr_override { + u64 val; + u64 mask; +}; + /* * @arm64_ftr_reg - Feature register * @strict_maskBits which should match across all CPUs for sanity. @@ -74,6 +79,7 @@ struct arm64_ftr_reg { u64 user_mask; u64 sys_val; u64 user_val; + struct arm64_ftr_override *override; const struct arm64_ftr_bits *ftr_bits; }; diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index e99eddec0a46..cb58c7c991ef 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -352,9 +352,12 @@ static const struct arm64_ftr_bits ftr_ctr[] = { ARM64_FTR_END, }; +static struct arm64_ftr_override no_override = { }; + struct arm64_ftr_reg arm64_ftr_reg_ctrel0 = { .name = "SYS_CTR_EL0", - .ftr_bits = ftr_ctr + .ftr_bits = ftr_ctr, + .override = &no_override, }; static const struct arm64_ftr_bits ftr_id_mmfr0[] = { @@ -544,13 +547,16 @@ static const struct arm64_ftr_bits ftr_raz[] = { ARM64_FTR_END, }; -#define ARM64_FTR_REG(id, table) { \ - .sys_id = id, \ - .reg = &(struct arm64_ftr_reg){\ - .name = #id,\ - .ftr_bits = &((table)[0]), \ +#define ARM64_FTR_REG_OVERRIDE(id, table, ovr) { \ + .sys_id = id, \ + .reg = &(struct arm64_ftr_reg){\ + .name = #id,\ + .override = (ovr), \ + .ftr_bits = &((table)[0]), \ }} +#define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, &no_override) + static const struct __ftr_reg_entry { u32 sys_id; struct arm64_ftr_reg*reg; @@ -770,6 +776,30 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) { u64 ftr_mask = arm64_ftr_mask(ftrp); s64 ftr_new = arm64_ftr_value(ftrp, new); + s64 ftr_ovr = arm64_ftr_value(ftrp, reg->override->val); + + if ((ftr_mask & reg->override->mask) == ftr_mask) { + s64 tmp = arm64_ftr_safe_value(ftrp, ftr_ovr, ftr_new); + char *str = NULL; + + if (ftr_ovr != tmp) { + /* Unsafe, remove the override */ + reg->override->mask &= ~ftr_mask; + reg->override->val &= ~ftr_mask; + tmp = ftr_ovr; + str = "ignoring override"; + } else if (ftr_new != tmp) { + /* Override was valid */ + ftr_new = tmp; + str = "forced"; + } + + if (str) + pr_warn("%s[%d:%d]: %s to %llx\n", + reg->name, + ftrp->shift + ftrp->width - 1, + ftrp->shift, str, tmp); + } val = arm64_ftr_set_value(ftrp, val, ftr_new); -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 07/21] arm64: Simplify init_el2_state to be non-VHE only
As init_el2_state is now nVHE only, let's simplify it and drop the VHE setup. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/el2_setup.h | 36 +++--- arch/arm64/kernel/head.S | 2 +- arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h index 540116de80bf..d77d358f9395 100644 --- a/arch/arm64/include/asm/el2_setup.h +++ b/arch/arm64/include/asm/el2_setup.h @@ -32,16 +32,14 @@ * to transparently mess with the EL0 bits via CNTKCTL_EL1 access in * EL2. */ -.macro __init_el2_timers mode -.ifeqs "\mode", "nvhe" +.macro __init_el2_timers mrs x0, cnthctl_el2 orr x0, x0, #3 // Enable EL1 physical timers msr cnthctl_el2, x0 -.endif msr cntvoff_el2, xzr// Clear virtual offset .endm -.macro __init_el2_debug mode +.macro __init_el2_debug mrs x1, id_aa64dfr0_el1 sbfxx0, x1, #ID_AA64DFR0_PMUVER_SHIFT, #4 cmp x0, #1 @@ -55,7 +53,6 @@ ubfxx0, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4 cbz x0, .Lskip_spe_\@ // Skip if SPE not present -.ifeqs "\mode", "nvhe" mrs_s x0, SYS_PMBIDR_EL1 // If SPE available at EL2, and x0, x0, #(1 << SYS_PMBIDR_EL1_P_SHIFT) cbnzx0, .Lskip_spe_el2_\@ // then permit sampling of physical @@ -66,10 +63,6 @@ mov x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT) orr x2, x2, x0 // If we don't have VHE, then // use EL1&0 translation. -.else - orr x2, x2, #MDCR_EL2_TPMS // For VHE, use EL2 translation - // and disable access from EL1 -.endif .Lskip_spe_\@: msr mdcr_el2, x2// Configure debug traps @@ -145,37 +138,24 @@ /** * Initialize EL2 registers to sane values. This should be called early on all - * cores that were booted in EL2. + * cores that were booted in EL2. Note that everything gets initialised as + * if VHE was not evailable. The kernel context will be upgraded to VHE + * if possible later on in the boot process * * Regs: x0, x1 and x2 are clobbered. */ -.macro init_el2_state mode -.ifnes "\mode", "vhe" -.ifnes "\mode", "nvhe" -.error "Invalid 'mode' argument" -.endif -.endif - +.macro init_el2_state __init_el2_sctlr - __init_el2_timers \mode - __init_el2_debug \mode + __init_el2_timers + __init_el2_debug __init_el2_lor __init_el2_stage2 __init_el2_gicv3 __init_el2_hstr - - /* -* When VHE is not in use, early init of EL2 needs to be done here. -* When VHE _is_ in use, EL1 will not be used in the host and -* requires no configuration, and all non-hyp-specific EL2 setup -* will be done via the _EL1 system register aliases in __cpu_setup. -*/ -.ifeqs "\mode", "nvhe" __init_el2_nvhe_idregs __init_el2_nvhe_cptr __init_el2_nvhe_sve __init_el2_nvhe_prepare_eret -.endif .endm #endif /* __ARM_KVM_INIT_H__ */ diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 07445fd976ef..36212c05df42 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -501,7 +501,7 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL) msr hcr_el2, x0 isb - init_el2_state nvhe + init_el2_state /* Hypervisor stub */ adr_l x0, __hyp_stub_vectors diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S index 31b060a44045..222cfc3e7190 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S @@ -189,7 +189,7 @@ SYM_CODE_START_LOCAL(__kvm_hyp_init_cpu) 2: msr SPsel, #1 // We want to use SP_EL{1,2} /* Initialize EL2 CPU state to sane values. */ - init_el2_state nvhe // Clobbers x0..x2 + init_el2_state // Clobbers x0..x2 /* Enable MMU, set vectors and stack. */ mov x0, x28 -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 06/21] arm64: Move VHE-specific SPE setup to mutate_to_vhe()
There isn't much that a VHE kernel needs on top of whatever has been done for nVHE, so let's move the little we need to the VHE stub (the SPE setup), and drop the init_el2_state macro. No expected functional change. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/kernel/hyp-stub.S | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index 373ed2213e1d..6b5c73cf9d52 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -92,9 +92,6 @@ SYM_CODE_START_LOCAL(mutate_to_vhe) msr hcr_el2, x0 isb - // Doesn't do much on VHE, but still, worth a shot - init_el2_state vhe - // Use the EL1 allocated stack, per-cpu offset mrs x0, sp_el1 mov sp, x0 @@ -107,6 +104,31 @@ SYM_CODE_START_LOCAL(mutate_to_vhe) mrs_s x0, SYS_VBAR_EL12 msr vbar_el1, x0 + // Fixup SPE configuration, if supported... + mrs x1, id_aa64dfr0_el1 + ubfxx1, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4 + mov x2, xzr + cbz x1, skip_spe + + // ... and not owned by EL3 + mrs_s x0, SYS_PMBIDR_EL1 + and x0, x0, #(1 << SYS_PMBIDR_EL1_P_SHIFT) + cbnzx0, skip_spe + + // Let the SPE driver in control of the sampling + mrs_s x0, SYS_PMSCR_EL1 + bic x0, x0, #(1 << SYS_PMSCR_EL2_PCT_SHIFT) + bic x0, x0, #(1 << SYS_PMSCR_EL2_PA_SHIFT) + msr_s SYS_PMSCR_EL1, x0 + mov x2, #MDCR_EL2_TPMS + +skip_spe: + // For VHE, use EL2 translation and disable access from EL1 + mrs x0, mdcr_el2 + bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT) + orr x0, x0, x2 + msr mdcr_el2, x0 + // Transfer the MM state from EL1 to EL2 mrs_s x0, SYS_TCR_EL12 msr tcr_el1, x0 -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 05/21] arm64: Initialise as nVHE before switching to VHE
As we are aiming to be able to control whether we enable VHE or not, let's always drop down to EL1 first, and only then upgrade to VHE if at all possible. This means that if the kernel is booted at EL2, we always start with a nVHE init, drop to EL1 to initialise the the kernel, and only then upgrade the kernel EL to EL2 if possible (the process is obviously shortened for secondary CPUs). The resume path is handled similarly to a secondary CPU boot. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/kernel/head.S | 38 ++-- arch/arm64/kernel/hyp-stub.S | 24 +++ arch/arm64/kernel/sleep.S| 1 + 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 28e9735302df..07445fd976ef 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -433,6 +433,7 @@ SYM_FUNC_START_LOCAL(__primary_switched) bl __pi_memset dsb ishst // Make zero page visible to PTW + bl switch_to_vhe #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) bl kasan_early_init #endif @@ -493,42 +494,6 @@ SYM_INNER_LABEL(init_el1, SYM_L_LOCAL) eret SYM_INNER_LABEL(init_el2, SYM_L_LOCAL) -#ifdef CONFIG_ARM64_VHE - /* -* Check for VHE being present. x2 being non-zero indicates that we -* do have VHE, and that the kernel is intended to run at EL2. -*/ - mrs x2, id_aa64mmfr1_el1 - ubfxx2, x2, #ID_AA64MMFR1_VHE_SHIFT, #4 -#else - mov x2, xzr -#endif - cbz x2, init_el2_nvhe - - /* -* When VHE _is_ in use, EL1 will not be used in the host and -* requires no configuration, and all non-hyp-specific EL2 setup -* will be done via the _EL1 system register aliases in __cpu_setup. -*/ - mov_q x0, HCR_HOST_VHE_FLAGS - msr hcr_el2, x0 - isb - - init_el2_state vhe - - isb - - mov_q x0, INIT_PSTATE_EL2 - msr spsr_el2, x0 - msr elr_el2, lr - mov w0, #BOOT_CPU_MODE_EL2 - eret - -SYM_INNER_LABEL(init_el2_nvhe, SYM_L_LOCAL) - /* -* When VHE is not in use, early init of EL2 and EL1 needs to be -* done here. -*/ mov_q x0, INIT_SCTLR_EL1_MMU_OFF msr sctlr_el1, x0 @@ -623,6 +588,7 @@ SYM_FUNC_START_LOCAL(secondary_startup) /* * Common entry point for secondary CPUs. */ + bl switch_to_vhe bl __cpu_secondary_check52bitva bl __cpu_setup // initialise processor adrpx1, swapper_pg_dir diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index 3f3dbbe8914d..373ed2213e1d 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -190,3 +190,27 @@ SYM_FUNC_START(__hyp_reset_vectors) hvc #0 ret SYM_FUNC_END(__hyp_reset_vectors) + +/* + * Entry point to switch to VHE if deemed capable + */ +SYM_FUNC_START(switch_to_vhe) +#ifdef CONFIG_ARM64_VHE + // Need to have booted at EL2 + adr_l x1, __boot_cpu_mode + ldr w0, [x1] + cmp w0, #BOOT_CPU_MODE_EL2 + b.ne1f + + // and still be at EL1 + mrs x0, CurrentEL + cmp x0, #CurrentEL_EL1 + b.ne1f + + // Turn the world upside down + mov x0, #HVC_VHE_RESTART + hvc #0 +1: +#endif + ret +SYM_FUNC_END(switch_to_vhe) diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S index 6bdef7362c0e..5bfd9b87f85d 100644 --- a/arch/arm64/kernel/sleep.S +++ b/arch/arm64/kernel/sleep.S @@ -100,6 +100,7 @@ SYM_FUNC_END(__cpu_suspend_enter) .pushsection ".idmap.text", "awx" SYM_CODE_START(cpu_resume) bl init_kernel_el + bl switch_to_vhe bl __cpu_setup /* enable the MMU early - so we can access sleep_save_stash by va */ adrpx1, swapper_pg_dir -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 04/21] arm64: Provide an 'upgrade to VHE' stub hypercall
As we are about to change the way a VHE system boots, let's provide the core helper, in the form of a stub hypercall that enables VHE and replicates the full EL1 context at EL2, thanks to EL1 and VHE-EL2 being extremely similar. On exception return, the kernel carries on at EL2. Fancy! Nothing calls this new hypercall yet, so no functional change. Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/virt.h | 7 +++- arch/arm64/kernel/hyp-stub.S | 76 ++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h index ee6a48df89d9..7379f35ae2c6 100644 --- a/arch/arm64/include/asm/virt.h +++ b/arch/arm64/include/asm/virt.h @@ -35,8 +35,13 @@ */ #define HVC_RESET_VECTORS 2 +/* + * HVC_VHE_RESTART - Upgrade the CPU from EL1 to EL2, if possible + */ +#define HVC_VHE_RESTART3 + /* Max number of HYP stub hypercalls */ -#define HVC_STUB_HCALL_NR 3 +#define HVC_STUB_HCALL_NR 4 /* Error returned when an invalid stub number is passed into x0 */ #define HVC_STUB_ERR 0xbadca11 diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index 160f5881a0b7..3f3dbbe8914d 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -8,9 +8,9 @@ #include #include -#include #include +#include #include #include #include @@ -47,10 +47,13 @@ SYM_CODE_END(__hyp_stub_vectors) SYM_CODE_START_LOCAL(el1_sync) cmp x0, #HVC_SET_VECTORS - b.ne2f + b.ne1f msr vbar_el2, x1 b 9f +1: cmp x0, #HVC_VHE_RESTART + b.eqmutate_to_vhe + 2: cmp x0, #HVC_SOFT_RESTART b.ne3f mov x0, x2 @@ -70,6 +73,75 @@ SYM_CODE_START_LOCAL(el1_sync) eret SYM_CODE_END(el1_sync) +// nVHE? No way! Give me the real thing! +SYM_CODE_START_LOCAL(mutate_to_vhe) + // Be prepared to fail + mov_q x0, HVC_STUB_ERR + + // Sanity check: MMU *must* be off + mrs x1, sctlr_el2 + tbnzx1, #0, 1f + + // Needs to be VHE capable, obviously + mrs x1, id_aa64mmfr1_el1 + ubfxx1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4 + cbz x1, 1f + + // Engage the VHE magic! + mov_q x0, HCR_HOST_VHE_FLAGS + msr hcr_el2, x0 + isb + + // Doesn't do much on VHE, but still, worth a shot + init_el2_state vhe + + // Use the EL1 allocated stack, per-cpu offset + mrs x0, sp_el1 + mov sp, x0 + mrs x0, tpidr_el1 + msr tpidr_el2, x0 + + // FP configuration, vectors + mrs_s x0, SYS_CPACR_EL12 + msr cpacr_el1, x0 + mrs_s x0, SYS_VBAR_EL12 + msr vbar_el1, x0 + + // Transfer the MM state from EL1 to EL2 + mrs_s x0, SYS_TCR_EL12 + msr tcr_el1, x0 + mrs_s x0, SYS_TTBR0_EL12 + msr ttbr0_el1, x0 + mrs_s x0, SYS_TTBR1_EL12 + msr ttbr1_el1, x0 + mrs_s x0, SYS_MAIR_EL12 + msr mair_el1, x0 + isb + + // Invalidate TLBs before enabling the MMU + tlbivmalle1 + dsb nsh + + // Enable the EL2 S1 MMU, as set up from EL1 + mrs_s x0, SYS_SCTLR_EL12 + set_sctlr_el1 x0 + + // Disable the EL1 S1 MMU for a good measure + mov_q x0, INIT_SCTLR_EL1_MMU_OFF + msr_s SYS_SCTLR_EL12, x0 + + // Hack the exception return to stay at EL2 + mrs x0, spsr_el1 + and x0, x0, #~PSR_MODE_MASK + mov x1, #PSR_MODE_EL2h + orr x0, x0, x1 + msr spsr_el1, x0 + + mov x0, xzr + +1: eret +SYM_CODE_END(mutate_to_vhe) + .macro invalid_vector label SYM_CODE_START_LOCAL(\label) b \label -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 00/21] arm64: Early CPU feature override, and applications to VHE, BTI and PAuth
It recently came to light that there is a need to be able to override some CPU features very early on, before the kernel is fully up and running. The reasons for this range from specific feature support (such as using Protected KVM on VHE HW, which is the main motivation for this work) to errata workaround (a feature is broken on a CPU and needs to be turned off, or rather not enabled). This series tries to offer a limited framework for this kind of problems, by allowing a set of options to be passed on the command-line and altering the feature set that the cpufeature subsystem exposes to the rest of the kernel. Note that this doesn't change anything for code that directly uses the CPU ID registers. The series completely changes the way a VHE-capable system boots, by *always* booting non-VHE first, and then upgrading to VHE when deemed capable. Although it sounds scary, this is actually simple to implement (and I wish I had done that five years ago). The "upgrade to VHE" path is then conditioned on the VHE feature not being disabled from the command-line. Said command-line parsing borrows a lot from the kaslr code, and subsequently allows the "nokaslr" option to be moved to the new infrastructure (though it all looks a bit... odd). Further patches now add support for disabling BTI and PAuth, the latter being based on an initial series by Srinivas Ramana[0]. There is some ongoing discussions about being able to disable MTE, but no clear resolution on that subject yet. This has been tested on multiple VHE and non-VHE systems. * From v4 [4]: - Documentation fixes - Moved the val/mask pair into a arm64_ftr_override structure, leading to simpler code - All arm64_ftr_reg now have a default override, which simplifies the code a bit further - Dropped some of the "const" attributes - Renamed init_shadow_regs() to init_feature_override() - Renamed struct reg_desc to struct ftr_set_desc - Refactored command-line parsing - Simplified handling of VHE being disabled on the cmdline - Turn EL1 S1 MMU off on switch to VHE - HVC_VHE_RESTART now returns an error code on failure - Added missing asmlinkage and dummy prototypes - Collected Acks and RBs from David, Catalin and Suzuki * From v3 [3]: - Fixed the VHE_RESTART stub (duh!) - Switched to using arm64_ftr_safe_value() instead of the user provided value - Per-feature override warning * From v2 [2]: - Simplify the VHE_RESTART stub - Fixed a number of spelling mistakes, and hopefully introduced a few more - Override features in __read_sysreg_by_encoding() - Allow both BTI and PAuth to be overridden on the command line - Rebased on -rc3 * From v1 [1]: - Fix SPE init on VHE when EL2 doesn't own SPE - Fix re-init when KASLR is used - Handle the resume path - Rebased to 5.11-rc2 [0] https://lore.kernel.org/r/1610152163-16554-1-git-send-email-sram...@codeaurora.org [1] https://lore.kernel.org/r/20201228104958.1848833-1-...@kernel.org [2] https://lore.kernel.org/r/20210104135011.2063104-1-...@kernel.org [3] https://lore.kernel.org/r/2021032811.2455113-1-...@kernel.org [4] https://lore.kernel.org/r/20210118094533.2874082-1-...@kernel.org Marc Zyngier (20): arm64: Fix labels in el2_setup macros arm64: Fix outdated TCR setup comment arm64: Turn the MMU-on sequence into a macro arm64: Provide an 'upgrade to VHE' stub hypercall arm64: Initialise as nVHE before switching to VHE arm64: Move VHE-specific SPE setup to mutate_to_vhe() arm64: Simplify init_el2_state to be non-VHE only arm64: Move SCTLR_EL1 initialisation to EL-agnostic code arm64: cpufeature: Add global feature override facility arm64: cpufeature: Use IDreg override in __read_sysreg_by_encoding() arm64: Extract early FDT mapping from kaslr_early_init() arm64: cpufeature: Add an early command-line cpufeature override facility arm64: Allow ID_AA64MMFR1_EL1.VH to be overridden from the command line arm64: Honor VHE being disabled from the command-line arm64: Add an aliasing facility for the idreg override arm64: Make kvm-arm.mode={nvhe, protected} an alias of id_aa64mmfr1.vh=0 KVM: arm64: Document HVC_VHE_RESTART stub hypercall arm64: Move "nokaslr" over to the early cpufeature infrastructure arm64: cpufeatures: Allow disabling of BTI from the command-line arm64: cpufeatures: Allow disabling of Pointer Auth from the command-line Srinivas Ramana (1): arm64: Defer enabling pointer authentication on boot core .../admin-guide/kernel-parameters.txt | 9 + Documentation/virt/kvm/arm/hyp-abi.rst| 9 + arch/arm64/include/asm/assembler.h| 17 ++ arch/arm64/include/asm/cpufeature.h | 11 + arch/arm64/include/asm/el2_setup.h| 60 ++ arch/arm64/include/asm/pointer_auth.h | 10 + arch/arm64/include/asm/setup.h| 11 + arch/arm64/include/asm/stackprotector.h | 1 + arch/arm64/include/asm/virt.h
[PATCH v5 03/21] arm64: Turn the MMU-on sequence into a macro
Turning the MMU on is a popular sport in the arm64 kernel, and we do it more than once, or even twice. As we are about to add even more, let's turn it into a macro. No expected functional change. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/include/asm/assembler.h | 17 + arch/arm64/kernel/head.S | 19 --- arch/arm64/mm/proc.S | 12 +--- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index bf125c591116..8cded93f99c3 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -675,6 +675,23 @@ USER(\label, icivau, \tmp2)// invalidate I line PoU .endif .endm +/* + * Set SCTLR_EL1 to the passed value, and invalidate the local icache + * in the process. This is called when setting the MMU on. + */ +.macro set_sctlr_el1, reg + msr sctlr_el1, \reg + isb + /* +* Invalidate the local I-cache so that any instructions fetched +* speculatively from the PoC are discarded, since they may have +* been dynamically patched at the PoU. +*/ + ic iallu + dsb nsh + isb +.endm + /* * Check whether to yield to another runnable task from kernel mode NEON code * (which runs with preemption disabled). diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index a0dc987724ed..28e9735302df 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -703,16 +703,9 @@ SYM_FUNC_START(__enable_mmu) offset_ttbr1 x1, x3 msr ttbr1_el1, x1 // load TTBR1 isb - msr sctlr_el1, x0 - isb - /* -* Invalidate the local I-cache so that any instructions fetched -* speculatively from the PoC are discarded, since they may have -* been dynamically patched at the PoU. -*/ - ic iallu - dsb nsh - isb + + set_sctlr_el1 x0 + ret SYM_FUNC_END(__enable_mmu) @@ -883,11 +876,7 @@ SYM_FUNC_START_LOCAL(__primary_switch) tlbivmalle1 // Remove any stale TLB entries dsb nsh - msr sctlr_el1, x19 // re-enable the MMU - isb - ic iallu // flush instructions fetched - dsb nsh // via old mapping - isb + set_sctlr_el1 x19 // re-enable the MMU bl __relocate_kernel #endif diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index ece785477bdc..c967bfd30d2b 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -291,17 +291,7 @@ skip_pgd: /* We're done: fire up the MMU again */ mrs x17, sctlr_el1 orr x17, x17, #SCTLR_ELx_M - msr sctlr_el1, x17 - isb - - /* -* Invalidate the local I-cache so that any instructions fetched -* speculatively from the PoC are discarded, since they may have -* been dynamically patched at the PoU. -*/ - ic iallu - dsb nsh - isb + set_sctlr_el1 x17 /* Set the flag to zero to indicate that we're all done */ str wzr, [flag_ptr] -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 02/21] arm64: Fix outdated TCR setup comment
The arm64 kernel has long be able to use more than 39bit VAs. Since day one, actually. Let's rewrite the offending comment. Signed-off-by: Marc Zyngier Acked-by: Catalin Marinas Acked-by: David Brazdil --- arch/arm64/mm/proc.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 1f7ee8c8b7b8..ece785477bdc 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -464,8 +464,8 @@ SYM_FUNC_START(__cpu_setup) #endif msr mair_el1, x5 /* -* Set/prepare TCR and TTBR. We use 512GB (39-bit) address range for -* both user and kernel. +* Set/prepare TCR and TTBR. TCR_EL1.T1SZ gets further +* adjusted if the kernel is compiled with 52bit VA support. */ mov_q x10, TCR_TxSZ(VA_BITS) | TCR_CACHE_FLAGS | TCR_SMP_FLAGS | \ TCR_TG_FLAGS | TCR_KASLR_FLAGS | TCR_ASID16 | \ -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 01/21] arm64: Fix labels in el2_setup macros
If someone happens to write the following code: b 1f init_el2_state vhe 1: [...] they will be in for a long debugging session, as the label "1f" will be resolved *inside* the init_el2_state macro instead of after it. Not really what one expects. Instead, rewite the EL2 setup macros to use unambiguous labels, thanks to the usual macro counter trick. Acked-by: Catalin Marinas Signed-off-by: Marc Zyngier Acked-by: David Brazdil --- arch/arm64/include/asm/el2_setup.h | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h index a7f5a1bbc8ac..540116de80bf 100644 --- a/arch/arm64/include/asm/el2_setup.h +++ b/arch/arm64/include/asm/el2_setup.h @@ -45,24 +45,24 @@ mrs x1, id_aa64dfr0_el1 sbfxx0, x1, #ID_AA64DFR0_PMUVER_SHIFT, #4 cmp x0, #1 - b.lt1f // Skip if no PMU present + b.lt.Lskip_pmu_\@ // Skip if no PMU present mrs x0, pmcr_el0// Disable debug access traps ubfxx0, x0, #11, #5 // to EL2 and allow access to -1: +.Lskip_pmu_\@: cselx2, xzr, x0, lt // all PMU counters from EL1 /* Statistical profiling */ ubfxx0, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4 - cbz x0, 3f // Skip if SPE not present + cbz x0, .Lskip_spe_\@ // Skip if SPE not present .ifeqs "\mode", "nvhe" mrs_s x0, SYS_PMBIDR_EL1 // If SPE available at EL2, and x0, x0, #(1 << SYS_PMBIDR_EL1_P_SHIFT) - cbnzx0, 2f // then permit sampling of physical + cbnzx0, .Lskip_spe_el2_\@ // then permit sampling of physical mov x0, #(1 << SYS_PMSCR_EL2_PCT_SHIFT | \ 1 << SYS_PMSCR_EL2_PA_SHIFT) msr_s SYS_PMSCR_EL2, x0 // addresses and physical counter -2: +.Lskip_spe_el2_\@: mov x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT) orr x2, x2, x0 // If we don't have VHE, then // use EL1&0 translation. @@ -71,7 +71,7 @@ // and disable access from EL1 .endif -3: +.Lskip_spe_\@: msr mdcr_el2, x2// Configure debug traps .endm @@ -79,9 +79,9 @@ .macro __init_el2_lor mrs x1, id_aa64mmfr1_el1 ubfxx0, x1, #ID_AA64MMFR1_LOR_SHIFT, 4 - cbz x0, 1f + cbz x0, .Lskip_lor_\@ msr_s SYS_LORC_EL1, xzr -1: +.Lskip_lor_\@: .endm /* Stage-2 translation */ @@ -93,7 +93,7 @@ .macro __init_el2_gicv3 mrs x0, id_aa64pfr0_el1 ubfxx0, x0, #ID_AA64PFR0_GIC_SHIFT, #4 - cbz x0, 1f + cbz x0, .Lskip_gicv3_\@ mrs_s x0, SYS_ICC_SRE_EL2 orr x0, x0, #ICC_SRE_EL2_SRE// Set ICC_SRE_EL2.SRE==1 @@ -103,7 +103,7 @@ mrs_s x0, SYS_ICC_SRE_EL2 // Read SRE back, tbz x0, #0, 1f // and check that it sticks msr_s SYS_ICH_HCR_EL2, xzr// Reset ICC_HCR_EL2 to defaults -1: +.Lskip_gicv3_\@: .endm .macro __init_el2_hstr @@ -128,14 +128,14 @@ .macro __init_el2_nvhe_sve mrs x1, id_aa64pfr0_el1 ubfxx1, x1, #ID_AA64PFR0_SVE_SHIFT, #4 - cbz x1, 1f + cbz x1, .Lskip_sve_\@ bic x0, x0, #CPTR_EL2_TZ// Also disable SVE traps msr cptr_el2, x0// Disable copro. traps to EL2 isb mov x1, #ZCR_ELx_LEN_MASK // SVE: Enable full vector msr_s SYS_ZCR_EL2, x1 // length for EL1. -1: +.Lskip_sve_\@: .endm .macro __init_el2_nvhe_prepare_eret -- 2.29.2 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm