Re: [PATCH v6 43/64] KVM: arm64: nv: arch_timer: Support hyp timer emulation

Marc Zyngier
2022-03-07T16:41:52Z

On Mon, 07 Mar 2022 16:24:58 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Mon, Mar 07, 2022 at 03:44:43PM +0000, Marc Zyngier wrote:
> > On 2022-03-07 15:23, Alexandru Elisei wrote:
> > > Hi,
> > >
> > > On Fri, Jan 28, 2022 at 12:18:51PM +0000, Marc Zyngier wrote:
> > > > From: Christoffer Dall <christoffer.dall@arm.com>
> > > >
> > > > Emulating EL2 also means emulating the EL2 timers. To do so, we expand
> > > > our timer framework to deal with at most 4 timers. At any given time,
> > > > two timers are using the HW timers, and the two others are purely
> > > > emulated.
> > > >
> > > > The role of deciding which is which at any given time is left to a
> > > > mapping function which is called every time we need to make such a
> > > > decision.
> > > >
> > > > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
> > > > [maz: added CNTVOFF support, general reworking for v4.8]
> > > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > > ---
> > > > arch/arm64/include/asm/kvm_host.h | 4 +
> > > > arch/arm64/kvm/arch_timer.c | 165
> > > > ++++++++++++++++++++++++++++--
> > > > arch/arm64/kvm/sys_regs.c | 7 +-
> > > > arch/arm64/kvm/trace_arm.h | 6 +-
> > > > arch/arm64/kvm/vgic/vgic.c | 15 +++
> > > > include/kvm/arm_arch_timer.h | 8 +-
> > > > include/kvm/arm_vgic.h | 1 +
> > > > 7 files changed, 194 insertions(+), 12 deletions(-)
> > > >
> > > [..]
> > > > @@ -1301,6 +1445,7 @@ static void set_timer_irqs(struct kvm *kvm,
> > > > int vtimer_irq, int ptimer_irq)
> > > > kvm_for_each_vcpu(i, vcpu, kvm) {
> > > > vcpu_vtimer(vcpu)->irq.irq = vtimer_irq;
> > > > vcpu_ptimer(vcpu)->irq.irq = ptimer_irq;
> > > > + /* TODO: Add support for hv/hp timers */
> > > > }
> > > > }
> > > >
> > > > @@ -1311,6 +1456,8 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu
> > > > *vcpu, struct kvm_device_attr *attr)
> > > > struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> > > > int irq;
> > > >
> > > > + /* TODO: Add support for hv/hp timers */
> > >
> > > Is the patch unfinished?
> >
> > Just like the rest of the kernel.
>
> That doesn't really answer my question. What I was asking if this is
> the patch that is intended to be merged or if you still want to add
> handling of those timers before merging it.

It depends what people's expectations are.

If you want full support for everything the architecture has, plus all
the features that KVM has such as save restore (which is what we this
comment is about) before things can go in, then the answer probably is
that I should go and erase that code as quickly as possible.

M.

--
Without deviation from the norm, progress is not possible.