From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966755AbdAJUKm (ORCPT ); Tue, 10 Jan 2017 15:10:42 -0500 Received: from outprodmail01.cc.columbia.edu ([128.59.72.39]:33349 "EHLO outprodmail01.cc.columbia.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965503AbdAJUKk (ORCPT ); Tue, 10 Jan 2017 15:10:40 -0500 MIME-Version: 1.0 In-Reply-To: <20170110194040.GP4348@cbox> References: <1482772326-29110-1-git-send-email-jintack@cs.columbia.edu> <1482772326-29110-9-git-send-email-jintack@cs.columbia.edu> <20170109121612.GG4348@cbox> <20170110194040.GP4348@cbox> From: Jintack Lim Date: Tue, 10 Jan 2017 15:10:36 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access To: Christoffer Dall Cc: kvmarm@lists.cs.columbia.edu, Marc Zyngier , Paolo Bonzini , rkrcmar@redhat.com, linux@armlinux.org.uk, Catalin Marinas , will.deacon@arm.com, andre.przywara@arm.com, linux-arm-kernel@lists.infradead.org, KVM General , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 X-No-Spam-Score: Local Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 10, 2017 at 2:40 PM, Christoffer Dall wrote: > On Tue, Jan 10, 2017 at 12:36:36PM -0500, Jintack Lim wrote: >> On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall >> wrote: >> > On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote: >> >> 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 >> >> --- >> >> 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(-) >> >> >> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >> >> index fd9e747..7cef94f 100644 >> >> --- a/arch/arm64/kvm/sys_regs.c >> >> +++ b/arch/arm64/kvm/sys_regs.c >> >> @@ -824,7 +824,15 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu, >> >> struct sys_reg_params *p, >> >> const struct sys_reg_desc *r) >> >> { >> >> - kvm_inject_undefined(vcpu); >> >> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu); >> >> + cycle_t now = kvm_phys_timer_read(); >> >> + >> >> + if (p->is_write) { >> >> + ptimer->cnt_cval = p->regval + now; >> >> + kvm_timer_emulate(vcpu, ptimer); >> > >> > 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. Ok, it makes sense. I'll respin! > > -Christoffer >