[PATCH v2 00/12] target/arm: Add vSPE support to KVM guest

[PATCH v2 00/12] target/arm: Add vSPE support to KVM guest

[Haibo Xu]

v2:
  - Rebased on Andrew's patches[3]
  - Added compat codes to enable vSPE only for the 5.2 and 
    later machine types [Andrew]
  - Changed to use the ID register bit to identify the spe
    feature [Andrew/Richard]
  - Added the missing field in AcpiMadtGenericCpuInterface
    definition [Auger]
  - Split the un-tested userspace irqchip support for vSPE
    into a separate patch [Andrew]
  - Added doc and qtest for vSPE [Andrew]

Many thanks to Andrew, Richard, Philippe and Auger for their comments. 

This series add support for SPE(Statistical Profiling Extension)[1]
in KVM guest. It's based on Andrew Murray's kernel KVM patches V2[2],
and has been tested to ensure that guest can use SPE with valid data.
E.g.

In host:
$ ./qemu-system-aarch64 \
        -cpu host -M virt,accel=kvm,gic-version=3 -nographic -m 2048M \
        -kernel ./Image-new \
        -initrd /boot/initrd.img-5.6.0-rc2+ \
        -append "root=/dev/vda rw console=ttyAMA0" -nodefaults -serial stdio\
        -drive if=none,file=./xenial.rootfs.ext4,id=hd0,format=raw \
        -device virtio-blk-device,drive=hd0  \

In guest:
$ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
        dd if=/dev/zero of=/dev/null count=1000
$ perf report --dump-raw-trace > spe_buf.txt

The spe_buf.txt should contain similar data as below:

. ... ARM SPE data: size 135944 bytes
.  00000000:  b0 f4 d3 29 10 00 80 ff a0                      PC 0xff80001029d3f4 el1 ns=1
.  00000009:  99 0b 00                                        LAT 11 ISSUE
.  0000000c:  98 0d 00                                        LAT 13 TOT
.  0000000f:  52 16 00                                        EV RETIRED L1D-ACCESS TLB-ACCESS
.  00000012:  49 00                                           LD
.  00000014:  b2 d0 40 d8 70 00 00 ff 00                      VA 0xff000070d840d0
.  0000001d:  9a 01 00                                        LAT 1 XLAT
.  00000020:  00 00 00                                        PAD
.  00000023:  71 a5 1f b3 20 14 00 00 00                      TS 86447955877
.  0000002c:  b0 7c f9 29 10 00 80 ff a0                      PC 0xff80001029f97c el1 ns=1
.  00000035:  99 02 00                                        LAT 2 ISSUE
.  00000038:  98 03 00                                        LAT 3 TOT
.  0000003b:  52 02 00                                        EV RETIRED
.  0000003e:  48 00                                           INSN-OTHER
.  00000040:  00 00 00                                        PAD
.  00000043:  71 ef 1f b3 20 14 00 00 00                      TS 86447955951
.  0000004c:  b0 f0 e9 29 10 00 80 ff a0                      PC 0xff80001029e9f0 el1 ns=1
.  00000055:  99 02 00                                        LAT 2 ISSUE
.  00000058:  98 03 00                                        LAT 3 TOT
.  0000005b:  52 02 00                                        EV RETIRED

If you want to disable the vSPE support, you can use the 'spe=off' cpu
property:

./qemu-system-aarch64 \
        -cpu host,spe=off -M virt,accel=kvm,gic-version=3 -nographic -m 2048M \
        -kernel ./Image-new \
        -initrd /boot/initrd.img-5.6.0-rc2+ \
        -append "root=/dev/vda rw console=ttyAMA0" -nodefaults -serial stdio\
        -drive if=none,file=./xenial.rootfs.ext4,id=hd0,format=raw \
        -device virtio-blk-device,drive=hd0  \

Note:
(1) Since the kernel patches are still under review, some of the macros
    in the header files may be changed after merging. We may need to
    update them accordingly. To be specific, if you want to have a try
    on this patch series, you needs to replace on the kernel 5.5-rc2 based
    series, and do minor changes:

    -#define KVM_CAP_ARM_SPE_V1 179
    +#define KVM_CAP_ARM_SPE_V1 184

(2) These patches only add vSPE support in KVM mode, for TCG mode, I'm
    not sure whether we need to support it.
(3) Just followed the 'sve' property, we only allow this feature to be
    removed from CPUs which enable it by default when the host cpu support it.

[1]https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/
   posts/statistical-profiling-extension-for-armv8-a
[2]https://www.spinics.net/lists/arm-kernel/msg776228.html
[3]https://www.mail-archive.com/qemu-devel@nongnu.org/msg727588.html

Haibo Xu (12):
  update Linux headers with new vSPE macros
  target/arm/kvm: spe: Add helper to detect SPE when using KVM
  target/arm/cpu: spe: Add an option to turn on/off vSPE support
  target/arm: spe: Only enable SPE from 5.2 compat machines.
  target/arm/kvm: spe: Unify device attr operation helper
  target/arm/kvm: spe: Add device init and set_irq operations
  hw/arm/virt: Move post cpu realize check into its own function
  hw/arm/virt: Move kvm pmu setup to virt_cpu_post_init
  hw/arm/virt: spe: Add SPE fdt binding for virt machine
  target/arm/cpu: spe: Enable spe to work with host cpu
  target/arm/kvm: spe: Enable userspace irqchip support.
  target/arm: spe: Add corresponding doc and test.

 docs/system/arm/cpu-features.rst |  20 ++++++
 hw/arm/virt-acpi-build.c         |   3 +
 hw/arm/virt.c                    | 116 +++++++++++++++++++++++--------
 include/hw/acpi/acpi-defs.h      |   3 +
 include/hw/arm/virt.h            |   2 +
 linux-headers/asm-arm64/kvm.h    |   4 ++
 linux-headers/linux/kvm.h        |   2 +
 target/arm/cpu.c                 |  11 +++
 target/arm/cpu.h                 |  17 +++++
 target/arm/cpu64.c               |  57 +++++++++++++++
 target/arm/kvm.c                 |  10 +++
 target/arm/kvm64.c               |  56 +++++++++++++--
 target/arm/kvm_arm.h             |  18 +++++
 target/arm/monitor.c             |   2 +-
 tests/qtest/arm-cpu-features.c   |   9 +++
 15 files changed, 294 insertions(+), 36 deletions(-)

-- 
2.17.1

[PATCH v2 01/12] update Linux headers with new vSPE macros

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 linux-headers/asm-arm64/kvm.h | 4 ++++
 linux-headers/linux/kvm.h     | 1 +
 2 files changed, 5 insertions(+)

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 9e34f0f875..802319ee02 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/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;
@@ -334,6 +335,9 @@ struct kvm_vcpu_events {
 #define   KVM_ARM_VCPU_TIMER_IRQ_PTIMER		1
 #define KVM_ARM_VCPU_PVTIME_CTRL	2
 #define   KVM_ARM_VCPU_PVTIME_IPA	0
+#define KVM_ARM_VCPU_SPE_V1_CTRL	3
+#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_VCPU2_SHIFT		28
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index a28c366737..8840cbb01c 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_ARM_SPE_V1 184
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.17.1

[PATCH v2 02/12] target/arm/kvm: spe: Add helper to detect SPE when using KVM

[Haibo Xu]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/kvm.c     |  5 +++++
 target/arm/kvm_arm.h | 13 +++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 8bb7318378..58f991e890 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -214,6 +214,11 @@ bool kvm_arm_pmu_supported(void)
     return kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
 }
 
+bool kvm_arm_spe_supported(void)
+{
+    return kvm_check_extension(kvm_state, KVM_CAP_ARM_SPE_V1);
+}
+
 int kvm_arm_get_max_vm_ipa_size(MachineState *ms)
 {
     KVMState *s = KVM_STATE(ms->accelerator);
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index adb38514bf..f79655674e 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -283,6 +283,14 @@ bool kvm_arm_aarch32_supported(void);
  */
 bool kvm_arm_pmu_supported(void);
 
+/**
+ * kvm_arm_spe_supported:
+ *
+ * Returns: true if the KVM VCPU can enable its SPE
+ * and false otherwise.
+ */
+bool kvm_arm_spe_supported(void);
+
 /**
  * kvm_arm_sve_supported:
  *
@@ -366,6 +374,11 @@ static inline bool kvm_arm_pmu_supported(void)
     return false;
 }
 
+static inline bool kvm_arm_spe_supported(void)
+{
+    return false;
+}
+
 static inline bool kvm_arm_sve_supported(void)
 {
     return false;
-- 
2.17.1

[PATCH v2 03/12] target/arm/cpu: spe: Add an option to turn on/off vSPE support

[Haibo Xu]

Adds a spe=[on/off] option to enable/disable vSPE support in
guest vCPU.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/cpu.c   |  6 ++++++
 target/arm/cpu.h   | 13 ++++++++++++
 target/arm/cpu64.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index c179e0752d..f211958eaa 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1310,6 +1310,12 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
+
+        arm_cpu_spe_finalize(cpu, &local_err);
+        if (local_err != NULL) {
+            error_propagate(errp, local_err);
+            return;
+        }
     }
 }
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a1c7d8ebae..baf2bbcee8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -24,6 +24,7 @@
 #include "hw/registerfields.h"
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
+#include "qapi/qapi-types-common.h"
 
 /* ARM processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO      (0)
@@ -196,9 +197,11 @@ typedef struct {
 #ifdef TARGET_AARCH64
 # define ARM_MAX_VQ    16
 void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
+void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp);
 #else
 # define ARM_MAX_VQ    1
 static inline void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp) { }
+static inline void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp) { }
 #endif
 
 typedef struct ARMVectorReg {
@@ -829,6 +832,8 @@ struct ARMCPU {
     bool has_el3;
     /* CPU has PMU (Performance Monitor Unit) */
     bool has_pmu;
+    /* CPU has SPE (Statistical Profiling Extension) */
+    OnOffAuto has_spe;
     /* CPU has VFP */
     bool has_vfp;
     /* CPU has Neon */
@@ -3869,6 +3874,14 @@ static inline bool isar_feature_aa64_pmu_8_4(const ARMISARegisters *id)
         FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) != 0xf;
 }
 
