From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2AC6C4338F for ; Fri, 30 Jul 2021 10:12:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 964B660EC0 for ; Fri, 30 Jul 2021 10:12:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238420AbhG3KMO (ORCPT ); Fri, 30 Jul 2021 06:12:14 -0400 Received: from mail.kernel.org ([198.145.29.99]:45908 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238274AbhG3KMK (ORCPT ); Fri, 30 Jul 2021 06:12:10 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B9C3F60EC0; Fri, 30 Jul 2021 10:12:05 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m9PUp-001yYr-Ea; Fri, 30 Jul 2021 11:12:03 +0100 Date: Fri, 30 Jul 2021 11:12:02 +0100 Message-ID: <877dh82jrh.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, Paolo Bonzini , Sean Christopherson , Peter Shier , Jim Mattson , David Matlack , Ricardo Koller , Jing Zhang , Raghavendra Rao Anata , James Morse , Alexandru Elisei , Suzuki K Poulose , linux-arm-kernel@lists.infradead.org, Andrew Jones Subject: Re: [PATCH v5 09/13] KVM: arm64: Allow userspace to configure a vCPU's virtual offset In-Reply-To: <20210729173300.181775-10-oupton@google.com> References: <20210729173300.181775-1-oupton@google.com> <20210729173300.181775-10-oupton@google.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/27.1 (x86_64-pc-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 X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com, seanjc@google.com, pshier@google.com, jmattson@google.com, dmatlack@google.com, ricarkol@google.com, jingzhangos@google.com, rananta@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, drjones@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, 29 Jul 2021 18:32:56 +0100, Oliver Upton wrote: > > Add a new vCPU attribute that allows userspace to directly manipulate > the virtual counter-timer offset. Exposing such an interface allows for > the precise migration of guest virtual counter-timers, as it is an > indepotent interface. > > Uphold the existing behavior of writes to CNTVOFF_EL2 for this new > interface, wherein a write to a single vCPU is broadcasted to all vCPUs > within a VM. > > Reviewed-by: Andrew Jones > Signed-off-by: Oliver Upton > --- > Documentation/virt/kvm/devices/vcpu.rst | 22 ++++++++ > arch/arm64/include/uapi/asm/kvm.h | 1 + > arch/arm64/kvm/arch_timer.c | 68 ++++++++++++++++++++++++- > 3 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst > index 0f46f2588905..ecbab7adc602 100644 > --- a/Documentation/virt/kvm/devices/vcpu.rst > +++ b/Documentation/virt/kvm/devices/vcpu.rst > @@ -139,6 +139,28 @@ configured values on other VCPUs. Userspace should configure the interrupt > numbers on at least one VCPU after creating all VCPUs and before running any > VCPUs. > > +2.2. ATTRIBUTE: KVM_ARM_VCPU_TIMER_OFFSET_VTIMER > +------------------------------------------------ > + > +:Parameters: Pointer to a 64-bit unsigned counter-timer offset. > + > +Returns: > + > + ======= ====================================== > + -EFAULT Error reading/writing the provided > + parameter address > + -ENXIO Attribute not supported > + ======= ====================================== > + > +Specifies the guest's virtual counter-timer offset from the host's > +virtual counter. The guest's virtual counter is then derived by > +the following equation: > + > + guest_cntvct = host_cntvct - KVM_ARM_VCPU_TIMER_OFFSET_VTIMER I still have a problem with this, specially as you later introduce a physical timer offset. My gut feeling is that the virtual offset should be relative to the physical counter *of the guest*, and not that of the host. The physical offset should be the only one that is relative to the host. Anything else should be deriving from it. If you don't set the ptimer offset, then the two definitions are strictly identical. It will also match the definition of a userspace-visible CNTVOFF_EL2 with NV, which is strictly relative to the guest view of the physical counter. > + > +KVM does not allow the use of varying offset values for different vCPUs; > +the last written offset value will be broadcasted to all vCPUs in a VM. > + Please document the effects of this attribute w.r.t. writing CNTVCT_EL0 from userspace. > 3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL > ================================== > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index b3edde68bc3e..008d0518d2b1 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -365,6 +365,7 @@ struct kvm_arm_copy_mte_tags { > #define KVM_ARM_VCPU_TIMER_CTRL 1 > #define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0 > #define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1 > +#define KVM_ARM_VCPU_TIMER_OFFSET_VTIMER 2 > #define KVM_ARM_VCPU_PVTIME_CTRL 2 > #define KVM_ARM_VCPU_PVTIME_IPA 0 > > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c > index 3df67c127489..d2b1b13af658 100644 > --- a/arch/arm64/kvm/arch_timer.c > +++ b/arch/arm64/kvm/arch_timer.c > @@ -1305,7 +1305,7 @@ static void set_timer_irqs(struct kvm *kvm, int vtimer_irq, int ptimer_irq) > } > } > > -int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *vtimer = vcpu_vtimer(vcpu); > @@ -1338,7 +1338,39 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return 0; > } > > -int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + u64 offset; > + > + if (get_user(offset, uaddr)) > + return -EFAULT; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + update_vtimer_cntvoff(vcpu, offset); Probably not a good idea if the timer is already enabled on any of the CPUs (we probably already have that problem, so let's fix it once and for all). > + break; > + default: > + return -ENXIO; > + } > + > + return 0; > +} > + > +int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_set_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_set_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > +int kvm_arm_timer_get_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *timer; > @@ -1359,11 +1391,43 @@ int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return put_user(irq, uaddr); > } > > +int kvm_arm_timer_get_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + struct arch_timer_context *timer; > + u64 offset; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + timer = vcpu_vtimer(vcpu); > + break; > + default: > + return -ENXIO; What is the rational for retrieving this offset the first place? I don't dislike the symmetry, but we already have an architectural way of getting it (read the counter registers). > + } > + > + offset = timer_get_offset(timer); > + return put_user(offset, uaddr); > +} > + > +int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_get_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_get_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > switch (attr->attr) { > case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > return 0; > } > Thanks, M. -- Without deviation from the norm, progress is not possible. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3FDBCC4320A for ; Fri, 30 Jul 2021 10:12:13 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id B23BD60EC0 for ; Fri, 30 Jul 2021 10:12:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B23BD60EC0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 1CC3C4B085; Fri, 30 Jul 2021 06:12:12 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mhC1JY8sWlOa; Fri, 30 Jul 2021 06:12:09 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id C0BEB4B0A1; Fri, 30 Jul 2021 06:12:09 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 35C8A4B0A1 for ; Fri, 30 Jul 2021 06:12:08 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eBIHBc7WNSqm for ; Fri, 30 Jul 2021 06:12:07 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id D7E1E4A3B4 for ; Fri, 30 Jul 2021 06:12:06 -0400 (EDT) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B9C3F60EC0; Fri, 30 Jul 2021 10:12:05 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m9PUp-001yYr-Ea; Fri, 30 Jul 2021 11:12:03 +0100 Date: Fri, 30 Jul 2021 11:12:02 +0100 Message-ID: <877dh82jrh.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Subject: Re: [PATCH v5 09/13] KVM: arm64: Allow userspace to configure a vCPU's virtual offset In-Reply-To: <20210729173300.181775-10-oupton@google.com> References: <20210729173300.181775-1-oupton@google.com> <20210729173300.181775-10-oupton@google.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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com, seanjc@google.com, pshier@google.com, jmattson@google.com, dmatlack@google.com, ricarkol@google.com, jingzhangos@google.com, rananta@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, drjones@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kvm@vger.kernel.org, Sean Christopherson , Peter Shier , Raghavendra Rao Anata , David Matlack , Paolo Bonzini , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Jim Mattson X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Thu, 29 Jul 2021 18:32:56 +0100, Oliver Upton wrote: > > Add a new vCPU attribute that allows userspace to directly manipulate > the virtual counter-timer offset. Exposing such an interface allows for > the precise migration of guest virtual counter-timers, as it is an > indepotent interface. > > Uphold the existing behavior of writes to CNTVOFF_EL2 for this new > interface, wherein a write to a single vCPU is broadcasted to all vCPUs > within a VM. > > Reviewed-by: Andrew Jones > Signed-off-by: Oliver Upton > --- > Documentation/virt/kvm/devices/vcpu.rst | 22 ++++++++ > arch/arm64/include/uapi/asm/kvm.h | 1 + > arch/arm64/kvm/arch_timer.c | 68 ++++++++++++++++++++++++- > 3 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst > index 0f46f2588905..ecbab7adc602 100644 > --- a/Documentation/virt/kvm/devices/vcpu.rst > +++ b/Documentation/virt/kvm/devices/vcpu.rst > @@ -139,6 +139,28 @@ configured values on other VCPUs. Userspace should configure the interrupt > numbers on at least one VCPU after creating all VCPUs and before running any > VCPUs. > > +2.2. ATTRIBUTE: KVM_ARM_VCPU_TIMER_OFFSET_VTIMER > +------------------------------------------------ > + > +:Parameters: Pointer to a 64-bit unsigned counter-timer offset. > + > +Returns: > + > + ======= ====================================== > + -EFAULT Error reading/writing the provided > + parameter address > + -ENXIO Attribute not supported > + ======= ====================================== > + > +Specifies the guest's virtual counter-timer offset from the host's > +virtual counter. The guest's virtual counter is then derived by > +the following equation: > + > + guest_cntvct = host_cntvct - KVM_ARM_VCPU_TIMER_OFFSET_VTIMER I still have a problem with this, specially as you later introduce a physical timer offset. My gut feeling is that the virtual offset should be relative to the physical counter *of the guest*, and not that of the host. The physical offset should be the only one that is relative to the host. Anything else should be deriving from it. If you don't set the ptimer offset, then the two definitions are strictly identical. It will also match the definition of a userspace-visible CNTVOFF_EL2 with NV, which is strictly relative to the guest view of the physical counter. > + > +KVM does not allow the use of varying offset values for different vCPUs; > +the last written offset value will be broadcasted to all vCPUs in a VM. > + Please document the effects of this attribute w.r.t. writing CNTVCT_EL0 from userspace. > 3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL > ================================== > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index b3edde68bc3e..008d0518d2b1 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -365,6 +365,7 @@ struct kvm_arm_copy_mte_tags { > #define KVM_ARM_VCPU_TIMER_CTRL 1 > #define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0 > #define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1 > +#define KVM_ARM_VCPU_TIMER_OFFSET_VTIMER 2 > #define KVM_ARM_VCPU_PVTIME_CTRL 2 > #define KVM_ARM_VCPU_PVTIME_IPA 0 > > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c > index 3df67c127489..d2b1b13af658 100644 > --- a/arch/arm64/kvm/arch_timer.c > +++ b/arch/arm64/kvm/arch_timer.c > @@ -1305,7 +1305,7 @@ static void set_timer_irqs(struct kvm *kvm, int vtimer_irq, int ptimer_irq) > } > } > > -int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *vtimer = vcpu_vtimer(vcpu); > @@ -1338,7 +1338,39 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return 0; > } > > -int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + u64 offset; > + > + if (get_user(offset, uaddr)) > + return -EFAULT; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + update_vtimer_cntvoff(vcpu, offset); Probably not a good idea if the timer is already enabled on any of the CPUs (we probably already have that problem, so let's fix it once and for all). > + break; > + default: > + return -ENXIO; > + } > + > + return 0; > +} > + > +int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_set_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_set_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > +int kvm_arm_timer_get_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *timer; > @@ -1359,11 +1391,43 @@ int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return put_user(irq, uaddr); > } > > +int kvm_arm_timer_get_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + struct arch_timer_context *timer; > + u64 offset; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + timer = vcpu_vtimer(vcpu); > + break; > + default: > + return -ENXIO; What is the rational for retrieving this offset the first place? I don't dislike the symmetry, but we already have an architectural way of getting it (read the counter registers). > + } > + > + offset = timer_get_offset(timer); > + return put_user(offset, uaddr); > +} > + > +int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_get_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_get_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > switch (attr->attr) { > case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > return 0; > } > Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C6C7C4338F for ; Fri, 30 Jul 2021 10:14:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EE45D61008 for ; Fri, 30 Jul 2021 10:14:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EE45D61008 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Uez29uxYoo9ly1k/1vz1YGdivyzNfTvQf8upkOfGnU4=; b=e7zuH5/0tSp0UH gnTUG8m6w5ugXehk4YmBdlKpbXMIwcDNTJ53U87LWXUmM0gJSfHE3jM0XPwKw1130qAE4tK+T0dlq G45GPNn4m/1L4vvBjTGjX5a4wQKggrX5T2EGfn2QCfUMgRUetyuxketXp5t2NT0BioDrhJzEuBx8T f3jnKbCWfaBKkF+pP8sie4z5SS0gETEfg5lZcjckSP7A7ZSyt7XyztOHNzLvFsSJodphkxPw2lhhZ B+iNY6RzcXmrBPFqtEGQrVjDYgGeJGqrP3KsXwFyiVnqfdi6Y1TpgytBk1iguLBZQimdYtQkB59JI jMgUTOHOuB5VyPLP8uLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9PV2-0086Vp-CQ; Fri, 30 Jul 2021 10:12:18 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m9PUs-0086U3-Iw for linux-arm-kernel@lists.infradead.org; Fri, 30 Jul 2021 10:12:08 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B9C3F60EC0; Fri, 30 Jul 2021 10:12:05 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m9PUp-001yYr-Ea; Fri, 30 Jul 2021 11:12:03 +0100 Date: Fri, 30 Jul 2021 11:12:02 +0100 Message-ID: <877dh82jrh.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, Paolo Bonzini , Sean Christopherson , Peter Shier , Jim Mattson , David Matlack , Ricardo Koller , Jing Zhang , Raghavendra Rao Anata , James Morse , Alexandru Elisei , Suzuki K Poulose , linux-arm-kernel@lists.infradead.org, Andrew Jones Subject: Re: [PATCH v5 09/13] KVM: arm64: Allow userspace to configure a vCPU's virtual offset In-Reply-To: <20210729173300.181775-10-oupton@google.com> References: <20210729173300.181775-1-oupton@google.com> <20210729173300.181775-10-oupton@google.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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com, seanjc@google.com, pshier@google.com, jmattson@google.com, dmatlack@google.com, ricarkol@google.com, jingzhangos@google.com, rananta@google.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, drjones@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210730_031206_756451_6D4E8E06 X-CRM114-Status: GOOD ( 39.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 29 Jul 2021 18:32:56 +0100, Oliver Upton wrote: > > Add a new vCPU attribute that allows userspace to directly manipulate > the virtual counter-timer offset. Exposing such an interface allows for > the precise migration of guest virtual counter-timers, as it is an > indepotent interface. > > Uphold the existing behavior of writes to CNTVOFF_EL2 for this new > interface, wherein a write to a single vCPU is broadcasted to all vCPUs > within a VM. > > Reviewed-by: Andrew Jones > Signed-off-by: Oliver Upton > --- > Documentation/virt/kvm/devices/vcpu.rst | 22 ++++++++ > arch/arm64/include/uapi/asm/kvm.h | 1 + > arch/arm64/kvm/arch_timer.c | 68 ++++++++++++++++++++++++- > 3 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst > index 0f46f2588905..ecbab7adc602 100644 > --- a/Documentation/virt/kvm/devices/vcpu.rst > +++ b/Documentation/virt/kvm/devices/vcpu.rst > @@ -139,6 +139,28 @@ configured values on other VCPUs. Userspace should configure the interrupt > numbers on at least one VCPU after creating all VCPUs and before running any > VCPUs. > > +2.2. ATTRIBUTE: KVM_ARM_VCPU_TIMER_OFFSET_VTIMER > +------------------------------------------------ > + > +:Parameters: Pointer to a 64-bit unsigned counter-timer offset. > + > +Returns: > + > + ======= ====================================== > + -EFAULT Error reading/writing the provided > + parameter address > + -ENXIO Attribute not supported > + ======= ====================================== > + > +Specifies the guest's virtual counter-timer offset from the host's > +virtual counter. The guest's virtual counter is then derived by > +the following equation: > + > + guest_cntvct = host_cntvct - KVM_ARM_VCPU_TIMER_OFFSET_VTIMER I still have a problem with this, specially as you later introduce a physical timer offset. My gut feeling is that the virtual offset should be relative to the physical counter *of the guest*, and not that of the host. The physical offset should be the only one that is relative to the host. Anything else should be deriving from it. If you don't set the ptimer offset, then the two definitions are strictly identical. It will also match the definition of a userspace-visible CNTVOFF_EL2 with NV, which is strictly relative to the guest view of the physical counter. > + > +KVM does not allow the use of varying offset values for different vCPUs; > +the last written offset value will be broadcasted to all vCPUs in a VM. > + Please document the effects of this attribute w.r.t. writing CNTVCT_EL0 from userspace. > 3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL > ================================== > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index b3edde68bc3e..008d0518d2b1 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -365,6 +365,7 @@ struct kvm_arm_copy_mte_tags { > #define KVM_ARM_VCPU_TIMER_CTRL 1 > #define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0 > #define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1 > +#define KVM_ARM_VCPU_TIMER_OFFSET_VTIMER 2 > #define KVM_ARM_VCPU_PVTIME_CTRL 2 > #define KVM_ARM_VCPU_PVTIME_IPA 0 > > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c > index 3df67c127489..d2b1b13af658 100644 > --- a/arch/arm64/kvm/arch_timer.c > +++ b/arch/arm64/kvm/arch_timer.c > @@ -1305,7 +1305,7 @@ static void set_timer_irqs(struct kvm *kvm, int vtimer_irq, int ptimer_irq) > } > } > > -int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *vtimer = vcpu_vtimer(vcpu); > @@ -1338,7 +1338,39 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return 0; > } > > -int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +int kvm_arm_timer_set_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + u64 offset; > + > + if (get_user(offset, uaddr)) > + return -EFAULT; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + update_vtimer_cntvoff(vcpu, offset); Probably not a good idea if the timer is already enabled on any of the CPUs (we probably already have that problem, so let's fix it once and for all). > + break; > + default: > + return -ENXIO; > + } > + > + return 0; > +} > + > +int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_set_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_set_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > +int kvm_arm_timer_get_attr_irq(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > int __user *uaddr = (int __user *)(long)attr->addr; > struct arch_timer_context *timer; > @@ -1359,11 +1391,43 @@ int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > return put_user(irq, uaddr); > } > > +int kvm_arm_timer_get_attr_offset(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + u64 __user *uaddr = (u64 __user *)(long)attr->addr; > + struct arch_timer_context *timer; > + u64 offset; > + > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + timer = vcpu_vtimer(vcpu); > + break; > + default: > + return -ENXIO; What is the rational for retrieving this offset the first place? I don't dislike the symmetry, but we already have an architectural way of getting it (read the counter registers). > + } > + > + offset = timer_get_offset(timer); > + return put_user(offset, uaddr); > +} > + > +int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > +{ > + switch (attr->attr) { > + case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > + case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + return kvm_arm_timer_get_attr_irq(vcpu, attr); > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > + return kvm_arm_timer_get_attr_offset(vcpu, attr); > + } > + > + return -ENXIO; > +} > + > int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > { > switch (attr->attr) { > case KVM_ARM_VCPU_TIMER_IRQ_VTIMER: > case KVM_ARM_VCPU_TIMER_IRQ_PTIMER: > + case KVM_ARM_VCPU_TIMER_OFFSET_VTIMER: > return 0; > } > Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel