Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Mon, 18 Sep 2023 12:29:26 +0100
Message-ID: <8634zben3d.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
Cc:     linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev,
        linux-arm-kernel@lists.infradead.org, Christoffer.Dall@arm.com,
        eauger@redhat.com, miguel.luis@oracle.com,
        darren@os.amperecomputing.com, scott@os.amperecomputing.com
Subject: Re: [PATCH v2 2/2] KVM: arm64: timers: Save restore CVAL of a ptimer across guest entry and exits
In-Reply-To: <ec322123-bfe7-6019-7f35-de326ee7b6c3@os.amperecomputing.com>
References: <20230904114218.590304-1-gankulkarni@os.amperecomputing.com>
        <20230904114218.590304-3-gankulkarni@os.amperecomputing.com>
        <86bkeadzf0.wl-maz@kernel.org>
        <ec322123-bfe7-6019-7f35-de326ee7b6c3@os.amperecomputing.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Fri, 15 Sep 2023 10:57:46 +0100,
Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote:
> 
> This patch did not work.
> After adding changes as in below diff, it is started working.

Thanks for looking into this.

> 
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c
> b/arch/arm64/kvm/hyp/vhe/switch.c
> index b0b07658f77d..91d2cfb03e26 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -117,7 +117,7 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>                         val = __vcpu_sys_reg(vcpu, CNTHP_CVAL_EL2);
> 
>                 if (map.direct_ptimer) {
> -                       write_sysreg_s(val, SYS_CNTP_CVAL_EL0);
> +                       write_sysreg_el0(val, SYS_CNTP_CVAL);

Duh, of course. Silly me.

>                         isb();
>                 }
>         }
> @@ -161,8 +161,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
> 
>         ___deactivate_traps(vcpu);
> 
> -       write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> -
>         if (has_cntpoff()) {
>                 struct timer_map map;
>                 u64 val, offset;
> @@ -173,7 +171,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
>                  * We're exiting the guest. Save the latest CVAL value
>                  * to memory and apply the offset now that TGE is set.
>                  */
> -               val = read_sysreg_s(SYS_CNTP_CVAL_EL0);
> +               val = read_sysreg_el0(SYS_CNTP_CVAL);
>                 if (map.direct_ptimer == vcpu_ptimer(vcpu))
>                         __vcpu_sys_reg(vcpu, CNTP_CVAL_EL0) = val;
>                 if (map.direct_ptimer == vcpu_hptimer(vcpu))
> @@ -182,12 +180,13 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
>                 offset = read_sysreg_s(SYS_CNTPOFF_EL2);
> 
>                 if (map.direct_ptimer && offset) {
> -                       offset = read_sysreg_s(SYS_CNTPOFF_EL2);
> -                       write_sysreg_s(val + offset, SYS_CNTP_CVAL_EL0);
> +                       write_sysreg_el0(val + offset, SYS_CNTP_CVAL);
>                         isb();
>                 }
>         }
> 
> +       write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);

Why moving the HCR_EL2 update? I don't grok what it changes. Or is it
that you end-up with spurious interrupts because your GIC is slow to
retire interrupts that are transiently pending?

Thanks,

	M.

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