From: Marc Zyngier <maz@kernel.org>
To: Steven Price <steven.price@arm.com>,
Will Deacon <will@kernel.org>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu
Cc: "Catalin Marinas" <catalin.marinas@arm.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>,
"Russell King" <linux@armlinux.org.uk>,
"James Morse" <james.morse@arm.com>,
"Julien Thierry" <julien.thierry.kdev@gmail.com>,
"Suzuki K Pouloze" <suzuki.poulose@arm.com>,
"Mark Rutland" <mark.rutland@arm.com>,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 07/10] KVM: arm64: Provide VCPU attributes for stolen time
Date: Fri, 30 Aug 2019 11:02:16 +0100 [thread overview]
Message-ID: <36104ec0-2237-fb0e-376f-ab50c23c6101@kernel.org> (raw)
In-Reply-To: <20190830084255.55113-8-steven.price@arm.com>
On 30/08/2019 09:42, Steven Price wrote:
> Allow user space to inform the KVM host where in the physical memory
> map the paravirtualized time structures should be located.
>
> User space can set an attribute on the VCPU providing the IPA base
> address of the stolen time structure for that VCPU. This must be
> repeated for every VCPU in the VM.
>
> The address is given in terms of the physical address visible to
> the guest and must be 64 byte aligned. The guest will discover the
> address via a hypercall.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 7 +++++
> arch/arm64/include/uapi/asm/kvm.h | 2 ++
> arch/arm64/kvm/guest.c | 9 ++++++
> include/uapi/linux/kvm.h | 2 ++
> virt/kvm/arm/pvtime.c | 47 +++++++++++++++++++++++++++++++
> 5 files changed, 67 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1697e63f6dd8..6af16b29a41f 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -489,6 +489,13 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
> long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
> int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
>
> +int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +
> static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
> {
> vcpu_arch->steal.base = GPA_INVALID;
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 9a507716ae2f..bde9f165ad3a 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -323,6 +323,8 @@ struct kvm_vcpu_events {
> #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_PVTIME_CTRL 2
> +#define KVM_ARM_VCPU_PVTIME_SET_IPA 0
>
> /* KVM_IRQ_LINE irq field index values */
> #define KVM_ARM_IRQ_TYPE_SHIFT 24
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dfd626447482..d3ac9d2fd405 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -858,6 +858,9 @@ int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_set_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_set_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> @@ -878,6 +881,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_get_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_get_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> @@ -898,6 +904,9 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_has_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_has_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5e3f12d5359e..265156a984f2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1222,6 +1222,8 @@ enum kvm_device_type {
> #define KVM_DEV_TYPE_ARM_VGIC_ITS KVM_DEV_TYPE_ARM_VGIC_ITS
> KVM_DEV_TYPE_XIVE,
> #define KVM_DEV_TYPE_XIVE KVM_DEV_TYPE_XIVE
> + KVM_DEV_TYPE_ARM_PV_TIME,
> +#define KVM_DEV_TYPE_ARM_PV_TIME KVM_DEV_TYPE_ARM_PV_TIME
> KVM_DEV_TYPE_MAX,
> };
>
> diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
> index d9d0dbc6994b..7b1834b98a68 100644
> --- a/virt/kvm/arm/pvtime.c
> +++ b/virt/kvm/arm/pvtime.c
> @@ -2,7 +2,9 @@
> // Copyright (C) 2019 Arm Ltd.
>
> #include <linux/arm-smccc.h>
> +#include <linux/kvm_host.h>
>
> +#include <asm/kvm_mmu.h>
> #include <asm/pvclock-abi.h>
>
> #include <kvm/arm_hypercalls.h>
> @@ -75,3 +77,48 @@ long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
>
> return vcpu->arch.steal.base;
> }
> +
> +int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + u64 __user *user = (u64 __user *)attr->addr;
> + u64 ipa;
> +
> + if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
> + return -ENXIO;
> +
> + if (get_user(ipa, user))
> + return -EFAULT;
> + if (ipa & 63)
nit: Please express this as !IS_ALIGNED(ipa, 64) instead.
> + return -EINVAL;
> + if (vcpu->arch.steal.base != GPA_INVALID)
> + return -EEXIST;
> + vcpu->arch.steal.base = ipa;
I'm still worried that you end-up not knowing whether the IPA is valid
or not at this stage, nor that we check about overlapping vcpus. How do
we validate that?
I also share Christoffer's concern that the memslot parsing may be
expensive on a system with multiple memslots. But maybe that can be
solved by adding some caching capabilities to your kvm_put_guest(),
should this become a problem.
> + return 0;
> +}
> +
> +int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + u64 __user *user = (u64 __user *)attr->addr;
> + u64 ipa;
> +
> + if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
It is a bit odd that this is using "SET_IPA" as a way to GET it.
> + return -ENXIO;
> +
> + ipa = vcpu->arch.steal.base;
> +
> + if (put_user(ipa, user))
> + return -EFAULT;
> + return 0;
> +}
> +
> +int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + switch (attr->attr) {
> + case KVM_ARM_VCPU_PVTIME_SET_IPA:
> + return 0;
> + }
> + return -ENXIO;
> +}
>
Thanks,
M.
--
Jazz is not dead, it just smells funny...
next prev parent reply other threads:[~2019-08-30 10:02 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 8:42 [PATCH v4 00/10] arm64: Stolen time support Steven Price
2019-08-30 8:42 ` [PATCH v4 01/10] KVM: arm64: Document PV-time interface Steven Price
2019-08-30 14:47 ` Andrew Jones
2019-08-30 15:25 ` Steven Price
2019-09-02 12:52 ` Andrew Jones
2019-09-04 13:55 ` Steven Price
2019-09-04 14:22 ` Andrew Jones
2019-09-04 15:07 ` Steven Price
2019-08-30 8:42 ` [PATCH v4 02/10] KVM: arm/arm64: Factor out hypercall handling from PSCI code Steven Price
2019-09-02 7:06 ` kbuild test robot
2019-09-04 15:05 ` Steven Price
2019-09-02 13:07 ` kbuild test robot
2019-08-30 8:42 ` [PATCH v4 03/10] KVM: arm64: Implement PV_FEATURES call Steven Price
2019-08-30 8:42 ` [PATCH v4 04/10] KVM: Implement kvm_put_guest() Steven Price
2019-08-30 8:42 ` [PATCH v4 05/10] KVM: arm64: Support stolen time reporting via shared structure Steven Price
2019-08-30 9:42 ` Christoffer Dall
2019-08-30 9:52 ` Steven Price
2019-09-03 9:14 ` Zenghui Yu
2019-09-04 15:53 ` Steven Price
2019-08-30 8:42 ` [PATCH v4 06/10] KVM: Allow kvm_device_ops to be const Steven Price
2019-08-30 8:42 ` [PATCH v4 07/10] KVM: arm64: Provide VCPU attributes for stolen time Steven Price
2019-08-30 10:02 ` Marc Zyngier [this message]
2019-08-30 15:04 ` Steven Price
2019-08-30 8:42 ` [PATCH v4 08/10] arm/arm64: Provide a wrapper for SMCCC 1.1 calls Steven Price
2019-08-30 8:42 ` [PATCH v4 09/10] arm/arm64: Make use of the SMCCC 1.1 wrapper Steven Price
2019-08-30 8:42 ` [PATCH v4 10/10] arm64: Retrieve stolen time as paravirtualized guest Steven Price
2019-09-03 8:47 ` Andrew Jones
2019-09-04 16:01 ` Steven Price
2019-09-03 8:03 ` [PATCH v4 00/10] arm64: Stolen time support Andrew Jones
2019-09-03 8:49 ` Andrew Jones
2019-09-04 16:02 ` Steven Price
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=36104ec0-2237-fb0e-376f-ab50c23c6101@kernel.org \
--to=maz@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=steven.price@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).