[RFC PATCH v7 12/28] KVM: arm64: Add SPE VCPU device attribute to set the interrupt number
Lorenzo Stoakes (ARM)
ljs at kernel.org
Tue Oct 20 10:20:29 PDT 2026
On Thu, Sep 03, 2026 at 05:06:07PM +0100, Alexandru Elisei wrote:
> From: Sudeep Holla <sudeep.holla at arm.com>
>
> Add KVM_ARM_VCPU_SPE_CTRL(KVM_ARM_VCPU_SPE_IRQ) to allow the user to set
> the interrupt number for the buffer management interrupt.
I know this is a bit of a theme of the review :P but feels a little short.
Be good to see why you want to do this, broadly what you are changing
(shouldn't duplicate stuff you can see from the diff but something like '
>
> [ Alexandru E: Split from "KVM: arm64: Add a new VCPU device control group
> for SPE" ]
>
> Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
> Signed-off-by: Alexandru Elisei <alexandru.elisei at arm.com>
I guess similar comment to before - is this a significant change? If so
then probably best to go Co-developed-by route, otherwise not a big deal.
> ---
> Documentation/virt/kvm/devices/vcpu.rst | 22 +++++++
> arch/arm64/include/asm/kvm_host.h | 2 +
> arch/arm64/include/asm/kvm_spe.h | 11 ++++
> arch/arm64/include/uapi/asm/kvm.h | 1 +
> arch/arm64/kvm/guest.c | 2 +
> arch/arm64/kvm/spe.c | 82 +++++++++++++++++++++++++
> 6 files changed, 120 insertions(+)
>
> diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst
> index e0a7fa35dfd2..d30b2f7fec9a 100644
> --- a/Documentation/virt/kvm/devices/vcpu.rst
> +++ b/Documentation/virt/kvm/devices/vcpu.rst
> @@ -307,3 +307,25 @@ From the destination VMM process:
> ===============================
>
> :Architectures: ARM64
> +
> +5.1 ATTRIBUTE: KVM_ARM_VCPU_SPE_IRQ
> +-----------------------------------
> +
> +:Parameters: in kvm_device_attr.addr the address for the Profiling Buffer
> + management interrupt number as a pointer to an int
> +
> +Returns:
Was going to say maybe it should be ':Returns:' but actually the doc seems
super inconsistent on this... :P so it doesn't really matter.
> +
> + ======= ==========================================================
> + -EFAULT Error accessing the buffer management interrupt number
> + -EINVAL Invalid interrupt number or not using an in-kernel irqchip
> + -ENODEV KVM_ARM_VCPU_HAS_SPE VCPU feature not set
Seems like a typo, should be KVM_ARM_VCPU_SPE_IRQ right?
> + -ENXIO SPE not supported or not properly configured
> + ======= ==========================================================
> +
> +Required.
> +
> +Specifies the Profiling Buffer management interrupt number. The interrupt number
> +must be a PPI and the interrupt number must be the same for each VCPU. Arm
> +recommends 21 as the interrupt number. SPE virtualization requires an in-kernel
> +vGIC implementation.
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7..e7332bd82bf3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -27,6 +27,7 @@
> #include <asm/fpsimd.h>
> #include <asm/kvm.h>
> #include <asm/kvm_asm.h>
> +#include <asm/kvm_spe.h>
> #include <asm/vncr_mapping.h>
>
> #define __KVM_HAVE_ARCH_INTC_INITIALIZED
> @@ -918,6 +919,7 @@ struct kvm_vcpu_arch {
> struct vgic_cpu vgic_cpu;
> struct arch_timer_cpu timer_cpu;
> struct kvm_pmu pmu;
> + struct kvm_vcpu_spe vcpu_spe;
>
> /* vcpu power state */
> struct kvm_mp_state mp_state;
> diff --git a/arch/arm64/include/asm/kvm_spe.h b/arch/arm64/include/asm/kvm_spe.h
> index 1327064d0984..8ded2d821bff 100644
> --- a/arch/arm64/include/asm/kvm_spe.h
> +++ b/arch/arm64/include/asm/kvm_spe.h
> @@ -6,7 +6,15 @@
> #ifndef __ARM64_KVM_SPE_H__
> #define __ARM64_KVM_SPE_H__
>
> +struct kvm;
> +struct kvm_vcpu;
> +struct kvm_device_attr;
> +
> #ifdef CONFIG_KVM_ARM_SPE
> +struct kvm_vcpu_spe {
> + int irq_num; /* Buffer management interrupt number */
> +};
I guess this struct will be expanded over time?
> +
> bool kvm_supports_spe(void);
>
> #define vcpu_has_spe(vcpu) \
> @@ -16,6 +24,9 @@ int kvm_spe_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> int kvm_spe_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> int kvm_spe_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> #else
NIT, and probably applicable to a previous patch, but useful to put /*
CONFIG_KVM_ARM_SPE */ there, certainly useful as things grow.
> +struct kvm_vcpu_spe {
> +};
> +
OK good this covers off the !CONFIG_KVM_ARM_SPE case.
> static __always_inline bool kvm_supports_spe(void)
> {
> return false;
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index e8a68e34be63..b9e5019181eb 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -447,6 +447,7 @@ enum {
> #define KVM_ARM_VCPU_PVTIME_CTRL 2
> #define KVM_ARM_VCPU_PVTIME_IPA 0
> #define KVM_ARM_VCPU_SPE_CTRL 3
> +#define KVM_ARM_VCPU_SPE_IRQ 0
>
> /* KVM_IRQ_LINE irq field index values */
> #define KVM_ARM_IRQ_VCPU2_SHIFT 28
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56c5f937fcdb..f953b235b458 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -920,7 +920,9 @@ int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,
> ret = kvm_arm_pvtime_set_attr(vcpu, attr);
> break;
> case KVM_ARM_VCPU_SPE_CTRL:
> + mutex_lock(&vcpu->kvm->arch.config_lock);
> ret = kvm_spe_set_attr(vcpu, attr);
> + mutex_unlock(&vcpu->kvm->arch.config_lock);
> break;
> default:
> ret = -ENXIO;
> diff --git a/arch/arm64/kvm/spe.c b/arch/arm64/kvm/spe.c
> index fe45ad4c703a..dd5f80318ba4 100644
> --- a/arch/arm64/kvm/spe.c
> +++ b/arch/arm64/kvm/spe.c
> @@ -59,17 +59,99 @@ bool kvm_supports_spe(void)
> return !list_empty(&spe_pmus);
> }
>
> +static bool kvm_spe_irq_is_valid(struct kvm *kvm, int irq)
> +{
> + struct kvm_vcpu *vcpu;
> + struct kvm_vcpu_spe *vcpu_spe;
> + unsigned long i;
> +
Maybe worth a small comment:
/* Every vCPU must have either the same or no IRQ set. */
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + vcpu_spe = &vcpu->arch.vcpu_spe;
> +
> + if (!vcpu_spe->irq_num)
> + continue;
> +
> + if (vcpu_spe->irq_num != irq)
> + return false;
> + }
> +
> + return true;
> +}
> +
> int kvm_spe_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> {
> + struct kvm_vcpu_spe *vcpu_spe = &vcpu->arch.vcpu_spe;
> + struct kvm *kvm = vcpu->kvm;
> +
> + lockdep_assert_held(&kvm->arch.config_lock);
> +
> + if (!vcpu_has_spe(vcpu))
> + return -ENODEV;
> +
> + switch (attr->attr) {
> + case KVM_ARM_VCPU_SPE_IRQ: {
> + int __user *uaddr = (int __user *)(long)attr->addr;
> + int irq;
> +
> + if (!irqchip_in_kernel(kvm))
> + return -EINVAL;
> +
> + if (get_user(irq, uaddr))
> + return -EFAULT;
> +
> + if (!irq_is_ppi(kvm, irq))
> + return -EINVAL;
> +
> + if (!kvm_spe_irq_is_valid(kvm, irq))
> + return -EINVAL;
What about changing the IRQ for vcpu? kvm_spe_irq_is_valid() will abort
even if only the current vCPU has an IRQ set.
Also what about the case of setting the current vCPU's IRQ to the same
value? Should that be -EBUSY?
PMU seems to do something like that, so maybe worth doing that for
consistency? Not exactly a huge thing, that though!
Also looking at PMU there's:
/* On GICv5, the PMUIRQ is architecturally mandated to be PPI 23 */
if (vgic_is_v5(kvm) && irq != KVM_ARMV8_PMU_GICV5_IRQ)
return false;
Is that something you have to worry about for SPE?
> +
> + vcpu_spe->irq_num = irq;
> + return 0;
> + }
> + }
> +
> return -ENXIO;
> }
>
> int kvm_spe_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> {
> + struct kvm_vcpu_spe *vcpu_spe = &vcpu->arch.vcpu_spe;
> + struct kvm *kvm = vcpu->kvm;
> +
> + if (!vcpu_has_spe(vcpu))
> + return -ENODEV;
> +
> + switch (attr->attr) {
> + case KVM_ARM_VCPU_SPE_IRQ: {
> + int __user *uaddr = (int __user *)(long)attr->addr;
> + int irq;
> +
> + if (!irqchip_in_kernel(kvm))
> + return -EINVAL;
> +
> + if (!vcpu_spe->irq_num)
> + return -ENXIO;
> +
> + irq = vcpu_spe->irq_num;
> + if (put_user(irq, uaddr))
> + return -EFAULT;
> +
> + return 0;
> + }
> + }
> +
> return -ENXIO;
> }
>
> int kvm_spe_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> {
> + if (!vcpu_has_spe(vcpu))
> + return -ENODEV;
> +
> + switch(attr->attr) {
> + case KVM_ARM_VCPU_SPE_IRQ:
> + return 0;
> + }
> +
> return -ENXIO;
> }
> --
> 2.43.0
>
>
--
Cheers, Lorenzo
More information about the linux-arm-kernel
mailing list