On 9/4/23 13:07, Andrew Jones wrote: > On Mon, Sep 04, 2023 at 10:18:05AM +0200, Claudio Fontana wrote: >> Hi, >> >> I think this discussion from ~2015 could potentially be be historically relevant for context, >> at the time we had the problem with CNTVOFF IIRC so KVM_REG_ARM_TIMER_CNT being read and rewritten causing time warps in the guest: >> >> https://patchwork.kernel.org/project/linux-arm-kernel/patch/1435157697-28579-1-git-send-email-marc.zyngier@xxxxxxx/ >> >> I could not remember or find if/where the problem was fixed in the end in QEMU, > > It's most likely commit 4b7a6bf402bd ("target-arm: kvm: Differentiate > registers based on write-back levels") Indeed, thanks! C > Thanks, > drew > >> >> Ciao, >> >> Claudio >> >> On 9/1/23 21:23, Colton Lewis wrote: >>> On Fri, Sep 01, 2023 at 09:35:47AM +0200, Andrew Jones wrote: >>>> On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote: >>>>> Due to recent KVM changes, QEMU is setting a ptimer offset resulting >>>>> in unintended trap and emulate access and a consequent performance >>>>> hit. Filter out the PTIMER_CNT register to restore trapless ptimer >>>>> access. >>>>> >>>>> Quoting Andrew Jones: >>>>> >>>>> Simply reading the CNT register and writing back the same value is >>>>> enough to set an offset, since the timer will have certainly moved >>>>> past whatever value was read by the time it's written. QEMU >>>>> frequently saves and restores all registers in the get-reg-list array, >>>>> unless they've been explicitly filtered out (with Linux commit >>>>> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to >>>>> restore trapless ptimer accesses, we need a QEMU patch to filter out >>>>> the register. >>>>> >>>>> See >>>>> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9 >>>> >>>> The link can be shorter with >>>> >>>> https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@xxxxxxxxxx/ >>> >>> I will keep that in mind next time. >>> >>>>> for additional context. >>>>> >>>>> Signed-off-by: Andrew Jones <andrew.jones@xxxxxxxxx> >>>> >>>> Thanks for the testing and posting, Colton. Please add your s-o-b and a >>>> Tested-by tag as well. >>> >>> Assuming it is sufficient to add here instead of reposting the whole patch: >>> >>> Signed-off-by: Colton Lewis <coltonlewis@xxxxxxxxxx> >>> Tested-by: Colton Lewis <coltonlewis@xxxxxxxxxx> >>> >>>>> --- >>>>> target/arm/kvm64.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c >>>>> index 4d904a1d11..2dd46e0a99 100644 >>>>> --- a/target/arm/kvm64.c >>>>> +++ b/target/arm/kvm64.c >>>>> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel { >>>>> */ >>>>> static const CPRegStateLevel non_runtime_cpregs[] = { >>>>> { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE }, >>>>> + { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE }, >>>>> }; >>>>> >>>>> int kvm_arm_cpreg_level(uint64_t regidx) >>>>> -- >>>>> 2.42.0.283.g2d96d420d3-goog >>>>> >>> >>
