From: Sudeep Holla <sudeep.holla@arm.com>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: kvm@vger.kernel.org, Suzuki K Pouloze <suzuki.poulose@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Julien Thierry <julien.thierry@arm.com>,
Will Deacon <will.deacon@arm.com>,
linux-kernel@vger.kernel.org,
Christoffer Dall <christoffer.dall@arm.com>,
James Morse <james.morse@arm.com>,
Sudeep Holla <sudeep.holla@arm.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 12/15] KVM: arm64: add a new vcpu device control group for SPEv1
Date: Fri, 24 May 2019 12:21:45 +0100 [thread overview]
Message-ID: <20190524112145.GC13121@e107155-lin> (raw)
In-Reply-To: <a2d64bf0-2424-83c5-d3c8-17affd59dd20@arm.com>
On Fri, May 24, 2019 at 11:37:51AM +0100, Marc Zyngier wrote:
> Hi Sudeep,
>
> On 23/05/2019 11:34, Sudeep Holla wrote:
> > To configure the virtual SPEv1 overflow interrupt number, we use the
> > vcpu kvm_device ioctl, encapsulating the KVM_ARM_VCPU_SPE_V1_IRQ
> > attribute within the KVM_ARM_VCPU_SPE_V1_CTRL group.
> >
> > After configuring the SPEv1, call the vcpu ioctl with attribute
> > KVM_ARM_VCPU_SPE_V1_INIT to initialize the SPEv1.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > Documentation/virtual/kvm/devices/vcpu.txt | 28 ++++
> > arch/arm64/include/asm/kvm_host.h | 2 +-
> > arch/arm64/include/uapi/asm/kvm.h | 4 +
> > arch/arm64/kvm/Makefile | 1 +
> > arch/arm64/kvm/guest.c | 9 ++
> > arch/arm64/kvm/reset.c | 3 +
> > include/kvm/arm_spe.h | 35 +++++
> > include/uapi/linux/kvm.h | 1 +
> > virt/kvm/arm/arm.c | 1 +
> > virt/kvm/arm/spe.c | 163 +++++++++++++++++++++
> > 10 files changed, 246 insertions(+), 1 deletion(-)
> > create mode 100644 virt/kvm/arm/spe.c
> >
> > diff --git a/Documentation/virtual/kvm/devices/vcpu.txt b/Documentation/virtual/kvm/devices/vcpu.txt
> > index 2b5dab16c4f2..d1ece488aeee 100644
> > --- a/Documentation/virtual/kvm/devices/vcpu.txt
> > +++ b/Documentation/virtual/kvm/devices/vcpu.txt
> > @@ -60,3 +60,31 @@ time to use the number provided for a given timer, overwriting any previously
> > 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.
> > +
> > +3. GROUP: KVM_ARM_VCPU_SPE_V1_CTRL
> > +Architectures: ARM64
> > +
> > +1.1. ATTRIBUTE: KVM_ARM_VCPU_SPE_V1_IRQ
> > +Parameters: in kvm_device_attr.addr the address for SPE buffer overflow interrupt
> > + is a pointer to an int
> > +Returns: -EBUSY: The SPE overflow interrupt is already set
> > + -ENXIO: The overflow interrupt not set when attempting to get it
> > + -ENODEV: SPEv1 not supported
> > + -EINVAL: Invalid SPE overflow interrupt number supplied or
> > + trying to set the IRQ number without using an in-kernel
> > + irqchip.
> > +
> > +A value describing the SPEv1 (Statistical Profiling Extension v1) overflow
> > +interrupt number for this vcpu. This interrupt should be PPI and the interrupt
> > +type and number must be same for each vcpu.
> > +
> > +1.2 ATTRIBUTE: KVM_ARM_VCPU_SPE_V1_INIT
> > +Parameters: no additional parameter in kvm_device_attr.addr
> > +Returns: -ENODEV: SPEv1 not supported or GIC not initialized
> > + -ENXIO: SPEv1 not properly configured or in-kernel irqchip not
> > + configured as required prior to calling this attribute
> > + -EBUSY: SPEv1 already initialized
> > +
> > +Request the initialization of the SPEv1. If using the SPEv1 with an in-kernel
> > +virtual GIC implementation, this must be done after initializing the in-kernel
> > +irqchip.
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 6921fdfd477b..fc4ead0774b3 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -50,7 +50,7 @@
> >
> > #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
> >
> > -#define KVM_VCPU_MAX_FEATURES 7
> > +#define KVM_VCPU_MAX_FEATURES 8
> >
> > #define KVM_REQ_SLEEP \
> > KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index 7b7ac0f6cec9..4c9e168de896 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -106,6 +106,7 @@ struct kvm_regs {
> > #define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */
> > #define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */
> > #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
> > +#define KVM_ARM_VCPU_SPE_V1 7 /* Support guest SPEv1 */
> >
> > struct kvm_vcpu_init {
> > __u32 target;
> > @@ -306,6 +307,9 @@ 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_SPE_V1_CTRL 2
> > +#define KVM_ARM_VCPU_SPE_V1_IRQ 0
> > +#define KVM_ARM_VCPU_SPE_V1_INIT 1
> >
> > /* KVM_IRQ_LINE irq field index values */
> > #define KVM_ARM_IRQ_TYPE_SHIFT 24
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 3ac1a64d2fb9..1ba6154dd8e1 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -35,3 +35,4 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o
> > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
> > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
> > kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
> > +kvm-$(CONFIG_KVM_ARM_SPE) += $(KVM)/arm/spe.o
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index 3ae2f82fca46..02c28a7eb332 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -848,6 +848,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_SPE_V1_CTRL:
> > + ret = kvm_arm_spe_v1_set_attr(vcpu, attr);
> > + break;
> > default:
> > ret = -ENXIO;
> > break;
> > @@ -868,6 +871,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_SPE_V1_CTRL:
> > + ret = kvm_arm_spe_v1_get_attr(vcpu, attr);
> > + break;
> > default:
> > ret = -ENXIO;
> > break;
> > @@ -888,6 +894,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_SPE_V1_CTRL:
> > + ret = kvm_arm_spe_v1_has_attr(vcpu, attr);
> > + break;
> > default:
> > ret = -ENXIO;
> > break;
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index 1140b4485575..33ce5248613e 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -91,6 +91,9 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > case KVM_CAP_ARM_INJECT_SERROR_ESR:
> > r = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
> > break;
> > + case KVM_CAP_ARM_SPE_V1:
> > + r = kvm_arm_support_spe_v1();
> > + break;
> > case KVM_CAP_SET_GUEST_DEBUG:
> > case KVM_CAP_VCPU_ATTRIBUTES:
> > r = 1;
> > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > index fdcb0df1e0fd..8c2e8f10a965 100644
> > --- a/include/kvm/arm_spe.h
> > +++ b/include/kvm/arm_spe.h
> > @@ -19,6 +19,9 @@ struct kvm_spe {
> > #ifdef CONFIG_KVM_ARM_SPE
> >
> > #define kvm_arm_spe_v1_ready(v) ((v)->arch.spe.ready)
> > +#define kvm_arm_spe_irq_initialized(v) \
> > + ((v)->arch.spe.irq >= VGIC_NR_SGIS && \
> > + (v)->arch.spe.irq <= VGIC_MAX_PRIVATE)
> >
> > static inline bool kvm_arm_support_spe_v1(void)
> > {
> > @@ -27,10 +30,42 @@ static inline bool kvm_arm_support_spe_v1(void)
> > return !!cpuid_feature_extract_unsigned_field(dfr0,
> > ID_AA64DFR0_PMSVER_SHIFT);
> > }
> > +
> > +int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr);
> > +int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr);
> > +int kvm_arm_spe_v1_has_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr);
> > +int kvm_arm_spe_v1_enable(struct kvm_vcpu *vcpu);
> > #else
> >
> > #define kvm_arm_spe_v1_ready(v) (false)
> > #define kvm_arm_support_spe_v1() (false)
> > +#define kvm_arm_spe_irq_initialized(v) (false)
> > +
> > +static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr)
> > +{
> > + return -ENXIO;
> > +}
> > +
> > +static inline int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr)
> > +{
> > + return -ENXIO;
> > +}
> > +
> > +static inline int kvm_arm_spe_v1_has_attr(struct kvm_vcpu *vcpu,
> > + struct kvm_device_attr *attr)
> > +{
> > + return -ENXIO;
> > +}
> > +
> > +static inline int kvm_arm_spe_v1_enable(struct kvm_vcpu *vcpu)
> > +{
> > + return 0;
> > +}
> > #endif /* CONFIG_KVM_ARM_SPE */
> >
> > #endif /* __ASM_ARM_KVM_SPE_H */
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 2fe12b40d503..698bcc2f96e3 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -993,6 +993,7 @@ struct kvm_ppc_resize_hpt {
> > #define KVM_CAP_ARM_SVE 170
> > #define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
> > #define KVM_CAP_ARM_PTRAUTH_GENERIC 172
> > +#define KVM_CAP_ARM_SPE_V1 173
> >
> > #ifdef KVM_CAP_IRQ_ROUTING
> >
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 90cedebaeb94..c5b711ef1cf8 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -34,6 +34,7 @@
> > #include <trace/events/kvm.h>
> > #include <kvm/arm_pmu.h>
> > #include <kvm/arm_psci.h>
> > +#include <kvm/arm_spe.h>
> >
> > #define CREATE_TRACE_POINTS
> > #include "trace.h"
> > diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> > new file mode 100644
> > index 000000000000..87f02ed92426
> > --- /dev/null
> > +++ b/virt/kvm/arm/spe.c
> > @@ -0,0 +1,163 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 ARM Ltd.
> > + */
> > +
> > +#include <linux/cpu.h>
> > +#include <linux/kvm.h>
> > +#include <linux/kvm_host.h>
> > +#include <linux/uaccess.h>
> > +#include <asm/kvm_emulate.h>
> > +#include <kvm/arm_spe.h>
> > +#include <kvm/arm_vgic.h>
> > +
> > +int kvm_arm_spe_v1_enable(struct kvm_vcpu *vcpu)
> > +{
> > + if (!vcpu->arch.spe.created)
> > + return 0;
> > +
> > + /*
> > + * A valid interrupt configuration for the SPE is either to have a
> > + * properly configured interrupt number and using an in-kernel irqchip.
> > + */
> > + if (irqchip_in_kernel(vcpu->kvm)) {
> > + int irq = vcpu->arch.spe.irq;
> > +
> > + if (!kvm_arm_spe_irq_initialized(vcpu))
> > + return -EINVAL;
> > +
> > + if (!irq_is_ppi(irq))
> > + return -EINVAL;
> > + }
> > +
> > + vcpu->arch.spe.ready = true;
>
> I don't think we should entertain the idea of using SPE without an
> in-kernel irqchip, nor on systems that do not have a GIC.
>
I agree, but sorry I didn't realise that this infrastructure is just
to deal with those scenario. I assume these in place for sanity check
the details we get from DT/ACPI. My assumption is completely wrong I
suppose.
> But there is a more fundamental issue here: I do not see how the SPE
> interrupt get injected in the guest. I've gone through the series twice,
> and I can't see how we go from a physical interrupt triggered by the HW
> on the host to a virtual interrupt injected in the guest.
>
I haven't been able to trigger error/overflow interrupt from the guest
so far on the models. I initial started taking PMU KVM implementation
as reference and soon realised it is quite different. IIUC, we don't
need to inject the interrupt and KVM takes care to set the corresponding
virtual INTID to the pending state on vCPU.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-05-24 11:21 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 10:34 [PATCH 00/15] arm64: KVM: add SPE profiling support for guest Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 01/15] KVM: arm64: add {read, write}_sysreg_elx_s versions for new registers Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 02/15] dt-bindings: ARM SPE: highlight the need for PPI partitions on heterogeneous systems Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 03/15] arm64: KVM: reset E2PB correctly in MDCR_EL2 when exiting the guest(VHE) Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 04/15] arm64: KVM: define SPE data structure for each vcpu Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 05/15] arm64: KVM: add access handler for SPE system registers Sudeep Holla
2019-05-24 11:36 ` Julien Thierry
2019-05-24 14:12 ` Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 06/15] arm64: KVM/VHE: enable the use PMSCR_EL12 on VHE systems Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 07/15] arm64: KVM: split debug save restore across vm/traps activation Sudeep Holla
2019-05-28 8:18 ` Julien Thierry
2019-05-23 10:34 ` [PATCH v2 08/15] arm64: KVM/debug: drop pmscr_el1 and use sys_regs[PMSCR_EL1] in kvm_cpu_context Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 09/15] arm64: KVM: add support to save/restore SPE profiling buffer controls Sudeep Holla
2019-05-29 8:26 ` Julien Thierry
2019-05-23 10:34 ` [PATCH v2 10/15] arm64: KVM: enable conditional save/restore full " Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 11/15] arm64: KVM/debug: trap all accesses to SPE controls at EL1 Sudeep Holla
2019-05-23 10:34 ` [PATCH v2 12/15] KVM: arm64: add a new vcpu device control group for SPEv1 Sudeep Holla
2019-05-24 10:37 ` Marc Zyngier
2019-05-24 11:21 ` Sudeep Holla [this message]
2019-05-24 12:07 ` Marc Zyngier
2019-05-23 10:35 ` [PATCH v2 13/15] KVM: arm64: enable SPE support Sudeep Holla
2019-05-23 10:35 ` [PATCH v2 14/15][KVMTOOL] update_headers: Sync kvm UAPI headers with linux v5.2-rc1 Sudeep Holla
2019-05-23 10:35 ` [PATCH v2 15/15][KVMTOOL] kvm: add a vcpu feature for SPEv1 support Sudeep Holla
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=20190524112145.GC13121@e107155-lin \
--to=sudeep.holla@arm.com \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@arm.com \
--cc=james.morse@arm.com \
--cc=julien.thierry@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=will.deacon@arm.com \
/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).