+/*
+ * Currently we don't differentiate between the ARMv8.2-SPE and ARMv8.3-SPE.
+ */
+static inline bool isar_feature_aa64_spe(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMSVER) != 0;
+}
+
 static inline bool isar_feature_aa64_rcpc_8_3(const ARMISARegisters *id)
 {
     return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, LRCPC) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 3c2b3d9599..4997c4a3c0 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -572,6 +572,55 @@ void aarch64_add_sve_properties(Object *obj)
     }
 }
 
+void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp)
+{
+    uint64_t t;
+    uint32_t value = 0;
+
+    if (cpu->has_spe == ON_OFF_AUTO_AUTO) {
+        if (kvm_enabled() && kvm_arm_spe_supported()) {
+            cpu->has_spe = ON_OFF_AUTO_ON;
+        } else {
+            cpu->has_spe = ON_OFF_AUTO_OFF;
+        }
+    } else if (cpu->has_spe == ON_OFF_AUTO_ON) {
+        if (!kvm_enabled() || !kvm_arm_spe_supported()) {
+            error_setg(errp, "'spe' cannot be enabled on this host");
+            return;
+        }
+    }
+
+    /*
+     * According to the ARM ARM, the ID_AA64DFR0[PMSVER] currently
+     * support 3 values:
+     *
+     * 0b0000: SPE not implemented
+     * 0b0001: ARMv8.2-SPE implemented
+     * 0b0010: ARMv8.3-SPE implemented
+     *
+     * But the kernel KVM API didn't expose all these 3 values, and
+     * we can only get whether the SPE feature is supported or not.
+     * So here we just set the PMSVER to 1 if this feature was supported.
+     */
+    if (cpu->has_spe == ON_OFF_AUTO_ON) {
+        value = 1;
+    }
+
+    t = cpu->isar.id_aa64dfr0;
+    t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, value);
+    cpu->isar.id_aa64dfr0 = t;
+}
+
+static bool arm_spe_get(Object *obj, Error **errp)
+{
+    return ARM_CPU(obj)->has_spe != ON_OFF_AUTO_OFF;
+}
+
+static void arm_spe_set(Object *obj, bool value, Error **errp)
+{
+    ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
+}
+
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
  * otherwise, a CPU with as many features enabled as our emulation supports.
  * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
@@ -721,6 +770,9 @@ static void aarch64_max_initfn(Object *obj)
     aarch64_add_sve_properties(obj);
     object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
                         cpu_max_set_sve_max_vq, NULL, NULL);
+
+    cpu->has_spe = ON_OFF_AUTO_AUTO;
+    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
 }
 
 static const ARMCPUInfo aarch64_cpus[] = {
-- 
2.17.1

[PATCH v2 04/12] target/arm: spe: Only enable SPE from 5.2 compat machines.

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 hw/arm/virt.c         | 7 +++++++
 include/hw/arm/virt.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index acf9bfbece..3f6d26c531 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1830,6 +1830,10 @@ static void machvirt_init(MachineState *machine)
             object_property_set_bool(cpuobj, "pmu", false, NULL);
         }
 
+        if (vmc->no_spe && object_property_find(cpuobj, "spe", NULL)) {
+            object_property_set_bool(cpuobj, "spe", false, NULL);
+        }
+
         if (object_property_find(cpuobj, "reset-cbar", NULL)) {
             object_property_set_int(cpuobj, "reset-cbar",
                                     vms->memmap[VIRT_CPUPERIPHS].base,
@@ -2553,8 +2557,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 2)
 
 static void virt_machine_5_1_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_5_2_options(mc);
     compat_props_add(mc->compat_props, hw_compat_5_1, hw_compat_5_1_len);
+    vmc->no_spe = true;
 }
 DEFINE_VIRT_MACHINE(5, 1)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index dff67e1bef..72c269aaa5 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -126,6 +126,7 @@ typedef struct {
     bool no_ged;   /* Machines < 4.2 has no support for ACPI GED device */
     bool kvm_no_adjvtime;
     bool acpi_expose_flash;
+    bool no_spe;
 } VirtMachineClass;
 
 typedef struct {
-- 
2.17.1

[PATCH v2 05/12] target/arm/kvm: spe: Unify device attr operation helper

[Haibo Xu]

From: Andrew Jones <drjones@redhat.com>

Rename kvm_arm_pmu_set_attr() to kvm_arm_set_device_attr(),
So both the vPMU and vSPE device can share the same API.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/kvm64.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index ef1e960285..8ffd31ffdf 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -397,19 +397,20 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
     return NULL;
 }
 
-static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
+static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr,
+                                    const char *name)
 {
     int err;
 
     err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
     if (err != 0) {
-        error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err));
+        error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err));
         return false;
     }
 
     err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
     if (err != 0) {
-        error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err));
+        error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err));
         return false;
     }
 
@@ -426,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
     if (!ARM_CPU(cs)->has_pmu) {
         return;
     }
-    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
+    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
         error_report("failed to init PMU");
         abort();
     }
@@ -443,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
     if (!ARM_CPU(cs)->has_pmu) {
         return;
     }
-    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
+    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
         error_report("failed to set irq for PMU");
         abort();
     }
-- 
2.17.1

[PATCH v2 06/12] target/arm/kvm: spe: Add device init and set_irq operations

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/kvm64.c   | 33 +++++++++++++++++++++++++++++++++
 target/arm/kvm_arm.h |  5 +++++
 2 files changed, 38 insertions(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 8ffd31ffdf..5a2032fc9e 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -450,6 +450,39 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
     }
 }
 
+void kvm_arm_spe_init(CPUState *cs)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_ARM_VCPU_SPE_V1_CTRL,
+        .attr = KVM_ARM_VCPU_SPE_V1_INIT,
+    };
+
+    if (!ARM_CPU(cs)->has_spe) {
+        return;
+    }
+    if (!kvm_arm_set_device_attr(cs, &attr, "SPE")) {
+        error_report("failed to init SPE");
+        abort();
+    }
+}
+
+void kvm_arm_spe_set_irq(CPUState *cs, int irq)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_ARM_VCPU_SPE_V1_CTRL,
+        .addr = (intptr_t)&irq,
+        .attr = KVM_ARM_VCPU_SPE_V1_IRQ,
+    };
+
+    if (!ARM_CPU(cs)->has_spe) {
+        return;
+    }
+    if (!kvm_arm_set_device_attr(cs, &attr, "SPE")) {
+        error_report("failed to set irq for SPE");
+        abort();
+    }
+}
+
 static int read_sys_reg32(int fd, uint32_t *pret, uint64_t id)
 {
     uint64_t ret;
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index f79655674e..bb155322eb 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -348,6 +348,8 @@ int kvm_arm_vgic_probe(void);
 
 void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
 void kvm_arm_pmu_init(CPUState *cs);
+void kvm_arm_spe_set_irq(CPUState *cs, int irq);
+void kvm_arm_spe_init(CPUState *cs);
 int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
 #else
@@ -397,6 +399,9 @@ static inline int kvm_arm_vgic_probe(void)
 static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {}
 static inline void kvm_arm_pmu_init(CPUState *cs) {}
 
+static inline void kvm_arm_spe_set_irq(CPUState *cs, int irq) {}
+static inline void kvm_arm_spe_init(CPUState *cs) {}
+
 static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) {}
 
 static inline void kvm_arm_get_virtual_time(CPUState *cs) {}
-- 
2.17.1

[PATCH v2 07/12] hw/arm/virt: Move post cpu realize check into its own function

[Haibo Xu]

From: Andrew Jones <drjones@redhat.com>

We'll add more to this new function in coming patches so we also
state the gic must be created and call it below create_gic().

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3f6d26c531..2ffcb073af 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1672,6 +1672,26 @@ static void finalize_gic_version(VirtMachineState *vms)
     }
 }
 
