Re: [PATCH v4 00/26] KVM/ARM: Add support for GICv4
On 09/10/17 18:52, Konrad Rzeszutek Wilk wrote: > On Fri, Oct 06, 2017 at 04:33:35PM +0100, Marc Zyngier wrote: >> This series implements full support for GICv4 in KVM, bringing direct >> injection of MSIs to arm and arm64, assuming you have the right >> hardware (which is quite unlikely). >> >> To get an idea of the design, I'd recommend you start with commit >> 7954907bedaf as well as patch #26, which try to shed some light on the >> approach that I've taken. And before that, please digest some of the >> GICv3/GICv4 architecture documentation[1] (less than 800 pages!). Once >> you feel reasonably insane, you'll be in the right mood to read the > > You may want to update the link: > > s/pfd/pdf/ > > (Does that mean you typed this by hand, my gosh, you really are breathing > this everyday) :-) I am, and I'm feeling pretty dizzy. You can't imagine the kind of fumes this thing generates... ;-) I probably copy/pasted the URL, edited something, messed up the URL, fixed it (sort of), and ended posting the crap (twice!) without even realizing. Oh well. I'll blame the spec, as always. Thanks for the heads up! 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 v4 00/26] KVM/ARM: Add support for GICv4
On Fri, Oct 06, 2017 at 04:33:35PM +0100, Marc Zyngier wrote: > This series implements full support for GICv4 in KVM, bringing direct > injection of MSIs to arm and arm64, assuming you have the right > hardware (which is quite unlikely). > > To get an idea of the design, I'd recommend you start with commit > 7954907bedaf as well as patch #26, which try to shed some light on the > approach that I've taken. And before that, please digest some of the > GICv3/GICv4 architecture documentation[1] (less than 800 pages!). Once > you feel reasonably insane, you'll be in the right mood to read the You may want to update the link: s/pfd/pdf/ (Does that mean you typed this by hand, my gosh, you really are breathing this everyday) :-) > code (no, I didn't bother changing the above, I already hit LWN/QotW > once). ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 13/20] KVM: arm/arm64: Set VCPU affinity for virt timer irq
On 23/09/17 01:42, Christoffer Dall wrote:
> As we are about to take physical interrupts for the virtual timer on the
> host but want to leave those active while running the VM (and let the VM
> deactivate them), we need to set the vtimer PPI affinity accordingly.
>
> Signed-off-by: Christoffer Dall
> ---
> virt/kvm/arm/arch_timer.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 4254f88..4275f8f 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -650,11 +650,20 @@ int kvm_timer_hyp_init(void)
> return err;
> }
>
> + err = irq_set_vcpu_affinity(host_vtimer_irq, kvm_get_running_vcpus());
> + if (err) {
> + kvm_err("kvm_arch_timer: error setting vcpu affinity\n");
> + goto out_free_irq;
> + }
> +
> kvm_info("virtual timer IRQ%d\n", host_vtimer_irq);
>
> cpuhp_setup_state(CPUHP_AP_KVM_ARM_TIMER_STARTING,
> "kvm/arm/timer:starting", kvm_timer_starting_cpu,
> kvm_timer_dying_cpu);
> + return 0;
> +out_free_irq:
> + free_percpu_irq(host_vtimer_irq, kvm_get_running_vcpus());
> return err;
> }
>
>
Reviewed-by: Marc Zyngier
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 v3 12/20] genirq: Document vcpu_info usage for percpu_devid interrupts
On 23/09/17 01:41, Christoffer Dall wrote: > It is currently unclear how to set the VCPU affinity for a percpu_devid > interrupt , since the Linux irq_data structure describes the state for > multiple interrupts, one for each physical CPU on the system. Since > each such interrupt can be associated with different VCPUs or none at > all, associating a single VCPU state with such an interrupt does not > capture the necessary semantics. > > The implementers of irq_set_affinity are the Intel and AMD IOMMUs, and > the ARM GIC irqchip. The Intel and AMD callers do not appear to use > percpu_devid interrupts, and the ARM GIC implementation only checks the > pointer against NULL vs. non-NULL. > > Therefore, simply update the function documentation to explain the > expected use in the context of percpu_devid interrupts, allowing future > changes or additions to irqchip implementers to do the right thing. > > This allows us to set the VCPU affinity for the virtual timer interrupt > in KVM/ARM, which is a percpu_devid (PPI) interrupt. > > Cc: Thomas Gleixner > Cc: Marc Zyngier > Signed-off-by: Christoffer Dall > --- > kernel/irq/manage.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 573dc52..2b2c94f 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -381,7 +381,8 @@ int irq_select_affinity_usr(unsigned int irq) > /** > * irq_set_vcpu_affinity - Set vcpu affinity for the interrupt > * @irq: interrupt number to set affinity > - * @vcpu_info: vCPU specific data > + * @vcpu_info: vCPU specific data or pointer to a percpu array of vCPU > + * specific data for percpu_devid interrupts > * > * This function uses the vCPU specific data to set the vCPU > * affinity for an irq. The vCPU specific data is passed from > Acked-by: Marc Zyngier 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 v3 11/20] KVM: arm/arm64: Move timer save/restore out of the hyp code
On 23/09/17 01:41, Christoffer Dall wrote: > As we are about to be lazy with saving and restoring the timer > registers, we prepare by moving all possible timer configuration logic > out of the hyp code. All virtual timer registers can be programmed from > EL1 and since the arch timer is always a level triggered interrupt we > can safely do this with interrupts disabled in the host kernel on the > way to the guest without taking vtimer interrupts in the host kernel > (yet). > > The downside is that the cntvoff register can only be programmed from > hyp mode, so we jump into hyp mode and back to program it. This is also > safe, because the host kernel doesn't use the virtual timer in the KVM > code. It may add a little performance performance penalty, but only > until following commits where we move this operation to vcpu load/put. > > Signed-off-by: Christoffer Dall > --- > arch/arm/include/asm/kvm_asm.h | 2 ++ > arch/arm/include/asm/kvm_hyp.h | 4 +-- > arch/arm/kvm/hyp/switch.c| 7 ++-- > arch/arm64/include/asm/kvm_asm.h | 2 ++ > arch/arm64/include/asm/kvm_hyp.h | 4 +-- > arch/arm64/kvm/hyp/switch.c | 6 ++-- > virt/kvm/arm/arch_timer.c| 40 ++ > virt/kvm/arm/hyp/timer-sr.c | 74 > +--- > 8 files changed, 87 insertions(+), 52 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h > index 14d68a4..36dd296 100644 > --- a/arch/arm/include/asm/kvm_asm.h > +++ b/arch/arm/include/asm/kvm_asm.h > @@ -68,6 +68,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, > phys_addr_t ipa); > extern void __kvm_tlb_flush_vmid(struct kvm *kvm); > extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu); > > +extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high); > + > extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); > > extern void __init_stage2_translation(void); > diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h > index 14b5903..ab20ffa 100644 > --- a/arch/arm/include/asm/kvm_hyp.h > +++ b/arch/arm/include/asm/kvm_hyp.h > @@ -98,8 +98,8 @@ > #define cntvoff_el2 CNTVOFF > #define cnthctl_el2 CNTHCTL > > -void __timer_save_state(struct kvm_vcpu *vcpu); > -void __timer_restore_state(struct kvm_vcpu *vcpu); > +void __timer_enable_traps(struct kvm_vcpu *vcpu); > +void __timer_disable_traps(struct kvm_vcpu *vcpu); > > void __vgic_v2_save_state(struct kvm_vcpu *vcpu); > void __vgic_v2_restore_state(struct kvm_vcpu *vcpu); > diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c > index ebd2dd4..330c9ce 100644 > --- a/arch/arm/kvm/hyp/switch.c > +++ b/arch/arm/kvm/hyp/switch.c > @@ -174,7 +174,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu) > __activate_vm(vcpu); > > __vgic_restore_state(vcpu); > - __timer_restore_state(vcpu); > + __timer_enable_traps(vcpu); > > __sysreg_restore_state(guest_ctxt); > __banked_restore_state(guest_ctxt); > @@ -191,7 +191,8 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu) > > __banked_save_state(guest_ctxt); > __sysreg_save_state(guest_ctxt); > - __timer_save_state(vcpu); > + __timer_disable_traps(vcpu); > + > __vgic_save_state(vcpu); > > __deactivate_traps(vcpu); > @@ -237,7 +238,7 @@ void __hyp_text __noreturn __hyp_panic(int cause) > > vcpu = (struct kvm_vcpu *)read_sysreg(HTPIDR); > host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context); > - __timer_save_state(vcpu); > + __timer_disable_traps(vcpu); > __deactivate_traps(vcpu); > __deactivate_vm(vcpu); > __banked_restore_state(host_ctxt); > diff --git a/arch/arm64/include/asm/kvm_asm.h > b/arch/arm64/include/asm/kvm_asm.h > index 26a64d0..ab4d0a9 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -55,6 +55,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, > phys_addr_t ipa); > extern void __kvm_tlb_flush_vmid(struct kvm *kvm); > extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu); > > +extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high); > + > extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); > > extern u64 __vgic_v3_get_ich_vtr_el2(void); > diff --git a/arch/arm64/include/asm/kvm_hyp.h > b/arch/arm64/include/asm/kvm_hyp.h > index 4572a9b..08d3bb6 100644 > --- a/arch/arm64/include/asm/kvm_hyp.h > +++ b/arch/arm64/include/asm/kvm_hyp.h > @@ -129,8 +129,8 @@ void __vgic_v3_save_state(struct kvm_vcpu *vcpu); > void __vgic_v3_restore_state(struct kvm_vcpu *vcpu); > int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu); > > -void __timer_save_state(struct kvm_vcpu *vcpu); > -void __timer_restore_state(struct kvm_vcpu *vcpu); > +void __timer_enable_traps(struct kvm_vcpu *vcpu); > +void __timer_disable_traps(struct kv
Re: [PATCH v3 10/20] KVM: arm/arm64: Move timer/vgic flush/sync under disabled irq
On 23/09/17 01:41, Christoffer Dall wrote:
> As we are about to play tricks with the timer to be more lazy in saving
> and restoring state, we need to move the timer sync and flush functions
> under a disabled irq section and since we have to flush the vgic state
> after the timer and PMU state, we do the whole flush/sync sequence with
> disabled irqs.
>
> The only downside is a slightly longer delay before being able to
> process hardware interrupts and run softirqs.
>
> Signed-off-by: Christoffer Dall
> ---
> virt/kvm/arm/arm.c | 26 +-
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index b9f68e4..27db222 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -654,11 +654,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
>
> kvm_pmu_flush_hwstate(vcpu);
>
> + local_irq_disable();
> +
> kvm_timer_flush_hwstate(vcpu);
> kvm_vgic_flush_hwstate(vcpu);
>
> - local_irq_disable();
> -
> /*
>* If we have a singal pending, or need to notify a userspace
>* irqchip about timer or PMU level changes, then we exit (and
> @@ -683,10 +683,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
> kvm_request_pending(vcpu)) {
> vcpu->mode = OUTSIDE_GUEST_MODE;
> - local_irq_enable();
> kvm_pmu_sync_hwstate(vcpu);
> kvm_timer_sync_hwstate(vcpu);
> kvm_vgic_sync_hwstate(vcpu);
> + local_irq_enable();
> preempt_enable();
> continue;
> }
> @@ -710,6 +710,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> kvm_arm_clear_debug(vcpu);
>
> /*
> + * We must sync the PMU and timer state before the vgic state so
> + * that the vgic can properly sample the updated state of the
> + * interrupt line.
> + */
> + kvm_pmu_sync_hwstate(vcpu);
> + kvm_timer_sync_hwstate(vcpu);
> +
> + kvm_vgic_sync_hwstate(vcpu);
> +
> + /*
>* We may have taken a host interrupt in HYP mode (ie
>* while executing the guest). This interrupt is still
>* pending, as we haven't serviced it yet!
> @@ -732,16 +742,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> guest_exit();
> trace_kvm_exit(ret, kvm_vcpu_trap_get_class(vcpu),
> *vcpu_pc(vcpu));
>
> - /*
> - * We must sync the PMU and timer state before the vgic state so
> - * that the vgic can properly sample the updated state of the
> - * interrupt line.
> - */
> - kvm_pmu_sync_hwstate(vcpu);
> - kvm_timer_sync_hwstate(vcpu);
> -
> - kvm_vgic_sync_hwstate(vcpu);
> -
> preempt_enable();
>
> ret = handle_exit(vcpu, run, ret);
>
Reviewed-by: Marc Zyngier
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 v3 09/20] KVM: arm/arm64: Use separate timer for phys timer emulation
On 23/09/17 01:41, Christoffer Dall wrote:
> We were using the same hrtimer for emulating the physical timer and for
> making sure a blocking VCPU thread would be eventually woken up. That
> worked fine in the previous arch timer design, but as we are about to
> actually use the soft timer expire function for the physical timer
> emulation, change the logic to use a dedicated hrtimer.
>
> This has the added benefit of not having to cancel any work in the sync
> path, which in turn allows us to run the flush and sync with IRQs
> disabled.
>
> Signed-off-by: Christoffer Dall
> ---
> include/kvm/arm_arch_timer.h | 3 +++
> virt/kvm/arm/arch_timer.c| 18 ++
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index dcbb2e1..16887c0 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -47,6 +47,9 @@ struct arch_timer_cpu {
> /* Work queued with the above timer expires */
> struct work_struct expired;
>
> + /* Physical timer emulation */
> + struct hrtimer phys_timer;
> +
> /* Background timer active */
> boolarmed;
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index c2e8326..7f87099 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -178,6 +178,12 @@ static enum hrtimer_restart kvm_bg_timer_expire(struct
> hrtimer *hrt)
> return HRTIMER_NORESTART;
> }
>
> +static enum hrtimer_restart kvm_phys_timer_expire(struct hrtimer *hrt)
> +{
> + WARN(1, "Timer only used to ensure guest exit - unexpected event.");
> + return HRTIMER_NORESTART;
> +}
> +
So what prevents this handler from actually firing? Is it that we cancel
the hrtimer while interrupts are still disabled, hence the timer never
fires? If that's the intention, then this patch is slightly out of
place, as we haven't moved the timer sync within the irq_disable() section.
Or am I missing something obvious?
> bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
> {
> u64 cval, now;
> @@ -255,7 +261,7 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
> }
>
> /* Schedule the background timer for the emulated timer. */
> -static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> +static void phys_timer_emulate(struct kvm_vcpu *vcpu,
> struct arch_timer_context *timer_ctx)
> {
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> @@ -267,7 +273,7 @@ static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> return;
>
> /* The timer has not yet expired, schedule a background timer */
> - soft_timer_start(&timer->bg_timer, kvm_timer_compute_delta(timer_ctx));
> + soft_timer_start(&timer->phys_timer,
> kvm_timer_compute_delta(timer_ctx));
> }
>
> /*
> @@ -424,7 +430,7 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
> kvm_timer_update_state(vcpu);
>
> /* Set the background timer for the physical timer emulation. */
> - kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
> + phys_timer_emulate(vcpu, vcpu_ptimer(vcpu));
>
> if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
> kvm_timer_flush_hwstate_user(vcpu);
> @@ -447,7 +453,7 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>* This is to cancel the background timer for the physical timer
>* emulation if it is set.
>*/
> - soft_timer_cancel(&timer->bg_timer, &timer->expired);
> + soft_timer_cancel(&timer->phys_timer, NULL);
Right, that now explains the "work" test in one of the previous patches.
>
> /*
>* The guest could have modified the timer registers or the timer
> @@ -507,6 +513,9 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
> hrtimer_init(&timer->bg_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
> timer->bg_timer.function = kvm_bg_timer_expire;
>
> + hrtimer_init(&timer->phys_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
> + timer->phys_timer.function = kvm_phys_timer_expire;
> +
> vtimer->irq.irq = default_vtimer_irq.irq;
> ptimer->irq.irq = default_ptimer_irq.irq;
> }
> @@ -615,6 +624,7 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>
> soft_timer_cancel(&timer->bg_timer, &timer->expired);
> + soft_timer_cancel(&timer->phys_timer, NULL);
> kvm_vgic_unmap_phys_irq(vcpu, vtimer->irq.irq);
> }
>
>
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 v3 08/20] KVM: arm/arm64: Rename soft timer to bg_timer
On 23/09/17 01:41, Christoffer Dall wrote: > As we are about to introduce a separate hrtimer for the physical timer, > call this timer bg_timer, because we refer to this timer as the > background timer in the code and comments elsewhere. > > Signed-off-by: Christoffer Dall Acked-by: Marc Zyngier 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 v3 07/20] KVM: arm/arm64: Make timer_arm and timer_disarm helpers more generic
On 23/09/17 01:41, Christoffer Dall wrote:
> We are about to add an additional soft timer to the arch timer state for
> a VCPU and would like to be able to reuse the functions to program and
> cancel a timer, so we make them slightly more generic and rename to make
> it more clear that these functions work on soft timers and not the
> hardware resource that this code is managing.
>
> Signed-off-by: Christoffer Dall
> ---
> virt/kvm/arm/arch_timer.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 8e89d63..871d8ae 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -56,26 +56,22 @@ u64 kvm_phys_timer_read(void)
> return timecounter->cc->read(timecounter->cc);
> }
>
> -static bool timer_is_armed(struct arch_timer_cpu *timer)
> +static bool soft_timer_is_armed(struct arch_timer_cpu *timer)
> {
> return timer->armed;
> }
>
> -/* timer_arm: as in "arm the timer", not as in ARM the company */
> -static void timer_arm(struct arch_timer_cpu *timer, u64 ns)
> +static void soft_timer_start(struct hrtimer *hrt, u64 ns)
> {
> - timer->armed = true;
> - hrtimer_start(&timer->timer, ktime_add_ns(ktime_get(), ns),
> + hrtimer_start(hrt, ktime_add_ns(ktime_get(), ns),
> HRTIMER_MODE_ABS);
> }
>
> -static void timer_disarm(struct arch_timer_cpu *timer)
> +static void soft_timer_cancel(struct hrtimer *hrt, struct work_struct *work)
> {
> - if (timer_is_armed(timer)) {
> - hrtimer_cancel(&timer->timer);
> - cancel_work_sync(&timer->expired);
> - timer->armed = false;
> - }
> + hrtimer_cancel(hrt);
> + if (work)
When can this happen? Something in a following patch?
> + cancel_work_sync(work);
> }
>
> static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
> @@ -271,7 +267,7 @@ static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
> return;
>
> /* The timer has not yet expired, schedule a background timer */
> - timer_arm(timer, kvm_timer_compute_delta(timer_ctx));
> + soft_timer_start(&timer->timer, kvm_timer_compute_delta(timer_ctx));
> }
>
> /*
> @@ -285,7 +281,7 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>
> - BUG_ON(timer_is_armed(timer));
> + BUG_ON(soft_timer_is_armed(timer));
>
> /*
>* No need to schedule a background timer if any guest timer has
> @@ -306,13 +302,16 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>* The guest timers have not yet expired, schedule a background timer.
>* Set the earliest expiration time among the guest timers.
>*/
> - timer_arm(timer, kvm_timer_earliest_exp(vcpu));
> + timer->armed = true;
> + soft_timer_start(&timer->timer, kvm_timer_earliest_exp(vcpu));
> }
>
> void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
> {
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> - timer_disarm(timer);
> +
> + soft_timer_cancel(&timer->timer, &timer->expired);
> + timer->armed = false;
> }
>
> static void kvm_timer_flush_hwstate_vgic(struct kvm_vcpu *vcpu)
> @@ -448,7 +447,7 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>* This is to cancel the background timer for the physical timer
>* emulation if it is set.
>*/
> - timer_disarm(timer);
> + soft_timer_cancel(&timer->timer, &timer->expired);
timer_disarm() used to set timer->armed to false, but that's not the
case any more. Don't we risk hitting the BUG_ON() in kvm_timer_schedule
if we hit WFI?
>
> /*
>* The guest could have modified the timer registers or the timer
> @@ -615,7 +614,7 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>
> - timer_disarm(timer);
> + soft_timer_cancel(&timer->timer, &timer->expired);
> kvm_vgic_unmap_phys_irq(vcpu, vtimer->irq.irq);
> }
>
>
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 v3 06/20] KVM: arm/arm64: Check that system supports split eoi/deactivate
On 23/09/17 01:41, Christoffer Dall wrote: > Some systems without proper firmware and/or hardware description data > don't support the split EOI and deactivate operation. > > On such systems, we cannot leave the physical interrupt active after the > timer handler on the host has run, so we cannot support KVM with an > in-kernel GIC with the timer changes we are about to introduce. > > This patch makes sure that trying to initialize the KVM GIC code will > fail on such systems. > > Cc: Marc Zyngier > Signed-off-by: Christoffer Dall > --- > drivers/irqchip/irq-gic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index f641e8e..ab12bf4 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -1420,7 +1420,8 @@ static void __init gic_of_setup_kvm_info(struct > device_node *node) > if (ret) > return; > > - gic_set_kvm_info(&gic_v2_kvm_info); > + if (static_key_true(&supports_deactivate)) > + gic_set_kvm_info(&gic_v2_kvm_info); > } > > int __init > Should we add the same level of checking on the ACPI path, just for the sake symmetry? Also, do we need to add the same thing for GICv3? Otherwise looks OK to me. 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 v3 05/20] KVM: arm/arm64: Support calling vgic_update_irq_pending from irq context
On 23/09/17 01:41, Christoffer Dall wrote:
> We are about to optimize our timer handling logic which involves
> injecting irqs to the vgic directly from the irq handler.
>
> Unfortunately, the injection path can take any AP list lock and irq lock
> and we must therefore make sure to use spin_lock_irqsave where ever
> interrupts are enabled and we are taking any of those locks, to avoid
> deadlocking between process context and the ISR.
>
> This changes a lot of the VGIC code, but The good news are that the
> changes are mostly mechanical.
>
> Signed-off-by: Christoffer Dall
> ---
> virt/kvm/arm/vgic/vgic-its.c | 17 +++-
> virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 +--
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 17 +++-
> virt/kvm/arm/vgic/vgic-mmio.c| 44 +
> virt/kvm/arm/vgic/vgic-v2.c | 5 ++--
> virt/kvm/arm/vgic/vgic-v3.c | 12
> virt/kvm/arm/vgic/vgic.c | 60
> +---
> virt/kvm/arm/vgic/vgic.h | 3 +-
> 8 files changed, 108 insertions(+), 72 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index f51c1e1..9f5e347 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -278,6 +278,7 @@ static int update_lpi_config(struct kvm *kvm, struct
> vgic_irq *irq,
> u64 propbase = GICR_PROPBASER_ADDRESS(kvm->arch.vgic.propbaser);
> u8 prop;
> int ret;
> + unsigned long flags;
>
> ret = kvm_read_guest(kvm, propbase + irq->intid - GIC_LPI_OFFSET,
>&prop, 1);
> @@ -285,15 +286,15 @@ static int update_lpi_config(struct kvm *kvm, struct
> vgic_irq *irq,
> if (ret)
> return ret;
>
> - spin_lock(&irq->irq_lock);
> + spin_lock_irqsave(&irq->irq_lock, flags);
>
> if (!filter_vcpu || filter_vcpu == irq->target_vcpu) {
> irq->priority = LPI_PROP_PRIORITY(prop);
> irq->enabled = LPI_PROP_ENABLE_BIT(prop);
>
> - vgic_queue_irq_unlock(kvm, irq);
> + vgic_queue_irq_unlock(kvm, irq, flags);
> } else {
> - spin_unlock(&irq->irq_lock);
> + spin_unlock_irqrestore(&irq->irq_lock, flags);
> }
>
> return 0;
> @@ -393,6 +394,7 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu
> *vcpu)
> int ret = 0;
> u32 *intids;
> int nr_irqs, i;
> + unsigned long flags;
>
> nr_irqs = vgic_copy_lpi_list(vcpu, &intids);
> if (nr_irqs < 0)
> @@ -420,9 +422,9 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu
> *vcpu)
> }
>
> irq = vgic_get_irq(vcpu->kvm, NULL, intids[i]);
> - spin_lock(&irq->irq_lock);
> + spin_lock_irqsave(&irq->irq_lock, flags);
> irq->pending_latch = pendmask & (1U << bit_nr);
> - vgic_queue_irq_unlock(vcpu->kvm, irq);
> + vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
> vgic_put_irq(vcpu->kvm, irq);
> }
>
> @@ -515,6 +517,7 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct
> vgic_its *its,
> {
> struct kvm_vcpu *vcpu;
> struct its_ite *ite;
> + unsigned long flags;
>
> if (!its->enabled)
> return -EBUSY;
> @@ -530,9 +533,9 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct
> vgic_its *its,
> if (!vcpu->arch.vgic_cpu.lpis_enabled)
> return -EBUSY;
>
> - spin_lock(&ite->irq->irq_lock);
> + spin_lock_irqsave(&ite->irq->irq_lock, flags);
> ite->irq->pending_latch = true;
> - vgic_queue_irq_unlock(kvm, ite->irq);
> + vgic_queue_irq_unlock(kvm, ite->irq, flags);
>
> return 0;
> }
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c
> b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> index b3d4a10..e21e2f4 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> @@ -74,6 +74,7 @@ static void vgic_mmio_write_sgir(struct kvm_vcpu
> *source_vcpu,
> int mode = (val >> 24) & 0x03;
> int c;
> struct kvm_vcpu *vcpu;
> + unsigned long flags;
>
> switch (mode) {
> case 0x0: /* as specified by targets */
> @@ -97,11 +98,11 @@ static void vgic_mmio_write_sgir(struct kvm_vcpu
> *source_vcpu,
>
> irq = vgic_get_irq(source_vcpu->kvm, vcpu, intid);
>
> - spin_lock(&irq->irq_lock);
> + spin_lock_irqsave(&irq->irq_lock, flags);
> irq->pending_latch = true;
> irq->source |= 1U << source_vcpu->vcpu_id;
>
> - vgic_queue_irq_unlock(source_vcpu->kvm, irq);
> + vgic_queue_irq_unlock(source_vcpu->kvm, irq, flags);
> vgic_put_irq(source_vcpu->kvm, irq);
> }
> }
> @@ -131,6 +132,7 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
> u32 intid = VGIC_ADDR_TO_INTID(addr, 8);
> u8 cpu_mask = GENMAS
Re: [PATCH v3 04/20] KVM: arm/arm64: Guard kvm_vgic_map_is_active against !vgic_initialized
On 23/09/17 01:41, Christoffer Dall wrote: > If the vgic is not initialized, don't try to grab its spinlocks or > traverse its data structures. > > This is important because we soon have to start considering the active > state of a virtual interrupts when doing vcpu_load, which may happen > early on before the vgic is initialized. > > Signed-off-by: Christoffer Dall > --- > virt/kvm/arm/vgic/vgic.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c > index fed717e..e1f7dbc 100644 > --- a/virt/kvm/arm/vgic/vgic.c > +++ b/virt/kvm/arm/vgic/vgic.c > @@ -777,6 +777,9 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, > unsigned int virt_irq) > struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq); > bool map_is_active; > > + if (!vgic_initialized(vcpu->kvm)) > + return false; > + > spin_lock(&irq->irq_lock); > map_is_active = irq->hw && irq->active; > spin_unlock(&irq->irq_lock); > Acked-by: Marc Zyngier 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 v3 03/20] arm64: Use the physical counter when available for read_cycles
On 23/09/17 01:41, Christoffer Dall wrote:
> Currently get_cycles() is hardwired to arch_counter_get_cntvct() on
> arm64, but as we move to using the physical timer for the in-kernel
> time-keeping, we need to make that more flexible.
>
> First, we need to make sure the physical counter can be read on equal
> terms to the virtual counter, which includes adding physical counter
> read functions for timers that require errata.
>
> Second, we need to make a choice between reading the physical vs virtual
> counter, depending on which timer is used for time keeping in the kernel
> otherwise. We can do this using a static key to avoid a performance
> penalty during runtime when reading the counter.
>
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Mark Rutland
> Cc: Marc Zyngier
> Signed-off-by: Christoffer Dall
Right. I should have read patch #3. I'm an idiot.
> ---
> arch/arm64/include/asm/arch_timer.h | 15 ---
> arch/arm64/include/asm/timex.h | 2 +-
> drivers/clocksource/arm_arch_timer.c | 32 ++--
> 3 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/arch_timer.h
> b/arch/arm64/include/asm/arch_timer.h
> index 1859a1c..c56d8cd 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -30,6 +30,8 @@
>
> #include
>
> +extern struct static_key_false arch_timer_phys_counter_available;
> +
> #if IS_ENABLED(CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND)
> extern struct static_key_false arch_timer_read_ool_enabled;
> #define needs_unstable_timer_counter_workaround() \
> @@ -52,6 +54,7 @@ struct arch_timer_erratum_workaround {
> const char *desc;
> u32 (*read_cntp_tval_el0)(void);
> u32 (*read_cntv_tval_el0)(void);
> + u64 (*read_cntpct_el0)(void);
> u64 (*read_cntvct_el0)(void);
> int (*set_next_event_phys)(unsigned long, struct clock_event_device *);
> int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
> @@ -148,10 +151,8 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>
> static inline u64 arch_counter_get_cntpct(void)
> {
> - u64 cval;
> isb();
> - asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
> - return cval;
> + return arch_timer_reg_read_stable(cntpct_el0);
> }
>
> static inline u64 arch_counter_get_cntvct(void)
> @@ -160,6 +161,14 @@ static inline u64 arch_counter_get_cntvct(void)
> return arch_timer_reg_read_stable(cntvct_el0);
> }
>
> +static inline u64 arch_counter_get_cycles(void)
> +{
> + if (static_branch_unlikely(&arch_timer_phys_counter_available))
> + return arch_counter_get_cntpct();
> + else
> + return arch_counter_get_cntvct();
> +}
> +
> static inline int arch_timer_arch_init(void)
> {
> return 0;
> diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h
> index 81a076e..c0d214c 100644
> --- a/arch/arm64/include/asm/timex.h
> +++ b/arch/arm64/include/asm/timex.h
> @@ -22,7 +22,7 @@
> * Use the current timer as a cycle counter since this is what we use for
> * the delay loop.
> */
> -#define get_cycles() arch_counter_get_cntvct()
> +#define get_cycles() arch_counter_get_cycles()
Why can't this be arch_timer_read_counter() instead? Is there any
measurable advantage in using a static key compared to a memory
indirection?
>
> #include
>
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index 9b3322a..f35da20 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -77,6 +77,9 @@ static bool arch_timer_mem_use_virtual;
> static bool arch_counter_suspend_stop;
> static bool vdso_default = true;
>
> +DEFINE_STATIC_KEY_FALSE(arch_timer_phys_counter_available);
> +EXPORT_SYMBOL_GPL(arch_timer_phys_counter_available);
> +
> static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
>
> static int __init early_evtstrm_cfg(char *buf)
> @@ -217,6 +220,11 @@ static u32 notrace fsl_a008585_read_cntv_tval_el0(void)
> return __fsl_a008585_read_reg(cntv_tval_el0);
> }
>
> +static u64 notrace fsl_a008585_read_cntpct_el0(void)
> +{
> + return __fsl_a008585_read_reg(cntpct_el0);
> +}
> +
> static u64 notrace fsl_a008585_read_cntvct_el0(void)
> {
> return __fsl_a008585_read_reg(cntvct_el0);
> @@ -258,6 +266,11 @@ static u32 notrace
> hisi_161010101_read_cntv_tval_el0(void)
> return __hisi_161010101_read_reg(cntv_tval_el0);
> }
>
> +static u64 notrace hisi_161010101_read_cntpct_el0(void)
> +{
> + return __hisi_161010101_read_reg(cntpct_el0);
> +}
> +
> static u64 notrace hisi_161010101_read_cntvct_el0(void)
> {
> return __hisi_161010101_read_reg(cntvct_el0);
> @@ -288,6 +301,15 @@ static struct ate_acpi_oem_info
> hisi_161010101_oem_info[] = {
> #endif
>
> #ifdef CONFIG_ARM64_ERRATUM_858921
> +static u64 notrace arm64_858921_read_cntpct_el0
Re: [PATCH v2 26/28] arm64/sve: Add documentation
On Mon, Oct 09, 2017 at 03:07:23PM +0100, Alex Bennée wrote: > > Dave Martin writes: > > > On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote: > >> > >> Dave Martin writes: > >> > >> > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote: > >> >> On 31/08/17 18:00, Dave Martin wrote: > >> >> > +9. System runtime configuration > >> >> > + > >> >> > + > >> >> > +* To mitigate the ABI impact of expansion of the signal frame, a > >> >> > policy > >> >> > + mechanism is provided for administrators, distro maintainers and > >> >> > developers > >> >> > + to set the default vector length for userspace processes: > >> >> > + > >> >> > +/proc/cpu/sve_default_vector_length > >> >> > >> >> > >> >> elsewhere in the patch series i see > >> >> > >> >> /proc/sys/abi/sve_default_vector_length > >> >> > >> >> is this supposed to be the same? > >> > > >> > Good spot, thanks! > >> > > >> > /proc/cpu/ was the old location: they should both say /proc/abi/. > >> > I'll fix it. > >> > >> Isn't /sys (or rather sysfs) the preferred location for modern control > >> knobs that mirror the kernels object model or is SVE a special case for > >> extending /proc? > > > > I couldn't figure out which kernel object this maps to. There's no > > device, no driver. This isn't even per-cpu. > > Hmm I can see: > > /sys/devices/system/cpu > > On both my x86 and arm64 systems - but I guess this is more ABIish than > CPU feature related. > > > sysctl is already used for similar knobs to this one, so I followed that > > precedent -- though if someone argues strongly enough it could be > > changed. > > > > Are there already examples of arch controls like this in sysfs? I > > wasn't aware of any, but I didn't look all that hard... > > Given the paucity of the /proc/sys/abi on both systems I guess this sort > of knob is rare enough that people haven't expressed a strong preference > for sysfs here. I have no objection to staying with /proc/sys/abi/. That was my thinking: sysctls tend to control the kernel, especially process behaviour, whereas /sys/ controls devices and subsystems. That's not a concrete rule though and not written down, and doubtless a major new set of sysctls would be shot down regardless of what they do. Part of the problem with /proc is that people historically put things in there that have random ad-hoc behaviour and semantics. The sysctl framework at least imposes some sanity here. There is also some support for specialising sysctls to user namespaces, which makes some sense in that /proc/sys/abi/* should probably be per- container -- though whether it's ever considered important enough to actually be implemented is another question. I certainly don't attempt to do it today. I don't know how sysfs interacts with namespaces, but probably it can. I guess I'll wait for someone to object loudly... Cheers ---Dave ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 02/20] arm64: Use physical counter for in-kernel reads
On 23/09/17 01:41, Christoffer Dall wrote:
> Using the physical counter allows KVM to retain the offset between the
> virtual and physical counter as long as it is actively running a VCPU.
>
> As soon as a VCPU is released, another thread is scheduled or we start
> running userspace applications, we reset the offset to 0, so that
> userspace accessing the virtual timer can still read the cirtual counter
s/cirtual/virtual/
> and get the same view of time as the kernel.
>
> This opens up potential improvements for KVM performance.
>
> VHE kernels or kernels continuing to use the virtual timer are
> unaffected.
>
> Signed-off-by: Christoffer Dall
> ---
> arch/arm64/include/asm/arch_timer.h | 9 -
> drivers/clocksource/arm_arch_timer.c | 3 +--
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/include/asm/arch_timer.h
> b/arch/arm64/include/asm/arch_timer.h
> index a652ce0..1859a1c 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -148,11 +148,10 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>
> static inline u64 arch_counter_get_cntpct(void)
> {
> - /*
> - * AArch64 kernel and user space mandate the use of CNTVCT.
> - */
> - BUG();
> - return 0;
> + u64 cval;
> + isb();
> + asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
> + return cval;
This would be just fine if we were blessed with quality HW. This is
unfortunately not the case, and we need a staggering amount of crap to
deal with timer errata.
I suggest you replace this with the (fully untested) following:
diff --git a/arch/arm64/include/asm/arch_timer.h
b/arch/arm64/include/asm/arch_timer.h
index a652ce0a5cb2..04275de614db 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -52,6 +52,7 @@ struct arch_timer_erratum_workaround {
const char *desc;
u32 (*read_cntp_tval_el0)(void);
u32 (*read_cntv_tval_el0)(void);
+ u64 (*read_cntpct_el0)(void);
u64 (*read_cntvct_el0)(void);
int (*set_next_event_phys)(unsigned long, struct clock_event_device *);
int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
@@ -148,11 +149,8 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
static inline u64 arch_counter_get_cntpct(void)
{
- /*
-* AArch64 kernel and user space mandate the use of CNTVCT.
-*/
- BUG();
- return 0;
+ isb();
+ return arch_timer_reg_read_stable(cntpct_el0);
}
static inline u64 arch_counter_get_cntvct(void)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index fd4b7f684bd0..5b41a96fa8dd 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -217,6 +217,11 @@ static u32 notrace fsl_a008585_read_cntv_tval_el0(void)
return __fsl_a008585_read_reg(cntv_tval_el0);
}
+static u64 notrace fsl_a008585_read_cntpct_el0(void)
+{
+ return __fsl_a008585_read_reg(cntpct_el0);
+}
+
static u64 notrace fsl_a008585_read_cntvct_el0(void)
{
return __fsl_a008585_read_reg(cntvct_el0);
@@ -258,6 +263,11 @@ static u32 notrace hisi_161010101_read_cntv_tval_el0(void)
return __hisi_161010101_read_reg(cntv_tval_el0);
}
+static u64 notrace hisi_161010101_read_cntpct_el0(void)
+{
+ return __hisi_161010101_read_reg(cntpct_el0);
+}
+
static u64 notrace hisi_161010101_read_cntvct_el0(void)
{
return __hisi_161010101_read_reg(cntvct_el0);
@@ -296,6 +306,15 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
new = read_sysreg(cntvct_el0);
return (((old ^ new) >> 32) & 1) ? old : new;
}
+
+static u64 notrace arm64_858921_read_cntpct_el0(void)
+{
+ u64 old, new;
+
+ old = read_sysreg(cntpct_el0);
+ new = read_sysreg(cntpct_el0);
+ return (((old ^ new) >> 32) & 1) ? old : new;
+}
#endif
#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
@@ -310,7 +329,7 @@ static void erratum_set_next_event_tval_generic(const int
access, unsigned long
struct clock_event_device *clk)
{
unsigned long ctrl;
- u64 cval = evt + arch_counter_get_cntvct();
+ u64 cval = evt + arch_timer_read_counter();
ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
ctrl |= ARCH_TIMER_CTRL_ENABLE;
@@ -346,6 +365,7 @@ static const struct arch_timer_erratum_workaround
ool_workarounds[] = {
.desc = "Freescale erratum a005858",
.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
+ .read_cntpct_el0 = fsl_a008585_read_cntpct_el0,
.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
.set_next_event_phys = erratum_set_next_event_tval_phys,
.set_next_event_virt = erratum_set_next_eve
[PATCH 04/10] arm64: KVM: PTE/PMD S2 XN bit definition
As we're about to make S2 page-tables eXecute Never by default, add the required bits for both PMDs and PTEs. Signed-off-by: Marc Zyngier --- arch/arm64/include/asm/pgtable-hwdef.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h index eb0c2bd90de9..af035331fb09 100644 --- a/arch/arm64/include/asm/pgtable-hwdef.h +++ b/arch/arm64/include/asm/pgtable-hwdef.h @@ -177,9 +177,11 @@ */ #define PTE_S2_RDONLY (_AT(pteval_t, 1) << 6) /* HAP[2:1] */ #define PTE_S2_RDWR(_AT(pteval_t, 3) << 6) /* HAP[2:1] */ +#define PTE_S2_XN (_AT(pteval_t, 2) << 53) /* XN[1:0] */ #define PMD_S2_RDONLY (_AT(pmdval_t, 1) << 6) /* HAP[2:1] */ #define PMD_S2_RDWR(_AT(pmdval_t, 3) << 6) /* HAP[2:1] */ +#define PMD_S2_XN (_AT(pmdval_t, 2) << 53) /* XN[1:0] */ /* * Memory Attribute override for Stage-2 (MemAttr[3:0]) -- 2.14.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 02/10] arm64: KVM: Add invalidate_icache_range helper
We currently tightly couple dcache clean with icache invalidation, but KVM could do without the initial flush to PoU, as we've already flushed things to PoC. Let's introduce invalidate_icache_range which is limited to invalidating the icache from the linear mapping (and thus has none of the userspace fault handling complexity), and wire it in KVM instead of flush_icache_range. Signed-off-by: Marc Zyngier --- arch/arm64/include/asm/cacheflush.h | 8 arch/arm64/include/asm/kvm_mmu.h| 4 ++-- arch/arm64/mm/cache.S | 24 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h index 76d1cc85d5b1..ad56406944c6 100644 --- a/arch/arm64/include/asm/cacheflush.h +++ b/arch/arm64/include/asm/cacheflush.h @@ -52,6 +52,13 @@ * - start - virtual start address * - end- virtual end address * + * invalidate_icache_range(start, end) + * + * Invalidate the I-cache in the region described by start, end. + * Linear mapping only! + * - start - virtual start address + * - end- virtual end address + * * __flush_cache_user_range(start, end) * * Ensure coherency between the I-cache and the D-cache in the @@ -66,6 +73,7 @@ * - size - region size */ extern void flush_icache_range(unsigned long start, unsigned long end); +extern void invalidate_icache_range(unsigned long start, unsigned long end); extern void __flush_dcache_area(void *addr, size_t len); extern void __inval_dcache_area(void *addr, size_t len); extern void __clean_dcache_area_poc(void *addr, size_t len); diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h index 4c4cb4f0e34f..48d31ca2ce9c 100644 --- a/arch/arm64/include/asm/kvm_mmu.h +++ b/arch/arm64/include/asm/kvm_mmu.h @@ -250,8 +250,8 @@ static inline void __coherent_icache_guest_page(struct kvm_vcpu *vcpu, /* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */ void *va = page_address(pfn_to_page(pfn)); - flush_icache_range((unsigned long)va, - (unsigned long)va + size); + invalidate_icache_range((unsigned long)va, + (unsigned long)va + size); } } diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S index 7f1dbe962cf5..0c330666a8c9 100644 --- a/arch/arm64/mm/cache.S +++ b/arch/arm64/mm/cache.S @@ -80,6 +80,30 @@ USER(9f, ic ivau, x4) // invalidate I line PoU ENDPROC(flush_icache_range) ENDPROC(__flush_cache_user_range) +/* + * invalidate_icache_range(start,end) + * + * Ensure that the I cache is invalid within specified region. This + * assumes that this is done on the linear mapping. Do not use it + * on a userspace range, as this may fault horribly. + * + * - start - virtual start address of region + * - end - virtual end address of region + */ +ENTRY(invalidate_icache_range) + icache_line_size x2, x3 + sub x3, x2, #1 + bic x4, x0, x3 +1: + ic ivau, x4// invalidate I line PoU + add x4, x4, x2 + cmp x4, x1 + b.lo1b + dsb ish + isb + ret +ENDPROC(invalidate_icache_range) + /* * __flush_dcache_area(kaddr, size) * -- 2.14.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 07/10] KVM: arm/arm64: Preserve Exec permission across R/W permission faults
So far, we loose the Exec property whenever we take permission
faults, as we always reconstruct the PTE/PMD from scratch. This
can be counter productive as we can end-up with the following
fault sequence:
X -> RO -> ROX -> RW -> RWX
Instead, we can lookup the existing PTE/PMD and clear the XN bit in the
new entry if it was already cleared in the old one, leadig to a much
nicer fault sequence:
X -> ROX -> RWX
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_mmu.h | 10 ++
arch/arm64/include/asm/kvm_mmu.h | 10 ++
virt/kvm/arm/mmu.c | 25 +
3 files changed, 45 insertions(+)
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index bf76150aad5f..ad442d86c23e 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -107,6 +107,11 @@ static inline bool kvm_s2pte_readonly(pte_t *pte)
return (pte_val(*pte) & L_PTE_S2_RDWR) == L_PTE_S2_RDONLY;
}
+static inline bool kvm_s2pte_exec(pte_t *pte)
+{
+ return !(pte_val(*pte) & L_PTE_XN);
+}
+
static inline void kvm_set_s2pmd_readonly(pmd_t *pmd)
{
pmd_val(*pmd) = (pmd_val(*pmd) & ~L_PMD_S2_RDWR) | L_PMD_S2_RDONLY;
@@ -117,6 +122,11 @@ static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
return (pmd_val(*pmd) & L_PMD_S2_RDWR) == L_PMD_S2_RDONLY;
}
+static inline bool kvm_s2pmd_exec(pmd_t *pmd)
+{
+ return !(pmd_val(*pmd) & PMD_SECT_XN);
+}
+
static inline bool kvm_page_empty(void *ptr)
{
struct page *ptr_page = virt_to_page(ptr);
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 60c420a5ac0d..e7af74b8b51a 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -203,6 +203,11 @@ static inline bool kvm_s2pte_readonly(pte_t *pte)
return (pte_val(*pte) & PTE_S2_RDWR) == PTE_S2_RDONLY;
}
+static inline bool kvm_s2pte_exec(pte_t *pte)
+{
+ return !(pte_val(*pte) & PTE_S2_XN);
+}
+
static inline void kvm_set_s2pmd_readonly(pmd_t *pmd)
{
kvm_set_s2pte_readonly((pte_t *)pmd);
@@ -213,6 +218,11 @@ static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
return kvm_s2pte_readonly((pte_t *)pmd);
}
+static inline bool kvm_s2pmd_exec(pmd_t *pmd)
+{
+ return !(pmd_val(*pmd) & PMD_S2_XN);
+}
+
static inline bool kvm_page_empty(void *ptr)
{
struct page *ptr_page = virt_to_page(ptr);
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 1911fadde88b..ccc6106764a6 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -926,6 +926,17 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct
kvm_mmu_memory_cache
return 0;
}
+static pte_t *stage2_get_pte(struct kvm *kvm, phys_addr_t addr)
+{
+ pmd_t *pmdp;
+
+ pmdp = stage2_get_pmd(kvm, NULL, addr);
+ if (!pmdp || pmd_none(*pmdp))
+ return NULL;
+
+ return pte_offset_kernel(pmdp, addr);
+}
+
static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
phys_addr_t addr, const pte_t *new_pte,
unsigned long flags)
@@ -1407,6 +1418,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
if (exec_fault) {
new_pmd = kvm_s2pmd_mkexec(new_pmd);
coherent_icache_guest_page(vcpu, pfn, PMD_SIZE);
+ } else if (fault_status == FSC_PERM) {
+ /* Preserve execute if XN was already cleared */
+ pmd_t *old_pmdp = stage2_get_pmd(kvm, NULL, fault_ipa);
+
+ if (old_pmdp && pmd_present(*old_pmdp) &&
+ kvm_s2pmd_exec(old_pmdp))
+ new_pmd = kvm_s2pmd_mkexec(new_pmd);
}
ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd);
@@ -1425,6 +1443,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
if (exec_fault) {
new_pte = kvm_s2pte_mkexec(new_pte);
coherent_icache_guest_page(vcpu, pfn, PAGE_SIZE);
+ } else if (fault_status == FSC_PERM) {
+ /* Preserve execute if XN was already cleared */
+ pte_t *old_ptep = stage2_get_pte(kvm, fault_ipa);
+
+ if (old_ptep && pte_present(*old_ptep) &&
+ kvm_s2pte_exec(old_ptep))
+ new_pte = kvm_s2pte_mkexec(new_pte);
}
ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, flags);
--
2.14.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 00/10] arm/arm64: KVM: limit icache invalidation to prefetch aborts
It was recently reported that on a VM restore, we seem to spend a
disproportionate amount of time invalidation the icache. This is
partially due to some HW behaviour, but also because we're being a bit
dumb and are invalidating the icache for every page we map at S2, even
if that on a data access.
The slightly better way of doing this is to mark the pages XN at S2,
and wait for the the guest to execute something in that page, at which
point we perform the invalidation. As it is likely that there is a lot
less instruction than data, we win (or so we hope).
We also take this opportunity to drop the extra dcache clean to the
PoU which is pretty useless, as we already clean all the way to the
PoC...
Running a bare metal test that touches 1GB of memory (using a 4kB
stride) leads to the following results on Seattle:
4.13:
do_fault_read.bin: 0.565885992 seconds time elapsed
do_fault_write.bin: 0.738296337 seconds time elapsed
do_fault_read_write.bin: 1.241812231 seconds time elapsed
4.14-rc3+patches:
do_fault_read.bin: 0.244961803 seconds time elapsed
do_fault_write.bin: 0.422740092 seconds time elapsed
do_fault_read_write.bin: 0.643402470 seconds time elapsed
We're almost halving the time of something that more or less looks
like a restore operation. Some larger systems will show much bigger
benefits as they become less impacted by the icache invalidation
(which is broadcast in the inner shareable domain).
I've also given it a test run on both Cubietruck and Jetson-TK1.
Tests are archived here:
https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvm-ws-tests.git/
I'd value some additional test results on HW I don't have access to.
Thanks,
M.
Marc Zyngier (10):
KVM: arm/arm64: Split dcache/icache flushing
arm64: KVM: Add invalidate_icache_range helper
arm: KVM: Add optimized PIPT icache flushing
arm64: KVM: PTE/PMD S2 XN bit definition
KVM: arm/arm64: Limit icache invalidation to prefetch aborts
KVM: arm/arm64: Only clean the dcache on translation fault
KVM: arm/arm64: Preserve Exec permission across R/W permission faults
KVM: arm/arm64: Drop vcpu parameter from
coherent_{d,i}cache_guest_page
KVM: arm/arm64: Detangle kvm_mmu.h from kvm_hyp.h
arm: KVM: Use common implementation for all flushes to PoC
arch/arm/include/asm/kvm_hyp.h | 3 +-
arch/arm/include/asm/kvm_mmu.h | 110 +++--
arch/arm/include/asm/pgtable.h | 4 +-
arch/arm/kvm/hyp/switch.c | 1 +
arch/arm/kvm/hyp/tlb.c | 1 +
arch/arm64/include/asm/cacheflush.h| 8 +++
arch/arm64/include/asm/kvm_hyp.h | 1 -
arch/arm64/include/asm/kvm_mmu.h | 37 +--
arch/arm64/include/asm/pgtable-hwdef.h | 2 +
arch/arm64/include/asm/pgtable-prot.h | 4 +-
arch/arm64/kvm/hyp/debug-sr.c | 1 +
arch/arm64/kvm/hyp/switch.c| 1 +
arch/arm64/kvm/hyp/tlb.c | 1 +
arch/arm64/mm/cache.S | 24 +++
virt/kvm/arm/hyp/vgic-v2-sr.c | 1 +
virt/kvm/arm/mmu.c | 68 +---
16 files changed, 213 insertions(+), 54 deletions(-)
--
2.14.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 06/10] KVM: arm/arm64: Only clean the dcache on translation fault
The only case where we actually need to perform a dache maintenance
is when we map the page for the first time, and subsequent permission
faults do not require cache maintenance. Let's make it conditional
on not being a permission fault (and thus a translation fault).
Signed-off-by: Marc Zyngier
---
virt/kvm/arm/mmu.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 1d47da22f75c..1911fadde88b 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1400,7 +1400,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
new_pmd = kvm_s2pmd_mkwrite(new_pmd);
kvm_set_pfn_dirty(pfn);
}
- coherent_dcache_guest_page(vcpu, pfn, PMD_SIZE);
+
+ if (fault_status != FSC_PERM)
+ coherent_dcache_guest_page(vcpu, pfn, PMD_SIZE);
if (exec_fault) {
new_pmd = kvm_s2pmd_mkexec(new_pmd);
@@ -1416,7 +1418,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
kvm_set_pfn_dirty(pfn);
mark_page_dirty(kvm, gfn);
}
- coherent_dcache_guest_page(vcpu, pfn, PAGE_SIZE);
+
+ if (fault_status != FSC_PERM)
+ coherent_dcache_guest_page(vcpu, pfn, PAGE_SIZE);
if (exec_fault) {
new_pte = kvm_s2pte_mkexec(new_pte);
--
2.14.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 10/10] arm: KVM: Use common implementation for all flushes to PoC
We currently have no less than three implementations for the
"flush to PoC" code. Let standardize on a single one. This
requires a bit of unpleasant moving around, and relies on
__kvm_flush_dcache_pte and co being #defines so that they can
call into coherent_dcache_guest_page...
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_mmu.h | 28
virt/kvm/arm/mmu.c | 20 ++--
2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index 5f1ac88a5951..011b0db85c02 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -235,31 +235,11 @@ static inline void __coherent_icache_guest_page(kvm_pfn_t
pfn,
}
}
-static inline void __kvm_flush_dcache_pte(pte_t pte)
-{
- void *va = kmap_atomic(pte_page(pte));
-
- kvm_flush_dcache_to_poc(va, PAGE_SIZE);
-
- kunmap_atomic(va);
-}
-
-static inline void __kvm_flush_dcache_pmd(pmd_t pmd)
-{
- unsigned long size = PMD_SIZE;
- kvm_pfn_t pfn = pmd_pfn(pmd);
-
- while (size) {
- void *va = kmap_atomic_pfn(pfn);
+#define __kvm_flush_dcache_pte(p) \
+ coherent_dcache_guest_page(pte_pfn((p)), PAGE_SIZE)
- kvm_flush_dcache_to_poc(va, PAGE_SIZE);
-
- pfn++;
- size -= PAGE_SIZE;
-
- kunmap_atomic(va);
- }
-}
+#define __kvm_flush_dcache_pmd(p) \
+ coherent_dcache_guest_page(pmd_pfn((p)), PMD_SIZE)
static inline void __kvm_flush_dcache_pud(pud_t pud)
{
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 5b495450e92f..ab027fdf76e7 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -70,6 +70,16 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm,
phys_addr_t ipa)
kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
}
+static void coherent_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
+{
+ __coherent_dcache_guest_page(pfn, size);
+}
+
+static void coherent_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
+{
+ __coherent_icache_guest_page(pfn, size);
+}
+
/*
* D-Cache management functions. They take the page table entries by
* value, as they are flushing the cache using the kernel mapping (or
@@ -1268,16 +1278,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm
*kvm,
kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
}
-static void coherent_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
- __coherent_dcache_guest_page(pfn, size);
-}
-
-static void coherent_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
- __coherent_icache_guest_page(pfn, size);
-}
-
static void kvm_send_hwpoison_signal(unsigned long address,
struct vm_area_struct *vma)
{
--
2.14.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 05/10] KVM: arm/arm64: Limit icache invalidation to prefetch aborts
We've so far eagerly invalidated the icache, no matter how
the page was faulted in (data or prefetch abort).
But we can easily track execution by setting the XN bits
in the S2 page tables, get the prefetch abort at HYP and
perform the icache invalidation at that time only.
As for most VMs, the instruction working set is pretty
small compared to the data set, this is likely to save
some traffic (specially as the invalidation is broadcast).
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_mmu.h| 12
arch/arm/include/asm/pgtable.h| 4 ++--
arch/arm64/include/asm/kvm_mmu.h | 12
arch/arm64/include/asm/pgtable-prot.h | 4 ++--
virt/kvm/arm/mmu.c| 19 +++
5 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index 6773dcf21bff..bf76150aad5f 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -85,6 +85,18 @@ static inline pmd_t kvm_s2pmd_mkwrite(pmd_t pmd)
return pmd;
}
+static inline pte_t kvm_s2pte_mkexec(pte_t pte)
+{
+ pte_val(pte) &= ~L_PTE_XN;
+ return pte;
+}
+
+static inline pmd_t kvm_s2pmd_mkexec(pmd_t pmd)
+{
+ pmd_val(pmd) &= ~PMD_SECT_XN;
+ return pmd;
+}
+
static inline void kvm_set_s2pte_readonly(pte_t *pte)
{
pte_val(*pte) = (pte_val(*pte) & ~L_PTE_S2_RDWR) | L_PTE_S2_RDONLY;
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 1c462381c225..9b6e77b9ab7e 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -102,8 +102,8 @@ extern pgprot_t pgprot_s2_device;
#define PAGE_HYP_EXEC _MOD_PROT(pgprot_kernel, L_PTE_HYP |
L_PTE_RDONLY)
#define PAGE_HYP_RO_MOD_PROT(pgprot_kernel, L_PTE_HYP |
L_PTE_RDONLY | L_PTE_XN)
#define PAGE_HYP_DEVICE_MOD_PROT(pgprot_hyp_device, L_PTE_HYP)
-#define PAGE_S2_MOD_PROT(pgprot_s2, L_PTE_S2_RDONLY)
-#define PAGE_S2_DEVICE _MOD_PROT(pgprot_s2_device, L_PTE_S2_RDONLY)
+#define PAGE_S2_MOD_PROT(pgprot_s2, L_PTE_S2_RDONLY |
L_PTE_XN)
+#define PAGE_S2_DEVICE _MOD_PROT(pgprot_s2_device, L_PTE_S2_RDONLY |
L_PTE_XN)
#define __PAGE_NONE__pgprot(_L_PTE_DEFAULT | L_PTE_RDONLY |
L_PTE_XN | L_PTE_NONE)
#define __PAGE_SHARED __pgprot(_L_PTE_DEFAULT | L_PTE_USER | L_PTE_XN)
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 48d31ca2ce9c..60c420a5ac0d 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -173,6 +173,18 @@ static inline pmd_t kvm_s2pmd_mkwrite(pmd_t pmd)
return pmd;
}
+static inline pte_t kvm_s2pte_mkexec(pte_t pte)
+{
+ pte_val(pte) &= ~PTE_S2_XN;
+ return pte;
+}
+
+static inline pmd_t kvm_s2pmd_mkexec(pmd_t pmd)
+{
+ pmd_val(pmd) &= ~PMD_S2_XN;
+ return pmd;
+}
+
static inline void kvm_set_s2pte_readonly(pte_t *pte)
{
pteval_t old_pteval, pteval;
diff --git a/arch/arm64/include/asm/pgtable-prot.h
b/arch/arm64/include/asm/pgtable-prot.h
index 0a5635fb0ef9..4e12dabd342b 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -60,8 +60,8 @@
#define PAGE_HYP_RO__pgprot(_PAGE_DEFAULT | PTE_HYP | PTE_RDONLY |
PTE_HYP_XN)
#define PAGE_HYP_DEVICE__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
-#define PAGE_S2__pgprot(PROT_DEFAULT |
PTE_S2_MEMATTR(MT_S2_NORMAL) | PTE_S2_RDONLY)
-#define PAGE_S2_DEVICE __pgprot(PROT_DEFAULT |
PTE_S2_MEMATTR(MT_S2_DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_UXN)
+#define PAGE_S2__pgprot(PROT_DEFAULT |
PTE_S2_MEMATTR(MT_S2_NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_DEVICE __pgprot(PROT_DEFAULT |
PTE_S2_MEMATTR(MT_S2_DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
#define PAGE_NONE __pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) |
PTE_PROT_NONE | PTE_RDONLY | PTE_PXN | PTE_UXN)
#define PAGE_SHARED__pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG |
PTE_PXN | PTE_UXN | PTE_WRITE)
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 9e5628388af8..1d47da22f75c 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1292,7 +1292,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
unsigned long fault_status)
{
int ret;
- bool write_fault, writable, hugetlb = false, force_pte = false;
+ bool write_fault, exec_fault, writable, hugetlb = false, force_pte =
false;
unsigned long mmu_seq;
gfn_t gfn = fault_ipa >> PAGE_SHIFT;
struct kvm *kvm = vcpu->kvm;
@@ -1304,7 +1304,10 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
unsigned long flags = 0;
write_fault = kvm_is_write_fault(vcpu);
- if (fault_stat
[PATCH 09/10] KVM: arm/arm64: Detangle kvm_mmu.h from kvm_hyp.h
kvm_hyp.h has an odd dependency on kvm_mmu.h, which makes the
opposite inclusion impossible. Let's start with breaking that
useless dependency.
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_hyp.h | 1 -
arch/arm/kvm/hyp/switch.c| 1 +
arch/arm/kvm/hyp/tlb.c | 1 +
arch/arm64/include/asm/kvm_hyp.h | 1 -
arch/arm64/kvm/hyp/debug-sr.c| 1 +
arch/arm64/kvm/hyp/switch.c | 1 +
arch/arm64/kvm/hyp/tlb.c | 1 +
virt/kvm/arm/hyp/vgic-v2-sr.c| 1 +
8 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index ad541f9ecc78..8b29faa119ba 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
#define __hyp_text __section(.hyp.text) notrace
diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
index ebd2dd46adf7..67e0a689c4b5 100644
--- a/arch/arm/kvm/hyp/switch.c
+++ b/arch/arm/kvm/hyp/switch.c
@@ -18,6 +18,7 @@
#include
#include
+#include
__asm__(".arch_extension virt");
diff --git a/arch/arm/kvm/hyp/tlb.c b/arch/arm/kvm/hyp/tlb.c
index 6d810af2d9fd..c0edd450e104 100644
--- a/arch/arm/kvm/hyp/tlb.c
+++ b/arch/arm/kvm/hyp/tlb.c
@@ -19,6 +19,7 @@
*/
#include
+#include
/**
* Flush per-VMID TLBs
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 4572a9b560fa..afbfbe0c12c5 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -20,7 +20,6 @@
#include
#include
-#include
#include
#define __hyp_text __section(.hyp.text) notrace
diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
index f5154ed3da6c..d3a13d57f2c5 100644
--- a/arch/arm64/kvm/hyp/debug-sr.c
+++ b/arch/arm64/kvm/hyp/debug-sr.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#define read_debug(r,n)read_sysreg(r##n##_el1)
#define write_debug(v,r,n) write_sysreg(v, r##n##_el1)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 945e79c641c4..c52f7094122f 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
static bool __hyp_text __fpsimd_enabled_nvhe(void)
diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 73464a96c365..131c7772703c 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -16,6 +16,7 @@
*/
#include
+#include
#include
static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
diff --git a/virt/kvm/arm/hyp/vgic-v2-sr.c b/virt/kvm/arm/hyp/vgic-v2-sr.c
index a3f18d362366..77ccd8e2090b 100644
--- a/virt/kvm/arm/hyp/vgic-v2-sr.c
+++ b/virt/kvm/arm/hyp/vgic-v2-sr.c
@@ -21,6 +21,7 @@
#include
#include
+#include
static void __hyp_text save_elrsr(struct kvm_vcpu *vcpu, void __iomem *base)
{
--
2.14.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 08/10] KVM: arm/arm64: Drop vcpu parameter from coherent_{d, i}cache_guest_page
The vcpu parameter isn't used for anything, and gets in the way of
further cleanups. Let's get rid of it.
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_mmu.h | 6 ++
arch/arm64/include/asm/kvm_mmu.h | 6 ++
virt/kvm/arm/mmu.c | 18 --
3 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index ad442d86c23e..5f1ac88a5951 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -150,8 +150,7 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu
*vcpu)
return (vcpu_cp15(vcpu, c1_SCTLR) & 0b101) == 0b101;
}
-static inline void __coherent_dcache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
+static inline void __coherent_dcache_guest_page(kvm_pfn_t pfn,
unsigned long size)
{
/*
@@ -177,8 +176,7 @@ static inline void __coherent_dcache_guest_page(struct
kvm_vcpu *vcpu,
}
}
-static inline void __coherent_icache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
+static inline void __coherent_icache_guest_page(kvm_pfn_t pfn,
unsigned long size)
{
u32 iclsz;
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index e7af74b8b51a..33dcc3c79574 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -252,8 +252,7 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu
*vcpu)
return (vcpu_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
}
-static inline void __coherent_dcache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
+static inline void __coherent_dcache_guest_page(kvm_pfn_t pfn,
unsigned long size)
{
void *va = page_address(pfn_to_page(pfn));
@@ -261,8 +260,7 @@ static inline void __coherent_dcache_guest_page(struct
kvm_vcpu *vcpu,
kvm_flush_dcache_to_poc(va, size);
}
-static inline void __coherent_icache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
+static inline void __coherent_icache_guest_page(kvm_pfn_t pfn,
unsigned long size)
{
if (icache_is_aliasing()) {
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index ccc6106764a6..5b495450e92f 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1268,16 +1268,14 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm
*kvm,
kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
}
-static void coherent_dcache_guest_page(struct kvm_vcpu *vcpu, kvm_pfn_t pfn,
- unsigned long size)
+static void coherent_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
{
- __coherent_dcache_guest_page(vcpu, pfn, size);
+ __coherent_dcache_guest_page(pfn, size);
}
-static void coherent_icache_guest_page(struct kvm_vcpu *vcpu, kvm_pfn_t pfn,
- unsigned long size)
+static void coherent_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
{
- __coherent_icache_guest_page(vcpu, pfn, size);
+ __coherent_icache_guest_page(pfn, size);
}
static void kvm_send_hwpoison_signal(unsigned long address,
@@ -1413,11 +1411,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
}
if (fault_status != FSC_PERM)
- coherent_dcache_guest_page(vcpu, pfn, PMD_SIZE);
+ coherent_dcache_guest_page(pfn, PMD_SIZE);
if (exec_fault) {
new_pmd = kvm_s2pmd_mkexec(new_pmd);
- coherent_icache_guest_page(vcpu, pfn, PMD_SIZE);
+ coherent_icache_guest_page(pfn, PMD_SIZE);
} else if (fault_status == FSC_PERM) {
/* Preserve execute if XN was already cleared */
pmd_t *old_pmdp = stage2_get_pmd(kvm, NULL, fault_ipa);
@@ -1438,11 +1436,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
}
if (fault_status != FSC_PERM)
- coherent_dcache_guest_page(vcpu, pfn, PAGE_SIZE);
+ coherent_dcache_guest_page(pfn, PAGE_SIZE);
if (exec_fault) {
new_pte = kvm_s2pte_mkexec(new_pte);
- coherent_icache_guest_page(vcpu, pfn, PAGE_SIZE);
+ coherent_icache_guest_page(pfn, PAGE_SIZE);
} else if (fault_status == FSC_PERM) {
/* Preserve execute if XN was already cleared */
pte_t *old_ptep = stage2_get_pte(kvm, fault_ipa);
--
2.
[PATCH 01/10] KVM: arm/arm64: Split dcache/icache flushing
As we're about to introduce opportunistic invalidation of the icache,
let's split dcache and icache flushing.
Signed-off-by: Marc Zyngier
---
arch/arm/include/asm/kvm_mmu.h | 60
arch/arm64/include/asm/kvm_mmu.h | 13 +++--
virt/kvm/arm/mmu.c | 20 ++
3 files changed, 67 insertions(+), 26 deletions(-)
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index fa6f2174276b..f553aa62d0c3 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -126,21 +126,12 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu
*vcpu)
return (vcpu_cp15(vcpu, c1_SCTLR) & 0b101) == 0b101;
}
-static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
- unsigned long size)
+static inline void __coherent_dcache_guest_page(struct kvm_vcpu *vcpu,
+ kvm_pfn_t pfn,
+ unsigned long size)
{
/*
-* If we are going to insert an instruction page and the icache is
-* either VIPT or PIPT, there is a potential problem where the host
-* (or another VM) may have used the same page as this guest, and we
-* read incorrect data from the icache. If we're using a PIPT cache,
-* we can invalidate just that page, but if we are using a VIPT cache
-* we need to invalidate the entire icache - damn shame - as written
-* in the ARM ARM (DDI 0406C.b - Page B3-1393).
-*
-* VIVT caches are tagged using both the ASID and the VMID and doesn't
-* need any kind of flushing (DDI 0406C.b - Page B3-1392).
+* Clean the dcache to the Point of Coherency.
*
* We need to do this through a kernel mapping (using the
* user-space mapping has proved to be the wrong
@@ -155,19 +146,52 @@ static inline void __coherent_cache_guest_page(struct
kvm_vcpu *vcpu,
kvm_flush_dcache_to_poc(va, PAGE_SIZE);
- if (icache_is_pipt())
- __cpuc_coherent_user_range((unsigned long)va,
- (unsigned long)va +
PAGE_SIZE);
-
size -= PAGE_SIZE;
pfn++;
kunmap_atomic(va);
}
+}
- if (!icache_is_pipt() && !icache_is_vivt_asid_tagged()) {
+static inline void __coherent_icache_guest_page(struct kvm_vcpu *vcpu,
+ kvm_pfn_t pfn,
+ unsigned long size)
+{
+ /*
+* If we are going to insert an instruction page and the icache is
+* either VIPT or PIPT, there is a potential problem where the host
+* (or another VM) may have used the same page as this guest, and we
+* read incorrect data from the icache. If we're using a PIPT cache,
+* we can invalidate just that page, but if we are using a VIPT cache
+* we need to invalidate the entire icache - damn shame - as written
+* in the ARM ARM (DDI 0406C.b - Page B3-1393).
+*
+* VIVT caches are tagged using both the ASID and the VMID and doesn't
+* need any kind of flushing (DDI 0406C.b - Page B3-1392).
+*/
+
+ VM_BUG_ON(size & ~PAGE_MASK);
+
+ if (icache_is_vivt_asid_tagged())
+ return;
+
+ if (!icache_is_pipt()) {
/* any kind of VIPT cache */
__flush_icache_all();
+ return;
+ }
+
+ /* PIPT cache. As for the d-side, use a temporary kernel mapping. */
+ while (size) {
+ void *va = kmap_atomic_pfn(pfn);
+
+ __cpuc_coherent_user_range((unsigned long)va,
+ (unsigned long)va + PAGE_SIZE);
+
+ size -= PAGE_SIZE;
+ pfn++;
+
+ kunmap_atomic(va);
}
}
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 672c8684d5c2..4c4cb4f0e34f 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -230,19 +230,26 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu
*vcpu)
return (vcpu_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
}
-static inline void __coherent_cache_guest_page(struct kvm_vcpu *vcpu,
- kvm_pfn_t pfn,
- unsigned long size)
+static inline void __coherent_dcache_guest_page(struct kvm_vcpu *vcpu,
+ kvm_pfn_t pfn,
+ unsigned long size)
{
void *va = page_address(pfn_to_page(pfn));
kvm_flush_dcache_to_poc(va, size);
+}
+static inline void __coherent_icache_gues
Re: [PATCH v2 26/28] arm64/sve: Add documentation
Dave Martin writes: > On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote: >> >> Dave Martin writes: >> >> > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote: >> >> On 31/08/17 18:00, Dave Martin wrote: >> >> > +9. System runtime configuration >> >> > + >> >> > + >> >> > +* To mitigate the ABI impact of expansion of the signal frame, a policy >> >> > + mechanism is provided for administrators, distro maintainers and >> >> > developers >> >> > + to set the default vector length for userspace processes: >> >> > + >> >> > +/proc/cpu/sve_default_vector_length >> >> >> >> >> >> elsewhere in the patch series i see >> >> >> >> /proc/sys/abi/sve_default_vector_length >> >> >> >> is this supposed to be the same? >> > >> > Good spot, thanks! >> > >> > /proc/cpu/ was the old location: they should both say /proc/abi/. >> > I'll fix it. >> >> Isn't /sys (or rather sysfs) the preferred location for modern control >> knobs that mirror the kernels object model or is SVE a special case for >> extending /proc? > > I couldn't figure out which kernel object this maps to. There's no > device, no driver. This isn't even per-cpu. Hmm I can see: /sys/devices/system/cpu On both my x86 and arm64 systems - but I guess this is more ABIish than CPU feature related. > sysctl is already used for similar knobs to this one, so I followed that > precedent -- though if someone argues strongly enough it could be > changed. > > Are there already examples of arch controls like this in sysfs? I > wasn't aware of any, but I didn't look all that hard... Given the paucity of the /proc/sys/abi on both systems I guess this sort of knob is rare enough that people haven't expressed a strong preference for sysfs here. I have no objection to staying with /proc/sys/abi/. -- Alex Bennée ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 26/28] arm64/sve: Add documentation
On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote: > > Dave Martin writes: > > > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote: > >> On 31/08/17 18:00, Dave Martin wrote: > >> > +9. System runtime configuration > >> > + > >> > + > >> > +* To mitigate the ABI impact of expansion of the signal frame, a policy > >> > + mechanism is provided for administrators, distro maintainers and > >> > developers > >> > + to set the default vector length for userspace processes: > >> > + > >> > +/proc/cpu/sve_default_vector_length > >> > >> > >> elsewhere in the patch series i see > >> > >> /proc/sys/abi/sve_default_vector_length > >> > >> is this supposed to be the same? > > > > Good spot, thanks! > > > > /proc/cpu/ was the old location: they should both say /proc/abi/. > > I'll fix it. > > Isn't /sys (or rather sysfs) the preferred location for modern control > knobs that mirror the kernels object model or is SVE a special case for > extending /proc? I couldn't figure out which kernel object this maps to. There's no device, no driver. This isn't even per-cpu. sysctl is already used for similar knobs to this one, so I followed that precedent -- though if someone argues strongly enough it could be changed. Are there already examples of arch controls like this in sysfs? I wasn't aware of any, but I didn't look all that hard... Cheers ---Dave ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 26/28] arm64/sve: Add documentation
Dave Martin writes: > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote: >> On 31/08/17 18:00, Dave Martin wrote: >> > +9. System runtime configuration >> > + >> > + >> > +* To mitigate the ABI impact of expansion of the signal frame, a policy >> > + mechanism is provided for administrators, distro maintainers and >> > developers >> > + to set the default vector length for userspace processes: >> > + >> > +/proc/cpu/sve_default_vector_length >> >> >> elsewhere in the patch series i see >> >> /proc/sys/abi/sve_default_vector_length >> >> is this supposed to be the same? > > Good spot, thanks! > > /proc/cpu/ was the old location: they should both say /proc/abi/. > I'll fix it. Isn't /sys (or rather sysfs) the preferred location for modern control knobs that mirror the kernels object model or is SVE a special case for extending /proc? > > ---Dave -- Alex Bennée ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH] KVM: remove printing of vcpu address
Code currently prints the address of the kvm_vcpu structure in an error
message. It is not immediately clear what value this address adds to
the error string, we can use the vcpu ID instead. Printing unnecessary
kernel addresses to dmesg poses a security risk.
Remove the address from error message output, show vcpu ID instead.
Signed-off-by: Tobin C. Harding
---
virt/kvm/arm/arch_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 8e89d63005c7..ca6c331cad28 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -88,7 +88,7 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void
*dev_id)
* interrupt at this point is a sure sign of some major
* breakage.
*/
- pr_warn("Unexpected interrupt %d on vcpu %p\n", irq, vcpu);
+ pr_warn("Unexpected interrupt %d on vcpu ID %d\n", irq, vcpu->vcpu_id);
return IRQ_HANDLED;
}
--
2.7.4
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
