The ARM architecture defines the EL1 physical timer and the virtual
timer, and it is reasonable for an OS to expect to be able to access
both. However, the current KVM implementation does not provide the EL1
physical timer to VMs but terminates VMs on access to the timer.
On VHE systems, this would be as simple as allowing full access to the
EL1 physical timer to VMs because the KVM host does not use the EL1
physical timer. However, on non-VHE systems, the KVM host already uses
the EL1 physical timer which prevents us from granting full access of
the EL1 physical timer to VMs.
This patchset enables VMs to use the EL1 physical timer through
trap-and-emulate. The KVM host emulates each EL1 physical timer
register access and sets up the background timer accordingly. When the
background timer expires, the KVM host injects EL1 physical timer
interrupts to the VM. Alternatively, it's also possible to allow VMs to
access the EL1 physical timer without trapping. However, this requires
somehow using the EL2 physical timer for the Linux host while running
the VM instead of the EL1 physical timer. Right now I just implemented
trap-and-emulate because this was straightforward to do, and I leave it
to future work to determine if transferring the EL1 physical timer state
to the EL2 timer provides any performance benefit.
This feature will be useful for any OS that wishes to access the EL1
physical timer. Nested virtualization is one of those use cases. A
nested hypervisor running inside a VM would think it has full access to
the hardware and naturally tries to use the EL1 physical timer as Linux
would do. Other nested hypervisors may try to use the EL2 physical timer
as Xen would do, but supporting the EL2 physical timer to the VM is out
of scope of this patchset. This patchset will make it easy to add the
EL2 timer support in the future, though.
Note, Linux VMs booting in EL1 will be unaffected by this patch set and
will continue to use only the virtual timer and this patch set will
therefore not introduce any performance degredation as a result of
trap-and-emulate.
Jintack Lim (8):
KVM: arm/arm64: Abstract virtual timer context into separate structure
KVM: arm/arm64: Decouple kvm timer functions from virtual timer
KVM: arm/arm64: Add the EL1 physical timer context
KVM: arm/arm64: Initialize the emulated EL1 physical timer
KVM: arm64: Add the EL1 physical timer access handler
KVM: arm/arm64: Update the physical timer interrupt level
KVM: arm/arm64: Set up a background timer for the physical timer
emulation
KVM: arm/arm64: Emulate the EL1 phys timer register access
arch/arm/kvm/arm.c | 3 +-
arch/arm/kvm/reset.c | 9 +-
arch/arm64/kvm/reset.c | 9 +-
arch/arm64/kvm/sys_regs.c | 63 +++++++++++++
include/kvm/arm_arch_timer.h | 43 +++++----
virt/kvm/arm/arch_timer.c | 214 ++++++++++++++++++++++++++++++-------------
virt/kvm/arm/hyp/timer-sr.c | 16 ++--
7 files changed, 264 insertions(+), 93 deletions(-)
--
1.9.1
@@ -45,6 +45,7 @@ struct arch_timer_context {structarch_timer_cpu{structarch_timer_contextvtimer;+structarch_timer_contextptimer;/* Background timer used when the guest is not running */structhrtimertimer;
KVM traps on the EL1 phys timer accesses from VMs, but it doesn't handle
those traps. This results in terminating VMs. Instead, set a handler for
the EL1 phys timer access, and inject an undefined exception as an
intermediate step.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
Now that we maintain the EL1 physical timer register states of the VM,
update the physical timer interrupt level along with the virtual one.
Note that the emulated EL1 physical timer is not mapped to any hardware
timer, so we let vgic know that.
With this commit, VMs are able to get the physical timer interrupts
while they are runnable. But they won't get interrupts once vcpus go to
sleep since we don't have code to wake vcpus up on the emulated physical
timer expiration yet.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm/kvm/arm.c | 3 +-
virt/kvm/arm/arch_timer.c | 76 ++++++++++++++++++++++++++++++++++++++---------
2 files changed, 64 insertions(+), 15 deletions(-)
@@ -211,6 +249,10 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)if(kvm_timer_should_fire(vcpu,vtimer)!=vtimer->irq.level)kvm_timer_update_mapped_irq(vcpu,!vtimer->irq.level,vtimer);+/* The emulated EL1 physical timer irq is not mapped to hardware */+if(kvm_timer_should_fire(vcpu,ptimer)!=ptimer->irq.level)+kvm_timer_update_irq(vcpu,!ptimer->irq.level,ptimer);+return0;}
@@ -223,26 +265,32 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu){structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;structarch_timer_context*vtimer=vcpu_vtimer(vcpu);+structarch_timer_context*ptimer=vcpu_ptimer(vcpu);BUG_ON(timer_is_armed(timer));/*-*Noneedtoscheduleabackgroundtimeriftheguesttimerhas+*Noneedtoscheduleabackgroundtimerifanyguesttimerhas*alreadyexpired,becausekvm_vcpu_blockwillreturnbeforeputting*thethreadtosleep.*/-if(kvm_timer_should_fire(vcpu,vtimer))+if(kvm_timer_should_fire(vcpu,vtimer)||+kvm_timer_should_fire(vcpu,ptimer))return;/*-*Ifthetimerisnotcapableofraisinginterrupts(disabledor+*Ifbothtimersarenotcapableofraisinginterrupts(disabledor*masked),thenthere'snomoreworkforustodo.*/-if(!kvm_timer_irq_can_fire(vtimer))+if(!kvm_timer_irq_can_fire(vtimer)&&+!kvm_timer_irq_can_fire(ptimer))return;-/* The timer has not yet expired, schedule a background timer */-timer_arm(timer,kvm_timer_compute_delta(vcpu,vtimer));+/*+*Theguesttimershavenotyetexpired,scheduleabackgroundtimer.+*Picksmallerexpirationtimebetweenphysandvirttimer.+*/+timer_arm(timer,kvm_timer_min_block(vcpu));}voidkvm_timer_unschedule(structkvm_vcpu*vcpu)
Set a background timer for the EL1 physical timer emulation while VMs are
running, so that VMs get interrupts for the physical timer in a timely
manner.
We still use just one background timer. When a VM is runnable, we use
the background timer for the physical timer emulation. When the VM is
about to be blocked, we use the background timer to wake up the vcpu at
the earliest timer expiration among timers the VM is using.
As a result, the assumption that the background timer is not armed while
VMs are running does not hold any more. So, remove BUG_ON()s and
WARN_ON()s accordingly.
Signed-off-by: Jintack Lim <redacted>
---
virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
@@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)if(kvm_timer_irq_can_fire(ptimer))min_phys=kvm_timer_compute_delta(vcpu,ptimer);-WARN_ON((min_virt==ULLONG_MAX)&&(min_phys==ULLONG_MAX));+/* If none of timers can fire, then return 0 */+if((min_virt==ULLONG_MAX)&&(min_phys==ULLONG_MAX))+return0;returnmin(min_virt,min_phys);}
@@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)}/*+*Schedulethebackgroundtimerfortheemulatedtimer.Thebackgroundtimer+*runswhenevervcpuisrunnableandthetimerisnotexpired.+*/+staticvoidkvm_timer_emulate(structkvm_vcpu*vcpu,+structarch_timer_context*timer_ctx)+{+structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;++if(kvm_timer_should_fire(vcpu,timer_ctx))+return;++if(!kvm_timer_irq_can_fire(timer_ctx))+return;++/* The timer has not yet expired, schedule a background timer */+timer_disarm(timer);+timer_arm(timer,kvm_timer_compute_delta(vcpu,timer_ctx));+}++/**Schedulethebackgroundtimerbeforecallingkvm_vcpu_block,sothatthis*threadisremovedfromitswaitqueueandmaderunnablewhenthere'satimer*interrupttohandle.
Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
Now the VM is able to use the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
include/kvm/arm_arch_timer.h | 3 +++
virt/kvm/arm/arch_timer.c | 4 ++--
3 files changed, 37 insertions(+), 5 deletions(-)
@@ -832,7 +840,21 @@ static bool access_cntp_ctl(struct kvm_vcpu *vcpu,structsys_reg_params*p,conststructsys_reg_desc*r){-kvm_inject_undefined(vcpu);+structarch_timer_context*ptimer=vcpu_ptimer(vcpu);++if(p->is_write){+/* ISTATUS bit is read-only */+ptimer->cnt_ctl=p->regval&~ARCH_TIMER_CTRL_IT_STAT;+kvm_timer_emulate(vcpu,ptimer);+}else{+cycle_tnow=kvm_phys_timer_read();++p->regval=ptimer->cnt_ctl;+/* Set ISTATUS bit if it's expired */+if(ptimer->cnt_cval<=now)+p->regval|=ARCH_TIMER_CTRL_IT_STAT;+}+returntrue;}
@@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)intkvm_reset_vcpu(structkvm_vcpu*vcpu){conststructkvm_irq_level*cpu_vtimer_irq;+conststructkvm_irq_level*cpu_ptimer_irq;conststructkvm_regs*cpu_reset;switch(vcpu->arch.target){
@@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)}cpu_vtimer_irq=&default_vtimer_irq;+cpu_ptimer_irq=&default_ptimer_irq;break;}
Abstract virtual timer context into a separate structure and change all
callers referring to timer registers, irq state and so on. No change in
functionality.
This is about to become very handy when adding the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
Acked-by: Christoffer Dall <redacted>
---
include/kvm/arm_arch_timer.h | 32 +++++++++---------
virt/kvm/arm/arch_timer.c | 77 ++++++++++++++++++++++----------------------
virt/kvm/arm/hyp/timer-sr.c | 16 ++++-----
3 files changed, 63 insertions(+), 62 deletions(-)
@@ -28,15 +28,23 @@ struct arch_timer_kvm {cycle_tcntvoff;};-structarch_timer_cpu{+structarch_timer_context{/* Registers: control register, timer value */-u32cntv_ctl;/* Saved/restored */-cycle_tcntv_cval;/* Saved/restored */+u32cnt_ctl;+cycle_tcnt_cval;++/* Timer IRQ */+structkvm_irq_levelirq;-/*-*Anythingthatisnotuseddirectlyfromassemblycodegoes-*here.-*/+/* Active IRQ state caching */+boolactive_cleared_last;++/* Is the timer enabled */+boolenabled;+};++structarch_timer_cpu{+structarch_timer_contextvtimer;/* Background timer used when the guest is not running */structhrtimertimer;
@@ -46,15 +54,6 @@ struct arch_timer_cpu {/* Background timer active */boolarmed;--/* Timer IRQ */-structkvm_irq_levelirq;--/* Active IRQ state caching */-boolactive_cleared_last;--/* Is the timer enabled */-boolenabled;};intkvm_timer_hyp_init(void);
@@ -76,4 +75,5 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,voidkvm_timer_vcpu_put(structkvm_vcpu*vcpu);+#define vcpu_vtimer(v) (&(v)->arch.timer_cpu.vtimer)#endif
@@ -24,12 +24,12 @@/* vcpu is already in the HYP VA space */void__hyp_text__timer_save_state(structkvm_vcpu*vcpu){-structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;+structarch_timer_context*vtimer=vcpu_vtimer(vcpu);u64val;-if(timer->enabled){-timer->cntv_ctl=read_sysreg_el0(cntv_ctl);-timer->cntv_cval=read_sysreg_el0(cntv_cval);+if(vtimer->enabled){+vtimer->cnt_ctl=read_sysreg_el0(cntv_ctl);+vtimer->cnt_cval=read_sysreg_el0(cntv_cval);}/* Disable the virtual timer */
Now that we have a separate structure for timer context, make functions
general so that they can work on any timer context, not just the virtual
timer context. This does not change the virtual timer functionality.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm/kvm/arm.c | 2 +-
include/kvm/arm_arch_timer.h | 3 +-
virt/kvm/arm/arch_timer.c | 65 +++++++++++++++++++++++++-------------------
3 files changed, 40 insertions(+), 30 deletions(-)
@@ -200,8 +208,8 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)if(!vgic_initialized(vcpu->kvm)||!vtimer->enabled)return-ENODEV;-if(kvm_timer_should_fire(vcpu)!=vtimer->irq.level)-kvm_timer_update_irq(vcpu,!vtimer->irq.level);+if(kvm_timer_should_fire(vcpu,vtimer)!=vtimer->irq.level)+kvm_timer_update_mapped_irq(vcpu,!vtimer->irq.level,vtimer);return0;}
@@ -214,6 +222,7 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)voidkvm_timer_schedule(structkvm_vcpu*vcpu){structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;+structarch_timer_context*vtimer=vcpu_vtimer(vcpu);BUG_ON(timer_is_armed(timer));
@@ -222,18 +231,18 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)*alreadyexpired,becausekvm_vcpu_blockwillreturnbeforeputting*thethreadtosleep.*/-if(kvm_timer_should_fire(vcpu))+if(kvm_timer_should_fire(vcpu,vtimer))return;/**Ifthetimerisnotcapableofraisinginterrupts(disabledor*masked),thenthere'snomoreworkforustodo.*/-if(!kvm_timer_irq_can_fire(vcpu))+if(!kvm_timer_irq_can_fire(vtimer))return;/* The timer has not yet expired, schedule a background timer */-timer_arm(timer,kvm_timer_compute_delta(vcpu));+timer_arm(timer,kvm_timer_compute_delta(vcpu,vtimer));}voidkvm_timer_unschedule(structkvm_vcpu*vcpu)
From: Christoffer Dall <hidden> Date: 2017-01-09 12:00:01
On Mon, Dec 26, 2016 at 12:12:00PM -0500, Jintack Lim wrote:
quoted hunk
Now that we have a separate structure for timer context, make functions
general so that they can work on any timer context, not just the virtual
timer context. This does not change the virtual timer functionality.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm/kvm/arm.c | 2 +-
include/kvm/arm_arch_timer.h | 3 +-
virt/kvm/arm/arch_timer.c | 65 +++++++++++++++++++++++++-------------------
3 files changed, 40 insertions(+), 30 deletions(-)
I'm wondering if we should move the cntvoff field to the
arch_timer_context struct so that all these functions can just take
the timer_ctx pointer without a vcpu pointer?
(That would pave the way for ever doing adjustments of the cntvoff on a
per-CPU basis if that should ever make sense, which it maybe won't but
still...)
The challenge would be to ensure that the cntvoff stays in sync between
all the contexts when being modified from userspace and during init.
Thoughts?
Thanks,
-Christoffer
@@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)intkvm_reset_vcpu(structkvm_vcpu*vcpu){conststructkvm_irq_level*cpu_vtimer_irq;+conststructkvm_irq_level*cpu_ptimer_irq;conststructkvm_regs*cpu_reset;switch(vcpu->arch.target){
@@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)}cpu_vtimer_irq=&default_vtimer_irq;+cpu_ptimer_irq=&default_ptimer_irq;break;}
Dubious comment the way it stands.
Does the rest of the code really support one timer being enabled while
another one not so?
In any case, the comment should explain the rationale as opposed to
reiterate what the following code line does, or just not be there.
From: Christoffer Dall <hidden> Date: 2017-01-09 12:13:53
On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
quoted hunk
Set a background timer for the EL1 physical timer emulation while VMs are
running, so that VMs get interrupts for the physical timer in a timely
manner.
We still use just one background timer. When a VM is runnable, we use
the background timer for the physical timer emulation. When the VM is
about to be blocked, we use the background timer to wake up the vcpu at
the earliest timer expiration among timers the VM is using.
As a result, the assumption that the background timer is not armed while
VMs are running does not hold any more. So, remove BUG_ON()s and
WARN_ON()s accordingly.
Signed-off-by: Jintack Lim <redacted>
---
virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
/*
* If the vcpu is blocked we want to wake it up so that it will see
* the timer has expired when entering the guest.
@@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx) /* * Returns minimal timer expiration time in ns among guest timers.- * Note that it will return inf time if none of timers can fire. */ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) {
@@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) if (kvm_timer_irq_can_fire(ptimer)) min_phys = kvm_timer_compute_delta(vcpu, ptimer);- WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));+ /* If none of timers can fire, then return 0 */+ if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))+ return 0;
Why didn't you have this semantics in the previous patch?
quoted hunk
return min(min_virt, min_phys);
}
@@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu) } /*+ * Schedule the background timer for the emulated timer. The background timer+ * runs whenever vcpu is runnable and the timer is not expired.+ */+static void kvm_timer_emulate(struct kvm_vcpu *vcpu,+ struct arch_timer_context *timer_ctx)+{+ struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;++ if (kvm_timer_should_fire(vcpu, timer_ctx))+ return;++ if (!kvm_timer_irq_can_fire(timer_ctx))+ return;++ /* The timer has not yet expired, schedule a background timer */+ timer_disarm(timer);+ timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));
I'm wondering what the effect of this thing really is. Isn't the soft
timer programmed in timer_arm() based on Linux's own timekeeping
schedule, such that the physical timer will be programmed to the next
tick, regardless of what you program here, so all you have to do is
check if you need to inject the phys timer on entry to the VM?
On the other hand, if this can cause Linux to program the phys timer to
expire sooner, then I guess it makes sense. Thinking about it, would
that be the case on a tickless system?
quoted hunk
+}++/* * Schedule the background timer before calling kvm_vcpu_block, so that this * thread is removed from its waitqueue and made runnable when there's a timer * interrupt to handle.
@@ -267,8 +285,6 @@ 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));- /* * No need to schedule a background timer if any guest timer has * already expired, because kvm_vcpu_block will return before putting
@@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu) * The guest timers have not yet expired, schedule a background timer. * Pick smaller expiration time between phys and virt timer. */+ timer_disarm(timer); timer_arm(timer, kvm_timer_min_block(vcpu)); } void kvm_timer_unschedule(struct kvm_vcpu *vcpu) { struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;+ timer_disarm(timer);++ /*+ * Now we return from the blocking. If we have any timer to emulate,+ * and it's not expired, set the background timer for it.+ */+ kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
hmm, this is only called when returning from the kvm_vcpu_block() path.
What about when you do vcpu_load/put, don't you need to schedule/cancel
it there too?
Maybe it's simpler to just program the soft timer during flush_hwstate
and cancel the timer during sync_hwstate. Does that work?
quoted hunk
}
/**
@@ -375,10 +399,6 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu) */ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu) {- struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;-- BUG_ON(timer_is_armed(timer));- /* * The guest could have modified the timer registers or the timer * could have expired, update the timer state.
From: Christoffer Dall <hidden> Date: 2017-01-09 12:14:59
On Mon, Dec 26, 2016 at 12:12:04PM -0500, Jintack Lim wrote:
Now that we maintain the EL1 physical timer register states of the VM,
update the physical timer interrupt level along with the virtual one.
Note that the emulated EL1 physical timer is not mapped to any hardware
timer, so we let vgic know that.
With this commit, VMs are able to get the physical timer interrupts
while they are runnable. But they won't get interrupts once vcpus go to
sleep since we don't have code to wake vcpus up on the emulated physical
timer expiration yet.
I'm not sure I understand. It looks to me like it's the other way
around and that this code supports waking up a sleeping VCPU sooner if
it has a physical timer scheduled, but doesn't yet support causing an
early exit from running the VPCU base on programing the physical timer?
Thanks,
-Christoffer
@@ -211,6 +249,10 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)if(kvm_timer_should_fire(vcpu,vtimer)!=vtimer->irq.level)kvm_timer_update_mapped_irq(vcpu,!vtimer->irq.level,vtimer);+/* The emulated EL1 physical timer irq is not mapped to hardware */+if(kvm_timer_should_fire(vcpu,ptimer)!=ptimer->irq.level)+kvm_timer_update_irq(vcpu,!ptimer->irq.level,ptimer);+return0;}
@@ -223,26 +265,32 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu){structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;structarch_timer_context*vtimer=vcpu_vtimer(vcpu);+structarch_timer_context*ptimer=vcpu_ptimer(vcpu);BUG_ON(timer_is_armed(timer));/*-*Noneedtoscheduleabackgroundtimeriftheguesttimerhas+*Noneedtoscheduleabackgroundtimerifanyguesttimerhas*alreadyexpired,becausekvm_vcpu_blockwillreturnbeforeputting*thethreadtosleep.*/-if(kvm_timer_should_fire(vcpu,vtimer))+if(kvm_timer_should_fire(vcpu,vtimer)||+kvm_timer_should_fire(vcpu,ptimer))return;/*-*Ifthetimerisnotcapableofraisinginterrupts(disabledor+*Ifbothtimersarenotcapableofraisinginterrupts(disabledor*masked),thenthere'snomoreworkforustodo.*/-if(!kvm_timer_irq_can_fire(vtimer))+if(!kvm_timer_irq_can_fire(vtimer)&&+!kvm_timer_irq_can_fire(ptimer))return;-/* The timer has not yet expired, schedule a background timer */-timer_arm(timer,kvm_timer_compute_delta(vcpu,vtimer));+/*+*Theguesttimershavenotyetexpired,scheduleabackgroundtimer.+*Picksmallerexpirationtimebetweenphysandvirttimer.+*/+timer_arm(timer,kvm_timer_min_block(vcpu));}voidkvm_timer_unschedule(structkvm_vcpu*vcpu)
From: Christoffer Dall <hidden> Date: 2017-01-09 12:16:30
On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
quoted hunk
Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
Now the VM is able to use the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
include/kvm/arm_arch_timer.h | 3 +++
virt/kvm/arm/arch_timer.c | 4 ++--
3 files changed, 37 insertions(+), 5 deletions(-)
Hmm, do we really need those calls here?
I guess I'm a little confused about exactly what the kvm_timer_emulate()
function is supposed to do, and it feels to me like these handlers
should just record what the guest is asking the kernel to do and the
logic of handling the additional timer should be moved into the run path
as much as possible.
Thanks,
-Christoffer
@@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)intkvm_reset_vcpu(structkvm_vcpu*vcpu){conststructkvm_irq_level*cpu_vtimer_irq;+conststructkvm_irq_level*cpu_ptimer_irq;conststructkvm_regs*cpu_reset;switch(vcpu->arch.target){
@@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)}cpu_vtimer_irq=&default_vtimer_irq;+cpu_ptimer_irq=&default_ptimer_irq;break;}
Dubious comment the way it stands.
Does the rest of the code really support one timer being enabled while
another one not so?
No. The code never check if the physical timer is enabled. I think
it's not necessary to set enable bit for this emulated physical timer.
We, however, may need to set/check this enable bit later if we decide
to give the EL1 physical timer to the guest instead of emulating it.
In any case, the comment should explain the rationale as opposed to
reiterate what the following code line does, or just not be there.
On Mon, Jan 9, 2017 at 7:14 AM, Christoffer Dall
[off-list ref] wrote:
On Mon, Dec 26, 2016 at 12:12:04PM -0500, Jintack Lim wrote:
quoted
Now that we maintain the EL1 physical timer register states of the VM,
update the physical timer interrupt level along with the virtual one.
Note that the emulated EL1 physical timer is not mapped to any hardware
timer, so we let vgic know that.
With this commit, VMs are able to get the physical timer interrupts
while they are runnable. But they won't get interrupts once vcpus go to
sleep since we don't have code to wake vcpus up on the emulated physical
timer expiration yet.
I'm not sure I understand. It looks to me like it's the other way
around and that this code supports waking up a sleeping VCPU sooner if
it has a physical timer scheduled, but doesn't yet support causing an
early exit from running the VPCU base on programing the physical timer?
Ah, you are right. This was really two commits: one that checks the
physical timer expiration on entry to the VM AND another one that sets
up the background timer when the VM goes to sleep.
I'll split this into two and put the correct commit messages.
Thanks,
Jintack
@@ -211,6 +249,10 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)if(kvm_timer_should_fire(vcpu,vtimer)!=vtimer->irq.level)kvm_timer_update_mapped_irq(vcpu,!vtimer->irq.level,vtimer);+/* The emulated EL1 physical timer irq is not mapped to hardware */+if(kvm_timer_should_fire(vcpu,ptimer)!=ptimer->irq.level)+kvm_timer_update_irq(vcpu,!ptimer->irq.level,ptimer);+return0;}
@@ -223,26 +265,32 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu){structarch_timer_cpu*timer=&vcpu->arch.timer_cpu;structarch_timer_context*vtimer=vcpu_vtimer(vcpu);+structarch_timer_context*ptimer=vcpu_ptimer(vcpu);BUG_ON(timer_is_armed(timer));/*-*Noneedtoscheduleabackgroundtimeriftheguesttimerhas+*Noneedtoscheduleabackgroundtimerifanyguesttimerhas*alreadyexpired,becausekvm_vcpu_blockwillreturnbeforeputting*thethreadtosleep.*/-if(kvm_timer_should_fire(vcpu,vtimer))+if(kvm_timer_should_fire(vcpu,vtimer)||+kvm_timer_should_fire(vcpu,ptimer))return;/*-*Ifthetimerisnotcapableofraisinginterrupts(disabledor+*Ifbothtimersarenotcapableofraisinginterrupts(disabledor*masked),thenthere'snomoreworkforustodo.*/-if(!kvm_timer_irq_can_fire(vtimer))+if(!kvm_timer_irq_can_fire(vtimer)&&+!kvm_timer_irq_can_fire(ptimer))return;-/* The timer has not yet expired, schedule a background timer */-timer_arm(timer,kvm_timer_compute_delta(vcpu,vtimer));+/*+*Theguesttimershavenotyetexpired,scheduleabackgroundtimer.+*Picksmallerexpirationtimebetweenphysandvirttimer.+*/+timer_arm(timer,kvm_timer_min_block(vcpu));}voidkvm_timer_unschedule(structkvm_vcpu*vcpu)--
On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
[off-list ref] wrote:
On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
quoted
Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
Now the VM is able to use the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
include/kvm/arm_arch_timer.h | 3 +++
virt/kvm/arm/arch_timer.c | 4 ++--
3 files changed, 37 insertions(+), 5 deletions(-)
Hmm, do we really need those calls here?
I guess I'm a little confused about exactly what the kvm_timer_emulate()
function is supposed to do, and it feels to me like these handlers
should just record what the guest is asking the kernel to do and the
logic of handling the additional timer should be moved into the run path
as much as possible.
I think it's a design decision. As you suggested, it's simple to do
set up the background timer on entry to the VM, cancel it on exit, but
since that's on the critical path it may have some impact on the
performance, especially the world switch cost. To avoid
canceling/setting up timer every world switch, I choose to schedule
the physical timer here. I haven't compared the cost of the two
alternatives, though.
Hi Christoffer,
thanks for the review!
On Mon, Jan 9, 2017 at 7:13 AM, Christoffer Dall
[off-list ref] wrote:
On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
quoted
Set a background timer for the EL1 physical timer emulation while VMs are
running, so that VMs get interrupts for the physical timer in a timely
manner.
We still use just one background timer. When a VM is runnable, we use
the background timer for the physical timer emulation. When the VM is
about to be blocked, we use the background timer to wake up the vcpu at
the earliest timer expiration among timers the VM is using.
As a result, the assumption that the background timer is not armed while
VMs are running does not hold any more. So, remove BUG_ON()s and
WARN_ON()s accordingly.
Signed-off-by: Jintack Lim <redacted>
---
virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
/*
* If the vcpu is blocked we want to wake it up so that it will see
* the timer has expired when entering the guest.
@@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx) /* * Returns minimal timer expiration time in ns among guest timers.- * Note that it will return inf time if none of timers can fire. */ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) {
@@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) if (kvm_timer_irq_can_fire(ptimer)) min_phys = kvm_timer_compute_delta(vcpu, ptimer);- WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));+ /* If none of timers can fire, then return 0 */+ if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))+ return 0;
Why didn't you have this semantics in the previous patch?
I should have put this in the previous patch, and I'll do that.
Just let you know, WARN_ON() in the previous patch was there because I
thought that the caller of this function is sure that one of the
timers are able to fire. But I think that's beyond the scope of this
function.
quoted
return min(min_virt, min_phys);
}
@@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu) } /*+ * Schedule the background timer for the emulated timer. The background timer+ * runs whenever vcpu is runnable and the timer is not expired.+ */+static void kvm_timer_emulate(struct kvm_vcpu *vcpu,+ struct arch_timer_context *timer_ctx)+{+ struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;++ if (kvm_timer_should_fire(vcpu, timer_ctx))+ return;++ if (!kvm_timer_irq_can_fire(timer_ctx))+ return;++ /* The timer has not yet expired, schedule a background timer */+ timer_disarm(timer);+ timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));
I'm wondering what the effect of this thing really is. Isn't the soft
timer programmed in timer_arm() based on Linux's own timekeeping
schedule, such that the physical timer will be programmed to the next
tick, regardless of what you program here, so all you have to do is
check if you need to inject the phys timer on entry to the VM?
On the other hand, if this can cause Linux to program the phys timer to
expire sooner, then I guess it makes sense. Thinking about it, would
that be the case on a tickless system?
I don't have a good answer for this, so I'll get back to you!
quoted
+}++/* * Schedule the background timer before calling kvm_vcpu_block, so that this * thread is removed from its waitqueue and made runnable when there's a timer * interrupt to handle.
@@ -267,8 +285,6 @@ 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));- /* * No need to schedule a background timer if any guest timer has * already expired, because kvm_vcpu_block will return before putting
@@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu) * The guest timers have not yet expired, schedule a background timer. * Pick smaller expiration time between phys and virt timer. */+ timer_disarm(timer); timer_arm(timer, kvm_timer_min_block(vcpu)); } void kvm_timer_unschedule(struct kvm_vcpu *vcpu) { struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;+ timer_disarm(timer);++ /*+ * Now we return from the blocking. If we have any timer to emulate,+ * and it's not expired, set the background timer for it.+ */+ kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
hmm, this is only called when returning from the kvm_vcpu_block() path.
What about when you do vcpu_load/put, don't you need to schedule/cancel
it there too?
We can do that, but I think that's not necessary. Firing the physical
timer while a vcpu is unloaded doesn't affect the task scheduling. Or
is it awkward to do so?
Maybe it's simpler to just program the soft timer during flush_hwstate
and cancel the timer during sync_hwstate. Does that work?
As far as I remember, it worked. I agree that it's simpler.
But as I mentioned in the patch [8/8] reply this *may* cause more overhead.
quoted
}
/**
@@ -375,10 +399,6 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu) */ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu) {- struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;-- BUG_ON(timer_is_armed(timer));- /* * The guest could have modified the timer registers or the timer * could have expired, update the timer state.--
@@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)intkvm_reset_vcpu(structkvm_vcpu*vcpu){conststructkvm_irq_level*cpu_vtimer_irq;+conststructkvm_irq_level*cpu_ptimer_irq;conststructkvm_regs*cpu_reset;switch(vcpu->arch.target){
@@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)}cpu_vtimer_irq=&default_vtimer_irq;+cpu_ptimer_irq=&default_ptimer_irq;break;}
Dubious comment the way it stands.
Does the rest of the code really support one timer being enabled while
another one not so?
No. The code never check if the physical timer is enabled. I think
it's not necessary to set enable bit for this emulated physical timer.
We, however, may need to set/check this enable bit later if we decide
to give the EL1 physical timer to the guest instead of emulating it.
It hink the semantics of the enable bit before your patches is more "is
the arch timer code at all enabled and working", so maybe you want to
preserve that and move the enabled bit out of the per-timer and per-vcpu
state?
Thanks,
-Christoffer
From: Christoffer Dall <hidden> Date: 2017-01-10 19:40:34
On Tue, Jan 10, 2017 at 01:47:49PM -0500, Jintack Lim wrote:
Hi Christoffer,
thanks for the review!
On Mon, Jan 9, 2017 at 7:13 AM, Christoffer Dall
[off-list ref] wrote:
quoted
On Mon, Dec 26, 2016 at 12:12:05PM -0500, Jintack Lim wrote:
quoted
Set a background timer for the EL1 physical timer emulation while VMs are
running, so that VMs get interrupts for the physical timer in a timely
manner.
We still use just one background timer. When a VM is runnable, we use
the background timer for the physical timer emulation. When the VM is
about to be blocked, we use the background timer to wake up the vcpu at
the earliest timer expiration among timers the VM is using.
As a result, the assumption that the background timer is not armed while
VMs are running does not hold any more. So, remove BUG_ON()s and
WARN_ON()s accordingly.
Signed-off-by: Jintack Lim <redacted>
---
virt/kvm/arm/arch_timer.c | 42 +++++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
/*
* If the vcpu is blocked we want to wake it up so that it will see
* the timer has expired when entering the guest.
@@ -139,7 +136,6 @@ static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx) /* * Returns minimal timer expiration time in ns among guest timers.- * Note that it will return inf time if none of timers can fire. */ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) {
@@ -153,7 +149,9 @@ static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu) if (kvm_timer_irq_can_fire(ptimer)) min_phys = kvm_timer_compute_delta(vcpu, ptimer);- WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));+ /* If none of timers can fire, then return 0 */+ if ((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX))+ return 0;
Why didn't you have this semantics in the previous patch?
I should have put this in the previous patch, and I'll do that.
Just let you know, WARN_ON() in the previous patch was there because I
thought that the caller of this function is sure that one of the
timers are able to fire. But I think that's beyond the scope of this
function.
quoted
quoted
return min(min_virt, min_phys);
}
@@ -257,6 +255,26 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu) } /*+ * Schedule the background timer for the emulated timer. The background timer+ * runs whenever vcpu is runnable and the timer is not expired.+ */+static void kvm_timer_emulate(struct kvm_vcpu *vcpu,+ struct arch_timer_context *timer_ctx)+{+ struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;++ if (kvm_timer_should_fire(vcpu, timer_ctx))+ return;++ if (!kvm_timer_irq_can_fire(timer_ctx))+ return;++ /* The timer has not yet expired, schedule a background timer */+ timer_disarm(timer);+ timer_arm(timer, kvm_timer_compute_delta(vcpu, timer_ctx));
I'm wondering what the effect of this thing really is. Isn't the soft
timer programmed in timer_arm() based on Linux's own timekeeping
schedule, such that the physical timer will be programmed to the next
tick, regardless of what you program here, so all you have to do is
check if you need to inject the phys timer on entry to the VM?
On the other hand, if this can cause Linux to program the phys timer to
expire sooner, then I guess it makes sense. Thinking about it, would
that be the case on a tickless system?
I don't have a good answer for this, so I'll get back to you!
quoted
quoted
+}++/* * Schedule the background timer before calling kvm_vcpu_block, so that this * thread is removed from its waitqueue and made runnable when there's a timer * interrupt to handle.
@@ -267,8 +285,6 @@ 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));- /* * No need to schedule a background timer if any guest timer has * already expired, because kvm_vcpu_block will return before putting
@@ -290,13 +306,21 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu) * The guest timers have not yet expired, schedule a background timer. * Pick smaller expiration time between phys and virt timer. */+ timer_disarm(timer); timer_arm(timer, kvm_timer_min_block(vcpu)); } void kvm_timer_unschedule(struct kvm_vcpu *vcpu) { struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;+ timer_disarm(timer);++ /*+ * Now we return from the blocking. If we have any timer to emulate,+ * and it's not expired, set the background timer for it.+ */+ kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
hmm, this is only called when returning from the kvm_vcpu_block() path.
What about when you do vcpu_load/put, don't you need to schedule/cancel
it there too?
We can do that, but I think that's not necessary. Firing the physical
timer while a vcpu is unloaded doesn't affect the task scheduling. Or
is it awkward to do so?
Not sure I understand your question.
quoted
Maybe it's simpler to just program the soft timer during flush_hwstate
and cancel the timer during sync_hwstate. Does that work?
As far as I remember, it worked. I agree that it's simpler.
But as I mentioned in the patch [8/8] reply this *may* cause more overhead.
I think you should go for something we can easily convince ourselves
will work initially, and then we can measure if there's significant
overhead and improve on that.
At least we should have a clear idea of the when to schedule software
timers and why.
We know from my RFC series to optimize the timer behavior that on some
systems, not having to read/write the timer registers on every put/load
can save us a few hundred cycles, so it's not extremely expensive, but
we should at the same time avoid it if possible.
Assuming most guests will use the virt timer for now, if we can just
have an initial check on both flush/sync if the ptimer is even enabled,
and only do extra work when that's the case, then I think that would be
a good start.
Thanks,
-Christoffer
From: Christoffer Dall <hidden> Date: 2017-01-10 19:40:45
On Tue, Jan 10, 2017 at 12:36:36PM -0500, Jintack Lim wrote:
On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
[off-list ref] wrote:
quoted
On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
quoted
Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
Now the VM is able to use the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
include/kvm/arm_arch_timer.h | 3 +++
virt/kvm/arm/arch_timer.c | 4 ++--
3 files changed, 37 insertions(+), 5 deletions(-)
Hmm, do we really need those calls here?
I guess I'm a little confused about exactly what the kvm_timer_emulate()
function is supposed to do, and it feels to me like these handlers
should just record what the guest is asking the kernel to do and the
logic of handling the additional timer should be moved into the run path
as much as possible.
I think it's a design decision. As you suggested, it's simple to do
set up the background timer on entry to the VM, cancel it on exit, but
since that's on the critical path it may have some impact on the
performance, especially the world switch cost. To avoid
canceling/setting up timer every world switch, I choose to schedule
the physical timer here. I haven't compared the cost of the two
alternatives, though.
I'd definitely like to avoid us scheduling soft timers on the host if
that's not even necessary in the first place, so I'd like to get that
clear first, and as I said on the previous patch I think it's better to
get a working solution that we understand firt, and then optimize on
that later based on real results.
-Christoffer
On Tue, Jan 10, 2017 at 2:40 PM, Christoffer Dall
[off-list ref] wrote:
On Tue, Jan 10, 2017 at 12:36:36PM -0500, Jintack Lim wrote:
quoted
On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
[off-list ref] wrote:
quoted
On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
quoted
Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
Now the VM is able to use the EL1 physical timer.
Signed-off-by: Jintack Lim <redacted>
---
arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
include/kvm/arm_arch_timer.h | 3 +++
virt/kvm/arm/arch_timer.c | 4 ++--
3 files changed, 37 insertions(+), 5 deletions(-)
Hmm, do we really need those calls here?
I guess I'm a little confused about exactly what the kvm_timer_emulate()
function is supposed to do, and it feels to me like these handlers
should just record what the guest is asking the kernel to do and the
logic of handling the additional timer should be moved into the run path
as much as possible.
I think it's a design decision. As you suggested, it's simple to do
set up the background timer on entry to the VM, cancel it on exit, but
since that's on the critical path it may have some impact on the
performance, especially the world switch cost. To avoid
canceling/setting up timer every world switch, I choose to schedule
the physical timer here. I haven't compared the cost of the two
alternatives, though.
I'd definitely like to avoid us scheduling soft timers on the host if
that's not even necessary in the first place, so I'd like to get that
clear first, and as I said on the previous patch I think it's better to
get a working solution that we understand firt, and then optimize on
that later based on real results.
@@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)intkvm_reset_vcpu(structkvm_vcpu*vcpu){conststructkvm_irq_level*cpu_vtimer_irq;+conststructkvm_irq_level*cpu_ptimer_irq;conststructkvm_regs*cpu_reset;switch(vcpu->arch.target){
@@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)}cpu_vtimer_irq=&default_vtimer_irq;+cpu_ptimer_irq=&default_ptimer_irq;break;}
Dubious comment the way it stands.
Does the rest of the code really support one timer being enabled while
another one not so?
No. The code never check if the physical timer is enabled. I think
it's not necessary to set enable bit for this emulated physical timer.
We, however, may need to set/check this enable bit later if we decide
to give the EL1 physical timer to the guest instead of emulating it.
It hink the semantics of the enable bit before your patches is more "is
the arch timer code at all enabled and working", so maybe you want to
preserve that and move the enabled bit out of the per-timer and per-vcpu
state?
From: Marc Zyngier <hidden> Date: 2017-01-17 17:09:17
On 26/12/16 17:11, Jintack Lim wrote:
The ARM architecture defines the EL1 physical timer and the virtual
timer, and it is reasonable for an OS to expect to be able to access
both. However, the current KVM implementation does not provide the EL1
physical timer to VMs but terminates VMs on access to the timer.
On VHE systems, this would be as simple as allowing full access to the
EL1 physical timer to VMs because the KVM host does not use the EL1
physical timer. However, on non-VHE systems, the KVM host already uses
the EL1 physical timer which prevents us from granting full access of
the EL1 physical timer to VMs.
This patchset enables VMs to use the EL1 physical timer through
trap-and-emulate. The KVM host emulates each EL1 physical timer
register access and sets up the background timer accordingly. When the
background timer expires, the KVM host injects EL1 physical timer
interrupts to the VM. Alternatively, it's also possible to allow VMs to
access the EL1 physical timer without trapping. However, this requires
somehow using the EL2 physical timer for the Linux host while running
the VM instead of the EL1 physical timer. Right now I just implemented
trap-and-emulate because this was straightforward to do, and I leave it
to future work to determine if transferring the EL1 physical timer state
to the EL2 timer provides any performance benefit.
This feature will be useful for any OS that wishes to access the EL1
physical timer. Nested virtualization is one of those use cases. A
nested hypervisor running inside a VM would think it has full access to
the hardware and naturally tries to use the EL1 physical timer as Linux
would do. Other nested hypervisors may try to use the EL2 physical timer
as Xen would do, but supporting the EL2 physical timer to the VM is out
of scope of this patchset. This patchset will make it easy to add the
EL2 timer support in the future, though.
Note, Linux VMs booting in EL1 will be unaffected by this patch set and
will continue to use only the virtual timer and this patch set will
therefore not introduce any performance degredation as a result of
trap-and-emulate.
Hi Jintack,
Any chance you could address Christoffer's comments and respin this
series? This looks like a good enhancement to our emulation, and
definitely a requirement for the nested work, so I'm obviously keen on it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Tue, Jan 17, 2017 at 12:09 PM, Marc Zyngier [off-list ref] wrote:
On 26/12/16 17:11, Jintack Lim wrote:
quoted
The ARM architecture defines the EL1 physical timer and the virtual
timer, and it is reasonable for an OS to expect to be able to access
both. However, the current KVM implementation does not provide the EL1
physical timer to VMs but terminates VMs on access to the timer.
On VHE systems, this would be as simple as allowing full access to the
EL1 physical timer to VMs because the KVM host does not use the EL1
physical timer. However, on non-VHE systems, the KVM host already uses
the EL1 physical timer which prevents us from granting full access of
the EL1 physical timer to VMs.
This patchset enables VMs to use the EL1 physical timer through
trap-and-emulate. The KVM host emulates each EL1 physical timer
register access and sets up the background timer accordingly. When the
background timer expires, the KVM host injects EL1 physical timer
interrupts to the VM. Alternatively, it's also possible to allow VMs to
access the EL1 physical timer without trapping. However, this requires
somehow using the EL2 physical timer for the Linux host while running
the VM instead of the EL1 physical timer. Right now I just implemented
trap-and-emulate because this was straightforward to do, and I leave it
to future work to determine if transferring the EL1 physical timer state
to the EL2 timer provides any performance benefit.
This feature will be useful for any OS that wishes to access the EL1
physical timer. Nested virtualization is one of those use cases. A
nested hypervisor running inside a VM would think it has full access to
the hardware and naturally tries to use the EL1 physical timer as Linux
would do. Other nested hypervisors may try to use the EL2 physical timer
as Xen would do, but supporting the EL2 physical timer to the VM is out
of scope of this patchset. This patchset will make it easy to add the
EL2 timer support in the future, though.
Note, Linux VMs booting in EL1 will be unaffected by this patch set and
will continue to use only the virtual timer and this patch set will
therefore not introduce any performance degredation as a result of
trap-and-emulate.
Hi Jintack,
Any chance you could address Christoffer's comments and respin this
series? This looks like a good enhancement to our emulation, and
definitely a requirement for the nested work, so I'm obviously keen on it.
Hi Marc,
Yes, I plan to respin this patch series this week.
Thanks,
Jintack
Thanks,
M.
--
Jazz is not dead. It just smells funny...