+static void virt_cpu_post_init(VirtMachineState *vms)
+{
+    bool aarch64;
+
+    aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
+
+    if (!kvm_enabled()) {
+        if (aarch64 && vms->highmem) {
+            int requested_pa_size = 64 - clz64(vms->highest_gpa);
+            int pamax = arm_pamax(ARM_CPU(first_cpu));
+
+            if (pamax < requested_pa_size) {
+                error_report("VCPU supports less PA bits (%d) than requested "
+                            "by the memory map (%d)", pamax, requested_pa_size);
+                exit(1);
+            }
+        }
+     }
+}
+
 static void machvirt_init(MachineState *machine)
 {
     VirtMachineState *vms = VIRT_MACHINE(machine);
@@ -1890,22 +1910,6 @@ static void machvirt_init(MachineState *machine)
     fdt_add_timer_nodes(vms);
     fdt_add_cpu_nodes(vms);
 
-   if (!kvm_enabled()) {
-        ARMCPU *cpu = ARM_CPU(first_cpu);
-        bool aarch64 = object_property_get_bool(OBJECT(cpu), "aarch64", NULL);
-
-        if (aarch64 && vms->highmem) {
-            int requested_pa_size, pamax = arm_pamax(cpu);
-
-            requested_pa_size = 64 - clz64(vms->highest_gpa);
-            if (pamax < requested_pa_size) {
-                error_report("VCPU supports less PA bits (%d) than requested "
-                            "by the memory map (%d)", pamax, requested_pa_size);
-                exit(1);
-            }
-        }
-    }
-
     memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base,
                                 machine->ram);
     if (machine->device_memory) {
@@ -1917,6 +1921,8 @@ static void machvirt_init(MachineState *machine)
 
     create_gic(vms);
 
+    virt_cpu_post_init(vms);
+
     fdt_add_pmu_nodes(vms);
 
     create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
-- 
2.17.1

[PATCH v2 08/12] hw/arm/virt: Move kvm pmu setup to virt_cpu_post_init

[Haibo Xu]

From: Andrew Jones <drjones@redhat.com>

Move the KVM PMU setup part of fdt_add_pmu_nodes() to
virt_cpu_post_init(), which is a more appropriate location. Now
fdt_add_pmu_nodes() is also named more appropriately, because it
no longer does anything but fdt node creation.

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2ffcb073af..6bacfb668d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -521,21 +521,12 @@ static void fdt_add_gic_node(VirtMachineState *vms)
 
 static void fdt_add_pmu_nodes(const VirtMachineState *vms)
 {
-    CPUState *cpu;
-    ARMCPU *armcpu;
+    ARMCPU *armcpu = ARM_CPU(first_cpu);
     uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
 
-    CPU_FOREACH(cpu) {
-        armcpu = ARM_CPU(cpu);
-        if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
-            return;
-        }
-        if (kvm_enabled()) {
-            if (kvm_irqchip_in_kernel()) {
-                kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ));
-            }
-            kvm_arm_pmu_init(cpu);
-        }
+    if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
+        assert(!object_property_get_bool(OBJECT(armcpu), "pmu", NULL));
+        return;
     }
 
     if (vms->gic_version == VIRT_GIC_VERSION_2) {
@@ -544,7 +535,6 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
                              (1 << vms->smp_cpus) - 1);
     }
 
-    armcpu = ARM_CPU(qemu_get_cpu(0));
     qemu_fdt_add_subnode(vms->fdt, "/pmu");
     if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
         const char compat[] = "arm,armv8-pmuv3";
@@ -1674,11 +1664,23 @@ static void finalize_gic_version(VirtMachineState *vms)
 
 static void virt_cpu_post_init(VirtMachineState *vms)
 {
-    bool aarch64;
+    bool aarch64, pmu;
+    CPUState *cpu;
 
     aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
+    pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
 
-    if (!kvm_enabled()) {
+    if (kvm_enabled()) {
+        CPU_FOREACH(cpu) {
+            if (pmu) {
+                assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU));
+                if (kvm_irqchip_in_kernel()) {
+                    kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ));
+                }
+                kvm_arm_pmu_init(cpu);
+            }
+        }
+    } else {
         if (aarch64 && vms->highmem) {
             int requested_pa_size = 64 - clz64(vms->highest_gpa);
             int pamax = arm_pamax(ARM_CPU(first_cpu));
-- 
2.17.1

[PATCH v2 09/12] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Haibo Xu]

Add a virtual SPE device for virt machine while using
PPI 5 for SPE overflow interrupt number which has already
selected in kvmtool.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 hw/arm/virt-acpi-build.c    |  3 +++
 hw/arm/virt.c               | 43 ++++++++++++++++++++++++++++++++++++-
 include/hw/acpi/acpi-defs.h |  3 +++
 include/hw/arm/virt.h       |  1 +
 target/arm/cpu.c            |  2 ++
 target/arm/cpu.h            |  2 ++
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9efd7a3881..3fd80fda53 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -665,6 +665,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
             gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
         }
+        if (cpu_isar_feature(aa64_spe, armcpu)) {
+            gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
+        }
         if (vms->virt) {
             gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
         }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6bacfb668d..bdb1ce925c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -545,6 +545,32 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
     }
 }
 
+static void fdt_add_spe_nodes(const VirtMachineState *vms)
+{
+    ARMCPU *armcpu = ARM_CPU(first_cpu);
+    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+
+    if (!cpu_isar_feature(aa64_spe, armcpu)) {
+        assert(!object_property_get_bool(OBJECT(armcpu), "spe", NULL));
+        return;
+    }
+
+    if (vms->gic_version == VIRT_GIC_VERSION_2) {
+        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+                             GIC_FDT_IRQ_PPI_CPU_WIDTH,
+                             (1 << vms->smp_cpus) - 1);
+    }
+
+    qemu_fdt_add_subnode(vms->fdt, "/spe");
+    if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
+        const char compat[] = "arm,statistical-profiling-extension-v1";
+        qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
+                         compat, sizeof(compat));
+        qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
+                               GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ, irqflags);
+    }
+}
+
 static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
 {
     DeviceState *dev;
@@ -717,6 +743,10 @@ static void create_gic(VirtMachineState *vms)
                                     qdev_get_gpio_in(vms->gic, ppibase
                                                      + VIRTUAL_PMU_IRQ));
 
+        qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
+                                    qdev_get_gpio_in(vms->gic, ppibase
+                                                     + VIRTUAL_SPE_IRQ));
+
         sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
         sysbus_connect_irq(gicbusdev, i + smp_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
@@ -1664,11 +1694,12 @@ static void finalize_gic_version(VirtMachineState *vms)
 
 static void virt_cpu_post_init(VirtMachineState *vms)
 {
-    bool aarch64, pmu;
+    bool aarch64, pmu, spe;
     CPUState *cpu;
 
     aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
     pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
+    spe = object_property_get_bool(OBJECT(first_cpu), "spe", NULL);
 
     if (kvm_enabled()) {
         CPU_FOREACH(cpu) {
@@ -1679,6 +1710,14 @@ static void virt_cpu_post_init(VirtMachineState *vms)
                 }
                 kvm_arm_pmu_init(cpu);
             }
+
+            if (spe) {
+                assert(ARM_CPU(cpu)->has_spe == ON_OFF_AUTO_ON);
+                if (kvm_irqchip_in_kernel()) {
+                    kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
+                }
+                kvm_arm_spe_init(cpu);
+            }
         }
     } else {
         if (aarch64 && vms->highmem) {
@@ -1927,6 +1966,8 @@ static void machvirt_init(MachineState *machine)
 
     fdt_add_pmu_nodes(vms);
 
+    fdt_add_spe_nodes(vms);
+
     create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
 
     if (vms->secure) {
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 38a42f409a..21e58f27c5 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -302,6 +302,9 @@ struct AcpiMadtGenericCpuInterface {
     uint32_t vgic_interrupt;
     uint64_t gicr_base_address;
     uint64_t arm_mpidr;
+    uint8_t  efficiency_class;
+    uint8_t  reserved2[1];
+    uint16_t spe_interrupt; /* ACPI 6.3 */
 } QEMU_PACKED;
 
 typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 72c269aaa5..6013b6d535 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -49,6 +49,7 @@
 #define ARCH_TIMER_NS_EL1_IRQ 14
 #define ARCH_TIMER_NS_EL2_IRQ 10
 
+#define VIRTUAL_SPE_IRQ 5
 #define VIRTUAL_PMU_IRQ 7
 
 #define PPI(irq) ((irq) + 16)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f211958eaa..786cc6134c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1041,6 +1041,8 @@ static void arm_cpu_initfn(Object *obj)
                              "gicv3-maintenance-interrupt", 1);
     qdev_init_gpio_out_named(DEVICE(cpu), &cpu->pmu_interrupt,
                              "pmu-interrupt", 1);
+    qdev_init_gpio_out_named(DEVICE(cpu), &cpu->spe_interrupt,
+                             "spe-interrupt", 1);
 #endif
 
     /* DTB consumers generally don't in fact care what the 'compatible'
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index baf2bbcee8..395a1e5df8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -800,6 +800,8 @@ struct ARMCPU {
     qemu_irq gicv3_maintenance_interrupt;
     /* GPIO output for the PMU interrupt */
     qemu_irq pmu_interrupt;
+    /* GPIO output for the SPE interrupt */
+    qemu_irq spe_interrupt;
 
     /* MemoryRegion to use for secure physical accesses */
     MemoryRegion *secure_memory;
-- 
2.17.1

[PATCH v2 10/12] target/arm/cpu: spe: Enable spe to work with host cpu

[Haibo Xu]

Turn on the spe cpu property by default if host cpu
support it, i.e. we can now do '-cpu max|host' to add
the vSPE, and '-cpu max|host,spe=off' to remove it.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/cpu.c   |  3 +++
 target/arm/cpu.h   |  2 ++
 target/arm/cpu64.c |  7 ++++++-
 target/arm/kvm64.c | 12 ++++++++++++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 786cc6134c..58f12d6eb5 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj)
     kvm_arm_set_cpu_features_from_host(cpu);
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         aarch64_add_sve_properties(obj);
+
+        cpu->has_spe = ON_OFF_AUTO_AUTO;
+        aarch64_add_spe_properties(obj);
     }
     arm_cpu_post_init(obj);
 }
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 395a1e5df8..5a3ea876c8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
 void aarch64_sve_change_el(CPUARMState *env, int old_el,
                            int new_el, bool el0_a64);
 void aarch64_add_sve_properties(Object *obj);
+void aarch64_add_spe_properties(Object *obj);
 
 /*
  * SVE registers are encoded in KVM's memory in an endianness-invariant format.
@@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o,
                                          int n, bool a)
 { }
 static inline void aarch64_add_sve_properties(Object *obj) { }
+static inline void aarch64_add_spe_properties(Object *obj) { }
 #endif
 
 #if !defined(CONFIG_TCG)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4997c4a3c0..d38c55e2ca 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp)
     ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
 }
 
+void aarch64_add_spe_properties(Object *obj)
+{
+    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
+}
+
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
  * otherwise, a CPU with as many features enabled as our emulation supports.
  * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
@@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj)
                         cpu_max_set_sve_max_vq, NULL, NULL);
 
     cpu->has_spe = ON_OFF_AUTO_AUTO;
-    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
+    aarch64_add_spe_properties(obj);
 }
 
 static const ARMCPUInfo aarch64_cpus[] = {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 5a2032fc9e..3f0a09c05b 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      */
     int fdarray[3];
     bool sve_supported;
+    bool spe_supported;
     uint64_t features = 0;
     uint64_t t;
     int err;
@@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     }
 
     sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
+    spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION,
+                                      KVM_CAP_ARM_SPE_V1) > 0;
 
     kvm_arm_destroy_scratch_host_vcpu(fdarray);
 
@@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
         t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
         ahcf->isar.id_aa64pfr0 = t;
     }
+    if (!spe_supported) {
+        t = ahcf->isar.id_aa64dfr0;
+        t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0);
+        ahcf->isar.id_aa64dfr0 = t;
+    }
 
     /*
      * We can assume any KVM supporting CPU is at least a v8
@@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
         assert(kvm_arm_sve_supported());
         cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
     }
+    if (cpu_isar_feature(aa64_spe, cpu)) {
+        assert(kvm_arm_spe_supported());
+        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1;
+    }
 
     /* Do KVM_ARM_VCPU_INIT ioctl */
     ret = kvm_arm_vcpu_init(cs);
-- 
2.17.1

[PATCH v2 11/12] target/arm/kvm: spe: Enable userspace irqchip support.

[Haibo Xu]

Since the current kernel patches haven't enabled the
userspace irqchip support, this patch is not verified yet!

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 linux-headers/linux/kvm.h | 1 +
 target/arm/kvm.c          | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 8840cbb01c..35ef0ae842 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1672,6 +1672,7 @@ struct kvm_assigned_msix_entry {
 #define KVM_ARM_DEV_EL1_VTIMER		(1 << 0)
 #define KVM_ARM_DEV_EL1_PTIMER		(1 << 1)
 #define KVM_ARM_DEV_PMU			(1 << 2)
+#define KVM_ARM_DEV_SPE			(1 << 3)
 
 struct kvm_hyperv_eventfd {
 	__u32 conn_id;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 58f991e890..7950ff1d83 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -820,6 +820,11 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
             switched_level &= ~KVM_ARM_DEV_PMU;
         }
 
+        if (switched_level & KVM_ARM_DEV_SPE) {
+            qemu_set_irq(cpu->spe_interrupt,
+                         !!(run->s.regs.device_irq_level & KVM_ARM_DEV_SPE));
+            switched_level &= ~KVM_ARM_DEV_SPE;
+        }
         if (switched_level) {
             qemu_log_mask(LOG_UNIMP, "%s: unhandled in-kernel device IRQ %x\n",
                           __func__, switched_level);
-- 
2.17.1

[PATCH v2 12/12] target/arm: spe: Add corresponding doc and test.

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 docs/system/arm/cpu-features.rst | 20 ++++++++++++++++++++
 target/arm/monitor.c             |  2 +-
 tests/qtest/arm-cpu-features.c   |  9 +++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index 2d5c06cd01..5b81b9a560 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -344,3 +344,23 @@ verbose command lines.  However, the recommended way to select vector
 lengths is to explicitly enable each desired length.  Therefore only
 example's (1), (4), and (6) exhibit recommended uses of the properties.
 
+SPE CPU Property
+==================
+
+The SPE CPU property `spe` is used to enable or disable the SPE feature,
+just as the `pmu` CPU property completely enables or disables the PMU.
+
+Currently, this property is only available with KVM mode, and is enabled
+by default if KVM support it. When KVM is enabled, if the host does not
+support SPE, then an error is generated when attempting to enable it.
+
+Following are 2 examples to use this property:
+
+  1) Disable SPE::
+
+     $ qemu-system-aarch64 -M virt,accel=kvm -cpu max,spe=off
+
+  2) Implicitly enable it with the `host` CPU type if host cpu
+     support it::
+
+     $ qemu-system-aarch64 -M virt,accel=kvm -cpu host
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index ba6e01abd0..1b8f08988a 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -99,7 +99,7 @@ QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16);
  * then the order that considers those dependencies must be used.
  */
 static const char *cpu_model_advertised_features[] = {
-    "aarch64", "pmu", "sve",
+    "aarch64", "pmu", "spe", "sve",
     "sve128", "sve256", "sve384", "sve512",
     "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
     "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 77b5e30a9c..4d393fb2e2 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -494,6 +494,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         bool kvm_supports_sve;
+        bool kvm_supports_spe;
         char max_name[8], name[8];
         uint32_t max_vq, vq;
         uint64_t vls;
@@ -512,8 +513,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
             "with KVM on this host", NULL);
 
         assert_has_feature(qts, "host", "sve");
+        assert_has_feature(qts, "host", "spe");
         resp = do_query_no_props(qts, "host");
         kvm_supports_sve = resp_get_feature(resp, "sve");
+        kvm_supports_spe = resp_get_feature(resp, "spe");
         vls = resp_get_sve_vls(resp);
         qobject_unref(resp);
 
@@ -573,10 +576,16 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         } else {
             g_assert(vls == 0);
         }
+
+        if (kvm_supports_spe) {
+                assert_set_feature(qts, "host", "spe", false);
+                assert_set_feature(qts, "host", "spe", true);
+        }
     } else {
         assert_has_not_feature(qts, "host", "aarch64");
         assert_has_not_feature(qts, "host", "pmu");
         assert_has_not_feature(qts, "host", "sve");
+        assert_has_not_feature(qts, "host", "spe");
     }
 
     qtest_quit(qts);
-- 
2.17.1

Re: [PATCH v2 02/12] target/arm/kvm: spe: Add helper to detect SPE when using KVM

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:20AM +0000, Haibo Xu wrote:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/kvm.c     |  5 +++++
>  target/arm/kvm_arm.h | 13 +++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 8bb7318378..58f991e890 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -214,6 +214,11 @@ bool kvm_arm_pmu_supported(void)
>      return kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
>  }
>

Reviewed-by: Andrew Jones <drjones@redhat.com>  

Re: [PATCH v2 03/12] target/arm/cpu: spe: Add an option to turn on/off vSPE support

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:21AM +0000, Haibo Xu wrote:
> Adds a spe=[on/off] option to enable/disable vSPE support in
> guest vCPU.
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/cpu.c   |  6 ++++++
>  target/arm/cpu.h   | 13 ++++++++++++
>  target/arm/cpu64.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 71 insertions(+)
>

Reviewed-by: Andrew Jones <drjones@redhat.com> 

Re: [PATCH v2 04/12] target/arm: spe: Only enable SPE from 5.2 compat machines.

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:22AM +0000, Haibo Xu wrote:
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  hw/arm/virt.c         | 7 +++++++
>  include/hw/arm/virt.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index acf9bfbece..3f6d26c531 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1830,6 +1830,10 @@ static void machvirt_init(MachineState *machine)
>              object_property_set_bool(cpuobj, "pmu", false, NULL);
>          }
>  
> +        if (vmc->no_spe && object_property_find(cpuobj, "spe", NULL)) {
> +            object_property_set_bool(cpuobj, "spe", false, NULL);
> +        }
> +
>          if (object_property_find(cpuobj, "reset-cbar", NULL)) {
>              object_property_set_int(cpuobj, "reset-cbar",
>                                      vms->memmap[VIRT_CPUPERIPHS].base,
> @@ -2553,8 +2557,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 2)
>  
>  static void virt_machine_5_1_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>      virt_machine_5_2_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_5_1, hw_compat_5_1_len);
> +    vmc->no_spe = true;
>  }
>  DEFINE_VIRT_MACHINE(5, 1)
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index dff67e1bef..72c269aaa5 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -126,6 +126,7 @@ typedef struct {
>      bool no_ged;   /* Machines < 4.2 has no support for ACPI GED device */
>      bool kvm_no_adjvtime;
>      bool acpi_expose_flash;
> +    bool no_spe;
>  } VirtMachineClass;
>  
>  typedef struct {
> -- 
> 2.17.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com> 

Re: [PATCH v2 05/12] target/arm/kvm: spe: Unify device attr operation helper

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:23AM +0000, Haibo Xu wrote:
> From: Andrew Jones <drjones@redhat.com>
> 
> Rename kvm_arm_pmu_set_attr() to kvm_arm_set_device_attr(),
> So both the vPMU and vSPE device can share the same API.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Looks like a faithful port of what I posted as a hunk of another patch, so
I'll accept the authorship. Please also add you s-b though.

Thanks,
drew

> ---
>  target/arm/kvm64.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index ef1e960285..8ffd31ffdf 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -397,19 +397,20 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
>      return NULL;
>  }
>  
> -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> +static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr,
> +                                    const char *name)
>  {
>      int err;
>  
>      err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
>      if (err != 0) {
> -        error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err));
> +        error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err));
>          return false;
>      }
>  
>      err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
>      if (err != 0) {
> -        error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err));
> +        error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err));
>          return false;
>      }
>  
> @@ -426,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
>      if (!ARM_CPU(cs)->has_pmu) {
>          return;
>      }
> -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> +    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
>          error_report("failed to init PMU");
>          abort();
>      }
> @@ -443,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
>      if (!ARM_CPU(cs)->has_pmu) {
>          return;
>      }
> -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> +    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
>          error_report("failed to set irq for PMU");
>          abort();
>      }
> -- 
> 2.17.1
> 
> 

Re: [PATCH v2 06/12] target/arm/kvm: spe: Add device init and set_irq operations

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:24AM +0000, Haibo Xu wrote:
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/kvm64.c   | 33 +++++++++++++++++++++++++++++++++
>  target/arm/kvm_arm.h |  5 +++++
>  2 files changed, 38 insertions(+)
>

Reviewed-by: Andrew Jones <drjones@redhat.com> 

Re: [PATCH v2 07/12] hw/arm/virt: Move post cpu realize check into its own function

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:25AM +0000, Haibo Xu wrote:
> From: Andrew Jones <drjones@redhat.com>
> 
> We'll add more to this new function in coming patches so we also
> state the gic must be created and call it below create_gic().
> 
> No functional change intended.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

This is the wrong version of this patch. You should use v2, for which
Peter has given his r-b

Thanks,
drew

Re: [PATCH v2 08/12] hw/arm/virt: Move kvm pmu setup to virt_cpu_post_init

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:26AM +0000, Haibo Xu wrote:
> From: Andrew Jones <drjones@redhat.com>
> 
> Move the KVM PMU setup part of fdt_add_pmu_nodes() to
> virt_cpu_post_init(), which is a more appropriate location. Now
> fdt_add_pmu_nodes() is also named more appropriately, because it
> no longer does anything but fdt node creation.
> 
> No functional change intended.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>

This looks like the correct version, but you could also pick up Peter's
r-b.

Thanks,
drew 

Re: [PATCH v2 09/12] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Andrew Jones]

This patch does much more than the summary "hw/arm/virt: spe: Add SPE fdt
binding for virt machine" says it does. Please revise the summary.

On Tue, Sep 08, 2020 at 08:13:27AM +0000, Haibo Xu wrote:
> Add a virtual SPE device for virt machine while using
> PPI 5 for SPE overflow interrupt number which has already
> selected in kvmtool.
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c    |  3 +++
>  hw/arm/virt.c               | 43 ++++++++++++++++++++++++++++++++++++-
>  include/hw/acpi/acpi-defs.h |  3 +++
>  include/hw/arm/virt.h       |  1 +
>  target/arm/cpu.c            |  2 ++
>  target/arm/cpu.h            |  2 ++
>  6 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9efd7a3881..3fd80fda53 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -665,6 +665,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
>              gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
>          }
> +        if (cpu_isar_feature(aa64_spe, armcpu)) {
> +            gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> +        }
>          if (vms->virt) {
>              gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
>          }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 6bacfb668d..bdb1ce925c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -545,6 +545,32 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
>      }
>  }
>  
> +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> +{
> +    ARMCPU *armcpu = ARM_CPU(first_cpu);
> +    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> +
> +    if (!cpu_isar_feature(aa64_spe, armcpu)) {
> +        assert(!object_property_get_bool(OBJECT(armcpu), "spe", NULL));
> +        return;
> +    }
> +
> +    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> +                             GIC_FDT_IRQ_PPI_CPU_WIDTH,
> +                             (1 << vms->smp_cpus) - 1);
> +    }
> +
> +    qemu_fdt_add_subnode(vms->fdt, "/spe");
> +    if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
> +        const char compat[] = "arm,statistical-profiling-extension-v1";
> +        qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> +                         compat, sizeof(compat));
> +        qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> +                               GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ, irqflags);
> +    }
> +}
> +
>  static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>  {
>      DeviceState *dev;
> @@ -717,6 +743,10 @@ static void create_gic(VirtMachineState *vms)
>                                      qdev_get_gpio_in(vms->gic, ppibase
>                                                       + VIRTUAL_PMU_IRQ));
>  
> +        qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> +                                    qdev_get_gpio_in(vms->gic, ppibase
> +                                                     + VIRTUAL_SPE_IRQ));
> +
>          sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
>          sysbus_connect_irq(gicbusdev, i + smp_cpus,
>                             qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> @@ -1664,11 +1694,12 @@ static void finalize_gic_version(VirtMachineState *vms)
>  
>  static void virt_cpu_post_init(VirtMachineState *vms)
>  {
> -    bool aarch64, pmu;
> +    bool aarch64, pmu, spe;
>      CPUState *cpu;
>  
>      aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
>      pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
> +    spe = object_property_get_bool(OBJECT(first_cpu), "spe", NULL);
>  
>      if (kvm_enabled()) {
>          CPU_FOREACH(cpu) {
> @@ -1679,6 +1710,14 @@ static void virt_cpu_post_init(VirtMachineState *vms)
>                  }
>                  kvm_arm_pmu_init(cpu);
>              }
> +
> +            if (spe) {
> +                assert(ARM_CPU(cpu)->has_spe == ON_OFF_AUTO_ON);
> +                if (kvm_irqchip_in_kernel()) {
> +                    kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> +                }
> +                kvm_arm_spe_init(cpu);

A later patch introduces userspace irqchip support. Should we avoid
allowing it until then to avoid breaking bisection?

> +            }
>          }
>      } else {
>          if (aarch64 && vms->highmem) {
> @@ -1927,6 +1966,8 @@ static void machvirt_init(MachineState *machine)
>  
>      fdt_add_pmu_nodes(vms);
>  
> +    fdt_add_spe_nodes(vms);
> +
>      create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
>  
>      if (vms->secure) {
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 38a42f409a..21e58f27c5 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -302,6 +302,9 @@ struct AcpiMadtGenericCpuInterface {
>      uint32_t vgic_interrupt;
>      uint64_t gicr_base_address;
>      uint64_t arm_mpidr;
> +    uint8_t  efficiency_class;
> +    uint8_t  reserved2[1];
> +    uint16_t spe_interrupt; /* ACPI 6.3 */
>  } QEMU_PACKED;
>  
>  typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 72c269aaa5..6013b6d535 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -49,6 +49,7 @@
>  #define ARCH_TIMER_NS_EL1_IRQ 14
>  #define ARCH_TIMER_NS_EL2_IRQ 10
>  
> +#define VIRTUAL_SPE_IRQ 5
>  #define VIRTUAL_PMU_IRQ 7
>  
>  #define PPI(irq) ((irq) + 16)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f211958eaa..786cc6134c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1041,6 +1041,8 @@ static void arm_cpu_initfn(Object *obj)
>                               "gicv3-maintenance-interrupt", 1);
>      qdev_init_gpio_out_named(DEVICE(cpu), &cpu->pmu_interrupt,
>                               "pmu-interrupt", 1);
> +    qdev_init_gpio_out_named(DEVICE(cpu), &cpu->spe_interrupt,
> +                             "spe-interrupt", 1);
>  #endif
>  
>      /* DTB consumers generally don't in fact care what the 'compatible'
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index baf2bbcee8..395a1e5df8 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -800,6 +800,8 @@ struct ARMCPU {
>      qemu_irq gicv3_maintenance_interrupt;
>      /* GPIO output for the PMU interrupt */
>      qemu_irq pmu_interrupt;
> +    /* GPIO output for the SPE interrupt */
> +    qemu_irq spe_interrupt;
>  
>      /* MemoryRegion to use for secure physical accesses */
>      MemoryRegion *secure_memory;
> -- 
> 2.17.1
> 
> 

Otherwise

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH v2 10/12] target/arm/cpu: spe: Enable spe to work with host cpu

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:28AM +0000, Haibo Xu wrote:
> Turn on the spe cpu property by default if host cpu
> support it, i.e. we can now do '-cpu max|host' to add
> the vSPE, and '-cpu max|host,spe=off' to remove it.
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/cpu.c   |  3 +++
>  target/arm/cpu.h   |  2 ++
>  target/arm/cpu64.c |  7 ++++++-
>  target/arm/kvm64.c | 12 ++++++++++++
>  4 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 786cc6134c..58f12d6eb5 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj)
>      kvm_arm_set_cpu_features_from_host(cpu);
>      if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
>          aarch64_add_sve_properties(obj);
> +
> +        cpu->has_spe = ON_OFF_AUTO_AUTO;
> +        aarch64_add_spe_properties(obj);

Why not put the assignment of has_spe into aarch64_add_spe_properties()?

>      }
>      arm_cpu_post_init(obj);
>  }
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 395a1e5df8..5a3ea876c8 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
>  void aarch64_sve_change_el(CPUARMState *env, int old_el,
>                             int new_el, bool el0_a64);
>  void aarch64_add_sve_properties(Object *obj);
> +void aarch64_add_spe_properties(Object *obj);
>  
>  /*
>   * SVE registers are encoded in KVM's memory in an endianness-invariant format.
> @@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o,
>                                           int n, bool a)
>  { }
>  static inline void aarch64_add_sve_properties(Object *obj) { }
> +static inline void aarch64_add_spe_properties(Object *obj) { }
>  #endif
>  
>  #if !defined(CONFIG_TCG)
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4997c4a3c0..d38c55e2ca 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp)
>      ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
>  }
>  
> +void aarch64_add_spe_properties(Object *obj)
> +{
> +    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
> +}
> +
>  /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
>   * otherwise, a CPU with as many features enabled as our emulation supports.
>   * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
> @@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj)
>                          cpu_max_set_sve_max_vq, NULL, NULL);
>  
>      cpu->has_spe = ON_OFF_AUTO_AUTO;
> -    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
> +    aarch64_add_spe_properties(obj);

If TCG doesn't support this cpu feature then this should be in the
kvm_enabled() part of this function.


>  }
>  
>  static const ARMCPUInfo aarch64_cpus[] = {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 5a2032fc9e..3f0a09c05b 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>       */
>      int fdarray[3];
>      bool sve_supported;
> +    bool spe_supported;
>      uint64_t features = 0;
>      uint64_t t;
>      int err;
> @@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>      }
>  
>      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
> +    spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION,
> +                                      KVM_CAP_ARM_SPE_V1) > 0;
>  
>      kvm_arm_destroy_scratch_host_vcpu(fdarray);
>  
> @@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
>          ahcf->isar.id_aa64pfr0 = t;
>      }
> +    if (!spe_supported) {
> +        t = ahcf->isar.id_aa64dfr0;
> +        t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0);
> +        ahcf->isar.id_aa64dfr0 = t;
> +    }
> 

If this works then there's a bug with the KVM implementation. get-one-reg
shouldn't expose SPE unless userspace has enabled it, which we're not
doing on the scratch vcpu. That's why we set, not clear, the ID bits for
SVE.

 
>      /*
>       * We can assume any KVM supporting CPU is at least a v8
> @@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          assert(kvm_arm_sve_supported());
>          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
>      }
> +    if (cpu_isar_feature(aa64_spe, cpu)) {
> +        assert(kvm_arm_spe_supported());
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1;
> +    }
>  
>      /* Do KVM_ARM_VCPU_INIT ioctl */
>      ret = kvm_arm_vcpu_init(cs);
> -- 
> 2.17.1
> 

Thanks,
drew

Re: [PATCH v2 11/12] target/arm/kvm: spe: Enable userspace irqchip support.

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:29AM +0000, Haibo Xu wrote:
> Since the current kernel patches haven't enabled the
> userspace irqchip support, this patch is not verified yet!
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  linux-headers/linux/kvm.h | 1 +
>  target/arm/kvm.c          | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 8840cbb01c..35ef0ae842 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1672,6 +1672,7 @@ struct kvm_assigned_msix_entry {
>  #define KVM_ARM_DEV_EL1_VTIMER		(1 << 0)
>  #define KVM_ARM_DEV_EL1_PTIMER		(1 << 1)
>  #define KVM_ARM_DEV_PMU			(1 << 2)
> +#define KVM_ARM_DEV_SPE			(1 << 3)

kernel header changes should be separate patches

>  
>  struct kvm_hyperv_eventfd {
>  	__u32 conn_id;
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 58f991e890..7950ff1d83 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -820,6 +820,11 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
>              switched_level &= ~KVM_ARM_DEV_PMU;
>          }
>  
> +        if (switched_level & KVM_ARM_DEV_SPE) {
> +            qemu_set_irq(cpu->spe_interrupt,
> +                         !!(run->s.regs.device_irq_level & KVM_ARM_DEV_SPE));
> +            switched_level &= ~KVM_ARM_DEV_SPE;
> +        }
>          if (switched_level) {
>              qemu_log_mask(LOG_UNIMP, "%s: unhandled in-kernel device IRQ %x\n",
>                            __func__, switched_level);
> -- 
> 2.17.1
> 

Otherwise 

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH v2 12/12] target/arm: spe: Add corresponding doc and test.

[Andrew Jones]

On Tue, Sep 08, 2020 at 08:13:30AM +0000, Haibo Xu wrote:
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  docs/system/arm/cpu-features.rst | 20 ++++++++++++++++++++
>  target/arm/monitor.c             |  2 +-
>  tests/qtest/arm-cpu-features.c   |  9 +++++++++
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
> index 2d5c06cd01..5b81b9a560 100644
> --- a/docs/system/arm/cpu-features.rst
> +++ b/docs/system/arm/cpu-features.rst
> @@ -344,3 +344,23 @@ verbose command lines.  However, the recommended way to select vector
>  lengths is to explicitly enable each desired length.  Therefore only
>  example's (1), (4), and (6) exhibit recommended uses of the properties.
>  
> +SPE CPU Property
> +==================

Too many '='

> +
> +The SPE CPU property `spe` is used to enable or disable the SPE feature,
> +just as the `pmu` CPU property completely enables or disables the PMU.
> +
> +Currently, this property is only available with KVM mode, and is enabled
> +by default if KVM support it. When KVM is enabled, if the host does not
> +support SPE, then an error is generated when attempting to enable it.
> +
> +Following are 2 examples to use this property:
> +
> +  1) Disable SPE::
> +
> +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu max,spe=off
> +
> +  2) Implicitly enable it with the `host` CPU type if host cpu
> +     support it::

if the host CPU supports it


Actually, I'm not sure we need to document this feature. We didn't bother
documenting pauth, since there wasn't anything special about it and
there's nothing special about this feature either.

> +
> +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu host
> diff --git a/target/arm/monitor.c b/target/arm/monitor.c
> index ba6e01abd0..1b8f08988a 100644
> --- a/target/arm/monitor.c
> +++ b/target/arm/monitor.c
> @@ -99,7 +99,7 @@ QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16);
>   * then the order that considers those dependencies must be used.
>   */
>  static const char *cpu_model_advertised_features[] = {
> -    "aarch64", "pmu", "sve",
> +    "aarch64", "pmu", "spe", "sve",
>      "sve128", "sve256", "sve384", "sve512",
>      "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
>      "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index 77b5e30a9c..4d393fb2e2 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -494,6 +494,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>  
>      if (g_str_equal(qtest_get_arch(), "aarch64")) {
>          bool kvm_supports_sve;
> +        bool kvm_supports_spe;
>          char max_name[8], name[8];
>          uint32_t max_vq, vq;
>          uint64_t vls;
> @@ -512,8 +513,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>              "with KVM on this host", NULL);
>  
>          assert_has_feature(qts, "host", "sve");
> +        assert_has_feature(qts, "host", "spe");
>          resp = do_query_no_props(qts, "host");
>          kvm_supports_sve = resp_get_feature(resp, "sve");
> +        kvm_supports_spe = resp_get_feature(resp, "spe");
>          vls = resp_get_sve_vls(resp);
>          qobject_unref(resp);
>  
> @@ -573,10 +576,16 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>          } else {
>              g_assert(vls == 0);
>          }
> +
> +        if (kvm_supports_spe) {
> +                assert_set_feature(qts, "host", "spe", false);
> +                assert_set_feature(qts, "host", "spe", true);
> +        }
>      } else {
>          assert_has_not_feature(qts, "host", "aarch64");
>          assert_has_not_feature(qts, "host", "pmu");
>          assert_has_not_feature(qts, "host", "sve");
> +        assert_has_not_feature(qts, "host", "spe");
>      }
>  
>      qtest_quit(qts);
> -- 
> 2.17.1
>

Otherwise

Reviewed-by: Andrew Jones <drjones@redhat.com> 

Re: [PATCH v2 05/12] target/arm/kvm: spe: Unify device attr operation helper

[Haibo Xu]

On Tue, 8 Sep 2020 at 18:56, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:23AM +0000, Haibo Xu wrote:
> > From: Andrew Jones <drjones@redhat.com>
> >
> > Rename kvm_arm_pmu_set_attr() to kvm_arm_set_device_attr(),
> > So both the vPMU and vSPE device can share the same API.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
>
> Looks like a faithful port of what I posted as a hunk of another patch, so
> I'll accept the authorship. Please also add you s-b though.
>
> Thanks,
> drew
>

Ok, will fix it in v3.

Thanks,
Haibo

> > ---
> >  target/arm/kvm64.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index ef1e960285..8ffd31ffdf 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -397,19 +397,20 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
> >      return NULL;
> >  }
> >
> > -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > +static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr,
> > +                                    const char *name)
> >  {
> >      int err;
> >
> >      err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
> >      if (err != 0) {
> > -        error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err));
> > +        error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err));
> >          return false;
> >      }
> >
> >      err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
> >      if (err != 0) {
> > -        error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err));
> > +        error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err));
> >          return false;
> >      }
> >
> > @@ -426,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
> >      if (!ARM_CPU(cs)->has_pmu) {
> >          return;
> >      }
> > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > +    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
> >          error_report("failed to init PMU");
> >          abort();
> >      }
> > @@ -443,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
> >      if (!ARM_CPU(cs)->has_pmu) {
> >          return;
> >      }
> > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > +    if (!kvm_arm_set_device_attr(cs, &attr, "PMU")) {
> >          error_report("failed to set irq for PMU");
> >          abort();
> >      }
> > --
> > 2.17.1
> >
> >
>

Re: [PATCH v2 11/12] target/arm/kvm: spe: Enable userspace irqchip support.

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:35, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:29AM +0000, Haibo Xu wrote:
> > Since the current kernel patches haven't enabled the
> > userspace irqchip support, this patch is not verified yet!
> >
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  linux-headers/linux/kvm.h | 1 +
> >  target/arm/kvm.c          | 5 +++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> > index 8840cbb01c..35ef0ae842 100644
> > --- a/linux-headers/linux/kvm.h
> > +++ b/linux-headers/linux/kvm.h
> > @@ -1672,6 +1672,7 @@ struct kvm_assigned_msix_entry {
> >  #define KVM_ARM_DEV_EL1_VTIMER               (1 << 0)
> >  #define KVM_ARM_DEV_EL1_PTIMER               (1 << 1)
> >  #define KVM_ARM_DEV_PMU                      (1 << 2)
> > +#define KVM_ARM_DEV_SPE                      (1 << 3)
>
> kernel header changes should be separate patches
>

Will move this line to patch 01 in v3.

Thanks,
Haibo

> >
> >  struct kvm_hyperv_eventfd {
> >       __u32 conn_id;
> > diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> > index 58f991e890..7950ff1d83 100644
> > --- a/target/arm/kvm.c
> > +++ b/target/arm/kvm.c
> > @@ -820,6 +820,11 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
> >              switched_level &= ~KVM_ARM_DEV_PMU;
> >          }
> >
> > +        if (switched_level & KVM_ARM_DEV_SPE) {
> > +            qemu_set_irq(cpu->spe_interrupt,
> > +                         !!(run->s.regs.device_irq_level & KVM_ARM_DEV_SPE));
> > +            switched_level &= ~KVM_ARM_DEV_SPE;
> > +        }
> >          if (switched_level) {
> >              qemu_log_mask(LOG_UNIMP, "%s: unhandled in-kernel device IRQ %x\n",
> >                            __func__, switched_level);
> > --
> > 2.17.1
> >
>
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>

Re: [PATCH v2 12/12] target/arm: spe: Add corresponding doc and test.

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:41, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:30AM +0000, Haibo Xu wrote:
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  docs/system/arm/cpu-features.rst | 20 ++++++++++++++++++++
> >  target/arm/monitor.c             |  2 +-
> >  tests/qtest/arm-cpu-features.c   |  9 +++++++++
> >  3 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
> > index 2d5c06cd01..5b81b9a560 100644
> > --- a/docs/system/arm/cpu-features.rst
> > +++ b/docs/system/arm/cpu-features.rst
> > @@ -344,3 +344,23 @@ verbose command lines.  However, the recommended way to select vector
> >  lengths is to explicitly enable each desired length.  Therefore only
> >  example's (1), (4), and (6) exhibit recommended uses of the properties.
> >
> > +SPE CPU Property
> > +==================
>
> Too many '='
>
> > +
> > +The SPE CPU property `spe` is used to enable or disable the SPE feature,
> > +just as the `pmu` CPU property completely enables or disables the PMU.
> > +
> > +Currently, this property is only available with KVM mode, and is enabled
> > +by default if KVM support it. When KVM is enabled, if the host does not
> > +support SPE, then an error is generated when attempting to enable it.
> > +
> > +Following are 2 examples to use this property:
> > +
> > +  1) Disable SPE::
> > +
> > +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu max,spe=off
> > +
> > +  2) Implicitly enable it with the `host` CPU type if host cpu
> > +     support it::
>
> if the host CPU supports it
>
>
> Actually, I'm not sure we need to document this feature. We didn't bother
> documenting pauth, since there wasn't anything special about it and
> there's nothing special about this feature either.
>

Yes, there is no special treatment for this feature, and it just
follows the syntax
of other vCPU features. Will remove this doc in v3.
Anyway, thanks so much for the review!

Regards,
Haibo

> > +
> > +     $ qemu-system-aarch64 -M virt,accel=kvm -cpu host
> > diff --git a/target/arm/monitor.c b/target/arm/monitor.c
> > index ba6e01abd0..1b8f08988a 100644
> > --- a/target/arm/monitor.c
> > +++ b/target/arm/monitor.c
> > @@ -99,7 +99,7 @@ QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16);
> >   * then the order that considers those dependencies must be used.
> >   */
> >  static const char *cpu_model_advertised_features[] = {
> > -    "aarch64", "pmu", "sve",
> > +    "aarch64", "pmu", "spe", "sve",
> >      "sve128", "sve256", "sve384", "sve512",
> >      "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
> >      "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
> > diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> > index 77b5e30a9c..4d393fb2e2 100644
> > --- a/tests/qtest/arm-cpu-features.c
> > +++ b/tests/qtest/arm-cpu-features.c
> > @@ -494,6 +494,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
> >
> >      if (g_str_equal(qtest_get_arch(), "aarch64")) {
> >          bool kvm_supports_sve;
> > +        bool kvm_supports_spe;
> >          char max_name[8], name[8];
> >          uint32_t max_vq, vq;
> >          uint64_t vls;
> > @@ -512,8 +513,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
> >              "with KVM on this host", NULL);
> >
> >          assert_has_feature(qts, "host", "sve");
> > +        assert_has_feature(qts, "host", "spe");
> >          resp = do_query_no_props(qts, "host");
> >          kvm_supports_sve = resp_get_feature(resp, "sve");
> > +        kvm_supports_spe = resp_get_feature(resp, "spe");
> >          vls = resp_get_sve_vls(resp);
> >          qobject_unref(resp);
> >
> > @@ -573,10 +576,16 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
> >          } else {
> >              g_assert(vls == 0);
> >          }
> > +
> > +        if (kvm_supports_spe) {
> > +                assert_set_feature(qts, "host", "spe", false);
> > +                assert_set_feature(qts, "host", "spe", true);
> > +        }
> >      } else {
> >          assert_has_not_feature(qts, "host", "aarch64");
> >          assert_has_not_feature(qts, "host", "pmu");
> >          assert_has_not_feature(qts, "host", "sve");
> > +        assert_has_not_feature(qts, "host", "spe");
> >      }
> >
> >      qtest_quit(qts);
> > --
> > 2.17.1
> >
>
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>

Re: [PATCH v2 07/12] hw/arm/virt: Move post cpu realize check into its own function

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:03, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:25AM +0000, Haibo Xu wrote:
> > From: Andrew Jones <drjones@redhat.com>
> >
> > We'll add more to this new function in coming patches so we also
> > state the gic must be created and call it below create_gic().
> >
> > No functional change intended.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
>
> This is the wrong version of this patch. You should use v2, for which
> Peter has given his r-b
>
> Thanks,
> drew
>

Will fix it in v3.

Thanks,
Haibo

Re: [PATCH v2 09/12] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:16, Andrew Jones <drjones@redhat.com> wrote:
>
>
> This patch does much more than the summary "hw/arm/virt: spe: Add SPE fdt
> binding for virt machine" says it does. Please revise the summary.
>

Will revise it in v3.

Thanks,
Haibo

> On Tue, Sep 08, 2020 at 08:13:27AM +0000, Haibo Xu wrote:
> > Add a virtual SPE device for virt machine while using
> > PPI 5 for SPE overflow interrupt number which has already
> > selected in kvmtool.
> >
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  hw/arm/virt-acpi-build.c    |  3 +++
> >  hw/arm/virt.c               | 43 ++++++++++++++++++++++++++++++++++++-
> >  include/hw/acpi/acpi-defs.h |  3 +++
> >  include/hw/arm/virt.h       |  1 +
> >  target/arm/cpu.c            |  2 ++
> >  target/arm/cpu.h            |  2 ++
> >  6 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 9efd7a3881..3fd80fda53 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -665,6 +665,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >          if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
> >              gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> >          }
> > +        if (cpu_isar_feature(aa64_spe, armcpu)) {
> > +            gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> > +        }
> >          if (vms->virt) {
> >              gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
> >          }
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 6bacfb668d..bdb1ce925c 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -545,6 +545,32 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
> >      }
> >  }
> >
> > +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> > +{
> > +    ARMCPU *armcpu = ARM_CPU(first_cpu);
> > +    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> > +
> > +    if (!cpu_isar_feature(aa64_spe, armcpu)) {
> > +        assert(!object_property_get_bool(OBJECT(armcpu), "spe", NULL));
> > +        return;
> > +    }
> > +
> > +    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > +        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> > +                             GIC_FDT_IRQ_PPI_CPU_WIDTH,
> > +                             (1 << vms->smp_cpus) - 1);
> > +    }
> > +
> > +    qemu_fdt_add_subnode(vms->fdt, "/spe");
> > +    if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
> > +        const char compat[] = "arm,statistical-profiling-extension-v1";
> > +        qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> > +                         compat, sizeof(compat));
> > +        qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> > +                               GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ, irqflags);
> > +    }
> > +}
> > +
> >  static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
> >  {
> >      DeviceState *dev;
> > @@ -717,6 +743,10 @@ static void create_gic(VirtMachineState *vms)
> >                                      qdev_get_gpio_in(vms->gic, ppibase
> >                                                       + VIRTUAL_PMU_IRQ));
> >
> > +        qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> > +                                    qdev_get_gpio_in(vms->gic, ppibase
> > +                                                     + VIRTUAL_SPE_IRQ));
> > +
> >          sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> >          sysbus_connect_irq(gicbusdev, i + smp_cpus,
> >                             qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > @@ -1664,11 +1694,12 @@ static void finalize_gic_version(VirtMachineState *vms)
> >
> >  static void virt_cpu_post_init(VirtMachineState *vms)
> >  {
> > -    bool aarch64, pmu;
> > +    bool aarch64, pmu, spe;
> >      CPUState *cpu;
> >
> >      aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
> >      pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
> > +    spe = object_property_get_bool(OBJECT(first_cpu), "spe", NULL);
> >
> >      if (kvm_enabled()) {
> >          CPU_FOREACH(cpu) {
> > @@ -1679,6 +1710,14 @@ static void virt_cpu_post_init(VirtMachineState *vms)
> >                  }
> >                  kvm_arm_pmu_init(cpu);
> >              }
> > +
> > +            if (spe) {
> > +                assert(ARM_CPU(cpu)->has_spe == ON_OFF_AUTO_ON);
> > +                if (kvm_irqchip_in_kernel()) {
> > +                    kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> > +                }
> > +                kvm_arm_spe_init(cpu);
>
> A later patch introduces userspace irqchip support. Should we avoid
> allowing it until then to avoid breaking bisection?
>

Yes, it's possible to break the bisection. To avoid it I think we can
move the above codes
block to a separate patch after adding the userspace irqchip support,
Or, just put the userspace
irqchip support patch before this patch. What's your opinion?

Thanks,
Haibo

> > +            }
> >          }
> >      } else {
> >          if (aarch64 && vms->highmem) {
> > @@ -1927,6 +1966,8 @@ static void machvirt_init(MachineState *machine)
> >
> >      fdt_add_pmu_nodes(vms);
> >
> > +    fdt_add_spe_nodes(vms);
> > +
> >      create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
> >
> >      if (vms->secure) {
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 38a42f409a..21e58f27c5 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -302,6 +302,9 @@ struct AcpiMadtGenericCpuInterface {
> >      uint32_t vgic_interrupt;
> >      uint64_t gicr_base_address;
> >      uint64_t arm_mpidr;
> > +    uint8_t  efficiency_class;
> > +    uint8_t  reserved2[1];
> > +    uint16_t spe_interrupt; /* ACPI 6.3 */
> >  } QEMU_PACKED;
> >
> >  typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index 72c269aaa5..6013b6d535 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -49,6 +49,7 @@
> >  #define ARCH_TIMER_NS_EL1_IRQ 14
> >  #define ARCH_TIMER_NS_EL2_IRQ 10
> >
> > +#define VIRTUAL_SPE_IRQ 5
> >  #define VIRTUAL_PMU_IRQ 7
> >
> >  #define PPI(irq) ((irq) + 16)
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index f211958eaa..786cc6134c 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1041,6 +1041,8 @@ static void arm_cpu_initfn(Object *obj)
> >                               "gicv3-maintenance-interrupt", 1);
> >      qdev_init_gpio_out_named(DEVICE(cpu), &cpu->pmu_interrupt,
> >                               "pmu-interrupt", 1);
> > +    qdev_init_gpio_out_named(DEVICE(cpu), &cpu->spe_interrupt,
> > +                             "spe-interrupt", 1);
> >  #endif
> >
> >      /* DTB consumers generally don't in fact care what the 'compatible'
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index baf2bbcee8..395a1e5df8 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -800,6 +800,8 @@ struct ARMCPU {
> >      qemu_irq gicv3_maintenance_interrupt;
> >      /* GPIO output for the PMU interrupt */
> >      qemu_irq pmu_interrupt;
> > +    /* GPIO output for the SPE interrupt */
> > +    qemu_irq spe_interrupt;
> >
> >      /* MemoryRegion to use for secure physical accesses */
> >      MemoryRegion *secure_memory;
> > --
> > 2.17.1
> >
> >
>
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>

Re: [PATCH v2 10/12] target/arm/cpu: spe: Enable spe to work with host cpu

[Haibo Xu]

On Tue, 8 Sep 2020 at 19:33, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Sep 08, 2020 at 08:13:28AM +0000, Haibo Xu wrote:
> > Turn on the spe cpu property by default if host cpu
> > support it, i.e. we can now do '-cpu max|host' to add
> > the vSPE, and '-cpu max|host,spe=off' to remove it.
> >
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > ---
> >  target/arm/cpu.c   |  3 +++
> >  target/arm/cpu.h   |  2 ++
> >  target/arm/cpu64.c |  7 ++++++-
> >  target/arm/kvm64.c | 12 ++++++++++++
> >  4 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 786cc6134c..58f12d6eb5 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj)
> >      kvm_arm_set_cpu_features_from_host(cpu);
> >      if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> >          aarch64_add_sve_properties(obj);
> > +
> > +        cpu->has_spe = ON_OFF_AUTO_AUTO;
> > +        aarch64_add_spe_properties(obj);
>
> Why not put the assignment of has_spe into aarch64_add_spe_properties()?
>

Yes, it could be. Will fix it in v3.

> >      }
> >      arm_cpu_post_init(obj);
> >  }
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 395a1e5df8..5a3ea876c8 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
> >  void aarch64_sve_change_el(CPUARMState *env, int old_el,
> >                             int new_el, bool el0_a64);
> >  void aarch64_add_sve_properties(Object *obj);
> > +void aarch64_add_spe_properties(Object *obj);
> >
> >  /*
> >   * SVE registers are encoded in KVM's memory in an endianness-invariant format.
> > @@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o,
> >                                           int n, bool a)
> >  { }
> >  static inline void aarch64_add_sve_properties(Object *obj) { }
> > +static inline void aarch64_add_spe_properties(Object *obj) { }
> >  #endif
> >
> >  #if !defined(CONFIG_TCG)
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 4997c4a3c0..d38c55e2ca 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp)
> >      ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> >  }
> >
> > +void aarch64_add_spe_properties(Object *obj)
> > +{
> > +    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
> > +}
> > +
> >  /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
> >   * otherwise, a CPU with as many features enabled as our emulation supports.
> >   * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
> > @@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj)
> >                          cpu_max_set_sve_max_vq, NULL, NULL);
> >
> >      cpu->has_spe = ON_OFF_AUTO_AUTO;
> > -    object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set);
> > +    aarch64_add_spe_properties(obj);
>
> If TCG doesn't support this cpu feature then this should be in the
> kvm_enabled() part of this function.
>
>

Will fix it in v3.

> >  }
> >
> >  static const ARMCPUInfo aarch64_cpus[] = {
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 5a2032fc9e..3f0a09c05b 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> >       */
> >      int fdarray[3];
> >      bool sve_supported;
> > +    bool spe_supported;
> >      uint64_t features = 0;
> >      uint64_t t;
> >      int err;
> > @@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> >      }
> >
> >      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
> > +    spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION,
> > +                                      KVM_CAP_ARM_SPE_V1) > 0;
> >
> >      kvm_arm_destroy_scratch_host_vcpu(fdarray);
> >
> > @@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> >          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
> >          ahcf->isar.id_aa64pfr0 = t;
> >      }
> > +    if (!spe_supported) {
> > +        t = ahcf->isar.id_aa64dfr0;
> > +        t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0);
> > +        ahcf->isar.id_aa64dfr0 = t;
> > +    }
> >
>
> If this works then there's a bug with the KVM implementation. get-one-reg
> shouldn't expose SPE unless userspace has enabled it, which we're not
> doing on the scratch vcpu. That's why we set, not clear, the ID bits for
> SVE.
>
>

You are right. I have printed the returned field, and it's 0.
Thanks for pointing this out, I will fix it in v3.

Regards,
Haibo

> >      /*
> >       * We can assume any KVM supporting CPU is at least a v8
> > @@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >          assert(kvm_arm_sve_supported());
> >          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
> >      }
> > +    if (cpu_isar_feature(aa64_spe, cpu)) {
> > +        assert(kvm_arm_spe_supported());
> > +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1;
> > +    }
> >
> >      /* Do KVM_ARM_VCPU_INIT ioctl */
> >      ret = kvm_arm_vcpu_init(cs);
> > --
> > 2.17.1
> >
>
> Thanks,
> drew
>

Re: [PATCH v2 09/12] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Andrew Jones]

On Wed, Sep 09, 2020 at 03:51:14PM +0800, Haibo Xu wrote:
> > > +
> > > +            if (spe) {
> > > +                assert(ARM_CPU(cpu)->has_spe == ON_OFF_AUTO_ON);
> > > +                if (kvm_irqchip_in_kernel()) {
> > > +                    kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> > > +                }
> > > +                kvm_arm_spe_init(cpu);
> >
> > A later patch introduces userspace irqchip support. Should we avoid
> > allowing it until then to avoid breaking bisection?
> >
> 
> Yes, it's possible to break the bisection. To avoid it I think we can
> move the above codes
> block to a separate patch after adding the userspace irqchip support,
> Or, just put the userspace
> irqchip support patch before this patch. What's your opinion?
>

This patch ca forbid SPE without kernel irqchip. Then the patch that adds
userspace irqchip support would also remove the restriction.

Thanks,
drew 

Re: [PATCH v2 09/12] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Haibo Xu]

On Wed, 9 Sep 2020 at 17:34, Andrew Jones <drjones@redhat.com> wrote:
>
> On Wed, Sep 09, 2020 at 03:51:14PM +0800, Haibo Xu wrote:
> > > > +
> > > > +            if (spe) {
> > > > +                assert(ARM_CPU(cpu)->has_spe == ON_OFF_AUTO_ON);
> > > > +                if (kvm_irqchip_in_kernel()) {
> > > > +                    kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> > > > +                }
> > > > +                kvm_arm_spe_init(cpu);
> > >
> > > A later patch introduces userspace irqchip support. Should we avoid
> > > allowing it until then to avoid breaking bisection?
> > >
> >
> > Yes, it's possible to break the bisection. To avoid it I think we can
> > move the above codes
> > block to a separate patch after adding the userspace irqchip support,
> > Or, just put the userspace
> > irqchip support patch before this patch. What's your opinion?
> >
>
> This patch ca forbid SPE without kernel irqchip. Then the patch that adds
> userspace irqchip support would also remove the restriction.
>
> Thanks,
> drew
>

Good idea! Will fix it in v3.